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");