From aa184548b1ee0a559fba7be658240053e270e16f Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Wed, 27 May 2026 11:36:52 -0300 Subject: [PATCH] fix(tui): complete vim word-end and line-end behavior (#24380) ## Why The TUI Vim composer currently diverges from normal Vim editing in two common workflows: pressing `e` repeatedly can remain stuck at an existing word end, and normal mode does not support `C` for changing through the end of the line. The existing `D` behavior also removes the newline when the cursor is already at the line boundary, which makes the new `C` action and existing deletion action surprising in multiline prompts. Closes #23926. Closes #24238. ## What Changed - Make normal-mode `e` advance from the current word end to the next word end, including for operator motions such as `de`. - Add configurable Vim normal-mode `change_to_line_end` behavior, bound to `C` by default, which deletes to the end of the current line and enters Insert mode. - Keep the newline intact when `D` or `C` is pressed at the end-of-line boundary. - Add regression coverage for repeated `e`, `de`, `C`, and the multiline `C`/`D` boundary behavior. - Regenerate the config schema and update the keymap picker snapshots for the new Vim action. ## How to Test 1. Run Codex with Vim composer mode enabled: ```bash cd codex-rs cargo run --bin codex -- -c tui.vim_mode_default=true ``` 2. Enter `alpha beta gamma`, press `Esc`, `0`, then press `e` repeatedly. Confirm the cursor advances through the ends of `alpha`, `beta`, and `gamma`. 3. Enter `hello world`, press `Esc`, `0`, `w`, then `C`. Confirm `world` is deleted and the composer enters Insert mode. 4. Enter a multiline prompt with `hello` above `world`, press `Esc`, `k`, `$`, and then `D`. Confirm the newline is preserved and the two lines do not join. 5. At the same boundary, press `C` and type `!`. Confirm the composer enters Insert mode and yields `hello!` above `world`, preserving the newline. Targeted automated verification: - `just fix -p codex-tui` - `just argument-comment-lint-from-source -p codex-tui -p codex-config` - `cargo insta pending-snapshots` reports no pending snapshots. - `just test -p codex-tui` validates the new Vim and keymap snapshot coverage, but the command remains red due to two reproducible unrelated failures in `app::tests::update_feature_flags_disabling_guardian_*`. ## Validation Note The workspace-wide `just argument-comment-lint` form is currently blocked during Bazel analysis by the existing LLVM `compiler-rt` missing `include/sanitizer/*.h` failure; package-scoped source linting for the changed Rust crates passed. --- codex-rs/config/src/tui_keymap.rs | 2 + codex-rs/core/config.schema.json | 10 ++ ...ts__vim_e_advances_from_each_word_end.snap | 12 ++ codex-rs/tui/src/bottom_pane/textarea.rs | 154 +++++++++++++++++- codex-rs/tui/src/keymap.rs | 10 ++ codex-rs/tui/src/keymap_setup/actions.rs | 3 + ...ap_setup__tests__keymap_picker_custom.snap | 2 +- ...ests__keymap_picker_fast_mode_enabled.snap | 2 +- ...p__tests__keymap_picker_first_actions.snap | 4 +- ...ap_setup__tests__keymap_picker_narrow.snap | 2 +- ...ymap_setup__tests__keymap_picker_wide.snap | 2 +- 11 files changed, 190 insertions(+), 13 deletions(-) create mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__textarea__tests__vim_e_advances_from_each_word_end.snap diff --git a/codex-rs/config/src/tui_keymap.rs b/codex-rs/config/src/tui_keymap.rs index 5b1e7d4cc..5848d1f44 100644 --- a/codex-rs/config/src/tui_keymap.rs +++ b/codex-rs/config/src/tui_keymap.rs @@ -223,6 +223,8 @@ pub struct TuiVimNormalKeymap { pub delete_char: Option, /// Delete from cursor to end of line (`D`). pub delete_to_line_end: Option, + /// Change from cursor to end of line and enter insert mode (`C`). + pub change_to_line_end: Option, /// Yank the entire line (`Y`). pub yank_line: Option, /// Paste after cursor (`p`). diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 6af30b407..06b7c56d5 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -2834,6 +2834,7 @@ "append_after_cursor": null, "append_line_end": null, "cancel_operator": null, + "change_to_line_end": null, "delete_char": null, "delete_to_line_end": null, "enter_insert": null, @@ -3500,6 +3501,7 @@ "append_after_cursor": null, "append_line_end": null, "cancel_operator": null, + "change_to_line_end": null, "delete_char": null, "delete_to_line_end": null, "enter_insert": null, @@ -3765,6 +3767,14 @@ ], "description": "Cancel a pending operator and return to normal mode." }, + "change_to_line_end": { + "allOf": [ + { + "$ref": "#/definitions/KeybindingsSpec" + } + ], + "description": "Change from cursor to end of line and enter insert mode (`C`)." + }, "delete_char": { "allOf": [ { diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__textarea__tests__vim_e_advances_from_each_word_end.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__textarea__tests__vim_e_advances_from_each_word_end.snap new file mode 100644 index 000000000..0a41c2d80 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__textarea__tests__vim_e_advances_from_each_word_end.snap @@ -0,0 +1,12 @@ +--- +source: tui/src/bottom_pane/textarea.rs +expression: "states.join(\"\\n\\n\")" +--- +alpha beta gamma + ^ + +alpha beta gamma + ^ + +alpha beta gamma + ^ diff --git a/codex-rs/tui/src/bottom_pane/textarea.rs b/codex-rs/tui/src/bottom_pane/textarea.rs index 96f6cf8cb..d931b14b2 100644 --- a/codex-rs/tui/src/bottom_pane/textarea.rs +++ b/codex-rs/tui/src/bottom_pane/textarea.rs @@ -755,7 +755,12 @@ impl TextArea { return; } if self.vim_normal_keymap.delete_to_line_end.is_pressed(event) { - self.kill_to_end_of_line(); + self.vim_kill_to_end_of_line(); + return; + } + if self.vim_normal_keymap.change_to_line_end.is_pressed(event) { + self.vim_kill_to_end_of_line(); + self.vim_mode = VimMode::Insert; return; } if self.vim_normal_keymap.yank_line.is_pressed(event) { @@ -914,7 +919,7 @@ impl TextArea { VimMotion::Down => self.move_cursor_down(), VimMotion::WordForward => self.set_cursor(self.beginning_of_next_word()), VimMotion::WordBackward => self.set_cursor(self.beginning_of_previous_word()), - VimMotion::WordEnd => self.set_cursor(self.end_of_next_word()), + VimMotion::WordEnd => self.set_cursor(self.vim_word_end_exclusive()), VimMotion::LineStart => self.set_cursor(self.beginning_of_current_line()), VimMotion::LineEnd => self.set_cursor(self.end_of_current_line()), } @@ -1007,6 +1012,13 @@ impl TextArea { } } + fn vim_kill_to_end_of_line(&mut self) { + let eol = self.end_of_current_line(); + if self.cursor_pos < eol { + self.kill_range(self.cursor_pos..eol); + } + } + pub fn kill_to_beginning_of_line(&mut self) { let bol = self.beginning_of_current_line(); let range = if self.cursor_pos == bol { @@ -1734,7 +1746,11 @@ impl TextArea { } pub(crate) fn end_of_next_word(&self) -> usize { - let suffix = &self.text[self.cursor_pos..]; + self.end_of_next_word_from(self.cursor_pos) + } + + fn end_of_next_word_from(&self, cursor_pos: usize) -> usize { + let suffix = &self.text[cursor_pos..]; let Some(first_non_ws) = suffix.find(|ch: char| !ch.is_whitespace()) else { return self.text.len(); }; @@ -1742,16 +1758,16 @@ impl TextArea { let run = &run[..run.find(char::is_whitespace).unwrap_or(run.len())]; let mut pieces = split_word_pieces(run).into_iter().peekable(); let Some((start, piece)) = pieces.next() else { - return self.cursor_pos + first_non_ws; + return cursor_pos + first_non_ws; }; - let word_start = self.cursor_pos + first_non_ws + start; + let word_start = cursor_pos + first_non_ws + start; let mut end = word_start + piece.len(); if piece.chars().all(is_word_separator) { while let Some((idx, piece)) = pieces.peek() { if !piece.chars().all(is_word_separator) { break; } - end = self.cursor_pos + first_non_ws + *idx + piece.len(); + end = cursor_pos + first_non_ws + *idx + piece.len(); pieces.next(); } } @@ -1759,8 +1775,22 @@ impl TextArea { self.adjust_pos_out_of_elements(end, /*prefer_start*/ false) } - fn vim_word_end_cursor(&self) -> usize { + fn vim_word_end_exclusive(&self) -> usize { let end = self.end_of_next_word(); + let target = if end > self.cursor_pos { + self.prev_atomic_boundary(end) + } else { + end + }; + if target == self.cursor_pos && end < self.text.len() { + self.end_of_next_word_from(end) + } else { + end + } + } + + fn vim_word_end_cursor(&self) -> usize { + let end = self.vim_word_end_exclusive(); if end > self.cursor_pos { self.prev_atomic_boundary(end) } else { @@ -2275,6 +2305,60 @@ mod tests { assert_eq!(t.cursor(), 11); } + #[test] + fn vim_shift_c_changes_to_line_end_and_enters_insert_mode() { + let mut t = ta_with("hello world\nnext line"); + t.set_cursor(/*pos*/ 6); + t.set_vim_enabled(/*enabled*/ true); + + t.input(KeyEvent::new(KeyCode::Char('c'), KeyModifiers::SHIFT)); + + assert_eq!(t.text(), "hello \nnext line"); + assert_eq!(t.vim_mode_label(), Some("Insert")); + assert_eq!(t.cursor(), 6); + assert_eq!(t.kill_buffer, "world"); + } + + #[test] + fn vim_uppercase_c_changes_to_line_end() { + let mut t = ta_with("hello world\nnext line"); + t.set_cursor(/*pos*/ 6); + t.set_vim_enabled(/*enabled*/ true); + + t.input(KeyEvent::new(KeyCode::Char('C'), KeyModifiers::NONE)); + + assert_eq!(t.text(), "hello \nnext line"); + assert_eq!(t.vim_mode_label(), Some("Insert")); + assert_eq!(t.cursor(), 6); + } + + #[test] + fn vim_d_at_line_end_does_not_remove_newline() { + let mut t = ta_with("hello\nworld"); + t.set_cursor(/*pos*/ "hello".len()); + t.set_vim_enabled(/*enabled*/ true); + + t.input(KeyEvent::new(KeyCode::Char('D'), KeyModifiers::NONE)); + + assert_eq!(t.text(), "hello\nworld"); + assert_eq!(t.vim_mode_label(), Some("Normal")); + assert_eq!(t.kill_buffer, ""); + } + + #[test] + fn vim_c_at_line_end_enters_insert_without_removing_newline() { + let mut t = ta_with("hello\nworld"); + t.set_cursor(/*pos*/ "hello".len()); + t.set_vim_enabled(/*enabled*/ true); + + t.input(KeyEvent::new(KeyCode::Char('C'), KeyModifiers::NONE)); + + assert_eq!(t.text(), "hello\nworld"); + assert_eq!(t.vim_mode_label(), Some("Insert")); + assert_eq!(t.cursor(), "hello".len()); + assert_eq!(t.kill_buffer, ""); + } + #[test] fn vim_shift_o_opens_line_above_with_shift_only_binding() { let mut t = ta_with("hello\nworld"); @@ -2348,6 +2432,62 @@ mod tests { assert_eq!(t.kill_buffer, "c"); } + #[test] + fn vim_e_advances_from_each_word_end() { + let mut t = ta_with("alpha beta gamma"); + t.set_cursor("alph".len()); // codespell:ignore alph + t.set_vim_enabled(/*enabled*/ true); + let mut states = Vec::new(); + + for _ in 0..3 { + t.input(KeyEvent::new(KeyCode::Char('e'), KeyModifiers::NONE)); + states.push(format!("{}\n{}^", t.text(), " ".repeat(t.cursor()))); + } + + insta::assert_snapshot!("vim_e_advances_from_each_word_end", states.join("\n\n")); + } + + #[test] + fn vim_delete_to_word_end_advances_from_existing_word_end() { + let mut t = ta_with("alpha beta gamma"); + t.set_cursor("alph".len()); // codespell:ignore alph + t.set_vim_enabled(/*enabled*/ true); + + t.input(KeyEvent::new(KeyCode::Char('d'), KeyModifiers::NONE)); + t.input(KeyEvent::new(KeyCode::Char('e'), KeyModifiers::NONE)); + + assert_eq!(t.text(), "alph gamma"); // codespell:ignore alph + assert_eq!(t.kill_buffer, "a beta"); + } + + #[test] + fn vim_e_from_word_end_can_land_on_trailing_space() { + let mut t = ta_with("alpha "); + t.set_cursor("alph".len()); // codespell:ignore alph + t.set_vim_enabled(/*enabled*/ true); + + t.input(KeyEvent::new(KeyCode::Char('e'), KeyModifiers::NONE)); + + assert_eq!(t.cursor(), "alpha ".len()); + } + + #[test] + fn vim_e_advances_across_atomic_element_word_ends() { + let mut t = TextArea::new(); + t.insert_str("alpha "); + t.insert_element(""); + t.insert_str(" gamma"); + let element_start = t.elements[0].range.start; + t.set_cursor("alph".len()); // codespell:ignore alph + t.set_vim_enabled(/*enabled*/ true); + + t.input(KeyEvent::new(KeyCode::Char('e'), KeyModifiers::NONE)); + assert_eq!(t.cursor(), element_start); + + t.input(KeyEvent::new(KeyCode::Char('e'), KeyModifiers::NONE)); + assert_eq!(t.cursor(), "alpha gamm".len()); + } + #[test] fn vim_dollar_lands_on_line_end_character() { let mut t = ta_with("abc\n123"); diff --git a/codex-rs/tui/src/keymap.rs b/codex-rs/tui/src/keymap.rs index 533e28f3a..bc54224c4 100644 --- a/codex-rs/tui/src/keymap.rs +++ b/codex-rs/tui/src/keymap.rs @@ -156,6 +156,7 @@ pub(crate) struct VimNormalKeymap { pub(crate) move_line_end: Vec, pub(crate) delete_char: Vec, pub(crate) delete_to_line_end: Vec, + pub(crate) change_to_line_end: Vec, pub(crate) yank_line: Vec, pub(crate) paste_after: Vec, pub(crate) start_delete_operator: Vec, @@ -469,6 +470,7 @@ impl RuntimeKeymap { move_line_end: resolve_local!(keymap, defaults, vim_normal, move_line_end), delete_char: resolve_local!(keymap, defaults, vim_normal, delete_char), delete_to_line_end: resolve_local!(keymap, defaults, vim_normal, delete_to_line_end), + change_to_line_end: resolve_local!(keymap, defaults, vim_normal, change_to_line_end), yank_line: resolve_local!(keymap, defaults, vim_normal, yank_line), paste_after: resolve_local!(keymap, defaults, vim_normal, paste_after), start_delete_operator: resolve_local!( @@ -786,6 +788,10 @@ impl RuntimeKeymap { shift(KeyCode::Char('d')), plain(KeyCode::Char('D')) ], + change_to_line_end: default_bindings![ + shift(KeyCode::Char('c')), + plain(KeyCode::Char('C')) + ], yank_line: default_bindings![shift(KeyCode::Char('y')), plain(KeyCode::Char('Y'))], paste_after: default_bindings![plain(KeyCode::Char('p'))], start_delete_operator: default_bindings![plain(KeyCode::Char('d'))], @@ -1176,6 +1182,10 @@ impl RuntimeKeymap { "delete_to_line_end", self.vim_normal.delete_to_line_end.as_slice(), ), + ( + "change_to_line_end", + self.vim_normal.change_to_line_end.as_slice(), + ), ("yank_line", self.vim_normal.yank_line.as_slice()), ("paste_after", self.vim_normal.paste_after.as_slice()), ( diff --git a/codex-rs/tui/src/keymap_setup/actions.rs b/codex-rs/tui/src/keymap_setup/actions.rs index 9ce827b8a..fb3ecd9f2 100644 --- a/codex-rs/tui/src/keymap_setup/actions.rs +++ b/codex-rs/tui/src/keymap_setup/actions.rs @@ -135,6 +135,7 @@ pub(super) const KEYMAP_ACTIONS: &[KeymapActionDescriptor] = &[ action("vim_normal", "Vim normal", "move_line_end", "Move to the end of the line."), action("vim_normal", "Vim normal", "delete_char", "Delete the character under the cursor."), action("vim_normal", "Vim normal", "delete_to_line_end", "Delete from cursor to end of line."), + action("vim_normal", "Vim normal", "change_to_line_end", "Change from cursor to end of line and enter insert mode."), action("vim_normal", "Vim normal", "yank_line", "Yank the entire line."), action("vim_normal", "Vim normal", "paste_after", "Paste after the cursor."), action("vim_normal", "Vim normal", "start_delete_operator", "Begin a delete operator and wait for a motion."), @@ -263,6 +264,7 @@ pub(super) fn binding_slot<'a>( ("vim_normal", "move_line_end") => Some(&mut keymap.vim_normal.move_line_end), ("vim_normal", "delete_char") => Some(&mut keymap.vim_normal.delete_char), ("vim_normal", "delete_to_line_end") => Some(&mut keymap.vim_normal.delete_to_line_end), + ("vim_normal", "change_to_line_end") => Some(&mut keymap.vim_normal.change_to_line_end), ("vim_normal", "yank_line") => Some(&mut keymap.vim_normal.yank_line), ("vim_normal", "paste_after") => Some(&mut keymap.vim_normal.paste_after), ("vim_normal", "start_delete_operator") => Some(&mut keymap.vim_normal.start_delete_operator), @@ -373,6 +375,7 @@ pub(super) fn bindings_for_action<'a>( ("vim_normal", "move_line_end") => Some(runtime_keymap.vim_normal.move_line_end.as_slice()), ("vim_normal", "delete_char") => Some(runtime_keymap.vim_normal.delete_char.as_slice()), ("vim_normal", "delete_to_line_end") => Some(runtime_keymap.vim_normal.delete_to_line_end.as_slice()), + ("vim_normal", "change_to_line_end") => Some(runtime_keymap.vim_normal.change_to_line_end.as_slice()), ("vim_normal", "yank_line") => Some(runtime_keymap.vim_normal.yank_line.as_slice()), ("vim_normal", "paste_after") => Some(runtime_keymap.vim_normal.paste_after.as_slice()), ("vim_normal", "start_delete_operator") => Some(runtime_keymap.vim_normal.start_delete_operator.as_slice()), 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 898108c75..bc41e50b2 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. - 93 actions, 1 customized, 2 unbound. + 94 actions, 1 customized, 2 unbound. [All] Common Customized (1) Unbound (2) App Composer Editor Vim Navigation 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 4d9440cb4..e0c1aa7ff 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. - 94 actions, 0 customized, 3 unbound. + 95 actions, 0 customized, 3 unbound. [All] Common Customized (0) Unbound (3) App Composer Editor Vim Navigation 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 58b1b8080..856ac05d1 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 (93 selectable) +tab: All (94 selectable) tab: Common (19 selectable) tab: Customized (0) (0 selectable) tab: Unbound (2) (2 selectable) tab: App (9 selectable) tab: Composer (5 selectable) tab: Editor (17 selectable) -tab: Vim (34 selectable) +tab: Vim (35 selectable) tab: Navigation (20 selectable) tab: Approval (8 selectable) tab: Debug (1 selectable) 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 6a98da923..57b013100 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. - 93 actions, 0 customized, 2 unbound. + 94 actions, 0 customized, 2 unbound. [All] Common Customized (0) Unbound (2) App Composer Editor Vim Navigation Approval Debug 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 9d651ee4f..4d1001364 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. - 93 actions, 0 customized, 2 unbound. + 94 actions, 0 customized, 2 unbound. [All] Common Customized (0) Unbound (2) App Composer Editor Vim Navigation Approval Debug