diff --git a/CHANGELOG.md b/CHANGELOG.md index 0dd0eef7..d548cf3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 session" means: paste it into `codex resume`, a bug report, or another tool. (#211) +### Changed + +- **The prompt editor's soft newline is now a rebindable action** — `Shift+Enter` + and `Alt+Enter` have inserted a literal newline into the command editor since + the multi-line prompt editor landed, but the chords were hardcoded in the key + handler: there was no `InsertNewline` to name in `keybindings`, and no way to + move the gesture to a chord of your own. The behaviour is unchanged out of the + box — both chords still insert, plain `Enter` still submits the whole buffer — + but it now runs through an `InsertNewline` action, so it appears in Settings → + Keybindings and can be rebound like anything else, and rebinding it retires + both defaults. Only the prompt editor answers it; with a full-screen program on + the pane the chord reaches the application exactly as before. `⌘⏎` fullscreen + and `⌘⇧⏎` pane zoom are untouched. + + Two smaller behaviour changes come with it, both aligning on what other + terminals do. With a completion menu open, the newline chords now insert a + newline and close the menu instead of accepting the highlighted candidate — + plain `Enter` remains the key that accepts it. And `Shift+Alt+Enter`, which + the old modifier test caught by accident, now submits like any other `Enter`: + keybindings match modifiers exactly, and no terminal treats that three-key + chord as a newline. (#182) + ### Fixed - **Rounded UI controls no longer square off their corners** diff --git a/docs/features.md b/docs/features.md index 1b0efa79..10d35378 100644 --- a/docs/features.md +++ b/docs/features.md @@ -10,7 +10,7 @@ - **Fuzzy history search** — ⌃ R shows what you ran, where, and whether it failed; turn it off (Settings → Terminal → Keyboard, or `history_search` in `config.json`) and ⌃ R goes to your shell instead, so an fzf / percol binding keeps working - **History from day one** — your existing shell history works as-is and carries across sessions - **Line editing** — click to place the caret, mouse selection, word motion, undo -- **Multi-line editing** — wrapped and multi-line commands edit in place; the grid shifts to keep the caret visible +- **Multi-line editing** — wrapped and multi-line commands edit in place; the grid shifts to keep the caret visible. ⇧ ⏎ · ⌥ ⏎ insert a newline instead of submitting (rebindable as `InsertNewline`); a plain submits the whole buffer ## In the window diff --git a/docs/features.zh-CN.md b/docs/features.zh-CN.md index 94730bb6..988bd1b1 100644 --- a/docs/features.zh-CN.md +++ b/docs/features.zh-CN.md @@ -10,7 +10,7 @@ - **模糊历史搜索** —— ⌃ R 看到每条命令在哪跑的、什么时候、有没有失败;关掉它(设置 → 终端 → 键盘,或 `config.json` 里的 `history_search`)后 ⌃ R 直接交给 shell,你绑的 fzf / percol 照常可用 - **历史开箱即用** —— 你已有的 shell 历史直接生效,并跨会话延续 - **行编辑** —— 点击定位光标、鼠标选区、词级移动、撤销 -- **多行编辑** —— 折行和多行命令原地编辑;网格自动上移,光标始终可见 +- **多行编辑** —— 折行和多行命令原地编辑;网格自动上移,光标始终可见。⇧ ⏎ · ⌥ ⏎ 插入换行而不提交(可改绑,动作名 `InsertNewline`),单独按 提交整个缓冲区 ## 窗口 diff --git a/src/terminal/view.rs b/src/terminal/view.rs index 4ddcf101..4edb8978 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -50,6 +50,11 @@ const GRID_PAD_Y: f32 = 4.; // context-menu row, and the Edit-menu item all dispatch the same action, so the // three can't drift (they did — the context menu's Paste used to skip the // image-paste branch that ⌘V had). +// +// `InsertNewline` is the exception to the "menu" part: it has no menu row (you +// do not reach for a menu mid-word), and exists so the prompt editor's soft +// newline is a *bindable* action rather than a hardcoded chord — see +// `insert_newline_action`. actions!( terminal, [ @@ -62,7 +67,8 @@ actions!( FindInTerminal, FindNext, FindPrevious, - ClearScrollback + ClearScrollback, + InsertNewline ] ); @@ -2403,14 +2409,11 @@ impl TerminalView { match key { "enter" => { - // Shift+Enter / Opt+Enter inserts a newline to author (or extend) - // a multi-line command; a plain Enter submits the whole buffer. - if (m.shift || m.alt) && !m.control && !m.platform { - self.cmd.insert_str("\n"); - self.history_nav = None; - cx.notify(); - return; - } + // Any Enter that reaches here submits. The soft newline that + // Shift+Enter / Opt+Enter authors is not handled inline: it is + // the `InsertNewline` action, dispatched by the keymap before + // the key ever reaches this dispatcher, so the chord can be + // rebound like every other action (#182). self.submit_command(cx); return; } @@ -3978,6 +3981,46 @@ impl TerminalView { } } + /// The `InsertNewline` action: insert a literal newline at the caret so the + /// user can author (or extend) a multi-line command, which plain Enter then + /// submits whole. Bound to Shift+Enter and Alt+Enter by default. + /// + /// Only the local command editor answers this. When the editor isn't holding + /// the line — a foreground application owns the screen, the search field has + /// focus — or while a reverse search owns the keyboard, we `propagate` + /// instead, so the chord takes the exact path it took before this action + /// existed: on to `on_key_down`, and from there to the widget or out to the + /// application as raw bytes. + /// + /// An open completion menu deliberately does *not* decline it. A newline + /// ends the word being completed, so the menu is closed and the newline + /// inserted — for both chords. Warp draws the same line: only a bare Enter + /// reaches the popup-acceptance path (`FixedBinding::new("enter", …)` → + /// `input_enter`), while Shift+Enter / Alt+Enter dispatch their own actions + /// that the editor resolves as a newline without the popup ever seeing them. + /// Plain Enter here still runs `accept_line`, which takes the highlighted + /// candidate — that path is untouched. + fn insert_newline_action(&mut self, cx: &mut Context) { + if !self.input_active() || self.reverse_search.is_some() { + cx.propagate(); + return; + } + // A key pressed while scrolled up must edit the line the viewport is + // showing, the way every editor key does (see `handle_editor_key`). + self.jump_to_prompt(); + self.close_completion(); + self.cursor_visible = true; + self.cmd.insert_str("\n"); + self.history_nav = None; + // This action bypasses `handle_editor_key`, so it has to repeat that + // dispatcher's per-key state resets itself — same reason `commit_text` + // does for the IME path. Without them the next ↑/↓ takes its column + // from a stale goal, and an ⌥. walk would continue across the newline. + self.editor_goal_col = None; + self.last_word_nav = None; + cx.notify(); + } + /// readline's accept-line, as the editor means it: with a completion /// candidate highlighted, take the candidate (a second stroke then runs the /// line); otherwise close any menu and submit. Shared by Enter and its @@ -6272,6 +6315,12 @@ impl Render for TerminalView { this.step_match(Direction::Left, cx); })) .on_action(cx.listener(|this, _: &ClearScrollback, _w, cx| this.clear_scrollback(cx))) + // Soft newline in the prompt editor (Shift+Enter / Alt+Enter by + // default). Propagates when the editor isn't holding the line, so a + // foreground application still sees the chord unchanged. + .on_action(cx.listener(|this, _: &InsertNewline, _w, cx| { + this.insert_newline_action(cx); + })) // Tab / Shift-Tab are claimed here (in the "Terminal" key context) so // they reach the shell instead of triggering Root's focus navigation. // Tab → HT (0x09); Shift-Tab → CSI Z (back-tab), the standard sequence. @@ -7965,6 +8014,32 @@ mod gpui_tests { (window, daemon_side) } + /// Report the shell as idle at its prompt and wait for the view to see it, + /// so `input_active()` is true and the local command editor owns the line. + fn prompt_ready( + window: &gpui::WindowHandle, + cx: &mut TestAppContext, + daemon: &mut UnixStream, + ) { + DaemonMsg::Prompt { + active: true, + at_prompt: true, + last_exit: None, + } + .encode(daemon) + .unwrap(); + for _ in 0..200 { + if window + .update(cx, |view, _, _| view.terminal.at_prompt()) + .unwrap() + { + return; + } + std::thread::sleep(std::time::Duration::from_millis(5)); + } + panic!("the prompt report never reached the view"); + } + /// A hover cell remembered while the pane was tall names a row the grid no /// longer has once the pane shrinks (a vertical split, a smaller window). /// Resolving it must decline rather than index the grid — this path runs @@ -8562,6 +8637,159 @@ mod gpui_tests { .unwrap(); } + /// The `InsertNewline` action puts a literal newline at the caret and leaves + /// the line unsubmitted; a plain Enter then ships the whole multi-line + /// buffer. Behaviour that used to be hardcoded on Shift+Enter (#182). + #[gpui::test] + fn insert_newline_action_extends_the_line_and_enter_submits_it(cx: &mut TestAppContext) { + // `submit_command` defers a history-file record; pin the config dir to + // the shared test scratch so nothing touches the real user history. + let dir = std::env::temp_dir().join(format!("tty7-covtest-{}", std::process::id())); + std::fs::create_dir_all(&dir).ok(); + crate::core::config::set_config_dir(dir); + + let (window, mut daemon) = harness(cx); + prompt_ready(&window, cx, &mut daemon); + + window + .update(cx, |view, _, cx| { + assert!(view.input_active(), "the editor owns an idle prompt"); + view.commit_text("echo a", cx); + view.insert_newline_action(cx); + view.commit_text("echo b", cx); + assert_eq!(view.cmd.text(), "echo a\necho b"); + + view.handle_editor_key(&key("enter"), cx); + assert!(view.cmd.is_empty(), "Enter submits the whole buffer"); + }) + .unwrap(); + assert_eq!( + next_input_until_timeout(&mut daemon), + // Submit sends each buffer line as a carriage return, the way a + // pasted multi-line command already goes out. + Some(b"echo a\recho b\r".to_vec()), + "the multi-line command reaches the PTY in one submit" + ); + } + + /// With a completion menu open the action still inserts, and closes the + /// menu: the newline ends the word being completed, so a menu still + /// filtered on the old word would be stale. Plain Enter keeps its own + /// meaning there — it accepts the highlighted candidate (#182). + #[gpui::test] + fn insert_newline_action_closes_the_completion_menu_but_enter_still_accepts( + cx: &mut TestAppContext, + ) { + let (window, mut daemon) = harness(cx); + prompt_ready(&window, cx, &mut daemon); + + let candidate = |text: &str| completion::Candidate { + text: text.to_string(), + kind: CandidateKind::Command, + start: 4, + end: 4, + description: None, + icon: None, + }; + + window + .update(cx, |view, _, cx| { + // Menu open on the word after "git ". + view.cmd.set_with_cursor("git ", 4); + view.open_completion(CompletionSession::new( + 4, + String::new(), + vec![candidate("status")], + )); + + view.insert_newline_action(cx); + assert!( + view.completion.is_none(), + "the newline ends the completed word, so the menu closes" + ); + assert_eq!(view.cmd.text(), "git \n"); + + // Plain Enter with a menu open is a different gesture: it takes + // the highlighted candidate rather than submitting or inserting. + view.cmd.set_with_cursor("git ", 4); + view.open_completion(CompletionSession::new( + 4, + String::new(), + vec![candidate("status")], + )); + view.handle_editor_key(&key("enter"), cx); + // Accepting a command candidate leaves the trailing space that + // starts the next word. + assert_eq!(view.cmd.text(), "git status "); + }) + .unwrap(); + } + + /// The action is the prompt editor's alone: with a foreground application on + /// the alternate screen it declines, so the chord takes its old path out to + /// the application instead of editing a line that isn't there. + #[gpui::test] + fn insert_newline_action_declines_when_the_editor_is_not_live(cx: &mut TestAppContext) { + let (window, _daemon) = harness(cx); + window + .update(cx, |view, _, cx| { + view.cmd.set("keep me"); + view.terminal.exited = true; // simplest input_active() = false + assert!(!view.input_active()); + view.insert_newline_action(cx); + assert_eq!(view.cmd.text(), "keep me", "no newline inserted"); + }) + .unwrap(); + } + + /// The check the tests above structurally can't make: with the *real* keymap + /// installed, both default chords have to actually reach the action. They + /// call `insert_newline_action` directly, so a wrong key context — or a + /// `NoAction` from a later `rebind` shadowing the chord — would leave every + /// one of them green while Shift+Enter silently submitted the line. This + /// drives the keystroke through GPUI's dispatch instead (#182). + #[gpui::test] + fn the_keymap_routes_both_newline_chords_to_the_action(cx: &mut TestAppContext) { + let (window, mut daemon) = harness(cx); + cx.update(|cx| crate::ui::keymap::init(cx)); + prompt_ready(&window, cx, &mut daemon); + window + .update(cx, |view, window, cx| { + window.activate_window(); + view.focus_handle.focus(window, cx); + view.commit_text("echo a", cx); + }) + .unwrap(); + + let mut vcx = gpui::VisualTestContext::from_window(window.into(), cx); + vcx.simulate_keystrokes("shift-enter"); + vcx.simulate_keystrokes("alt-enter"); + window + .update(cx, |view, _, _| { + assert_eq!( + view.cmd.text(), + "echo a\n\n", + "both chords dispatched InsertNewline instead of submitting" + ); + }) + .unwrap(); + + // And again after a rebind, which is when the suppression bindings go in: + // the `NoAction` retiring the old chord must not outrank the identical + // one being re-installed alongside it. + cx.update(|cx| crate::ui::keymap::rebind(cx)); + vcx.simulate_keystrokes("shift-enter"); + window + .update(cx, |view, _, _| { + assert_eq!( + view.cmd.text(), + "echo a\n\n\n", + "the chord survives a rebind" + ); + }) + .unwrap(); + } + /// The Ctrl+R flow end-to-end at the editor dispatcher: Ctrl+R opens the /// search, typed text (the IME/commit path) edits the query with fuzzy /// matching, Enter loads the selection into the editor without running it. diff --git a/src/ui/app.rs b/src/ui/app.rs index 508ce048..5b79d75e 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -5885,8 +5885,13 @@ impl Tty7App { /// the takeover so the user can undo it with a reset. fn assign_keybinding(&mut self, action: String, spec: String, cx: &mut Context) { // Find the current owner of this exact keystroke, if it isn't `action`. + // The extra chords installed alongside the table (an action can ship a + // second default it has no row for) count as owners too — miss them and + // the new binding would quietly take a chord off an action whose + // Settings row still advertises its first one. let displaced = crate::ui::keymap::effective_bindings(cx) .into_iter() + .chain(crate::ui::keymap::extra_bindings(cx)) .find(|(a, k)| *k == spec && *a != action) .map(|(a, _)| a); let note = displaced.as_ref().map(|other| { @@ -7777,6 +7782,28 @@ mod keybinding_gpui_tests { wait_for_binding(&mut vcx, "CloseActiveTab", "secondary-b x"); } + // Taking a chord an action holds only as an *extra* default (Alt+Enter, the + // second one Insert Newline ships without a table row of its own) displaces + // it like any other owner: the chord stops inserting newlines either way, so + // the unset has to be written and the takeover said out loud. + #[gpui::test] + fn recording_an_extra_default_chord_displaces_its_owner(cx: &mut TestAppContext) { + let (app, mut vcx) = harness(cx); + begin_capture(&app, &mut vcx, "NewTab"); + vcx.simulate_keystrokes("alt-enter"); + wait_for_binding(&mut vcx, "NewTab", "alt-enter"); + wait_for_binding(&mut vcx, "InsertNewline", ""); + + let note = app.update_in(&mut vcx, |app, _, _| { + app.active_settings().and_then(|s| s.rebinding_note.clone()) + }); + assert!( + note.as_deref() + .is_some_and(|n| n.contains("Insert Newline")), + "the takeover note must name the action that lost the chord (got {note:?})" + ); + } + // Esc during capture cancels without touching config. #[gpui::test] fn escape_cancels_capture_without_writing(cx: &mut TestAppContext) { diff --git a/src/ui/keymap.rs b/src/ui/keymap.rs index 484e5e2c..03ac1478 100644 --- a/src/ui/keymap.rs +++ b/src/ui/keymap.rs @@ -7,15 +7,19 @@ use gpui::{App, Global, KeyBinding, Keystroke, NoAction}; use crate::core::actions::*; use crate::core::config::Config; -use crate::terminal::view::{ClearScrollback, FindInTerminal, FindNext, FindPrevious}; +use crate::terminal::view::{ + ClearScrollback, FindInTerminal, FindNext, FindPrevious, InsertNewline, +}; use crate::ui::theme::set_menus; /// The set of keystrokes currently installed for app actions, remembered so a /// later [`rebind`] can neutralize them with `NoAction` bindings instead of /// clearing the whole keymap (which would also wipe gpui-component's own input / -/// list / menu bindings). Stored as a GPUI global. +/// list / menu bindings). Each entry carries the key context its binding was +/// installed in (see [`action_context`]) so the `NoAction` can be scoped the +/// same way. Stored as a GPUI global. #[derive(Default)] -struct BoundKeystrokes(Vec); +struct BoundKeystrokes(Vec<(String, Option<&'static str>)>); impl Global for BoundKeystrokes {} /// Install the application menu bar, keybindings, and global actions. @@ -53,12 +57,19 @@ pub fn rebind(cx: &mut App) { .unwrap_or_default(); let effective = effective_bindings(cx); - // Neutralize each old keystroke (global NoAction is enabled in every - // context, so it also suppresses the Terminal-scoped ClearScrollback). + // Neutralize each old keystroke with a `NoAction` in the same context its + // binding was installed in. The scope matters: GPUI's `binding_enabled` + // gives a *context-less* binding the maximum match depth (`contexts.len()`), + // so a global `NoAction` outranks every context-scoped binding on that + // chord — and a matched `NoAction` discards the rest of the matches — which + // would kill bindings we never installed (gpui-component's own Input-scoped + // `shift-enter` while the search field has focus). Scoped the same way, the + // `NoAction` still suppresses our own old binding: that one sits at the + // same depth with a lower index, and index breaks the tie. let mut bindings: Vec = previous .iter() - .filter(|k| keystroke_is_valid(k)) - .map(|k| KeyBinding::new(k, NoAction {}, None)) + .filter(|(k, _)| keystroke_is_valid(k)) + .map(|(k, ctx)| KeyBinding::new(k, NoAction {}, *ctx)) .collect(); bindings.extend(action_bindings(&effective)); cx.bind_keys(bindings); @@ -74,10 +85,65 @@ pub fn rebind(cx: &mut App) { set_menus(cx); } +/// `InsertNewline`'s primary default: Windows Terminal's chord for a soft +/// newline in the prompt editor, and the one the table in [`default_bindings`] +/// carries. +const INSERT_NEWLINE_DEFAULT: &str = "shift-enter"; + +/// `InsertNewline`'s second default: iTerm2's chord for the same gesture. Both +/// have inserted a newline since the multi-line editor landed, and both must +/// keep doing so — but a binding spec can't express alternatives (whitespace in +/// a spec means a *sequence*, `ctrl-b n`-style), and the effective table holds +/// exactly one keystroke per action. So this chord is installed alongside the +/// table's, and only while `InsertNewline` still sits on its primary default: +/// rebinding the action in Settings or `config.json` retires both old chords, +/// which is what a user who moves the binding expects. +/// +/// Only these two. GPUI matches a binding on exact modifier equality +/// (`Keystroke::should_match`), so Shift+Alt+Enter — which the old hardcoded +/// `(m.shift || m.alt)` test happened to catch — maps to no action and submits +/// like any other Enter. That matches the reference: Warp's key table is a +/// `(ctrl, alt, shift, key)` tuple whose newline arms are exactly Shift+Enter, +/// Alt+Enter and Ctrl+J, so its `(false, true, true, "enter")` falls through +/// too, and its GUI compares whole keystrokes for equality just as gpui does. +const INSERT_NEWLINE_ALT_DEFAULT: &str = "alt-enter"; + +/// The extra keystroke an effective table installs beyond its one-per-action +/// rows — today only [`INSERT_NEWLINE_ALT_DEFAULT`]. `None` once the action has +/// been rebound off its default. +fn extra_keystrokes(effective: &[(String, String)]) -> Vec<(&'static str, &'static str)> { + let on_default = effective + .iter() + .any(|(a, k)| a == "InsertNewline" && k == INSERT_NEWLINE_DEFAULT); + if on_default { + vec![("InsertNewline", INSERT_NEWLINE_ALT_DEFAULT)] + } else { + Vec::new() + } +} + +/// The extra `(action, keystroke)` pairs the current effective table installs +/// beyond its one-row-per-action rows. [`effective_bindings`] stays one row per +/// action because Settings renders from it, so anything that has to reason +/// about which chords are actually *live* — conflict detection when the user +/// records a shortcut — must consult this alongside it, or it would miss the +/// extra chord and silently drop it. +pub(crate) fn extra_bindings(cx: &App) -> Vec<(String, String)> { + extra_keystrokes(&effective_bindings(cx)) + .into_iter() + .map(|(a, k)| (a.to_string(), k.to_string())) + .collect() +} + /// Build the `KeyBinding`s for an effective table, skipping unbound rows (empty /// keystroke) and any that fail validation. fn action_bindings(effective: &[(String, String)]) -> Vec { let mut bindings = Vec::new(); + for (action, key) in extra_keystrokes(effective) { + if let Some(b) = make_binding(action, key) { + bindings.push(b); + } + } for (action, key) in effective { if key.is_empty() { continue; // an action with no assigned key @@ -94,13 +160,17 @@ fn action_bindings(effective: &[(String, String)]) -> Vec { bindings } -/// The valid, non-empty keystrokes an effective table actually installs — the -/// list [`rebind`] remembers so it can suppress them on the next change. -fn bound_keystrokes(effective: &[(String, String)]) -> Vec { +/// The valid, non-empty keystrokes an effective table actually installs, each +/// paired with the key context it is installed in — the list [`rebind`] +/// remembers so it can suppress them, in that same context, on the next change. +fn bound_keystrokes(effective: &[(String, String)]) -> Vec<(String, Option<&'static str>)> { + let extras = extra_keystrokes(effective); effective .iter() + .map(|(a, k)| (a.as_str(), k.as_str())) + .chain(extras.iter().map(|(a, k)| (*a, *k))) .filter(|(_, k)| !k.is_empty() && keystroke_is_valid(k)) - .map(|(_, k)| k.clone()) + .map(|(a, k)| (k.to_string(), action_context(a))) .collect() } @@ -243,6 +313,11 @@ pub(crate) fn default_bindings() -> Vec<(&'static str, &'static str)> { ), // Like Terminal.app / iTerm2 / Ghostty ⌘K: wipe the screen + scrollback. ("ClearScrollback", "secondary-k"), + // Soft newline in the prompt editor: author a multi-line command without + // submitting it. Shift+Enter is Windows Terminal's chord and the primary + // default; Alt+Enter (iTerm2's) is installed alongside it — see + // `INSERT_NEWLINE_ALT_DEFAULT` for why it can't live in this table. + ("InsertNewline", INSERT_NEWLINE_DEFAULT), ("OpenSettings", "secondary-,"), // Help → Keyboard Shortcuts, on the ⌘/ that editors and browsers use for // "show me the shortcuts". Off macOS `secondary-/` is Ctrl+/, which some @@ -541,6 +616,24 @@ fn keystroke_is_valid(s: &str) -> bool { any } +/// The key context an action's binding is installed in, or `None` for a global +/// one. The single source of truth for that decision: [`make_binding`] builds +/// the binding with it and [`bound_keystrokes`] records it, so the `NoAction` +/// [`rebind`] later uses to retire a chord lands in the same scope as the +/// binding it retires. +/// +/// Terminal-scoped means the handler lives on the terminal surface, so the +/// "Terminal" context keeps the chord inert on the settings / home pages +/// instead of binding a dead global chord there. +fn action_context(action: &str) -> Option<&'static str> { + match action { + "FindInTerminal" | "FindNext" | "FindPrevious" | "ClearScrollback" | "InsertNewline" => { + Some("Terminal") + } + _ => None, + } +} + /// Build a `KeyBinding` for a known action name + (already-validated) keystroke. /// Returns `None` for an unrecognized action name. fn make_binding(action: &str, keystroke: &str) -> Option { @@ -615,16 +708,13 @@ fn make_binding(action: &str, keystroke: &str) -> Option { "ShowRightPanelOutline" => KeyBinding::new(keystroke, ShowRightPanelOutline, None), "ShowRightPanelChanges" => KeyBinding::new(keystroke, ShowRightPanelChanges, None), "ShowRightPanelFiles" => KeyBinding::new(keystroke, ShowRightPanelFiles, None), - // Terminal-scoped (the handler lives on the terminal surface): the "Terminal" - // context keeps ⌘K inert in the settings tab / home page instead of binding a - // dead global chord there. - // Terminal-scoped like ClearScrollback: the handlers live on the terminal - // surface, so the "Terminal" context keeps these inert on the settings / - // home pages instead of binding a dead global chord there. - "FindInTerminal" => KeyBinding::new(keystroke, FindInTerminal, Some("Terminal")), - "FindNext" => KeyBinding::new(keystroke, FindNext, Some("Terminal")), - "FindPrevious" => KeyBinding::new(keystroke, FindPrevious, Some("Terminal")), - "ClearScrollback" => KeyBinding::new(keystroke, ClearScrollback, Some("Terminal")), + // Terminal-scoped — see `action_context`, which owns that decision so the + // context here and the one `rebind` neutralizes with can't drift apart. + "FindInTerminal" => KeyBinding::new(keystroke, FindInTerminal, action_context(action)), + "FindNext" => KeyBinding::new(keystroke, FindNext, action_context(action)), + "FindPrevious" => KeyBinding::new(keystroke, FindPrevious, action_context(action)), + "ClearScrollback" => KeyBinding::new(keystroke, ClearScrollback, action_context(action)), + "InsertNewline" => KeyBinding::new(keystroke, InsertNewline, action_context(action)), "OpenSettings" => KeyBinding::new(keystroke, OpenSettings, None), "ShowKeyboardShortcuts" => KeyBinding::new(keystroke, ShowKeyboardShortcuts, None), "About" => KeyBinding::new(keystroke, About, None), @@ -734,6 +824,134 @@ mod tests { } } + #[test] + fn insert_newline_ships_both_default_chords() { + // Shift+Enter and Alt+Enter have both inserted a soft newline since the + // multi-line editor landed; exposing the gesture as an action must not + // cost either one. Shift+Enter is the table row, Alt+Enter the extra. + let effective: Vec<(String, String)> = default_bindings() + .into_iter() + .map(|(a, k)| (a.to_string(), k.to_string())) + .collect(); + assert_eq!( + effective + .iter() + .find(|(a, _)| a == "InsertNewline") + .map(|(_, k)| k.as_str()), + Some("shift-enter") + ); + assert_eq!( + extra_keystrokes(&effective), + vec![("InsertNewline", "alt-enter")] + ); + // Both are real, installable bindings, and both are remembered so a + // later `rebind` can neutralize them. + for key in ["shift-enter", "alt-enter"] { + assert!(keystroke_is_valid(key), "{key} does not parse"); + assert!(make_binding("InsertNewline", key).is_some()); + assert!( + bound_keystrokes(&effective).iter().any(|(k, _)| k == key), + "{key} is not remembered as installed" + ); + } + assert_eq!(key_tokens("shift-enter"), vec![SHIFT, "⏎"]); + } + + #[test] + fn rebinding_insert_newline_retires_both_default_chords() { + // Move the action off its default and the second chord goes with it — + // otherwise Alt+Enter would keep inserting newlines behind the user's + // back after they deliberately moved the binding. + let effective = vec![("InsertNewline".to_string(), "ctrl-o".to_string())]; + assert!(extra_keystrokes(&effective).is_empty()); + assert_eq!( + bound_keystrokes(&effective), + vec![("ctrl-o".to_string(), Some("Terminal"))] + ); + + // Unbinding it entirely (an empty override) installs nothing at all. + let unbound = vec![("InsertNewline".to_string(), String::new())]; + assert!(extra_keystrokes(&unbound).is_empty()); + assert!(action_bindings(&unbound).is_empty()); + } + + #[test] + fn bound_keystrokes_remember_the_context_each_binding_was_installed_in() { + // `rebind` retires an old chord with a `NoAction`, and a context-less + // one gets GPUI's *maximum* match depth — it would outrank, and discard, + // deeper bindings on that chord that we never installed (gpui-component + // binds `shift-enter` in its own "Input" context, which is what steps to + // the previous match in the terminal's search field). So each remembered + // keystroke carries the scope its binding actually had. + let effective = vec![ + ("InsertNewline".to_string(), "shift-enter".to_string()), + ("NewTab".to_string(), "secondary-t".to_string()), + ]; + assert_eq!( + bound_keystrokes(&effective), + vec![ + ("shift-enter".to_string(), Some("Terminal")), + ("secondary-t".to_string(), None), + // The extra chord is scoped by its action, like any other row. + ("alt-enter".to_string(), Some("Terminal")), + ] + ); + } + + #[test] + fn action_context_matches_the_scope_make_binding_installs() { + // The two must agree for every action, or `rebind` would neutralize a + // chord in a scope the binding never had — leaving the old binding live + // (too narrow) or shadowing unrelated ones (too wide). + let extra_actions = extra_keystrokes( + &default_bindings() + .into_iter() + .map(|(a, k)| (a.to_string(), k.to_string())) + .collect::>(), + ); + let actions = default_bindings() + .into_iter() + .map(|(a, _)| a) + .chain(extra_actions.into_iter().map(|(a, _)| a)); + for action in actions { + let binding = + make_binding(action, "f13").unwrap_or_else(|| panic!("no arm for {action}")); + let installed = binding.predicate().map(|p| p.to_string()); + assert_eq!( + installed.as_deref(), + action_context(action), + "{action} is installed in a different context than `action_context` reports" + ); + } + } + + #[test] + fn secondary_enter_chords_are_distinct_from_insert_newline() { + // The window bindings the reporter called out (#182) must stay put: + // ⌘⏎ / ⌘⇧⏎ are different keystrokes from the bare ⇧⏎ and ⌥⏎. + let defaults = default_bindings(); + let key_of = |action: &str| { + defaults + .iter() + .find(|(a, _)| *a == action) + .map(|(_, k)| *k) + .unwrap() + }; + assert_eq!(key_of("ToggleFullscreen"), "secondary-enter"); + assert_eq!(key_of("ToggleMaximizePane"), "secondary-shift-enter"); + for window_chord in ["secondary-enter", "secondary-shift-enter"] { + assert_ne!(window_chord, INSERT_NEWLINE_DEFAULT); + assert_ne!(window_chord, INSERT_NEWLINE_ALT_DEFAULT); + } + // Modifier matching is exact, so the three-key chord is nobody's: it + // inserts nothing and submits, as it does in Warp. Spelled out here so + // a future reader doesn't "restore" it as a missing default. + for chord in [INSERT_NEWLINE_DEFAULT, INSERT_NEWLINE_ALT_DEFAULT] { + assert_ne!(chord, "shift-alt-enter"); + assert_ne!(chord, "alt-shift-enter"); + } + } + #[test] fn spec_from_keystroke_round_trips_through_parse() { // A recorded keystroke → spec string → parsed keystroke must be stable,