mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
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.
This commit is contained in:
committed by
GitHub
Unverified
parent
f20904c4d6
commit
aa184548b1
@@ -223,6 +223,8 @@ pub struct TuiVimNormalKeymap {
|
||||
pub delete_char: Option<KeybindingsSpec>,
|
||||
/// Delete from cursor to end of line (`D`).
|
||||
pub delete_to_line_end: Option<KeybindingsSpec>,
|
||||
/// Change from cursor to end of line and enter insert mode (`C`).
|
||||
pub change_to_line_end: Option<KeybindingsSpec>,
|
||||
/// Yank the entire line (`Y`).
|
||||
pub yank_line: Option<KeybindingsSpec>,
|
||||
/// Paste after cursor (`p`).
|
||||
|
||||
@@ -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": [
|
||||
{
|
||||
|
||||
+12
@@ -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
|
||||
^
|
||||
@@ -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("<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 <element> gamm".len());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn vim_dollar_lands_on_line_end_character() {
|
||||
let mut t = ta_with("abc\n123");
|
||||
|
||||
@@ -156,6 +156,7 @@ pub(crate) struct VimNormalKeymap {
|
||||
pub(crate) move_line_end: Vec<KeyBinding>,
|
||||
pub(crate) delete_char: Vec<KeyBinding>,
|
||||
pub(crate) delete_to_line_end: Vec<KeyBinding>,
|
||||
pub(crate) change_to_line_end: Vec<KeyBinding>,
|
||||
pub(crate) yank_line: Vec<KeyBinding>,
|
||||
pub(crate) paste_after: Vec<KeyBinding>,
|
||||
pub(crate) start_delete_operator: Vec<KeyBinding>,
|
||||
@@ -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()),
|
||||
(
|
||||
|
||||
@@ -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()),
|
||||
|
||||
+1
-1
@@ -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
|
||||
|
||||
|
||||
+1
-1
@@ -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
|
||||
|
||||
|
||||
+2
-2
@@ -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)
|
||||
|
||||
+1
-1
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user