From 21c16161191ff965031d1ac41fe9046011610743 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Tue, 13 Oct 2026 19:46:45 +0000 Subject: [PATCH] Support Command/Super shortcuts in the TUI keymap (#53754) ## What changed - Accept `super` modifiers and normalize `cmd` and `command` aliases to `super`. Support capturing, displaying, and using these bindings in shortcuts, chords, and leaders. - Add `super-left`, `super-right`, and `super-backspace` editor defaults for moving to the start or end of a line and deleting to its start. - Honor safe Super-modified editor shortcuts during startup without treating them as text or inserting a pending paste newline. - Save keymap edits through `config/value/write` when connected to a local daemon so it can validate bindings before they are saved or applied. Preserve the configuration and active keymap if the daemon rejects the edit. ## Testing Add coverage for modifier aliases, shortcut capture and labels, chords and leaders, line editing, startup input, and daemon acceptance or rejection of keymap edits. Extend the startup PTY test with Command editing sequences. GitOrigin-RevId: 8ed74f8253c89d423d5dfe87aa092844ce834454 --- codex-rs/config/src/tui_keymap.rs | 34 ++++++-- codex-rs/tui/src/app/event_dispatch.rs | 45 ++++++++-- .../app/tests/local_command_scroll_tests.rs | 82 +++++++++++++++++++ .../app/tests/session_lifecycle_requests.rs | 13 +++ ...tests__keymap_daemon_rejected_binding.snap | 5 ++ codex-rs/tui/src/bottom_pane/textarea.rs | 16 ++++ codex-rs/tui/src/key_hint.rs | 21 +++-- codex-rs/tui/src/key_hint_label_tests.rs | 1 + codex-rs/tui/src/keymap.rs | 22 +++-- codex-rs/tui/src/keymap/chords.rs | 4 +- codex-rs/tui/src/keymap/chords_tests.rs | 11 +++ codex-rs/tui/src/keymap/leader.rs | 9 +- codex-rs/tui/src/keymap_setup.rs | 40 ++++++++- ...ap_setup__tests__keymap_picker_editor.snap | 39 +++++++++ codex-rs/tui/src/startup_draft_input.rs | 24 ++++-- codex-rs/tui/src/startup_draft_tests.rs | 32 +++++--- codex-rs/tui/src/tui_startup_tests.rs | 8 +- 17 files changed, 347 insertions(+), 59 deletions(-) create mode 100644 codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__local_command_scroll_tests__keymap_daemon_rejected_binding.snap create mode 100644 codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_editor.snap diff --git a/codex-rs/config/src/tui_keymap.rs b/codex-rs/config/src/tui_keymap.rs index 47360c88c4..f0e5b13f0d 100644 --- a/codex-rs/config/src/tui_keymap.rs +++ b/codex-rs/config/src/tui_keymap.rs @@ -576,7 +576,7 @@ pub struct TuiKeymap { /// Normalize one user-entered key spec into canonical storage format. /// -/// The output always orders modifiers as `ctrl-alt-shift-` when present +/// The output always orders modifiers as `ctrl-alt-shift-super-` when present /// and applies accepted aliases (`escape` -> `esc`, `pageup` -> `page-up`). /// Inputs that cannot be represented unambiguously are rejected. /// @@ -635,8 +635,12 @@ See the Codex keymap documentation for supported actions and examples." )); } - let mut modifiers = - BTreeMap::<&str, bool>::from([("ctrl", false), ("alt", false), ("shift", false)]); + let mut modifiers = BTreeMap::<&str, bool>::from([ + ("ctrl", false), + ("alt", false), + ("shift", false), + ("super", false), + ]); let mut key_segments = Vec::new(); let mut saw_key = false; @@ -645,6 +649,7 @@ See the Codex keymap documentation for supported actions and examples." "ctrl" | "control" => Some("ctrl"), "alt" | "option" => Some("alt"), "shift" => Some("shift"), + "super" | "cmd" | "command" => Some("super"), _ => None, }; @@ -668,10 +673,12 @@ See the Codex keymap documentation for supported actions and examples." )); } - if key_segments - .iter() - .any(|segment| matches!(*segment, "ctrl" | "control" | "alt" | "option" | "shift")) - { + if key_segments.iter().any(|segment| { + matches!( + *segment, + "ctrl" | "control" | "alt" | "option" | "shift" | "super" | "cmd" | "command" + ) + }) { return Err(format!( "invalid keybinding `{raw}`: modifiers must come before the key (for example `ctrl-a`)." )); @@ -688,6 +695,9 @@ See the Codex keymap documentation for supported actions and examples." if modifiers.get("shift").copied().unwrap_or(false) { normalized.push("shift".to_string()); } + if modifiers.get("super").copied().unwrap_or(false) { + normalized.push("super".to_string()); + } normalized.push(key); Ok(normalized.join("-")) } @@ -908,6 +918,16 @@ mod tests { assert_eq!(normalize_keybinding_spec("f24"), Ok("f24".to_string())); assert!(normalize_keybinding_spec("f25").is_err()); } + + #[test] + fn command_modifier_aliases_normalize_to_super() { + for spec in ["cmd-left", "command-left", "super-left"] { + assert_eq!( + normalize_keybinding_spec(spec), + Ok("super-left".to_string()) + ); + } + } } #[cfg(test)] diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index e5bd89312b..b7f6a94f51 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -3425,7 +3425,7 @@ impl App { key, intent, } => { - self.apply_keymap_capture(context, action, key, intent) + self.apply_keymap_capture(app_server, context, action, key, intent) .await; self.merge_startup_warnings(tui, &history_cell::StartupWarningsCell::default()); } @@ -3510,6 +3510,7 @@ impl App { async fn apply_keymap_capture( &mut self, + app_server: &mut AppServerSession, context: String, action: String, key: String, @@ -3559,13 +3560,41 @@ impl App { } }; - let edit = - crate::legacy_core::config::edit::keymap_bindings_edit(&context, &action, &bindings); - match ConfigEditsBuilder::for_config_path(self.local_settings.user_config_path.as_path()) - .with_edits([edit]) - .apply() - .await - { + let save = if matches!(self.app_server_target, AppServerTarget::LocalDaemon { .. }) { + // Let the daemon validate shared config before writing: older daemons may + // not understand bindings introduced by this TUI, including new defaults. + app_server + .request_handle() + .request_typed::( + codex_app_server_protocol::ClientRequest::ConfigValueWrite { + request_id: app_server.next_request_id(), + params: codex_app_server_protocol::ConfigValueWriteParams { + key_path: format!("tui.keymap.{context}.{action}"), + value: match bindings.as_slice() { + [binding] => serde_json::json!(binding), + bindings => serde_json::json!(bindings), + }, + merge_strategy: codex_app_server_protocol::MergeStrategy::Replace, + file_path: Some( + self.local_settings.user_config_path.display().to_string(), + ), + expected_version: None, + }, + }, + ) + .await + .map(|_| ()) + .map_err(anyhow::Error::new) + } else { + let edit = crate::legacy_core::config::edit::keymap_bindings_edit( + &context, &action, &bindings, + ); + ConfigEditsBuilder::for_config_path(self.local_settings.user_config_path.as_path()) + .with_edits([edit]) + .apply() + .await + }; + match save { Ok(()) => { self.cancel_pending_key_chord(); self.local_settings.tui.keymap = keymap_config.clone(); diff --git a/codex-rs/tui/src/app/tests/local_command_scroll_tests.rs b/codex-rs/tui/src/app/tests/local_command_scroll_tests.rs index 45ee9e7cf9..b4b41fbdd3 100644 --- a/codex-rs/tui/src/app/tests/local_command_scroll_tests.rs +++ b/codex-rs/tui/src/app/tests/local_command_scroll_tests.rs @@ -285,6 +285,88 @@ async fn settings_confirmations_reveal_their_history_output() -> Result<()> { Ok(()) } +#[tokio::test] +async fn keymap_daemon_validates_before_saving_or_applying_bindings() -> Result<()> { + use super::session_lifecycle_requests::HistoryCapabilities; + use super::session_lifecycle_requests::recorded_params; + use super::session_lifecycle_requests::start_recording_app_server_with_history; + + for capabilities in [ + HistoryCapabilities::Current, + HistoryCapabilities::SuperBindingsUnsupported, + ] { + let (mut app, mut events, _op_rx) = make_test_app_with_channels().await; + let mut tui = crate::tui::test_support::make_test_tui()?; + let path = app.local_settings.user_config_path.clone(); + std::fs::write(&path, "# retained config\n")?; + let original_keymap = app.local_settings.tui.keymap.clone(); + let (mut server, requests, proxy) = start_recording_app_server_with_history( + &app.config, + capabilities, + /*blocked_thread_list*/ None, + /*failed_thread_name*/ None, + crate::app_server_session::ThreadParamsMode::Embedded, + LoaderOverrides::without_managed_config_for_tests(), + ) + .await?; + app.app_server_target = AppServerTarget::LocalDaemon { + endpoint: crate::resolve_remote_addr("ws://127.0.0.1:4500")?, + allow_embedded_fallback: false, + }; + app.handle_event( + &mut tui, + &mut server, + AppEvent::KeymapCaptured { + context: "editor".into(), + action: "move_line_start".into(), + key: "f12".into(), + intent: crate::app_event::KeymapEditIntent::AddAlternate, + }, + ) + .await?; + while let Ok(event) = events.try_recv() { + app.handle_event(&mut tui, &mut server, event).await?; + } + assert_eq!( + recorded_params(&requests, "config/value/write"), + vec![serde_json::json!({ + "keyPath": "tui.keymap.editor.move_line_start", + "value": ["home", "ctrl-a", "super-left", "f12"], + "mergeStrategy": "replace", + "filePath": path.display().to_string(), + "expectedVersion": null, + })], + ); + if capabilities == HistoryCapabilities::Current { + let saved: codex_config::config_toml::ConfigToml = + toml::from_str(&std::fs::read_to_string(&path)?)?; + assert_eq!(saved.tui.unwrap().keymap, app.local_settings.tui.keymap); + assert_eq!( + crate::keymap_setup::active_binding_specs(&app.keymap, "editor", "move_line_start") + .expect("editor bindings"), + vec!["home", "ctrl-a", "super-left", "f12"], + ); + } else { + assert_eq!(std::fs::read_to_string(&path)?, "# retained config\n"); + assert_eq!(app.local_settings.tui.keymap, original_keymap); + assert_eq!( + crate::keymap_setup::active_binding_specs(&app.keymap, "editor", "move_line_start") + .expect("editor bindings"), + vec!["home", "ctrl-a", "super-left"], + ); + let error = app.transcript_cells.last().expect("save error"); + insta::assert_snapshot!( + "keymap_daemon_rejected_binding", + lines_to_single_string(&error.display_lines(/*width*/ 80)), + ); + } + server.start_thread(&app.config).await?; + server.shutdown().await?; + proxy.await??; + } + Ok(()) +} + #[tokio::test] async fn local_command_and_inline_error_reveal_their_history_output() -> Result<()> { let (mut app, mut events, _op_rx) = make_test_app_with_channels().await; diff --git a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs index bca048a4ea..ed67f0bf9d 100644 --- a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs +++ b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs @@ -377,6 +377,7 @@ pub(super) enum HistoryCapabilities { ThreadStartDuringLegacyShutdown, ConfigReadUnsupported(i64), ConfigReadFails, + SuperBindingsUnsupported, ConfigReadUnknownVoice, VoiceCatalogCustom, VoiceCatalogUnavailable, @@ -655,6 +656,18 @@ pub(super) async fn start_recording_app_server_with_realtime_speech( message: "config temporarily unavailable".to_string(), }, }) + } else if history_capabilities == HistoryCapabilities::SuperBindingsUnsupported + && request.method == "config/value/write" + { + JSONRPCMessage::Error(JSONRPCError { + id: request_id, + error: JSONRPCErrorError { + code: -32600, + data: None, + message: "Invalid configuration: unknown key `super-left`" + .to_string(), + }, + }) } else if history_capabilities == HistoryCapabilities::VoiceCatalogUnavailable && request.method == "thread/realtime/listVoices" { diff --git a/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__local_command_scroll_tests__keymap_daemon_rejected_binding.snap b/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__local_command_scroll_tests__keymap_daemon_rejected_binding.snap new file mode 100644 index 0000000000..45836d8537 --- /dev/null +++ b/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__local_command_scroll_tests__keymap_daemon_rejected_binding.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/app/tests/local_command_scroll_tests.rs +expression: lines_to_single_string(&error.display_lines(80)) +--- +■ Failed to save shortcut: config/value/write failed: Invalid configuration: unknown key `super-left` (code -32600) diff --git a/codex-rs/tui/src/bottom_pane/textarea.rs b/codex-rs/tui/src/bottom_pane/textarea.rs index 6a21beba8f..7a2540d993 100644 --- a/codex-rs/tui/src/bottom_pane/textarea.rs +++ b/codex-rs/tui/src/bottom_pane/textarea.rs @@ -3158,6 +3158,22 @@ mod tests { assert_eq!(t.cursor(), 3); } + #[test] + fn super_line_editing_defaults_follow_macos_conventions() { + let mut textarea = ta_with("first\nsecond third"); + textarea.set_cursor("first\nsecond".len()); + + textarea.input(KeyEvent::new(KeyCode::Left, KeyModifiers::SUPER)); + assert_eq!(textarea.cursor(), "first\n".len()); + + textarea.input(KeyEvent::new(KeyCode::Right, KeyModifiers::SUPER)); + assert_eq!(textarea.cursor(), textarea.text().len()); + + textarea.input(KeyEvent::new(KeyCode::Backspace, KeyModifiers::SUPER)); + assert_eq!(textarea.text(), "first\n"); + assert_eq!(textarea.cursor(), "first\n".len()); + } + #[test] fn kill_current_line_removes_current_line_linewise() { let mut t = ta_with("abc\ndef\nghi"); diff --git a/codex-rs/tui/src/key_hint.rs b/codex-rs/tui/src/key_hint.rs index 001d401046..723ddc4e26 100644 --- a/codex-rs/tui/src/key_hint.rs +++ b/codex-rs/tui/src/key_hint.rs @@ -23,19 +23,21 @@ use ratatui::style::Style; use ratatui::text::Span; #[cfg(any(test, target_os = "macos"))] -const MODIFIER_LABELS: [(KeyModifiers, &str); 3] = [ +const MODIFIER_LABELS: [(KeyModifiers, &str); 4] = [ (KeyModifiers::CONTROL, "⌃"), (KeyModifiers::SHIFT, "⇧"), (KeyModifiers::ALT, "⌥"), + (KeyModifiers::SUPER, "⌘"), ]; #[cfg(not(any(test, target_os = "macos")))] -const MODIFIER_LABELS: [(KeyModifiers, &str); 3] = [ +const MODIFIER_LABELS: [(KeyModifiers, &str); 4] = [ #[cfg(target_os = "linux")] (KeyModifiers::CONTROL, "^"), #[cfg(not(target_os = "linux"))] (KeyModifiers::CONTROL, "ctrl"), (KeyModifiers::SHIFT, "shift"), (KeyModifiers::ALT, "alt"), + (KeyModifiers::SUPER, "super"), ]; /// One concrete key event that can trigger a TUI action. @@ -215,9 +217,9 @@ impl KeyBindingListExt for [KeyBinding] { /// Searchable pickers use this to avoid stealing plain printable characters for /// navigation when the same character might be a valid query. For example, a /// list may bind `j` and `k` for movement, but a searchable list must let -/// plain `j` update the query while still allowing `Ctrl+J` to move. Calling -/// this after normalizing keybindings would blur that distinction and cause -/// printable search input to disappear. +/// plain `j` update the query while still allowing `Ctrl+J` or `Cmd+J` to move. +/// Calling this after normalizing keybindings would blur that distinction and +/// cause printable search input to disappear. pub(crate) fn is_plain_text_key_event(event: KeyEvent) -> bool { matches!( event, @@ -228,6 +230,7 @@ pub(crate) fn is_plain_text_key_event(event: KeyEvent) -> bool { } if !ch.is_ascii_control() && !modifiers.contains(KeyModifiers::CONTROL) && !modifiers.contains(KeyModifiers::ALT) + && !modifiers.contains(KeyModifiers::SUPER) ) } @@ -344,6 +347,14 @@ mod tests { assert!(!bindings.is_pressed(KeyEvent::new(KeyCode::Char('c'), KeyModifiers::NONE))); } + #[test] + fn super_char_event_is_not_plain_text() { + assert!(!is_plain_text_key_event(KeyEvent::new( + KeyCode::Char('j'), + KeyModifiers::SUPER, + ))); + } + #[test] fn shifted_letter_binding_matches_uppercase_char_events() { let binding = shift(KeyCode::Char('a')); diff --git a/codex-rs/tui/src/key_hint_label_tests.rs b/codex-rs/tui/src/key_hint_label_tests.rs index d42aa07e15..4d93806b1f 100644 --- a/codex-rs/tui/src/key_hint_label_tests.rs +++ b/codex-rs/tui/src/key_hint_label_tests.rs @@ -14,6 +14,7 @@ fn modifier_combinations_stay_compact_inside_chords() { (KeyModifiers::ALT, "⌥t"), (KeyModifiers::CONTROL | KeyModifiers::SHIFT, "⌃⇧t"), (KeyModifiers::CONTROL | KeyModifiers::ALT, "⌃⌥t"), + (KeyModifiers::SUPER, "⌘t"), ]; for (modifiers, expected) in cases { let prefix = KeyBinding::new(KeyCode::Char('t'), modifiers); diff --git a/codex-rs/tui/src/keymap.rs b/codex-rs/tui/src/keymap.rs index 841b3fef70..a2294dd90f 100644 --- a/codex-rs/tui/src/keymap.rs +++ b/codex-rs/tui/src/keymap.rs @@ -1719,8 +1719,16 @@ impl RuntimeKeymap { raw(KeyBinding::new(KeyCode::Right, KeyModifiers::ALT)), raw(KeyBinding::new(KeyCode::Right, KeyModifiers::CONTROL)) ], - move_line_start: default_bindings![plain(KeyCode::Home), ctrl(KeyCode::Char('a'))], - move_line_end: default_bindings![plain(KeyCode::End), ctrl(KeyCode::Char('e'))], + move_line_start: default_bindings![ + plain(KeyCode::Home), + ctrl(KeyCode::Char('a')), + raw(KeyBinding::new(KeyCode::Left, KeyModifiers::SUPER)) + ], + move_line_end: default_bindings![ + plain(KeyCode::End), + ctrl(KeyCode::Char('e')), + raw(KeyBinding::new(KeyCode::Right, KeyModifiers::SUPER)) + ], delete_backward: default_bindings![ plain(KeyCode::Backspace), shift(KeyCode::Backspace), @@ -1753,7 +1761,10 @@ impl RuntimeKeymap { )), alt(KeyCode::Char('d')) ], - kill_line_start: default_bindings![ctrl(KeyCode::Char('u'))], + kill_line_start: default_bindings![ + ctrl(KeyCode::Char('u')), + raw(KeyBinding::new(KeyCode::Backspace, KeyModifiers::SUPER)) + ], kill_whole_line: default_bindings![], kill_line_end: default_bindings![ctrl(KeyCode::Char('k'))], yank: default_bindings![ctrl(KeyCode::Char('y'))], @@ -2681,6 +2692,7 @@ fn parse_keybinding(spec: &str) -> Option { "ctrl" => modifiers |= KeyModifiers::CONTROL, "alt" => modifiers |= KeyModifiers::ALT, "shift" => modifiers |= KeyModifiers::SHIFT, + "super" => modifiers |= KeyModifiers::SUPER, other => { key_name = Some(other.to_string()); break; @@ -2742,11 +2754,11 @@ mod tests { #[test] fn parses_canonical_binding() { - let binding = parse_keybinding("ctrl-alt-shift-a").expect("binding should parse"); + let binding = parse_keybinding("ctrl-alt-shift-super-a").expect("binding should parse"); assert_eq!(binding.parts().0, KeyCode::Char('a')); assert_eq!( binding.parts().1, - KeyModifiers::CONTROL | KeyModifiers::ALT | KeyModifiers::SHIFT + KeyModifiers::CONTROL | KeyModifiers::ALT | KeyModifiers::SHIFT | KeyModifiers::SUPER ); } diff --git a/codex-rs/tui/src/keymap/chords.rs b/codex-rs/tui/src/keymap/chords.rs index fc6855fba5..4c32e26127 100644 --- a/codex-rs/tui/src/keymap/chords.rs +++ b/codex-rs/tui/src/keymap/chords.rs @@ -769,13 +769,13 @@ Windows. Choose a different chord and retry.", } if matches!(prefix_key, KeyCode::Char(_)) - && !crate::key_hint::has_ctrl_or_alt(prefix_modifiers) + && crate::key_hint::is_plain_text_key_event(KeyEvent::new(prefix_key, prefix_modifiers)) && !binding.action.context.allows_plain_chord_prefix() && binding.action.context != KeymapContext::Agents { return Err(format!( "Invalid `{path}` = `{}`: a chord prefix outside Vim or the command center must use ctrl, \ -alt, or a non-character key so ordinary text input is not intercepted.", +alt, super, or a non-character key so ordinary text input is not intercepted.", binding.spec )); } diff --git a/codex-rs/tui/src/keymap/chords_tests.rs b/codex-rs/tui/src/keymap/chords_tests.rs index a83149a8f8..3ba4131e7e 100644 --- a/codex-rs/tui/src/keymap/chords_tests.rs +++ b/codex-rs/tui/src/keymap/chords_tests.rs @@ -189,6 +189,17 @@ fn allows_shared_prefixes_with_distinct_completions() { RuntimeKeymap::from_config(&config).expect("prefix trie can branch on the second stroke"); } +#[test] +fn allows_super_modified_character_prefixes_and_leaders() { + let mut config = TuiKeymap::default(); + config.global.leader = Some(binding("super-k")); + config.global.open_transcript = Some(binding("leader super-t")); + config.global.copy = Some(binding("super-o super-c")); + + RuntimeKeymap::from_config(&config) + .expect("Super-modified characters do not intercept ordinary text input"); +} + #[test] fn allows_plain_prefixes_in_vim_and_repeated_chords_in_all_contexts() { let mut config = TuiKeymap::default(); diff --git a/codex-rs/tui/src/keymap/leader.rs b/codex-rs/tui/src/keymap/leader.rs index 6bbe0de5cf..bd3807fc50 100644 --- a/codex-rs/tui/src/keymap/leader.rs +++ b/codex-rs/tui/src/keymap/leader.rs @@ -9,6 +9,7 @@ use super::resolve_bindings; use crate::key_hint; use codex_config::types::TuiKeymap; use crossterm::event::KeyCode; +use crossterm::event::KeyEvent; use itertools::Itertools; pub(super) fn resolve(config: &TuiKeymap, chords: &mut RuntimeChordKeymap) -> Result<(), String> { @@ -42,11 +43,13 @@ pub(super) fn resolve(config: &TuiKeymap, chords: &mut RuntimeChordKeymap) -> Re let (key, modifiers) = prefix.parts(); let label = prefix.display_label(); if matches!(key, KeyCode::Char(_)) - && (!key_hint::has_ctrl_or_alt(modifiers) || key_hint::is_altgr(modifiers)) + && (key_hint::is_plain_text_key_event(KeyEvent::new(key, modifiers)) + || key_hint::is_altgr(modifiers)) { return Err(format!( - "Invalid `tui.keymap.global.leader` = `{label}`: use ctrl, alt, or a non-character \ -key so ordinary text input is not intercepted. Ctrl-alt characters may be AltGr input on Windows." + "Invalid `tui.keymap.global.leader` = `{label}`: use ctrl, alt, super, or a \ +non-character key so ordinary text input is not intercepted. Ctrl-alt characters may be AltGr \ +input on Windows." )); } #[cfg(unix)] diff --git a/codex-rs/tui/src/keymap_setup.rs b/codex-rs/tui/src/keymap_setup.rs index e9bba1e9dd..7d8faa0b6e 100644 --- a/codex-rs/tui/src/keymap_setup.rs +++ b/codex-rs/tui/src/keymap_setup.rs @@ -668,10 +668,11 @@ fn key_parts_to_config_key_spec( let (code, normalized_modifiers) = crate::key_hint::normalize_key_parts(code, modifiers); modifiers = normalized_modifiers; - let supported_modifiers = KeyModifiers::CONTROL | KeyModifiers::ALT | KeyModifiers::SHIFT; + let supported_modifiers = + KeyModifiers::CONTROL | KeyModifiers::ALT | KeyModifiers::SHIFT | KeyModifiers::SUPER; if !modifiers.difference(supported_modifiers).is_empty() { return Err( - "Only ctrl, alt, and shift modifiers can be stored in `tui.keymap`.".to_string(), + "Only ctrl, alt, shift, and super modifiers can be stored in `tui.keymap`.".to_string(), ); } @@ -732,6 +733,9 @@ fn format_key_spec(modifiers: KeyModifiers, key: &str) -> String { if modifiers.contains(KeyModifiers::SHIFT) { parts.push("shift"); } + if modifiers.contains(KeyModifiers::SUPER) { + parts.push("super"); + } parts.push(key); parts.join("-") } @@ -1282,6 +1286,15 @@ mod tests { assert_snapshot!("keymap_picker_narrow", render_picker(params, /*width*/ 78)); } + #[test] + fn picker_editor_render_snapshot() { + let runtime = RuntimeKeymap::defaults(); + let mut params = build_keymap_picker_params(&runtime, &TuiKeymap::default()); + params.initial_tab_id = Some("editor-shortcuts".to_string()); + + assert_snapshot!("keymap_picker_editor", render_picker(params, /*width*/ 120)); + } + #[test] fn picker_custom_render_snapshot() { let keymap = @@ -1758,12 +1771,12 @@ mod tests { fn key_capture_serializes_modifier_order_for_config() { let event = KeyEvent::new( KeyCode::Char('K'), - KeyModifiers::CONTROL | KeyModifiers::ALT, + KeyModifiers::CONTROL | KeyModifiers::ALT | KeyModifiers::SUPER, ); assert_eq!( key_event_to_config_key_spec(event), - Ok("ctrl-alt-shift-k".to_string()) + Ok("ctrl-alt-shift-super-k".to_string()) ); } @@ -1957,6 +1970,25 @@ mod tests { ); } + #[test] + fn add_alternate_preserves_super_default_binding() { + let runtime = RuntimeKeymap::defaults(); + let outcome = keymap_with_edit( + &TuiKeymap::default(), + &runtime, + "editor", + "move_line_start", + "f12", + &KeymapEditIntent::AddAlternate, + ) + .expect("add alternate"); + + let KeymapEditOutcome::Updated { bindings, .. } = outcome else { + panic!("expected updated keymap"); + }; + assert_eq!(bindings, vec!["home", "ctrl-a", "super-left", "f12"]); + } + #[test] fn add_alternate_duplicate_is_noop() { let runtime = RuntimeKeymap::defaults(); diff --git a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_editor.snap b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_editor.snap new file mode 100644 index 0000000000..dac53fd0d8 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_editor.snap @@ -0,0 +1,39 @@ +--- +source: tui/src/keymap_setup.rs +expression: "render_picker(params, 120)" +--- + + Keymap + Inline editor movement and editing shortcuts. + 17 actions. + + All Common Customized (0) Unbound (5) App Composer Editor Vim Navigation Agents Approval › + + Type to search shortcuts + +› Editor Insert Newline ctrl-j, ctrl-m, enter, shift-enter, alt-enter + Editor Move Left left, ctrl-b + Editor Move Right right, ctrl-f + Editor Move Up up, ctrl-p + Editor Move Down down, ctrl-n + Editor Move Word Left alt-b, alt-left, ctrl-left + Editor Move Word Right alt-f, alt-right, ctrl-right + Editor Move Line Start home, ctrl-a, super-left + Editor Move Line End end, ctrl-e, super-right + Editor Delete Backward backspace, shift-backspace, ctrl-h + Editor Delete Forward delete, shift-delete, ctrl-d + Editor Delete Backward Word alt-backspace, ctrl-backspace, ctrl-shift-backspace, ctrl-w, ctrl-alt-h + Editor Delete Forward Word alt-delete, ctrl-delete, ctrl-shift-delete, alt-d + Editor Kill Line Start ctrl-u, super-backspace + Editor - Kill Whole Line unbound + Editor Kill Line End ctrl-k + Editor Yank ctrl-y + + + + + + + + + left/right group · enter edit shortcut · * custom · - unbound · esc close diff --git a/codex-rs/tui/src/startup_draft_input.rs b/codex-rs/tui/src/startup_draft_input.rs index b37e70957e..2031500404 100644 --- a/codex-rs/tui/src/startup_draft_input.rs +++ b/codex-rs/tui/src/startup_draft_input.rs @@ -35,7 +35,12 @@ impl StartupDraftPump { modifiers, kind: KeyEventKind::Press | KeyEventKind::Repeat, .. - }) => !key_hint::has_ctrl_or_alt(*modifiers), + }) => { + !key_hint::has_ctrl_or_alt(*modifiers) + && !modifiers.intersects( + KeyModifiers::SUPER | KeyModifiers::HYPER | KeyModifiers::META, + ) + } TuiEvent::Key(KeyEvent { code: KeyCode::Enter, modifiers, @@ -228,13 +233,14 @@ pub(super) fn handle_startup_draft_key( } } - if key_hint::has_ctrl_or_alt(key.modifiers) && !bottom_pane.is_safe_startup_editor_key(key) - || key.code == KeyCode::Enter && !bottom_pane.is_safe_startup_editor_key(key) - || key - .modifiers - .intersects(KeyModifiers::SUPER | KeyModifiers::HYPER | KeyModifiers::META) - || !bottom_pane.is_safe_startup_editor_key(key) - && !matches!( + let is_safe_startup_editor_key = bottom_pane.is_safe_startup_editor_key(key); + if !is_safe_startup_editor_key + && (key_hint::has_ctrl_or_alt(key.modifiers) + || key.code == KeyCode::Enter + || key + .modifiers + .intersects(KeyModifiers::SUPER | KeyModifiers::HYPER | KeyModifiers::META) + || !matches!( key.code, KeyCode::Char(_) | KeyCode::Enter @@ -247,7 +253,7 @@ pub(super) fn handle_startup_draft_key( | KeyCode::End | KeyCode::Backspace | KeyCode::Delete - ) + )) { return Ok(()); } diff --git a/codex-rs/tui/src/startup_draft_tests.rs b/codex-rs/tui/src/startup_draft_tests.rs index 2cb7703f40..03a76ab430 100644 --- a/codex-rs/tui/src/startup_draft_tests.rs +++ b/codex-rs/tui/src/startup_draft_tests.rs @@ -619,19 +619,22 @@ fn startup_draft_allows_local_editor_shortcuts_without_startup_actions() { } #[cfg(target_os = "windows")] -#[test] -fn startup_draft_preserves_windows_altgr_text_input() { +#[tokio::test] +async fn startup_draft_preserves_windows_altgr_text_input() { let mut pump = startup_test_pump(std::iter::empty()); - handle_startup_draft_key( - &mut pump.bottom_pane, - KeyEvent::new( + pump.bottom_pane.insert_str("first"); + pump.pending_paste_newline = Some((tokio::time::Instant::now(), "\n".to_string())); + let mut tui = crate::tui::test_support::make_test_tui().expect("create test terminal"); + pump.handle_event( + &mut tui, + TuiEvent::Key(KeyEvent::new( KeyCode::Char('@'), KeyModifiers::CONTROL | KeyModifiers::ALT, - ), + )), ) .expect("preserve printable AltGr input"); pump.bottom_pane.flush_composer_paste_burst(); - assert_eq!(pump.bottom_pane.composer_text(), "@"); + assert_eq!(pump.bottom_pane.composer_text(), "first\n@"); } #[tokio::test] @@ -684,7 +687,7 @@ async fn startup_draft_applies_paste_burst_preferences_without_losing_buffered_i } #[tokio::test] -async fn startup_draft_applies_editor_keymap_without_enabling_vim() { +async fn startup_draft_applies_super_editor_keymap_without_inserting_pending_newline() { let codex_home = tempfile::tempdir().expect("create temporary Codex home"); let config = ConfigBuilder::default() .codex_home(codex_home.path().to_path_buf()) @@ -694,7 +697,7 @@ async fn startup_draft_applies_editor_keymap_without_enabling_vim() { let mut settings = crate::local_settings::LocalSettings::from(&config); settings.tui.vim_mode_default = true; settings.tui.keymap.editor.move_line_start = Some(codex_config::types::KeybindingsSpec::One( - codex_config::types::KeybindingSpec("ctrl-z".to_string()), + codex_config::types::KeybindingSpec("super-z".to_string()), )); let mut pump = startup_test_pump(std::iter::empty()); pump.bottom_pane.insert_str("draft"); @@ -707,13 +710,16 @@ async fn startup_draft_applies_editor_keymap_without_enabling_vim() { .expect("insert ordinary text despite the configured Vim mode"); pump.bottom_pane.flush_composer_paste_burst(); assert_eq!(pump.bottom_pane.composer_text(), "draftx"); - handle_startup_draft_key( - &mut pump.bottom_pane, - KeyEvent::new(KeyCode::Char('z'), KeyModifiers::CONTROL), + pump.pending_paste_newline = Some((tokio::time::Instant::now(), "\n".to_string())); + let mut tui = crate::tui::test_support::make_test_tui().expect("create test terminal"); + pump.handle_event( + &mut tui, + TuiEvent::Key(KeyEvent::new(KeyCode::Char('z'), KeyModifiers::SUPER)), ) .expect("honor a configured safe editor shortcut"); + pump.bottom_pane.flush_composer_paste_burst(); assert_eq!(pump.bottom_pane.composer_cursor(), 0); - let mut tui = crate::tui::test_support::make_test_tui().expect("create test terminal"); + assert_eq!(pump.bottom_pane.composer_text(), "draftx"); pump.handle_event(&mut tui, TuiEvent::Key(KeyEvent::from(KeyCode::Enter))) .expect("confirm draft"); pump.apply_settings(&settings, config.cwd.as_path()); diff --git a/codex-rs/tui/src/tui_startup_tests.rs b/codex-rs/tui/src/tui_startup_tests.rs index b0e27aa648..859a097323 100644 --- a/codex-rs/tui/src/tui_startup_tests.rs +++ b/codex-rs/tui/src/tui_startup_tests.rs @@ -93,8 +93,10 @@ fn startup_preserves_typeahead_and_discards_buffered_action_key_after_first_draw let result = (|| -> Result<(), String> { wait_for_file(&draft_ready)?; + // Warp emits Cmd+Left, Cmd+Right, and Cmd+Backspace using these Kitty protocol + // sequences: CSI 1;9D, CSI 1;9C, and CSI 127;9u. master - .write_all(b"abc\r\x1b[DZ") + .write_all(b"abc\r\x1b[DZ\x1b[C\x1b[127;9umiddle\x1b[1;9DL\x1b[1;9CR") .map_err(|err| format!("failed to queue editable startup input: {err}"))?; std::fs::write(&draft_queued, []) .map_err(|err| format!("failed to signal editable startup input: {err}"))?; @@ -285,10 +287,10 @@ async fn startup_typeahead_pty_child() { tui.pause_events(); let draft = startup_draft.into_draft(); assert_eq!( - draft.text, "abZ!c", + draft.text, "LmiddleR!", "startup Enter must not submit the draft" ); - assert_eq!(draft.cursor, 3, "startup cursor edits must survive handoff"); + assert_eq!(draft.cursor, 8, "startup cursor edits must survive handoff"); assert!(draft.local_images.is_empty()); tui.draw(u16::MAX, |_| {}).expect("draw actionable screen");