diff --git a/CHANGELOG.md b/CHANGELOG.md index 0dd0eef7..5d8ec895 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,20 @@ 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. (#182) + ### Fixed - **Rounded UI controls no longer square off their corners** diff --git a/src/terminal/view.rs b/src/terminal/view.rs index 4ddcf101..a00866b1 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,30 @@ 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 when another local widget has claimed the keyboard (completion + /// picker, reverse search), 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. + fn insert_newline_action(&mut self, cx: &mut Context) { + if !self.input_active() || self.completion.is_some() || 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.cursor_visible = true; + self.cmd.insert_str("\n"); + self.history_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 +6299,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. @@ -8562,6 +8595,74 @@ 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); + // A prompt report engages the editor, so `input_active()` is true. + DaemonMsg::Prompt { + active: true, + at_prompt: true, + last_exit: None, + } + .encode(&mut daemon) + .unwrap(); + for _ in 0..200 { + if window + .update(cx, |view, _, _| view.terminal.at_prompt()) + .unwrap() + { + break; + } + std::thread::sleep(std::time::Duration::from_millis(5)); + } + + 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" + ); + } + + /// 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 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/keymap.rs b/src/ui/keymap.rs index 484e5e2c..d4c25d11 100644 --- a/src/ui/keymap.rs +++ b/src/ui/keymap.rs @@ -7,7 +7,9 @@ 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 @@ -74,10 +76,44 @@ 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. +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() + } +} + /// 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 @@ -99,8 +135,10 @@ fn action_bindings(effective: &[(String, String)]) -> Vec { fn bound_keystrokes(effective: &[(String, String)]) -> Vec { effective .iter() - .filter(|(_, k)| !k.is_empty() && keystroke_is_valid(k)) - .map(|(_, k)| k.clone()) + .map(|(_, k)| k.as_str()) + .chain(extra_keystrokes(effective).into_iter().map(|(_, k)| k)) + .filter(|k| !k.is_empty() && keystroke_is_valid(k)) + .map(str::to_string) .collect() } @@ -243,6 +281,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 @@ -625,6 +668,7 @@ fn make_binding(action: &str, keystroke: &str) -> Option { "FindNext" => KeyBinding::new(keystroke, FindNext, Some("Terminal")), "FindPrevious" => KeyBinding::new(keystroke, FindPrevious, Some("Terminal")), "ClearScrollback" => KeyBinding::new(keystroke, ClearScrollback, Some("Terminal")), + "InsertNewline" => KeyBinding::new(keystroke, InsertNewline, Some("Terminal")), "OpenSettings" => KeyBinding::new(keystroke, OpenSettings, None), "ShowKeyboardShortcuts" => KeyBinding::new(keystroke, ShowKeyboardShortcuts, None), "About" => KeyBinding::new(keystroke, About, None), @@ -734,6 +778,74 @@ 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()]); + + // 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 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); + } + } + #[test] fn spec_from_keystroke_round_trips_through_parse() { // A recorded keystroke → spec string → parsed keystroke must be stable,