diff --git a/CHANGELOG.md b/CHANGELOG.md index c45e91b8..28f2643a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -113,6 +113,44 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **Windows and Linux stop taking keys the shell needs** — `secondary` means + Cmd on macOS and Ctrl everywhere else, and the default keymap was carried over + from macOS unchanged. That put window actions straight on top of terminal + control codes: Ctrl+D could not send EOF (it split the pane), Ctrl+[ could not + send ESC (it cycled panes), and Ctrl+W, Ctrl+K, Ctrl+P, Ctrl+J, Ctrl+T, + Ctrl+Q and Ctrl+S were all spoken for. These are window-level bindings with no + context, so they matched before the terminal ever saw the key — the code that + sends EOF was there, just unreachable. + + Off macOS the rule is now that `ctrl-`, `ctrl-[`, `ctrl-]`, `ctrl-\` + and `ctrl-space` belong to the terminal, and window actions live on + `ctrl-shift-*` — the convention GNOME Terminal, Konsole, Windows Terminal and + WezTerm already share. A test enforces it, so the next binding added cannot + quietly reintroduce the problem. + + | action | was | now | + |---|---|---| + | Focus previous / next pane | Ctrl+[ / Ctrl+] | Ctrl+Shift+[ / Ctrl+Shift+] | + | Split right / down | Ctrl+D / Ctrl+Shift+D | Ctrl+Shift+D / Ctrl+Alt+Shift+D | + | Close tab | Ctrl+W | Ctrl+Shift+W | + | New tab | Ctrl+T | Ctrl+Shift+T | + | Reopen closed tab | Ctrl+Shift+T | Alt+Shift+T | + | Clear scrollback | Ctrl+K | Ctrl+Shift+K | + | Command palette | Ctrl+P | Ctrl+Shift+P | + | Toggle right panel | Ctrl+J | Ctrl+Shift+J | + | Toggle left panel | unbound | Ctrl+Shift+B | + | Quit | Ctrl+Q | Ctrl+Shift+Q | + | Fullscreen | Ctrl+Enter | F11 | + | Activate tab 1-9 | Ctrl+1-9 | Alt+1-9 | + | Focus pane by direction | Ctrl+Alt+arrow | Alt+arrow | + + Ctrl+S keeps saving in the code panel but now falls through to the terminal + when the editor does not have focus, so a shell still receives XOFF. + Ctrl+C, Ctrl+V and Ctrl+X are unchanged — Ctrl+C still copies only when there + is a selection and sends SIGINT otherwise — and **Shift+Insert** now pastes. + macOS bindings are untouched. Anything you rebound yourself still wins; only + the defaults moved. (#269) + - **The machine that runs your panes now owns their layout** — the workspace, tab and pane tree has moved out of the app and into the background service, so one machine has one tree that every client of it reads: the window on it, a @@ -231,6 +269,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **⌃R on a remote machine searches that machine's history** — tty7 owns ⌃R at + the prompt and shows its own fuzzy menu, but the store behind it had no notion + of *where* a command had run. Every pane read one file, so ssh'ing to a server + and reaching for ⌃R offered the commands you had typed on your laptop — + worse than offering nothing, since the answers look plausible until you run + one. History is now kept per machine: the local store stays where it was, and + each remote gets its own, keyed by the target you connected to. On a remote + workspace tty7 also reads the far end's own `~/.zsh_history` and + `~/.bash_history` through the same channel it already uses for git and file + listings, so the first ⌃R on a freshly connected box has something in it + rather than starting empty. Switching a pane between machines swaps the store + under it, and the local history file is untouched by the upgrade. (#269) + +- **A Windows clipboard pastes like every other clipboard** — text copied on + Windows carries `\r\n`, and a bracketed paste forwarded it byte for byte. vim + counts CR and LF as two line breaks, so pasting a block of code into it left a + blank line under every line — bad enough to make tty7 unusable for editing. + Bracketed pastes now fold `\r\n` down to a single `\n`, which is exactly what + the same paste already produced on Linux and macOS. The non-bracketed path is + untouched: with no paste mode to distinguish text from typing, a line break + still has to arrive as the CR the Return key sends. (#269) + - **Closing every window before quitting no longer loses your place** — launch only ever restored a workspace that still had a window at quit, so closing them one by one and relaunching came up on the empty home page, with no hint that diff --git a/src/terminal/history.rs b/src/terminal/history.rs index 39de3963..eb52f1e8 100644 --- a/src/terminal/history.rs +++ b/src/terminal/history.rs @@ -40,9 +40,65 @@ pub struct History { pub meta: HashMap, } -pub fn load() -> History { - let mut raw: Vec = load_shell_history(); - if let Some(path) = config_path("history") +#[derive(Clone, Default, PartialEq, Eq, Debug)] +pub enum Scope { + #[default] + Local, + Remote(String), +} + +impl Scope { + pub fn remote(label: &str) -> Scope { + let label = label.trim(); + if label.is_empty() { + Scope::Local + } else { + Scope::Remote(label.to_string()) + } + } + + pub fn is_local(&self) -> bool { + matches!(self, Scope::Local) + } + + fn file(&self) -> Option { + match self { + Scope::Local => config_path("history"), + Scope::Remote(label) => config_path("history.d").map(|d| d.join(file_stem(label))), + } + } +} + +fn file_stem(label: &str) -> String { + let mut safe: String = label + .chars() + .map(|c| { + if c.is_ascii_alphanumeric() || c == '-' || c == '.' { + c + } else { + '_' + } + }) + .collect(); + safe.truncate(48); + format!("{safe}-{:016x}", tty7_core::host::fnv1a64(label.as_bytes())) +} + +pub fn load(scope: &Scope) -> History { + load_with_shell_files(scope, Vec::new()) +} + +pub fn load_with_shell_files(scope: &Scope, shell_files: Vec>) -> History { + let mut raw: Vec = if scope.is_local() { + load_shell_history() + } else { + let mut out = Vec::new(); + for bytes in &shell_files { + parse_shell_history(&String::from_utf8_lossy(bytes), &mut out); + } + out + }; + if let Some(path) = scope.file() && let Ok(content) = std::fs::read_to_string(&path) { raw.extend(content.lines().map(parse_own_line)); @@ -50,6 +106,10 @@ pub fn load() -> History { normalize(raw) } +pub fn shell_history_names() -> [&'static str; 2] { + [".zsh_history", ".bash_history"] +} + fn looks_absolute(p: &str) -> bool { match p.as_bytes() { [b'/' | b'\\', ..] => true, @@ -151,11 +211,11 @@ pub fn format_ago(now: u64, ts: u64) -> String { format!("{n}{unit}") } -pub fn append(cmd: &str, cwd: Option<&Path>, ts: u64, exit: Option) { +pub fn append(scope: &Scope, cmd: &str, cwd: Option<&Path>, ts: u64, exit: Option) { if cmd.contains('\n') { return; } - let Some(path) = config_path("history") else { + let Some(path) = scope.file() else { return; }; if let Some(parent) = path.parent() { @@ -570,11 +630,17 @@ mod tests { fn append_then_load_recovers_the_command_and_metadata() { crate::core::config::pin_test_config_dir(); - append("bad\ncmd", None, 1_700_000_000, None); + append(&Scope::Local, "bad\ncmd", None, 1_700_000_000, None); let unique = format!("tty7_cov_marker_{}", std::process::id()); - append(&unique, Some(Path::new("/tmp")), 1_700_000_123, Some(1)); - let loaded = load(); + append( + &Scope::Local, + &unique, + Some(Path::new("/tmp")), + 1_700_000_123, + Some(1), + ); + let loaded = load(&Scope::Local); assert!( loaded.entries.iter().any(|e| e == &unique), "appended command should be recalled by load()" @@ -607,6 +673,7 @@ mod tests { std::thread::spawn(move || { for i in 0..25 { append( + &Scope::Local, &format!("{tag}_{t}_{i}"), Some(Path::new("/tmp")), 1_700_000_000, @@ -620,7 +687,7 @@ mod tests { h.join().unwrap(); } - let loaded = load(); + let loaded = load(&Scope::Local); for t in 0..8 { for i in 0..25 { let cmd = format!("{tag}_{t}_{i}"); @@ -632,18 +699,108 @@ mod tests { } } + #[test] + fn a_remote_scope_never_serves_the_local_machine_s_history() { + crate::core::config::pin_test_config_dir(); + + let tag = format!("tty7_scope_{}", std::process::id()); + let here = Scope::Local; + let there = Scope::remote("me@box"); + append(&here, &format!("{tag}_local"), None, 1_700_000_000, Some(0)); + append( + &there, + &format!("{tag}_remote"), + None, + 1_700_000_001, + Some(0), + ); + + let local = load(&here); + let remote = load(&there); + assert!(local.entries.iter().any(|e| e == &format!("{tag}_local"))); + assert!(remote.entries.iter().any(|e| e == &format!("{tag}_remote"))); + assert!( + !remote.entries.iter().any(|e| e == &format!("{tag}_local")), + "a remote pane must not be offered commands from this machine" + ); + assert!( + !local.entries.iter().any(|e| e == &format!("{tag}_remote")), + "the local pane must not be offered commands from the far end" + ); + } + + #[test] + fn two_remotes_keep_their_own_stores() { + crate::core::config::pin_test_config_dir(); + + let tag = format!("tty7_twohosts_{}", std::process::id()); + let a = Scope::remote("me@alpha"); + let b = Scope::remote("me@beta"); + append(&a, &format!("{tag}_a"), None, 1_700_000_000, Some(0)); + append(&b, &format!("{tag}_b"), None, 1_700_000_001, Some(0)); + + assert!( + !load(&a).entries.iter().any(|e| e == &format!("{tag}_b")), + "one host's history must not leak into another's" + ); + assert!(!load(&b).entries.iter().any(|e| e == &format!("{tag}_a"))); + } + + #[test] + fn a_remote_scope_reads_the_far_end_s_own_shell_history() { + crate::core::config::pin_test_config_dir(); + + let scope = Scope::remote("me@readfile"); + let zsh = b": 1700000000:0;systemctl status nginx\n".to_vec(); + let bash = b"journalctl -u nginx\n".to_vec(); + let loaded = load_with_shell_files(&scope, vec![zsh, bash]); + + assert!( + loaded.entries.iter().any(|e| e == "systemctl status nginx"), + "the far end's zsh history should be searchable" + ); + assert!(loaded.entries.iter().any(|e| e == "journalctl -u nginx")); + assert_eq!( + loaded.meta.get("systemctl status nginx"), + Some(&EntryMeta { + ts: Some(1_700_000_000), + exit: None, + }), + "zsh's second field is elapsed seconds, not an exit code" + ); + } + + #[test] + fn a_label_that_is_not_a_filename_still_gets_its_own_file() { + let slashes = file_stem("me@box:/srv/../weird"); + assert!( + !slashes.contains(['/', '\\', ':', '@']), + "a scope file name must not escape its directory: {slashes}" + ); + assert_ne!(file_stem("me@alpha"), file_stem("me@beta")); + assert_eq!(file_stem("me@alpha"), file_stem("me@alpha")); + assert!(file_stem(&"x".repeat(400)).len() < 80); + } + + #[test] + fn an_empty_label_falls_back_to_local_rather_than_a_nameless_file() { + assert_eq!(Scope::remote(""), Scope::Local); + assert_eq!(Scope::remote(" "), Scope::Local); + } + #[test] fn append_rejects_a_cwd_that_would_break_the_line_format() { crate::core::config::pin_test_config_dir(); let unique = format!("tty7_nlcwd_marker_{}", std::process::id()); append( + &Scope::Local, &unique, Some(Path::new("/tmp/evil\n/tmp/tail")), 1_700_000_000, None, ); - let loaded = load(); + let loaded = load(&Scope::Local); assert!(loaded.entries.iter().any(|e| e == &unique)); assert!(loaded.cwds.get(&unique).is_none_or(|d| d.is_empty())); assert!(!loaded.entries.iter().any(|e| e == "/tmp/evil")); diff --git a/src/terminal/view.rs b/src/terminal/view.rs index 3b38fa19..f270fd37 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -149,6 +149,7 @@ pub struct TerminalView { history_meta: std::collections::HashMap, history_ranked: Vec, history_frecency: Vec, + history_scope: super::history::Scope, ranked_cwd: Option, history_nav: Option, history_stash: String, @@ -251,6 +252,8 @@ const INTEGRATION_NOTICE_TIMEOUT: std::time::Duration = std::time::Duration::fro const OPPORTUNISTIC_GIT_GAP: std::time::Duration = std::time::Duration::from_millis(1500); +const MAX_HISTORY_BYTES: u64 = 4 << 20; + #[derive(Clone, Copy, PartialEq, Eq, Debug)] enum GitRefresh { Edge, @@ -308,6 +311,7 @@ fn ring_system_bell() -> bool { fn paste_bytes(text: &str, bracketed: bool) -> Vec { if bracketed { + let text = text.replace("\r\n", "\n"); let mut bytes = b"\x1b[200~".to_vec(); bytes.extend(text.bytes().filter(|&b| b != 0x1b)); bytes.extend_from_slice(b"\x1b[201~"); @@ -670,7 +674,7 @@ impl TerminalView { window.focus(&focus_handle, cx); - let history = super::history::load(); + let history = super::history::load(&super::history::Scope::Local); let history_ranked = super::history::rank_by_frecency( &history.entries, &history.counts, @@ -739,6 +743,7 @@ impl TerminalView { history_meta: history.meta, history_ranked, history_frecency, + history_scope: super::history::Scope::Local, ranked_cwd: None, history_nav: None, history_stash: String::new(), @@ -2097,6 +2102,95 @@ impl TerminalView { } else if tool_activity { self.refresh_git_status(cwd_now, GitRefresh::Opportunistic, cx); } + + self.follow_history_scope(cx); + } + + fn desired_history_scope(&self) -> super::history::Scope { + if let Some(ctx) = self.remote_context() { + return super::history::Scope::remote(&ctx.target); + } + if !self.host_id.is_local() { + return super::history::Scope::remote(&format!("host-{:016x}", self.host_id.0)); + } + super::history::Scope::Local + } + + fn follow_history_scope(&mut self, cx: &mut Context) { + let scope = self.desired_history_scope(); + if scope == self.history_scope { + return; + } + self.flush_pending_history(); + self.history_scope = scope.clone(); + self.history.clear(); + self.history_counts.clear(); + self.history_cwds.clear(); + self.history_meta.clear(); + self.history_ranked.clear(); + self.history_frecency.clear(); + self.history_nav = None; + self.reverse_search = None; + cx.notify(); + + let shell_files = self.remote_shell_history_sources(cx); + let loading = scope.clone(); + cx.spawn(async move |this, cx| { + let loaded = cx + .background_spawn(async move { + let files = shell_files + .into_iter() + .filter_map(|(host, path)| host.read_file(&path, MAX_HISTORY_BYTES).ok()) + .collect(); + super::history::load_with_shell_files(&loading, files) + }) + .await; + this.update(cx, |view, cx| { + if view.history_scope != scope { + return; + } + view.history = loaded.entries; + view.history_counts = loaded.counts; + view.history_cwds = loaded.cwds; + view.history_meta = loaded.meta; + let cwd = view.ranked_cwd.clone(); + view.rerank_history(cwd.as_deref()); + cx.notify(); + }) + .ok(); + }) + .detach(); + } + + fn remote_shell_history_sources( + &self, + cx: &mut Context, + ) -> Vec<(crate::ui::host_ops::SharedHost, std::path::PathBuf)> { + if self.history_scope.is_local() || self.host_id.is_local() { + return Vec::new(); + } + // The Host reaches the workspace machine's home directory and nothing + // beyond it. A pane that has ssh'ed onward from there (remote_context) + // is scoped to the *inner* target, and seeding that scope from the + // workspace host's ~/.zsh_history would offer commands from the wrong + // box — the exact confusion scoping exists to prevent. Those panes + // start from what tty7 recorded for the inner target, like bare ssh. + if self.remote_context().is_some() { + return Vec::new(); + } + let Some(host) = self.host(cx) else { + return Vec::new(); + }; + if !host.is_connected() { + return Vec::new(); + } + let Some(home) = crate::ui::remote_connect::HostLinks::home(cx, self.host_id) else { + return Vec::new(); + }; + super::history::shell_history_names() + .into_iter() + .map(|name| (std::sync::Arc::clone(&host), host.join(&home, name))) + .collect() } fn refresh_git_status( @@ -2662,7 +2756,7 @@ impl TerminalView { { m.exit = exit; } - super::history::append(&p.line, p.cwd.as_deref(), p.ts, exit); + super::history::append(&self.history_scope, &p.line, p.cwd.as_deref(), p.ts, exit); } fn ghost_suggestion(&self) -> Option { @@ -5149,6 +5243,20 @@ mod tests { ); } + #[test] + fn paste_bytes_folds_crlf_so_a_windows_clipboard_pastes_like_any_other() { + assert_eq!( + paste_bytes("a\r\nb\r\n", true), + b"\x1b[200~a\nb\n\x1b[201~".to_vec(), + "CRLF must reach the app as one line break, not two" + ); + assert_eq!( + paste_bytes("a\r\nb", true), + paste_bytes("a\nb", true), + "a Windows clipboard must paste exactly like a Unix one" + ); + } + #[test] fn submit_bytes_sends_a_multi_line_command_as_one_bracketed_paste() { assert_eq!( diff --git a/src/ui/app.rs b/src/ui/app.rs index 614c8202..1f158b0f 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -5167,6 +5167,10 @@ impl Render for Tty7App { this.toggle_code_panel(window, cx) })) .on_action(cx.listener(|this, _: &EditorSave, window, cx| { + if !this.editor_has_focus(window, cx) { + cx.propagate(); + return; + } this.editor_save_active(window, cx) })) .on_action(cx.listener(|_, _: &Quit, _, cx| cx.quit())) diff --git a/src/ui/keymap.rs b/src/ui/keymap.rs index 09314b8b..0562dbe2 100644 --- a/src/ui/keymap.rs +++ b/src/ui/keymap.rs @@ -3,7 +3,7 @@ use gpui::{App, Global, KeyBinding, Keystroke, NoAction}; use crate::core::actions::*; use crate::core::config::Config; use crate::terminal::view::{ - ClearScrollback, FindInTerminal, FindNext, FindPrevious, InsertNewline, + ClearScrollback, FindInTerminal, FindNext, FindPrevious, InsertNewline, PasteText, }; use crate::ui::theme::set_menus; @@ -17,6 +17,9 @@ pub fn init(cx: &mut App) { bindings.push(KeyBinding::new("secondary-+", IncreaseFontSize, None)); bindings.push(KeyBinding::new("tab", SendTab, Some("Terminal"))); bindings.push(KeyBinding::new("shift-tab", SendBackTab, Some("Terminal"))); + if cfg!(not(target_os = "macos")) { + bindings.push(KeyBinding::new("shift-insert", PasteText, Some("Terminal"))); + } cx.bind_keys(bindings); cx.set_global(BoundKeystrokes(bound_keystrokes(&effective))); @@ -99,11 +102,22 @@ fn bound_keystrokes(effective: &[(String, String)]) -> Vec<(String, Option<&'sta .collect() } +fn per_platform(mac: &'static str, other: &'static str) -> &'static str { + if cfg!(target_os = "macos") { + mac + } else { + other + } +} + pub(crate) fn default_bindings() -> Vec<(&'static str, &'static str)> { vec![ - ("NewTab", "secondary-t"), + ("NewTab", per_platform("secondary-t", "secondary-shift-t")), ("NewWorkspace", "secondary-shift-n"), - ("CloseActiveTab", "secondary-w"), + ( + "CloseActiveTab", + per_platform("secondary-w", "secondary-shift-w"), + ), ("RenameTab", ""), ("NewWorktreeTab", ""), ("CloseOtherTabs", ""), @@ -120,14 +134,35 @@ pub(crate) fn default_bindings() -> Vec<(&'static str, &'static str)> { ("DeleteWorkspace", ""), ("RenameWorkspace", ""), ("ToggleSwitcher", "secondary-shift-o"), - ("SplitRight", "secondary-d"), - ("SplitDown", "secondary-shift-d"), - ("FocusNextPane", "secondary-]"), - ("FocusPrevPane", "secondary-["), - ("FocusPaneLeft", "secondary-alt-left"), - ("FocusPaneRight", "secondary-alt-right"), - ("FocusPaneUp", "secondary-alt-up"), - ("FocusPaneDown", "secondary-alt-down"), + ( + "SplitRight", + per_platform("secondary-d", "secondary-shift-d"), + ), + ( + "SplitDown", + per_platform("secondary-shift-d", "secondary-alt-shift-d"), + ), + ( + "FocusNextPane", + per_platform("secondary-]", "secondary-shift-]"), + ), + ( + "FocusPrevPane", + per_platform("secondary-[", "secondary-shift-["), + ), + ( + "FocusPaneLeft", + per_platform("secondary-alt-left", "alt-left"), + ), + ( + "FocusPaneRight", + per_platform("secondary-alt-right", "alt-right"), + ), + ("FocusPaneUp", per_platform("secondary-alt-up", "alt-up")), + ( + "FocusPaneDown", + per_platform("secondary-alt-down", "alt-down"), + ), ("ResizePaneLeft", ""), ("ResizePaneRight", ""), ("ResizePaneUp", ""), @@ -136,15 +171,15 @@ pub(crate) fn default_bindings() -> Vec<(&'static str, &'static str)> { ("SwapPanePrev", ""), ("NextTab", "ctrl-tab"), ("PrevTab", "ctrl-shift-tab"), - ("ActivateTab1", "secondary-1"), - ("ActivateTab2", "secondary-2"), - ("ActivateTab3", "secondary-3"), - ("ActivateTab4", "secondary-4"), - ("ActivateTab5", "secondary-5"), - ("ActivateTab6", "secondary-6"), - ("ActivateTab7", "secondary-7"), - ("ActivateTab8", "secondary-8"), - ("ActivateTab9", "secondary-9"), + ("ActivateTab1", per_platform("secondary-1", "alt-1")), + ("ActivateTab2", per_platform("secondary-2", "alt-2")), + ("ActivateTab3", per_platform("secondary-3", "alt-3")), + ("ActivateTab4", per_platform("secondary-4", "alt-4")), + ("ActivateTab5", per_platform("secondary-5", "alt-5")), + ("ActivateTab6", per_platform("secondary-6", "alt-6")), + ("ActivateTab7", per_platform("secondary-7", "alt-7")), + ("ActivateTab8", per_platform("secondary-8", "alt-8")), + ("ActivateTab9", per_platform("secondary-9", "alt-9")), ("SelectWorkspace1", ""), ("SelectWorkspace2", ""), ("SelectWorkspace3", ""), @@ -157,20 +192,25 @@ pub(crate) fn default_bindings() -> Vec<(&'static str, &'static str)> { ("IncreaseFontSize", "secondary-="), ("DecreaseFontSize", "secondary--"), ("ResetFontSize", "secondary-0"), - ("TogglePalette", "secondary-p"), - ("ReopenClosedTab", "secondary-shift-t"), + ( + "TogglePalette", + per_platform("secondary-p", "secondary-shift-p"), + ), + ( + "ReopenClosedTab", + per_platform("secondary-shift-t", "alt-shift-t"), + ), ("ToggleMaximizePane", "secondary-shift-enter"), - ("ToggleFullscreen", "secondary-enter"), + ("ToggleFullscreen", per_platform("secondary-enter", "f11")), ("ToggleTabSidebar", ""), ( "ToggleLeftPanel", - if cfg!(target_os = "macos") { - "secondary-b" - } else { - "" - }, + per_platform("secondary-b", "secondary-shift-b"), + ), + ( + "ToggleRightPanel", + per_platform("secondary-j", "secondary-shift-j"), ), - ("ToggleRightPanel", "secondary-j"), ( "FindInTerminal", if cfg!(target_os = "macos") { @@ -195,7 +235,10 @@ pub(crate) fn default_bindings() -> Vec<(&'static str, &'static str)> { "shift-f3" }, ), - ("ClearScrollback", "secondary-k"), + ( + "ClearScrollback", + per_platform("secondary-k", "secondary-shift-k"), + ), ("InsertNewline", INSERT_NEWLINE_DEFAULT), ("OpenSettings", "secondary-,"), ( @@ -243,7 +286,7 @@ pub(crate) fn default_bindings() -> Vec<(&'static str, &'static str)> { ("EditorSave", "secondary-s"), ("OpenSshProfiles", ""), ("RestartSshSession", "secondary-shift-r"), - ("Quit", "secondary-q"), + ("Quit", per_platform("secondary-q", "secondary-shift-q")), ] } @@ -705,7 +748,10 @@ mod tests { .map(|(_, k)| *k) .unwrap() }; - assert_eq!(key_of("ToggleFullscreen"), "secondary-enter"); + assert_eq!( + key_of("ToggleFullscreen"), + per_platform("secondary-enter", "f11") + ); assert_eq!(key_of("ToggleMaximizePane"), "secondary-shift-enter"); for window_chord in ["secondary-enter", "secondary-shift-enter"] { assert_ne!(window_chord, INSERT_NEWLINE_DEFAULT); @@ -717,6 +763,55 @@ mod tests { } } + #[test] + fn no_default_binding_sits_on_a_terminal_control_code() { + // The invariant is "no default may *swallow* a terminal control code". + // EditorSave deliberately stays on Ctrl+S: its handler in `app.rs` calls + // `cx.propagate()` whenever the editor does not have focus, so the + // keystroke falls through to the terminal as XOFF instead of dying at + // the window. Anything added here must have such a fall-through. + const FALLS_THROUGH_TO_TERMINAL: [&str; 1] = ["EditorSave"]; + for (action, spec) in default_bindings() { + if FALLS_THROUGH_TO_TERMINAL.contains(&action) { + continue; + } + for chord in spec.split_whitespace() { + let ks = Keystroke::parse(chord).expect("default chords parse"); + let m = &ks.modifiers; + if !m.control || m.alt || m.shift || m.platform || m.function { + continue; + } + let steals = ks.key.len() == 1 + && ks + .key + .chars() + .next() + .is_some_and(|c| c.is_ascii_alphabetic() || "[]\\".contains(c)) + || ks.key == "space"; + assert!( + !steals, + "{action} is bound to {chord}, which the shell needs as a control code \ + (Ctrl+[ is ESC, Ctrl+D is EOF, Ctrl+W deletes a word). \ + Window actions belong on ctrl-shift-* off macOS." + ); + } + } + } + + #[test] + fn every_default_chord_is_claimed_by_exactly_one_action() { + let mut seen: Vec<(&str, &str)> = Vec::new(); + for (action, spec) in default_bindings() { + if spec.is_empty() { + continue; + } + if let Some((other, _)) = seen.iter().find(|(_, s)| *s == spec) { + panic!("{action} and {other} both claim {spec}"); + } + seen.push((action, spec)); + } + } + #[test] fn spec_from_keystroke_round_trips_through_parse() { for spec in [ @@ -774,7 +869,10 @@ mod gpui_tests { }; assert_eq!(key_of("NewTab"), "secondary-shift-n"); assert_eq!(key_of("SplitRight"), "ctrl-b %"); - assert_eq!(key_of("TogglePalette"), "secondary-p"); + assert_eq!( + key_of("TogglePalette"), + per_platform("secondary-p", "secondary-shift-p") + ); cx.global_mut::().keybinding_preset = "default".to_string(); rebind(cx); @@ -783,7 +881,7 @@ mod gpui_tests { eff.iter() .find(|(a, _)| a == "SplitRight") .map(|(_, k)| k.as_str()), - Some("secondary-d") + Some(per_platform("secondary-d", "secondary-shift-d")) ); }); }