From ccc98b90a9ee96bb705a1ff967c15af29c5dcee9 Mon Sep 17 00:00:00 2001 From: l0ng-ai Date: Thu, 30 Jul 2026 23:11:07 +0800 Subject: [PATCH] fix(input): stop the Windows/Linux keymap and CRLF pastes from breaking the shell (#270) * fix(input): stop the Windows/Linux keymap and CRLF pastes from breaking the shell Two problems a Windows user hit within an hour of installing tty7, both from code that only ever had macOS in mind. A bracketed paste forwarded the clipboard byte for byte. Text copied on Windows carries \r\n, and vim counts CR and LF as two line breaks, so pasting code into it left a blank line under every line. Fold \r\n to \n inside the paste markers so a Windows clipboard produces exactly what the same paste already produced on Linux and macOS. The non-bracketed path keeps converting to CR: with no paste mode to tell text from typing, a line break has to arrive as the byte Return sends. `secondary` means Cmd on macOS and Ctrl everywhere else, and the default keymap was carried over from macOS unchanged, which put window actions on top of terminal control codes. Ctrl+D could not send EOF, Ctrl+[ could not send ESC, and Ctrl+W, Ctrl+K, Ctrl+P, Ctrl+J, Ctrl+T, Ctrl+Q and Ctrl+S were all taken. These bindings carry no context, so they match before the terminal sees the key -- apply_readline_ctrl already sends EOF for ^D, it was just unreachable. Off macOS, ctrl- and ctrl-[ ] \ space now belong to the terminal and window actions move to ctrl-shift-*, the convention GNOME Terminal, Konsole, Windows Terminal and WezTerm share. Tabs move to Alt+1-9 and pane focus to Alt+arrow, since Ctrl+Alt+arrow is workspace switching under GNOME. Ctrl+S keeps saving in the code panel but propagates when the editor is unfocused, so a shell still gets XOFF. Shift+Insert pastes. macOS is untouched, and a user's own rebindings still win over the defaults. A test walks the default table and rejects any chord that lands on a control code, so the next binding added cannot quietly bring this back; a second one rejects two actions claiming the same chord, which is how the reshuffle caught NewTab colliding with ReopenClosedTab. Refs #269 * fix(history): keep one history store per machine, not one per install tty7 owns ^R at the prompt and shows its own fuzzy menu, but `history.rs` had no notion of where a command had run: `load()` read one file and `append()` wrote one file, so an ssh session was offered the commands typed on the local machine. That is worse than an empty menu -- the entries look plausible until one of them runs against the wrong box. History is now scoped. The local store keeps its existing path, so nobody loses what they have; each remote gets its own file under `history.d/`, named after the target with a hash appended so a label that is not a filename ("me@box:/srv") cannot escape the directory or collide with another host. A pane picks its scope from the innermost remote it can see -- the ssh target reported by shell integration, else its host id -- and follows it when that changes, clearing the menu and reloading in the background rather than showing the previous machine's entries while it waits. A reload that lands after the pane has moved on again is dropped. On a remote workspace, where a Host impl exists, the far end's own ~/.zsh_history and ~/.bash_history are read through the same channel git status and file listings already use, so the first ^R on a freshly connected box has something in it. Bare ssh has no Host to read through and falls back to what tty7 recorded itself for that target, which is still the right machine's commands. Refs #269 * fix(test): the control-code guard must admit Ctrl+S, which falls through The guard forbade every bare ctrl-letter default, but EditorSave staying on Ctrl+S is the design: its handler propagates whenever the editor does not have focus, so the terminal still receives XOFF. The guard's real invariant is "no default may swallow a control code" -- an action that falls through does not swallow. EditorSave is exempted by name, with the condition for earning a spot written next to the list. The test is vacuous on macOS (secondary parses to Cmd), which is why the contradiction only surfaced on the Linux runner. Co-Authored-By: Claude Fable 5 * fix(history): never seed an inner ssh target from the workspace host's files remote_shell_history_sources gated only on "scope is remote and host is remote", so a pane that ssh'ed onward from a remote workspace -- scoped to the inner target -- was seeded from the workspace host's ~/.zsh_history: another machine's commands in the menu, the exact confusion scoping exists to prevent. The Host can only reach the workspace machine's home, so seeding is now reserved for the host's own scope; a pane with a remote_context falls back to what tty7 recorded for that target, exactly like bare ssh from a local pane. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Co-authored-by: thomas Co-authored-by: Claude Fable 5 --- CHANGELOG.md | 60 ++++++++++++++ src/terminal/history.rs | 177 +++++++++++++++++++++++++++++++++++++--- src/terminal/view.rs | 112 ++++++++++++++++++++++++- src/ui/app.rs | 4 + src/ui/keymap.rs | 166 +++++++++++++++++++++++++++++-------- 5 files changed, 473 insertions(+), 46 deletions(-) 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")) ); }); }