From b192442fc7f56aebbe065030561263cd67ffa44b Mon Sep 17 00:00:00 2001 From: Benjamin Carlsson Date: Tue, 1 Sep 2026 21:13:22 +0000 Subject: [PATCH] Add redo support to Vim composer history (#42140) ## What changed - Add a bounded redo stack for complete composer drafts, including paste payloads and image attachments. `Ctrl+R` now reapplies the last undone edit in Vim normal mode, while new edits clear stale redo history. - Add the configurable `vim_normal.redo` action to the keymap schema and picker. Preserve reverse prompt-history search in Vim insert mode and non-Vim mode, and honor explicitly configured bindings. - Keep pending edit snapshots outside the shared undo/redo byte budget so canceled commands cannot evict committed history. ## Testing - Cover grouped edits, Vim searches, direct draft changes, history limits, empty redo stacks, and custom or unbound shortcuts. GitOrigin-RevId: 0cf5bb376ff865ddc0a77705dcb322ef6b57b3a3 --- codex-rs/config/src/tui_keymap.rs | 2 + codex-rs/core/config.schema.json | 10 ++ codex-rs/tui/src/bottom_pane/chat_composer.rs | 7 +- .../chat_composer/history_search.rs | 3 + .../bottom_pane/chat_composer/vim_history.rs | 34 +++- .../chat_composer/vim_history_tests.rs | 168 +++++++++++++++++- ...r__tests__vim_empty_redo_stays_normal.snap | 14 ++ codex-rs/tui/src/keymap.rs | 101 +++++++---- codex-rs/tui/src/keymap/bindings.rs | 1 + codex-rs/tui/src/keymap_setup.rs | 16 +- codex-rs/tui/src/keymap_setup/actions.rs | 2 + ...ap_setup__tests__keymap_picker_custom.snap | 2 +- ...ests__keymap_picker_fast_mode_enabled.snap | 2 +- ...p__tests__keymap_picker_first_actions.snap | 4 +- ...up__tests__keymap_picker_global_chord.snap | 2 +- ...ap_setup__tests__keymap_picker_narrow.snap | 2 +- ...ymap_setup__tests__keymap_picker_redo.snap | 5 + ...ymap_setup__tests__keymap_picker_wide.snap | 2 +- 18 files changed, 318 insertions(+), 59 deletions(-) create mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__vim_empty_redo_stays_normal.snap create mode 100644 codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_redo.snap diff --git a/codex-rs/config/src/tui_keymap.rs b/codex-rs/config/src/tui_keymap.rs index 1c1c998ce7..54348c4971 100644 --- a/codex-rs/config/src/tui_keymap.rs +++ b/codex-rs/config/src/tui_keymap.rs @@ -271,6 +271,8 @@ pub struct TuiVimNormalKeymap { pub start_change_operator: Option, /// Undo the last complete edit (`u`). pub undo: Option, + /// Redo the last undone edit (`ctrl-r`). + pub redo: Option, /// Cancel a pending operator and return to normal mode. pub cancel_operator: Option, } diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index ec4e457335..e1bc30dc85 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -3944,6 +3944,7 @@ "open_line_above": null, "open_line_below": null, "paste_after": null, + "redo": null, "repeat_last_change": null, "replace_char": null, "start_change_operator": null, @@ -4771,6 +4772,7 @@ "open_line_above": null, "open_line_below": null, "paste_after": null, + "redo": null, "repeat_last_change": null, "replace_char": null, "start_change_operator": null, @@ -5234,6 +5236,14 @@ ], "description": "Paste after cursor (`p`)." }, + "redo": { + "allOf": [ + { + "$ref": "#/definitions/KeybindingsSpec" + } + ], + "description": "Redo the last undone edit (`ctrl-r`)." + }, "repeat_last_change": { "allOf": [ { diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index e1456aa679..953f2e9263 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -63,8 +63,11 @@ //! `Ctrl+R` opens a reverse incremental search mode. The footer becomes the search input; once the //! query is non-empty, the composer body previews the current match. `Enter` accepts the preview as //! an editable draft and `Esc` restores the draft that was active when search started. -//! Vim undo snapshots the complete draft and groups direct edits with active Vim transactions. -//! Canceled history previews restore both; accepting another prompt resets undo history. +//! Vim undo/redo snapshots complete drafts and groups direct edits with active Vim transactions. +//! An active edit keeps one separately capped snapshot; canceling does not evict committed history. +//! Canceled history previews restore history and active commands; accepting another prompt resets them. +//! Normal-mode Ctrl+R redoes an edit, or does nothing when redo is empty. Insert-mode Ctrl+R +//! keeps prompt-history search; explicitly configured keybindings retain precedence. //! Vim queries stay draft-local. //! //! Slash commands are staged for local history instead of being recorded immediately. Command diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/history_search.rs b/codex-rs/tui/src/bottom_pane/chat_composer/history_search.rs index 9072f9d4b6..3f767d3ca9 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer/history_search.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer/history_search.rs @@ -800,6 +800,9 @@ mod tests { .history .record_local_submission(HistoryEntry::new("git status".to_string())); composer.set_vim_enabled(/*enabled*/ true); + let mut keymap = crate::keymap::RuntimeKeymap::defaults(); + keymap.vim_normal.redo.clear(); + composer.set_keymap_bindings(&keymap); let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Char('r'), KeyModifiers::CONTROL)); for ch in ['g', 'i', 't'] { diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/vim_history.rs b/codex-rs/tui/src/bottom_pane/chat_composer/vim_history.rs index b750469b1f..9cd4e27110 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer/vim_history.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer/vim_history.rs @@ -1,8 +1,10 @@ -//! Bounded Vim undo history for the complete composer-owned draft. +//! Bounded Vim undo/redo history for the complete composer-owned draft. //! //! A textarea does not own pending paste payloads, image attachments, or mention targets. Keeping //! edit transactions here lets one snapshot restore those values together with visible text. Keys //! and direct composer-owned changes share the same bounded transaction state. +//! Committed snapshots share a byte budget. One pending edit snapshot has its own matching cap, +//! so canceled commands cannot evict committed history. use std::collections::VecDeque; @@ -18,6 +20,7 @@ const MAX_VIM_UNDO_BYTES: usize = 1024 * 1024; #[derive(Debug, Default)] pub(super) struct VimHistory { undo: VecDeque, + redo: VecDeque, pending: Option, } @@ -48,28 +51,46 @@ impl VimHistory { || self .undo .iter() - .chain(self.pending.iter()) + .chain(self.redo.iter()) .map(ComposerDraft::vim_history_bytes) .sum::() > MAX_VIM_UNDO_BYTES { - self.undo.pop_front(); + if self.undo.pop_front().is_none() { + self.redo.pop_front(); + } } } } impl ChatComposer { - /// Apply undo only before normal-mode commands begin. + /// Consume undo and redo before normal-mode commands, even when their history is empty. pub(super) fn handle_vim_history_key(&mut self, event: KeyEvent) -> bool { if !self.draft.textarea.is_vim_normal_mode() || self.draft.textarea.is_vim_operator_pending() || self.popups.active() - || !self.vim_normal_keymap.undo.is_pressed(event) { return false; } - if let Some(snapshot) = self.vim_history.undo.pop_back() { + let redo = self.vim_normal_keymap.redo.is_pressed(event); + let undo = self.vim_normal_keymap.undo.is_pressed(event); + if !undo && !redo { + return false; + } + + let snapshot = if redo { + self.vim_history.redo.pop_back() + } else { + self.vim_history.undo.pop_back() + }; + if let Some(snapshot) = snapshot { + let current = self.snapshot_draft(); + if redo { + self.vim_history.undo.push_back(current); + } else { + self.vim_history.redo.push_back(current); + } let history = std::mem::take(&mut self.vim_history); let mut vim_state = VimPersistentState::default(); self.draft @@ -188,6 +209,7 @@ impl ChatComposer { return; } + self.vim_history.redo.clear(); self.vim_history.undo.push_back(snapshot); self.vim_history.trim(); } diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/vim_history_tests.rs b/codex-rs/tui/src/bottom_pane/chat_composer/vim_history_tests.rs index d9696c4f90..1c2b265da0 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer/vim_history_tests.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer/vim_history_tests.rs @@ -1,4 +1,4 @@ -//! Composer undo transactions preserve rich draft data and active Vim commands. +//! Composer undo/redo transactions preserve rich draft data and active Vim commands. use std::path::PathBuf; @@ -46,7 +46,7 @@ fn ctrl_r(composer: &mut ChatComposer) { } #[test] -fn undo_groups_complete_vim_edits() { +fn undo_and_redo_group_complete_vim_edits() { for (original, command, edited) in [ ("one two", "cwX", "X two"), ("abc", "iXYZ", "XYZabc"), @@ -70,6 +70,8 @@ fn undo_groups_complete_vim_edits() { (composer.current_text(), composer.cursor()), (original.to_owned(), 0) ); + ctrl_r(&mut composer); + assert_eq!(composer.current_text(), edited); } } @@ -90,7 +92,7 @@ fn undo_preserves_repeat_and_pending_operator_bindings() { } #[test] -fn undo_preserves_confirmed_vim_search() { +fn undo_and_redo_preserve_confirmed_vim_search() { let mut composer = vim_composer("alpha beta alpha"); keys(&mut composer, "/alpha\n0xun"); assert_eq!( @@ -105,6 +107,13 @@ fn undo_preserves_confirmed_vim_search() { (composer.current_text(), composer.cursor()), ("Zalpha beta alpha".to_owned(), 12) ); + keys(&mut composer, "u"); + ctrl_r(&mut composer); + keys(&mut composer, "n"); + assert_eq!( + (composer.current_text(), composer.cursor()), + ("Zalpha beta alpha".to_owned(), 1) + ); } #[test] @@ -114,10 +123,12 @@ fn externally_canceled_search_operator_finishes_the_undo_transaction() { assert!(composer.cancel_vim_search()); keys(&mut composer, "uxu"); assert_eq!(composer.current_text(), "abc"); + ctrl_r(&mut composer); + assert_eq!(composer.current_text(), "bc"); } #[test] -fn normal_mode_explicit_paste_is_its_own_undoable_edit() { +fn normal_mode_explicit_paste_is_its_own_edit_and_replaces_stale_redo() { let mut composer = vim_composer("abc"); keys(&mut composer, "x"); composer.handle_paste("Z".to_owned()); @@ -126,23 +137,43 @@ fn normal_mode_explicit_paste_is_its_own_undoable_edit() { assert_eq!(composer.current_text(), "bc"); keys(&mut composer, "u"); assert_eq!(composer.current_text(), "abc"); + + composer.handle_paste("Z".to_owned()); + assert_eq!(composer.current_text(), "Zabc"); + assert!(composer.vim_history.redo.is_empty()); + ctrl_r(&mut composer); + assert_eq!(composer.current_text(), "Zabc"); + assert!(!composer.history_search_active()); + keys(&mut composer, "u"); + assert_eq!(composer.current_text(), "abc"); + ctrl_r(&mut composer); + assert_eq!(composer.current_text(), "Zabc"); } #[test] fn normal_mode_direct_insert_and_image_attachment_are_undoable() { let mut composer = vim_composer("abc"); + keys(&mut composer, "xu"); composer.insert_str("Z"); assert_eq!(composer.current_text(), "Zabc"); + assert!(composer.vim_history.redo.is_empty()); keys(&mut composer, "u"); assert_eq!(composer.current_text(), "abc"); + ctrl_r(&mut composer); + assert_eq!(composer.current_text(), "Zabc"); + keys(&mut composer, "u"); let path = PathBuf::from("example.png"); composer.attach_image(path.clone()); + let with_image = composer.draft_snapshot(); assert_eq!(composer.local_image_paths(), vec![path]); + assert!(composer.vim_history.redo.is_empty()); keys(&mut composer, "u"); assert_eq!(composer.current_text(), "abc"); assert!(composer.local_image_paths().is_empty()); - keys(&mut composer, "\nABC"); + ctrl_r(&mut composer); + assert_eq!(composer.draft_snapshot(), with_image); + keys(&mut composer, "u\nABC"); assert_eq!(composer.current_text(), "ABC"); escape(&mut composer); keys(&mut composer, "u"); @@ -157,13 +188,19 @@ fn deleting_a_selected_remote_image_is_one_undoable_vim_edit() { let mut composer = vim_composer("abc"); let url = "https://example.com/remote.png".to_owned(); composer.set_remote_image_urls(vec![url.clone()]); + keys(&mut composer, "xu"); + assert_eq!(composer.vim_history.redo.len(), 1); let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE)); assert!(composer.vim_history.pending.is_none()); let _ = composer.handle_key_event(KeyEvent::new(key, KeyModifiers::NONE)); assert!(composer.remote_image_urls().is_empty()); + assert!(composer.vim_history.redo.is_empty()); keys(&mut composer, "u"); assert_eq!(composer.remote_image_urls(), vec![url]); assert_eq!(composer.current_text(), "abc"); + ctrl_r(&mut composer); + assert!(composer.remote_image_urls().is_empty()); + assert_eq!(composer.current_text(), "abc"); } } @@ -175,10 +212,89 @@ fn direct_edits_join_the_active_vim_insert_transaction() { composer.attach_image(PathBuf::from("example.png")); keys(&mut composer, "D"); escape(&mut composer); + let edited = composer.draft_snapshot(); assert_eq!(composer.vim_history.undo.len(), 1); keys(&mut composer, "u"); assert_eq!(composer.current_text(), "abc"); assert!(composer.local_image_paths().is_empty()); + ctrl_r(&mut composer); + assert_eq!(composer.draft_snapshot(), edited); +} + +#[test] +fn vim_search_query_paste_does_not_invalidate_draft_redo() { + let mut composer = vim_composer("alpha beta alpha"); + keys(&mut composer, "xu/"); + composer.handle_paste("beta".to_owned()); + assert_eq!(composer.current_text(), "alpha beta alpha"); + assert_eq!( + composer.draft.textarea.vim_query().unwrap().editor.text(), + "beta" + ); + assert!(composer.vim_history.pending.is_none()); + + escape(&mut composer); + ctrl_r(&mut composer); + assert_eq!(composer.current_text(), "lpha beta alpha"); +} + +#[test] +fn vim_redo_keeps_its_binding_when_history_is_empty() { + let mut composer = vim_composer("abc"); + let before = composer.draft_snapshot(); + ctrl_r(&mut composer); + assert_eq!(composer.draft_snapshot(), before); + assert!(!composer.history_search_active()); + keys(&mut composer, "xu"); + assert_eq!(composer.current_text(), "abc"); + ctrl_r(&mut composer); + assert_eq!(composer.current_text(), "bc"); + assert!(!composer.history_search_active()); + let after = composer.draft_snapshot(); + ctrl_r(&mut composer); + assert_eq!(composer.draft_snapshot(), after); + assert!(composer.draft.textarea.is_vim_normal_mode()); + assert!(!composer.history_search_active()); + super::super::tests::snapshot_composer_state_with_width( + "vim_empty_redo_stays_normal", + /*width*/ 60, + /*enhanced_keys_supported*/ true, + |target| *target = composer, + ); + + let original = "a".repeat(super::MAX_VIM_UNDO_BYTES / 2 + 1); + let mut composer = vim_composer(&original); + keys(&mut composer, "xu"); + for command in ["d", "i"] { + keys(&mut composer, command); + escape(&mut composer); + assert_eq!(composer.vim_history.redo.len(), 1); + } + ctrl_r(&mut composer); + assert_eq!(composer.current_text(), original[1..]); + assert!(!composer.history_search_active()); + + keys(&mut composer, "uiZ"); + escape(&mut composer); + assert!(composer.vim_history.redo.is_empty()); + keys(&mut composer, "x"); + assert_eq!(composer.vim_history.undo.len(), 1); + keys(&mut composer, "u"); + assert_eq!(composer.current_text(), format!("Z{original}")); +} + +#[test] +fn ctrl_r_searches_history_in_insert_and_non_vim_modes() { + let mut composer = vim_composer("draft"); + keys(&mut composer, "i"); + ctrl_r(&mut composer); + assert!(composer.history_search_active()); + escape(&mut composer); + assert_eq!(composer.draft.textarea.vim_mode_label(), Some("Insert")); + + composer.set_vim_enabled(/*enabled*/ false); + ctrl_r(&mut composer); + assert!(composer.history_search_active()); } #[test] @@ -197,6 +313,9 @@ fn removing_the_shell_prefix_is_its_own_undoable_edit() { #[test] fn canceled_reverse_history_preview_preserves_vim_undo_and_repeat() { let mut composer = vim_composer("alpha beta alpha"); + let mut keymap = RuntimeKeymap::defaults(); + keymap.vim_normal.redo.clear(); + composer.set_keymap_bindings(&keymap); composer .history .record_local_submission(HistoryEntry::new("archived prompt".to_owned())); @@ -252,18 +371,23 @@ fn accepted_reverse_history_preview_starts_a_fresh_vim_edit_history() { } #[test] -fn undo_uses_configured_bindings_and_supports_unbinding() { +fn undo_and_redo_use_configured_bindings_and_support_unbinding() { let mut composer = vim_composer("abc"); let mut keymap = RuntimeKeymap::defaults(); keymap.vim_normal.undo = vec![crate::key_hint::plain(KeyCode::Char('z'))]; + keymap.vim_normal.redo = vec![crate::key_hint::plain(KeyCode::Char('v'))]; composer.set_keymap_bindings(&keymap); keys(&mut composer, "xu"); assert_eq!(composer.current_text(), "bc"); keys(&mut composer, "z"); assert_eq!(composer.current_text(), "abc"); + keys(&mut composer, "v"); + assert_eq!(composer.current_text(), "bc"); + keys(&mut composer, "z"); keymap.vim_normal.undo.clear(); + keymap.vim_normal.redo.clear(); composer.set_keymap_bindings(&keymap); - keys(&mut composer, "xz"); + keys(&mut composer, "xzv"); assert_eq!(composer.current_text(), "bc"); let mut config = TuiKeymap::default(); @@ -278,6 +402,23 @@ fn undo_uses_configured_bindings_and_supports_unbinding() { keys(&mut composer, "u"); assert!(!composer.history_search_active()); + let mut composer = vim_composer("abc"); + keys(&mut composer, "xu"); + let mut config = TuiKeymap::default(); + config.composer.history_search_previous = + Some(KeybindingsSpec::One(KeybindingSpec("ctrl-r".to_owned()))); + composer.set_keymap_bindings(&RuntimeKeymap::from_config(&config).unwrap()); + ctrl_r(&mut composer); + assert!(composer.history_search_active()); + escape(&mut composer); + config.vim_normal.redo = Some(KeybindingsSpec::One(KeybindingSpec("ctrl-r".to_owned()))); + composer.set_keymap_bindings(&RuntimeKeymap::from_config(&config).unwrap()); + ctrl_r(&mut composer); + assert!(!composer.history_search_active()); + assert_eq!(composer.current_text(), "bc"); + ctrl_r(&mut composer); + assert!(!composer.history_search_active()); + let mut composer = vim_composer("abc"); let mut config = TuiKeymap::default(); config.composer.submit = Some(KeybindingsSpec::One(KeybindingSpec("u".to_owned()))); @@ -294,7 +435,7 @@ fn undo_uses_configured_bindings_and_supports_unbinding() { } #[test] -fn undo_restores_deleted_large_paste_payload() { +fn undo_and_redo_restore_deleted_large_paste_payload() { let mut composer = vim_composer("alpha "); composer.draft.textarea.set_cursor("alpha ".len()); keys(&mut composer, "i"); @@ -311,10 +452,13 @@ fn undo_restores_deleted_large_paste_payload() { composer.current_text_with_pending(), format!("alpha {pasted}") ); + ctrl_r(&mut composer); + assert_eq!(composer.current_text(), "alpha "); + assert!(composer.draft.pending_pastes.is_empty()); } #[test] -fn undo_restores_deleted_image_attachment() { +fn undo_and_redo_restore_deleted_image_attachment() { let mut composer = vim_composer("alpha "); composer.draft.textarea.set_cursor("alpha ".len()); composer.attach_image(PathBuf::from("example.png")); @@ -325,6 +469,9 @@ fn undo_restores_deleted_image_attachment() { assert!(composer.local_image_paths().is_empty()); keys(&mut composer, "u"); assert_eq!(composer.draft_snapshot(), before); + ctrl_r(&mut composer); + assert_eq!(composer.current_text(), "alpha "); + assert!(composer.local_image_paths().is_empty()); } #[test] @@ -359,6 +506,9 @@ fn find_jump_and_search_navigation_never_snapshot_the_draft() { } assert_eq!(composer.current_text(), text); } + let mut keymap = RuntimeKeymap::defaults(); + keymap.vim_normal.redo.clear(); + composer.set_keymap_bindings(&keymap); composer .history .record_local_submission(HistoryEntry::new("alpha archived".to_owned())); diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__vim_empty_redo_stays_normal.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__vim_empty_redo_stays_normal.snap new file mode 100644 index 0000000000..cbbd27f11f --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__vim_empty_redo_stays_normal.snap @@ -0,0 +1,14 @@ +--- +source: tui/src/bottom_pane/chat_composer.rs +assertion_line: 5360 +expression: terminal.backend() +--- +" " +"› bc " +" " +" " +" " +" " +" " +" " +" 100% context left | Vim: Normal " diff --git a/codex-rs/tui/src/keymap.rs b/codex-rs/tui/src/keymap.rs index 9adc438438..2a008a8210 100644 --- a/codex-rs/tui/src/keymap.rs +++ b/codex-rs/tui/src/keymap.rs @@ -211,6 +211,7 @@ pub(crate) struct VimNormalKeymap { pub(crate) start_yank_operator: Vec, pub(crate) start_change_operator: Vec, pub(crate) undo: Vec, + pub(crate) redo: Vec, pub(crate) cancel_operator: Vec, } @@ -759,6 +760,7 @@ impl RuntimeKeymap { start_change_operator ), undo: resolve_local!(keymap, defaults, vim_normal, undo), + redo: resolve_local!(keymap, defaults, vim_normal, redo), cancel_operator: resolve_local!(keymap, defaults, vim_normal, cancel_operator), }; @@ -892,6 +894,7 @@ impl RuntimeKeymap { vim_normal.start_change_operator.as_slice(), ), (keymap.vim_normal.undo.as_ref(), vim_normal.undo.as_slice()), + (keymap.vim_normal.redo.as_ref(), vim_normal.redo.as_slice()), ( keymap.vim_normal.cancel_operator.as_ref(), vim_normal.cancel_operator.as_slice(), @@ -1365,22 +1368,33 @@ impl RuntimeKeymap { approval, }; - if keymap.vim_normal.undo.is_none() { - let configured: Vec<_> = runtime_action_bindings(&resolved) - .filter(|action| { - action.id.context.overlaps(KeymapContext::VimNormal) - && bindings::configured_binding_for_action(keymap, action.id) - .is_some_and(Option::is_some) - }) - .flat_map(|action| action.bindings.iter().copied()) - .collect(); - resolved.vim_normal.undo.retain(|binding| { - !configured.contains(binding) - && !resolved.chords.bindings.iter().any(|chord| { - chord.action.context.overlaps(KeymapContext::VimNormal) - && chord.chord.prefix.parts() == binding.parts() - }) - }); + let configured: Vec<_> = runtime_action_bindings(&resolved) + .filter(|action| { + action.id.context.overlaps(KeymapContext::VimNormal) + && bindings::configured_binding_for_action(keymap, action.id) + .is_some_and(Option::is_some) + }) + .flat_map(|action| action.bindings.iter().copied()) + .collect(); + for (setting, bindings) in [ + ( + keymap.vim_normal.undo.as_ref(), + &mut resolved.vim_normal.undo, + ), + ( + keymap.vim_normal.redo.as_ref(), + &mut resolved.vim_normal.redo, + ), + ] { + if setting.is_none() { + bindings.retain(|binding| { + !configured.contains(binding) + && !resolved.chords.bindings.iter().any(|chord| { + chord.action.context.overlaps(KeymapContext::VimNormal) + && chord.chord.prefix.parts() == binding.parts() + }) + }); + } } resolved.configure_vim_search(keymap)?; resolved.validate_conflicts()?; @@ -1559,6 +1573,7 @@ impl RuntimeKeymap { start_yank_operator: default_bindings![plain(KeyCode::Char('y'))], start_change_operator: default_bindings![plain(KeyCode::Char('c'))], undo: default_bindings![plain(KeyCode::Char('u'))], + redo: default_bindings![ctrl(KeyCode::Char('r'))], cancel_operator: default_bindings![plain(KeyCode::Esc)], }, vim_search: VimSearchKeymap::default(), @@ -2982,24 +2997,35 @@ mod tests { #[test] fn configured_legacy_bindings_prune_new_history_defaults() { - for (context, action, binding) in [ - ("vim_normal", "move_left", "u"), - ("vim_normal", "move_left", "u g"), - ("vim_search", "forward", "u"), - ("vim_search", "forward", "u g"), - ("composer", "submit", "u"), - ("global", "submit", "u"), - ("global", "queue", "u"), - ("global", "toggle_shortcuts", "u"), - ] { - let keymap = serde_json::from_value(serde_json::json!({ - (context): { (action): binding } - })) - .expect("config should deserialize"); - - let runtime = RuntimeKeymap::from_config(&keymap).expect("config should parse"); - - assert_eq!(runtime.vim_normal.undo, Vec::new()); + for key in ["u", "ctrl-r"] { + for (context, action, suffix) in [ + ("vim_normal", "move_left", ""), + ("vim_normal", "move_left", " g"), + ("vim_search", "forward", ""), + ("vim_search", "forward", " g"), + ("composer", "submit", ""), + ("global", "submit", ""), + ("global", "queue", ""), + ("global", "toggle_shortcuts", ""), + ] { + let mut keymap: TuiKeymap = serde_json::from_value(serde_json::json!({ + (context): { (action): format!("{key}{suffix}") } + })) + .expect("config should deserialize"); + if key == "ctrl-r" + && (!suffix.is_empty() || matches!(context, "composer" | "global")) + { + // Ctrl+R chords and main-surface actions must unbind the older history key. + keymap.composer.history_search_previous = Some(KeybindingsSpec::Many(vec![])); + } + let runtime = RuntimeKeymap::from_config(&keymap).expect("config should parse"); + let bindings = if key == "u" { + runtime.vim_normal.undo + } else { + runtime.vim_normal.redo + }; + assert_eq!(bindings, Vec::new()); + } } } @@ -3009,16 +3035,23 @@ mod tests { keymap.vim_normal.move_left = Some(one("u")); keymap.vim_normal.undo = Some(one("u")); expect_conflict(&keymap, "move_left", "undo"); + + keymap.vim_normal.move_left = Some(one("ctrl-r")); + keymap.vim_normal.undo = None; + keymap.vim_normal.redo = Some(one("ctrl-r")); + expect_conflict(&keymap, "move_left", "redo"); } #[test] fn explicit_empty_arrays_unbind_vim_history_actions() { let mut keymap = TuiKeymap::default(); keymap.vim_normal.undo = Some(KeybindingsSpec::Many(vec![])); + keymap.vim_normal.redo = Some(KeybindingsSpec::Many(vec![])); let runtime = RuntimeKeymap::from_config(&keymap).expect("config should parse"); assert!(runtime.vim_normal.undo.is_empty()); + assert!(runtime.vim_normal.redo.is_empty()); } #[test] diff --git a/codex-rs/tui/src/keymap/bindings.rs b/codex-rs/tui/src/keymap/bindings.rs index ca83f16327..4903f63bd4 100644 --- a/codex-rs/tui/src/keymap/bindings.rs +++ b/codex-rs/tui/src/keymap/bindings.rs @@ -320,6 +320,7 @@ define_runtime_action_bindings! { start_yank_operator, start_change_operator, undo, + redo, cancel_operator, ], "vim_search" => VimSearch, vim_search, vim_search [forward, backward, next, previous], diff --git a/codex-rs/tui/src/keymap_setup.rs b/codex-rs/tui/src/keymap_setup.rs index 3c8b4ecd57..acdfcdc14c 100644 --- a/codex-rs/tui/src/keymap_setup.rs +++ b/codex-rs/tui/src/keymap_setup.rs @@ -1121,7 +1121,7 @@ mod tests { } #[test] - fn picker_repeat_last_change_render_snapshot() { + fn picker_repeat_and_redo_render_snapshots() { let runtime = RuntimeKeymap::defaults(); let params = build_keymap_picker_params_for_selected_action( &runtime, @@ -1136,6 +1136,20 @@ mod tests { .expect("repeat-last-change row should render"); assert_snapshot!("keymap_picker_repeat_last_change", repeat_row); + + let params = build_keymap_picker_params_for_selected_action( + &runtime, + &TuiKeymap::default(), + "vim_normal", + "redo", + ); + let rendered = render_picker(params, /*width*/ 120); + let redo_row = rendered + .lines() + .find(|line| line.contains("Redo")) + .expect("redo row should render"); + + assert_snapshot!("keymap_picker_redo", redo_row); } #[test] diff --git a/codex-rs/tui/src/keymap_setup/actions.rs b/codex-rs/tui/src/keymap_setup/actions.rs index 3ad272e0fd..4d84a41b7a 100644 --- a/codex-rs/tui/src/keymap_setup/actions.rs +++ b/codex-rs/tui/src/keymap_setup/actions.rs @@ -154,6 +154,7 @@ pub(super) const KEYMAP_ACTIONS: &[KeymapActionDescriptor] = &[ action("vim_normal", "Vim normal", "start_yank_operator", "Begin a yank operator and wait for a motion."), action("vim_normal", "Vim normal", "start_change_operator", "Begin a change operator and wait for a motion or text object."), action("vim_normal", "Vim normal", "undo", "Undo the last complete edit."), + action("vim_normal", "Vim normal", "redo", "Redo the last undone edit."), action("vim_normal", "Vim normal", "cancel_operator", "Cancel a pending Vim operator."), action("vim_search", "Vim search", "forward", "Search forward in the active buffer."), action("vim_search", "Vim search", "backward", "Search backward in the active buffer."), @@ -325,6 +326,7 @@ pub(super) fn binding_slot<'a>( ("vim_normal", "start_yank_operator") => Some(&mut keymap.vim_normal.start_yank_operator), ("vim_normal", "start_change_operator") => Some(&mut keymap.vim_normal.start_change_operator), ("vim_normal", "undo") => Some(&mut keymap.vim_normal.undo), + ("vim_normal", "redo") => Some(&mut keymap.vim_normal.redo), ("vim_normal", "cancel_operator") => Some(&mut keymap.vim_normal.cancel_operator), ("vim_search", "forward") => Some(&mut keymap.vim_search.forward), ("vim_search", "backward") => Some(&mut keymap.vim_search.backward), diff --git a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_custom.snap b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_custom.snap index b057c11c90..1b467c8ab7 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_custom.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_custom.snap @@ -5,7 +5,7 @@ expression: "render_picker(params, 120)" Keymap All configurable shortcuts. - 136 actions, 1 customized, 5 unbound. + 137 actions, 1 customized, 5 unbound. [All] Common Customized (1) Unbound (5) App Composer Editor Vim Navigation Agents Approval Debug diff --git a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_fast_mode_enabled.snap b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_fast_mode_enabled.snap index 2c4456772d..52d8426766 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_fast_mode_enabled.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_fast_mode_enabled.snap @@ -5,7 +5,7 @@ expression: "render_picker(params, 120)" Keymap All configurable shortcuts. - 137 actions, 0 customized, 6 unbound. + 138 actions, 0 customized, 6 unbound. [All] Common Customized (0) Unbound (6) App Composer Editor Vim Navigation Agents Approval Debug diff --git a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_first_actions.snap b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_first_actions.snap index cf2365caab..913edbba4f 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_first_actions.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_first_actions.snap @@ -2,14 +2,14 @@ source: tui/src/keymap_setup.rs expression: snapshot --- -tab: All (136 selectable) +tab: All (137 selectable) tab: Common (20 selectable) tab: Customized (0) (0 selectable) tab: Unbound (5) (5 selectable) tab: App (14 selectable) tab: Composer (5 selectable) tab: Editor (17 selectable) -tab: Vim (67 selectable) +tab: Vim (68 selectable) tab: Navigation (20 selectable) tab: Agents (5 selectable) tab: Approval (8 selectable) diff --git a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_global_chord.snap b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_global_chord.snap index 3eb29aff66..4c79a4fa95 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_global_chord.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_global_chord.snap @@ -5,7 +5,7 @@ expression: "render_picker(params, 120)" Keymap All configurable shortcuts. - 136 actions, 1 customized, 5 unbound. + 137 actions, 1 customized, 5 unbound. [All] Common Customized (1) Unbound (5) App Composer Editor Vim Navigation Agents Approval Debug diff --git a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_narrow.snap b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_narrow.snap index 651dc681e0..adc5c15800 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_narrow.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_narrow.snap @@ -5,7 +5,7 @@ expression: "render_picker(params, 78)" Keymap All configurable shortcuts. - 136 actions, 0 customized, 5 unbound. + 137 actions, 0 customized, 5 unbound. [All] Common Customized (0) Unbound (5) App Composer Editor Vim Navigation Agents Approval Debug diff --git a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_redo.snap b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_redo.snap new file mode 100644 index 0000000000..c257b7516e --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_redo.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/keymap_setup.rs +expression: redo_row +--- +› Vim normal Redo ctrl-r diff --git a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_wide.snap b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_wide.snap index 35e7d5f991..8d25bd8a52 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_wide.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_wide.snap @@ -5,7 +5,7 @@ expression: "render_picker(params, 120)" Keymap All configurable shortcuts. - 136 actions, 0 customized, 5 unbound. + 137 actions, 0 customized, 5 unbound. [All] Common Customized (0) Unbound (5) App Composer Editor Vim Navigation Agents Approval Debug