From 98ee29c7cd3f6257c2d7d623599da5ae2ceaec05 Mon Sep 17 00:00:00 2001 From: Benjamin Carlsson Date: Wed, 26 Aug 2026 04:12:27 +0000 Subject: [PATCH] Add Vim character find and till motions (#40785) ## What changed - Add line-local `f`, `F`, `t`, and `T` motions to Vim normal mode. - Support the motions with `c`, `d`, and `y`, including dot-repeat for changes. - Expose separate configurable normal-mode and operator-motion bindings, while preserving existing custom keymaps that use the new defaults. - Keep character targeting aligned to extended grapheme boundaries and skip embedded composer elements. ## Testing - Cover forward and backward motion and operator ranges, missing and adjacent targets, cancellation, dot-repeat, custom bindings, shifted terminal key events, grapheme clusters, and embedded elements. GitOrigin-RevId: 2610cf9a13538d9ae9b890c01417ca25316c1d60 --- codex-rs/config/src/tui_keymap.rs | 16 ++ codex-rs/core/config.schema.json | 80 +++++++ codex-rs/tui/src/bottom_pane/textarea.rs | 6 +- codex-rs/tui/src/bottom_pane/textarea/vim.rs | 13 ++ .../src/bottom_pane/textarea/vim_commands.rs | 169 +++++++++++++- .../textarea/vim_commands_tests.rs | 208 ++++++++++++++++++ codex-rs/tui/src/keymap.rs | 184 ++++++++++++++++ codex-rs/tui/src/keymap/bindings.rs | 8 + codex-rs/tui/src/keymap/chords.rs | 2 +- codex-rs/tui/src/keymap_setup/actions.rs | 16 ++ ...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_wide.snap | 2 +- 16 files changed, 703 insertions(+), 13 deletions(-) diff --git a/codex-rs/config/src/tui_keymap.rs b/codex-rs/config/src/tui_keymap.rs index fbd2fba603..8e6f430b32 100644 --- a/codex-rs/config/src/tui_keymap.rs +++ b/codex-rs/config/src/tui_keymap.rs @@ -235,6 +235,14 @@ pub struct TuiVimNormalKeymap { pub move_line_start: Option, /// Move cursor to end of line (`$`). pub move_line_end: Option, + /// Find the next character on the current line (`f`). + pub find_forward: Option, + /// Find the previous character on the current line (`F`). + pub find_backward: Option, + /// Stop before the next character on the current line (`t`). + pub till_forward: Option, + /// Stop after the previous character on the current line (`T`). + pub till_backward: Option, /// Delete character under cursor (`x`). pub delete_char: Option, /// Replace the character under the cursor (`r`). @@ -292,6 +300,14 @@ pub struct TuiVimOperatorKeymap { pub motion_line_start: Option, /// Motion: to end of line (`$`). pub motion_line_end: Option, + /// Motion: find the next character on the current line (`f`). + pub motion_find_forward: Option, + /// Motion: find the previous character on the current line (`F`). + pub motion_find_backward: Option, + /// Motion: stop before the next character on the current line (`t`). + pub motion_till_forward: Option, + /// Motion: stop after the previous character on the current line (`T`). + pub motion_till_backward: Option, /// Select an inner text object after an operator. pub select_inner_text_object: Option, /// Select an around text object after an operator. diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index bc51420f1a..cc80ed561a 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -3807,6 +3807,8 @@ "delete_char": null, "delete_to_line_end": null, "enter_insert": null, + "find_backward": null, + "find_forward": null, "insert_line_start": null, "move_down": null, "move_left": null, @@ -3826,16 +3828,22 @@ "start_delete_operator": null, "start_yank_operator": null, "substitute_char": null, + "till_backward": null, + "till_forward": null, "yank_line": null }, "vim_operator": { "cancel": null, "delete_line": null, "motion_down": null, + "motion_find_backward": null, + "motion_find_forward": null, "motion_left": null, "motion_line_end": null, "motion_line_start": null, "motion_right": null, + "motion_till_backward": null, + "motion_till_forward": null, "motion_up": null, "motion_word_backward": null, "motion_word_end": null, @@ -4615,6 +4623,8 @@ "delete_char": null, "delete_to_line_end": null, "enter_insert": null, + "find_backward": null, + "find_forward": null, "insert_line_start": null, "move_down": null, "move_left": null, @@ -4634,6 +4644,8 @@ "start_delete_operator": null, "start_yank_operator": null, "substitute_char": null, + "till_backward": null, + "till_forward": null, "yank_line": null } }, @@ -4647,10 +4659,14 @@ "cancel": null, "delete_line": null, "motion_down": null, + "motion_find_backward": null, + "motion_find_forward": null, "motion_left": null, "motion_line_end": null, "motion_line_start": null, "motion_right": null, + "motion_till_backward": null, + "motion_till_forward": null, "motion_up": null, "motion_word_backward": null, "motion_word_end": null, @@ -4933,6 +4949,22 @@ ], "description": "Enter insert mode at cursor (`i`)." }, + "find_backward": { + "allOf": [ + { + "$ref": "#/definitions/KeybindingsSpec" + } + ], + "description": "Find the previous character on the current line (`F`)." + }, + "find_forward": { + "allOf": [ + { + "$ref": "#/definitions/KeybindingsSpec" + } + ], + "description": "Find the next character on the current line (`f`)." + }, "insert_line_start": { "allOf": [ { @@ -5085,6 +5117,22 @@ ], "description": "Delete character under cursor and enter insert mode (`s`)." }, + "till_backward": { + "allOf": [ + { + "$ref": "#/definitions/KeybindingsSpec" + } + ], + "description": "Stop after the previous character on the current line (`T`)." + }, + "till_forward": { + "allOf": [ + { + "$ref": "#/definitions/KeybindingsSpec" + } + ], + "description": "Stop before the next character on the current line (`t`)." + }, "yank_line": { "allOf": [ { @@ -5124,6 +5172,22 @@ ], "description": "Motion: down one line (`j`)." }, + "motion_find_backward": { + "allOf": [ + { + "$ref": "#/definitions/KeybindingsSpec" + } + ], + "description": "Motion: find the previous character on the current line (`F`)." + }, + "motion_find_forward": { + "allOf": [ + { + "$ref": "#/definitions/KeybindingsSpec" + } + ], + "description": "Motion: find the next character on the current line (`f`)." + }, "motion_left": { "allOf": [ { @@ -5156,6 +5220,22 @@ ], "description": "Motion: right (`l`)." }, + "motion_till_backward": { + "allOf": [ + { + "$ref": "#/definitions/KeybindingsSpec" + } + ], + "description": "Motion: stop after the previous character on the current line (`T`)." + }, + "motion_till_forward": { + "allOf": [ + { + "$ref": "#/definitions/KeybindingsSpec" + } + ], + "description": "Motion: stop before the next character on the current line (`t`)." + }, "motion_up": { "allOf": [ { diff --git a/codex-rs/tui/src/bottom_pane/textarea.rs b/codex-rs/tui/src/bottom_pane/textarea.rs index 3749001040..aaebb46889 100644 --- a/codex-rs/tui/src/bottom_pane/textarea.rs +++ b/codex-rs/tui/src/bottom_pane/textarea.rs @@ -311,7 +311,7 @@ impl TextArea { } match self.vim_pending { VimPending::None => KeymapContext::VimNormal, - VimPending::Replace => KeymapContext::Editor, + VimPending::Replace | VimPending::Find { .. } => KeymapContext::Editor, VimPending::Operator(_) => KeymapContext::VimOperator, VimPending::TextObject { .. } => KeymapContext::VimTextObject, } @@ -718,7 +718,7 @@ impl TextArea { self.handle_vim_text_object(operator, scope, event); return; } - VimPending::Replace => { + VimPending::Replace | VimPending::Find { .. } => { self.handle_vim_pending_command(pending, event); return; } @@ -876,7 +876,7 @@ impl TextArea { self.start_vim_edit(VimAction::Change(VimEditTarget::Line)); return true; } - false + self.handle_vim_operator_command(op, event) } fn handle_vim_text_object( diff --git a/codex-rs/tui/src/bottom_pane/textarea/vim.rs b/codex-rs/tui/src/bottom_pane/textarea/vim.rs index 50b24ebc8a..8573831398 100644 --- a/codex-rs/tui/src/bottom_pane/textarea/vim.rs +++ b/codex-rs/tui/src/bottom_pane/textarea/vim.rs @@ -28,6 +28,19 @@ pub(super) enum VimPending { scope: VimTextObjectScope, }, Replace, + Find { + motion: VimFindMotion, + operator: Option, + }, +} + +/// Line-local character motions shared by navigation, operators, and dot replay. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(super) enum VimFindMotion { + Forward, + Backward, + TillForward, + TillBackward, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] diff --git a/codex-rs/tui/src/bottom_pane/textarea/vim_commands.rs b/codex-rs/tui/src/bottom_pane/textarea/vim_commands.rs index 1cee3b7b3b..284933e096 100644 --- a/codex-rs/tui/src/bottom_pane/textarea/vim_commands.rs +++ b/codex-rs/tui/src/bottom_pane/textarea/vim_commands.rs @@ -1,4 +1,8 @@ -//! Semantic Vim editing transactions and complete-change replay. +//! Semantic Vim editing transactions, character find/till motions, and complete-change replay. +//! +//! `f`/`F` land on a matching grapheme; `t`/`T` stop just before/after it. Forward operator +//! motions include the destination, while backward motions exclude the original cursor. +//! All four motions share these boundaries for navigation, `c`/`d`/`y`, and semantic `.` replay. use super::TextArea; use super::VimMode; @@ -7,10 +11,13 @@ use super::VimOperator; use super::VimPending; use super::VimTextObject; use super::VimTextObjectScope; +use super::vim::VimFindMotion; use crate::key_hint::KeyBindingListExt; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyModifiers; +use unicode_segmentation::GraphemeCursor; +use unicode_segmentation::UnicodeSegmentation; #[derive(Clone, Debug)] pub(crate) enum VimEdit { @@ -41,6 +48,10 @@ pub(super) enum VimEditTarget { scope: VimTextObjectScope, object: VimTextObject, }, + Find { + motion: VimFindMotion, + target: char, + }, } #[derive(Clone, Copy, Debug)] @@ -246,6 +257,11 @@ impl TextArea { }; self.apply_vim_operator_to_range(operator, range); } + VimEditTarget::Find { motion, target } => { + if !self.find_vim_character(motion, Some(operator), target) { + return false; + } + } } if operator == VimOperator::Change { return self.vim_mode == VimMode::Insert; @@ -306,7 +322,53 @@ impl TextArea { } return true; } - false + if self.vim_normal_keymap.find_forward.is_pressed(event) { + self.start_vim_find(VimFindMotion::Forward, /*operator*/ None); + } else if self.vim_normal_keymap.find_backward.is_pressed(event) { + self.start_vim_find(VimFindMotion::Backward, /*operator*/ None); + } else if self.vim_normal_keymap.till_forward.is_pressed(event) { + self.start_vim_find(VimFindMotion::TillForward, /*operator*/ None); + } else if self.vim_normal_keymap.till_backward.is_pressed(event) { + self.start_vim_find(VimFindMotion::TillBackward, /*operator*/ None); + } else { + return false; + } + true + } + + pub(super) fn handle_vim_operator_command( + &mut self, + operator: VimOperator, + event: KeyEvent, + ) -> bool { + if self + .vim_operator_keymap + .motion_find_forward + .is_pressed(event) + { + self.start_vim_find(VimFindMotion::Forward, Some(operator)); + } else if self + .vim_operator_keymap + .motion_find_backward + .is_pressed(event) + { + self.start_vim_find(VimFindMotion::Backward, Some(operator)); + } else if self + .vim_operator_keymap + .motion_till_forward + .is_pressed(event) + { + self.start_vim_find(VimFindMotion::TillForward, Some(operator)); + } else if self + .vim_operator_keymap + .motion_till_backward + .is_pressed(event) + { + self.start_vim_find(VimFindMotion::TillBackward, Some(operator)); + } else { + return false; + } + true } pub(super) fn handle_vim_pending_command(&mut self, pending: VimPending, event: KeyEvent) { @@ -316,9 +378,112 @@ impl TextArea { self.start_vim_edit(VimAction::Replace(ch)); } } + VimPending::Find { motion, operator } => { + if let Some(ch) = vim_command_char(event) { + match operator { + Some(VimOperator::Delete) => { + self.start_vim_edit(VimAction::Delete(VimEditTarget::Find { + motion, + target: ch, + })); + } + Some(VimOperator::Change) => { + self.start_vim_edit(VimAction::Change(VimEditTarget::Find { + motion, + target: ch, + })); + } + Some(VimOperator::Yank) | None => { + self.find_vim_character(motion, operator, ch); + } + } + } + } VimPending::None | VimPending::Operator(_) | VimPending::TextObject { .. } => {} } } + + fn start_vim_find(&mut self, motion: VimFindMotion, operator: Option) { + self.vim_pending = VimPending::Find { motion, operator }; + } + + fn find_vim_character( + &mut self, + motion: VimFindMotion, + operator: Option, + target: char, + ) -> bool { + let origin = self.cursor_pos; + let found = match motion { + VimFindMotion::Forward | VimFindMotion::TillForward => { + let line_end = self.end_of_current_line(); + if origin >= line_end { + return false; + } + let start = self.next_atomic_boundary(origin); + self.text[start..line_end] + .grapheme_indices(/*is_extended*/ true) + .find(|(offset, grapheme)| { + grapheme.starts_with(target) && self.is_vim_command_target(start + offset) + }) + .map(|(offset, grapheme)| start + offset..start + offset + grapheme.len()) + } + VimFindMotion::Backward | VimFindMotion::TillBackward => { + let line_start = self.beginning_of_current_line(); + self.text[line_start..origin] + .grapheme_indices(/*is_extended*/ true) + .rev() + .find(|(offset, grapheme)| { + grapheme.starts_with(target) + && self.is_vim_command_target(line_start + offset) + }) + .map(|(offset, grapheme)| { + let start = line_start + offset; + start..start + grapheme.len() + }) + } + }; + let Some(position) = found else { + return false; + }; + if let Some(operator) = operator { + let range = match motion { + VimFindMotion::Forward => origin..position.end, + VimFindMotion::Backward => position.start..origin, + VimFindMotion::TillForward => origin..position.start, + VimFindMotion::TillBackward => position.end..origin, + }; + if operator == VimOperator::Yank { + self.set_cursor(range.start); + } + self.apply_vim_operator_to_range(operator, range); + } else { + let destination = match motion { + VimFindMotion::Forward | VimFindMotion::Backward => position.start, + VimFindMotion::TillForward => { + let previous = self.text[..position.start] + .grapheme_indices(/*is_extended*/ true) + .next_back() + .map_or(origin, |(offset, _)| offset); + self.find_element_containing(previous) + .map_or(previous, |idx| self.elements[idx].range.start) + } + VimFindMotion::TillBackward => position.end, + }; + self.set_cursor(destination); + } + true + } + + fn is_vim_command_target(&self, position: usize) -> bool { + !self + .elements + .iter() + .any(|element| element.range.contains(&position)) + && GraphemeCursor::new(position, self.text.len(), /*is_extended*/ true) + .is_boundary(&self.text, /*chunk_start*/ 0) + .unwrap_or(false) + } } fn vim_command_char(event: KeyEvent) -> Option { diff --git a/codex-rs/tui/src/bottom_pane/textarea/vim_commands_tests.rs b/codex-rs/tui/src/bottom_pane/textarea/vim_commands_tests.rs index 512cbd7aa9..868a0127df 100644 --- a/codex-rs/tui/src/bottom_pane/textarea/vim_commands_tests.rs +++ b/codex-rs/tui/src/bottom_pane/textarea/vim_commands_tests.rs @@ -300,6 +300,186 @@ fn change_accepts_word_line_and_repeated_operator_motions() { assert_eq!(textarea.text(), " world"); } +#[test] +fn character_find_is_line_local_and_works_as_operator_motion() { + let mut textarea = vim_textarea("alpha beta\nbeta", /*cursor*/ 0); + keys(&mut textarea, "fb"); + assert_eq!(textarea.cursor(), "alpha ".len()); + keys(&mut textarea, "Fa"); + assert_eq!(textarea.cursor(), "alpha".len() - 1); + keys(&mut textarea, "fz"); + assert_eq!(textarea.cursor(), "alpha".len() - 1); + + for (motion, cursor, destination, range) in [ + ("fr", 0, 3, 0..4), + ("tr", 0, 2, 0..3), + ("Fb", 3, 1, 1..3), + ("Tb", 3, 2, 2..3), + ] { + let text = "abcrdef"; + let mut textarea = vim_textarea(text, cursor); + keys(&mut textarea, motion); + assert_eq!((textarea.text(), textarea.cursor()), (text, destination)); + for operator in ['c', 'd', 'y'] { + let mut textarea = vim_textarea(text, cursor); + keys(&mut textarea, &format!("{operator}{motion}")); + let expected = if operator == 'y' { + text.to_owned() + } else { + format!("{}{}", &text[..range.start], &text[range.end..]) + }; + let mode = if operator == 'c' { "Insert" } else { "Normal" }; + assert_eq!( + ( + textarea.text(), + textarea.cursor(), + textarea.vim_mode_label(), + textarea.kill_buffer.as_str() + ), + ( + expected.as_str(), + range.start, + Some(mode), + &text[range.clone()] + ), + "{operator}{motion}", + ); + } + } + + let mut textarea = vim_textarea("\nabc", /*cursor*/ 0); + keys(&mut textarea, "fx"); + assert_eq!(textarea.text(), "\nabc"); + assert_eq!(textarea.cursor(), 0); +} + +#[test] +fn find_and_till_handle_missing_cancelled_and_adjacent_targets() { + for motion in ['f', 'F', 't', 'T'] { + for operator in ["", "c", "d", "y"] { + let mut textarea = vim_textarea("abc\nz", /*cursor*/ 1); + keys(&mut textarea, &format!("{operator}{motion}z")); + assert_eq!( + ( + textarea.text(), + textarea.cursor(), + textarea.vim_mode_label() + ), + ("abc\nz", 1, Some("Normal")) + ); + keys(&mut textarea, &format!("{operator}{motion}")); + escape(&mut textarea); + assert!(!textarea.is_vim_operator_pending()); + } + } + for (cursor, command, expected) in [(0, "ctb", "bc"), (2, "cTb", "abc")] { + let mut textarea = vim_textarea("abc", cursor); + keys(&mut textarea, command); + assert_eq!( + (textarea.text(), textarea.vim_mode_label()), + (expected, Some("Insert")) + ); + } +} + +#[test] +fn dot_repeat_replays_character_find_operators() { + let mut textarea = vim_textarea("one:two:three", /*cursor*/ 0); + keys(&mut textarea, "df:."); + assert_eq!(textarea.text(), "three"); + + for (cursor, command, repeat, expected) in [ + (0, "cfrX", "w.", "X X"), + (0, "ctrX", "w.", "Xr Xr"), + (3, "cFbX", concat!("w", "lll."), "aXr aXr"), + (3, "cTbX", concat!("w", "lll."), "abXr abXr"), + ] { + let mut textarea = vim_textarea("abcr abcr", cursor); + keys(&mut textarea, command); + escape(&mut textarea); + keys(&mut textarea, repeat); + assert_eq!( + (textarea.text(), textarea.vim_mode_label()), + (expected, Some("Normal")) + ); + } +} + +#[test] +fn character_find_never_splits_extended_graphemes() { + let mut textarea = vim_textarea("a👩‍💻z", /*cursor*/ 0); + keys(&mut textarea, "f💻"); + assert_eq!(textarea.cursor(), 0); + keys(&mut textarea, "f👩rX"); + assert_eq!(textarea.text(), "aXz"); + for (text, command, expected, cursor) in [ + ("a👩‍💻r", "tr", "a👩‍💻r", 1), + ("aहिr", "tr", "aहिr", 1), + ("aहिr", "dfह", "r", 0), + ] { + let mut textarea = vim_textarea(text, /*cursor*/ 0); + keys(&mut textarea, command); + assert_eq!((textarea.text(), textarea.cursor()), (expected, cursor)); + } + let mut textarea = vim_textarea("a", /*cursor*/ 0); + textarea.set_cursor(/*pos*/ 1); + textarea.insert_element("[image]"); + textarea.insert_str("r"); + textarea.set_cursor(/*pos*/ 0); + keys(&mut textarea, "t]"); + assert_eq!(textarea.cursor(), 0); + keys(&mut textarea, "tr"); + assert_eq!(textarea.cursor(), 1); + textarea.set_cursor(/*pos*/ 0); + keys(&mut textarea, "ctr"); + assert_eq!((textarea.text(), textarea.elements.len()), ("r", 0)); + + let prefix = "earlier line\na"; + for (command, cursor) in [("Fa", prefix.len() - 1), ("Ta", prefix.len())] { + let mut textarea = vim_textarea(prefix, /*cursor*/ 0); + textarea.set_cursor(prefix.len()); + textarea.insert_element("[aaaa]"); + textarea.insert_str("z"); + textarea.set_cursor(textarea.text().len() - 1); + keys(&mut textarea, command); + assert_eq!( + (textarea.text(), textarea.cursor()), + ("earlier line\na[aaaa]z", cursor) + ); + } +} + +#[test] +fn character_find_and_operator_motion_use_configured_bindings() { + let mut keymap = crate::keymap::RuntimeKeymap::defaults(); + keymap.vim_normal.find_forward = vec![crate::key_hint::plain(KeyCode::Char('z'))]; + keymap.vim_operator.motion_find_forward = vec![crate::key_hint::plain(KeyCode::Char('z'))]; + keymap.vim_normal.till_forward = vec![crate::key_hint::plain(KeyCode::Char('q'))]; + keymap.vim_operator.motion_till_forward = vec![crate::key_hint::plain(KeyCode::Char('q'))]; + for (motion, cursor, expected) in [("zb", 6, "eta gamma"), ("qb", 5, "beta gamma")] { + let mut textarea = vim_textarea("alpha beta gamma", /*cursor*/ 0); + textarea.set_keymap_bindings(&keymap); + keys(&mut textarea, motion); + assert_eq!(textarea.cursor(), cursor); + textarea.set_cursor(/*pos*/ 0); + keys(&mut textarea, &format!("d{motion}")); + assert_eq!(textarea.text(), expected); + } +} + +#[test] +fn uppercase_commands_accept_shift_only_terminal_events() { + let mut textarea = vim_textarea("alpha\nbeta", /*cursor*/ 0); + textarea.set_cursor("alpha\nbe".len()); + textarea.input(KeyEvent::new(KeyCode::Char('f'), KeyModifiers::SHIFT)); + keys(&mut textarea, "b"); + assert_eq!(textarea.cursor(), "alpha\n".len()); + textarea.set_cursor("alpha\nbe".len()); + textarea.input(KeyEvent::new(KeyCode::Char('t'), KeyModifiers::SHIFT)); + keys(&mut textarea, "b"); + assert_eq!(textarea.cursor(), "alpha\nb".len()); +} + #[test] fn pending_replacement_owns_escape_before_turn_interruption() { let mut textarea = vim_textarea("alpha", /*cursor*/ 0); @@ -354,3 +534,31 @@ fn dot_repeat_has_visual_snapshot_coverage() { ^ "###); } + +#[test] +fn find_and_navigation_have_visual_snapshot_coverage() { + let mut textarea = vim_textarea("alpha beta\ngamma delta", /*cursor*/ 0); + let mut states = Vec::new(); + for command in ["tb", "fb", "Ta", "Fa"] { + keys(&mut textarea, command); + states.push(format!( + "{command}: {}\n{}^", + textarea.text().replace('\n', "\\n"), + " ".repeat(textarea.cursor()) + )); + } + insta::assert_snapshot!(states.join("\n\n"), @r###" + tb: alpha beta\ngamma delta + ^ + + fb: alpha beta\ngamma delta + ^ + + Ta: alpha beta\ngamma delta + ^ + + Fa: alpha beta\ngamma delta + ^ + + "###); +} diff --git a/codex-rs/tui/src/keymap.rs b/codex-rs/tui/src/keymap.rs index 6c7edf7e3e..25da976a15 100644 --- a/codex-rs/tui/src/keymap.rs +++ b/codex-rs/tui/src/keymap.rs @@ -186,6 +186,10 @@ pub(crate) struct VimNormalKeymap { pub(crate) move_word_end: Vec, pub(crate) move_line_start: Vec, pub(crate) move_line_end: Vec, + pub(crate) find_forward: Vec, + pub(crate) find_backward: Vec, + pub(crate) till_forward: Vec, + pub(crate) till_backward: Vec, pub(crate) delete_char: Vec, pub(crate) replace_char: Vec, pub(crate) repeat_last_change: Vec, @@ -219,6 +223,10 @@ pub(crate) struct VimOperatorKeymap { pub(crate) motion_word_end: Vec, pub(crate) motion_line_start: Vec, pub(crate) motion_line_end: Vec, + pub(crate) motion_find_forward: Vec, + pub(crate) motion_find_backward: Vec, + pub(crate) motion_till_forward: Vec, + pub(crate) motion_till_backward: Vec, pub(crate) select_inner_text_object: Vec, pub(crate) select_around_text_object: Vec, pub(crate) cancel: Vec, @@ -716,6 +724,10 @@ impl RuntimeKeymap { move_word_end: resolve_local!(keymap, defaults, vim_normal, move_word_end), move_line_start: resolve_local!(keymap, defaults, vim_normal, move_line_start), move_line_end: resolve_local!(keymap, defaults, vim_normal, move_line_end), + find_forward: resolve_local!(keymap, defaults, vim_normal, find_forward), + find_backward: resolve_local!(keymap, defaults, vim_normal, find_backward), + till_forward: resolve_local!(keymap, defaults, vim_normal, till_forward), + till_backward: resolve_local!(keymap, defaults, vim_normal, till_backward), delete_char: resolve_local!(keymap, defaults, vim_normal, delete_char), replace_char: resolve_local!(keymap, defaults, vim_normal, replace_char), repeat_last_change: resolve_local!(keymap, defaults, vim_normal, repeat_last_change), @@ -801,6 +813,22 @@ impl RuntimeKeymap { keymap.vim_normal.move_line_end.as_ref(), vim_normal.move_line_end.as_slice(), ), + ( + keymap.vim_normal.find_forward.as_ref(), + vim_normal.find_forward.as_slice(), + ), + ( + keymap.vim_normal.find_backward.as_ref(), + vim_normal.find_backward.as_slice(), + ), + ( + keymap.vim_normal.till_forward.as_ref(), + vim_normal.till_forward.as_slice(), + ), + ( + keymap.vim_normal.till_backward.as_ref(), + vim_normal.till_backward.as_slice(), + ), ( keymap.vim_normal.delete_char.as_ref(), vim_normal.delete_char.as_slice(), @@ -879,6 +907,34 @@ impl RuntimeKeymap { }) }); } + for (configured, bindings) in [ + ( + keymap.vim_normal.find_forward.as_ref(), + &mut vim_normal.find_forward, + ), + ( + keymap.vim_normal.find_backward.as_ref(), + &mut vim_normal.find_backward, + ), + ( + keymap.vim_normal.till_forward.as_ref(), + &mut vim_normal.till_forward, + ), + ( + keymap.vim_normal.till_backward.as_ref(), + &mut vim_normal.till_backward, + ), + ] { + if configured.is_none() { + bindings.retain(|binding| { + !configured_vim_normal_bindings_to_preserve.contains(binding) + && !chords.bindings.iter().any(|chord| { + chord.action.context == KeymapContext::VimNormal + && chord.chord.prefix.parts() == binding.parts() + }) + }); + } + } let mut vim_operator = VimOperatorKeymap { delete_line: resolve_local!(keymap, defaults, vim_operator, delete_line), @@ -902,6 +958,30 @@ impl RuntimeKeymap { motion_word_end: resolve_local!(keymap, defaults, vim_operator, motion_word_end), motion_line_start: resolve_local!(keymap, defaults, vim_operator, motion_line_start), motion_line_end: resolve_local!(keymap, defaults, vim_operator, motion_line_end), + motion_find_forward: resolve_local!( + keymap, + defaults, + vim_operator, + motion_find_forward + ), + motion_find_backward: resolve_local!( + keymap, + defaults, + vim_operator, + motion_find_backward + ), + motion_till_forward: resolve_local!( + keymap, + defaults, + vim_operator, + motion_till_forward + ), + motion_till_backward: resolve_local!( + keymap, + defaults, + vim_operator, + motion_till_backward + ), select_inner_text_object: resolve_local!( keymap, defaults, @@ -962,6 +1042,22 @@ impl RuntimeKeymap { keymap.vim_operator.motion_line_end.as_ref(), vim_operator.motion_line_end.as_slice(), ), + ( + keymap.vim_operator.motion_find_forward.as_ref(), + vim_operator.motion_find_forward.as_slice(), + ), + ( + keymap.vim_operator.motion_find_backward.as_ref(), + vim_operator.motion_find_backward.as_slice(), + ), + ( + keymap.vim_operator.motion_till_forward.as_ref(), + vim_operator.motion_till_forward.as_slice(), + ), + ( + keymap.vim_operator.motion_till_backward.as_ref(), + vim_operator.motion_till_backward.as_slice(), + ), ( keymap.vim_operator.cancel.as_ref(), vim_operator.cancel.as_slice(), @@ -978,6 +1074,34 @@ impl RuntimeKeymap { .select_around_text_object .retain(|binding| !configured_vim_operator_bindings_to_preserve.contains(binding)); } + for (configured, bindings) in [ + ( + keymap.vim_operator.motion_find_forward.as_ref(), + &mut vim_operator.motion_find_forward, + ), + ( + keymap.vim_operator.motion_find_backward.as_ref(), + &mut vim_operator.motion_find_backward, + ), + ( + keymap.vim_operator.motion_till_forward.as_ref(), + &mut vim_operator.motion_till_forward, + ), + ( + keymap.vim_operator.motion_till_backward.as_ref(), + &mut vim_operator.motion_till_backward, + ), + ] { + if configured.is_none() { + bindings.retain(|binding| { + !configured_vim_operator_bindings_to_preserve.contains(binding) + && !chords.bindings.iter().any(|chord| { + chord.action.context == KeymapContext::VimOperator + && chord.chord.prefix.parts() == binding.parts() + }) + }); + } + } let vim_text_object = VimTextObjectKeymap { word: resolve_local!(keymap, defaults, vim_text_object, word), @@ -1340,6 +1464,10 @@ impl RuntimeKeymap { plain(KeyCode::Char('$')), shift(KeyCode::Char('$')) ], + find_forward: default_bindings![plain(KeyCode::Char('f'))], + find_backward: default_bindings![shift(KeyCode::Char('f'))], + till_forward: default_bindings![plain(KeyCode::Char('t'))], + till_backward: default_bindings![shift(KeyCode::Char('t'))], delete_char: default_bindings![plain(KeyCode::Char('x'))], replace_char: default_bindings![plain(KeyCode::Char('r'))], repeat_last_change: default_bindings![plain(KeyCode::Char('.'))], @@ -1374,6 +1502,10 @@ impl RuntimeKeymap { plain(KeyCode::Char('$')), shift(KeyCode::Char('$')) ], + motion_find_forward: default_bindings![plain(KeyCode::Char('f'))], + motion_find_backward: default_bindings![shift(KeyCode::Char('f'))], + motion_till_forward: default_bindings![plain(KeyCode::Char('t'))], + motion_till_backward: default_bindings![shift(KeyCode::Char('t'))], select_inner_text_object: default_bindings![plain(KeyCode::Char('i'))], select_around_text_object: default_bindings![plain(KeyCode::Char('a'))], cancel: default_bindings![plain(KeyCode::Esc)], @@ -1875,6 +2007,10 @@ impl RuntimeKeymap { self.vim_normal.move_line_start.as_slice(), ), ("move_line_end", self.vim_normal.move_line_end.as_slice()), + ("find_forward", self.vim_normal.find_forward.as_slice()), + ("find_backward", self.vim_normal.find_backward.as_slice()), + ("till_forward", self.vim_normal.till_forward.as_slice()), + ("till_backward", self.vim_normal.till_backward.as_slice()), ("delete_char", self.vim_normal.delete_char.as_slice()), ("replace_char", self.vim_normal.replace_char.as_slice()), ( @@ -1943,6 +2079,22 @@ impl RuntimeKeymap { "motion_line_end", self.vim_operator.motion_line_end.as_slice(), ), + ( + "motion_find_forward", + self.vim_operator.motion_find_forward.as_slice(), + ), + ( + "motion_find_backward", + self.vim_operator.motion_find_backward.as_slice(), + ), + ( + "motion_till_forward", + self.vim_operator.motion_till_forward.as_slice(), + ), + ( + "motion_till_backward", + self.vim_operator.motion_till_backward.as_slice(), + ), ( "select_inner_text_object", self.vim_operator.select_inner_text_object.as_slice(), @@ -2988,6 +3140,38 @@ mod tests { assert_eq!(runtime.vim_normal.repeat_last_change, Vec::new()); } + #[test] + fn configured_legacy_vim_bindings_prune_new_navigation_defaults() { + let mut keymap = TuiKeymap::default(); + keymap.vim_normal.move_left = Some(one("f")); + keymap.vim_normal.move_right = Some(one("t")); + keymap.vim_normal.move_up = Some(one("shift-f")); + keymap.vim_operator.motion_right = Some(one("shift-t")); + + let runtime = RuntimeKeymap::from_config(&keymap).expect("config should parse"); + + assert_eq!(runtime.vim_normal.find_forward, Vec::new()); + assert_eq!(runtime.vim_normal.find_backward, Vec::new()); + assert_eq!(runtime.vim_normal.till_forward, Vec::new()); + assert_eq!(runtime.vim_operator.motion_till_backward, Vec::new()); + } + + #[test] + fn explicit_vim_navigation_binding_still_conflicts_with_legacy_binding() { + for key in ["f", "t"] { + let mut keymap = TuiKeymap::default(); + keymap.vim_normal.move_left = Some(one(key)); + let action = if key == "f" { + keymap.vim_normal.find_forward = Some(one(key)); + "find_forward" + } else { + keymap.vim_normal.till_forward = Some(one(key)); + "till_forward" + }; + expect_conflict(&keymap, "move_left", action); + } + } + #[test] fn configured_vim_normal_chord_prefix_prunes_new_repeat_default() { let mut keymap = TuiKeymap::default(); diff --git a/codex-rs/tui/src/keymap/bindings.rs b/codex-rs/tui/src/keymap/bindings.rs index ab57dc0c89..91796d2e5d 100644 --- a/codex-rs/tui/src/keymap/bindings.rs +++ b/codex-rs/tui/src/keymap/bindings.rs @@ -285,6 +285,10 @@ define_runtime_action_bindings! { move_word_end, move_line_start, move_line_end, + find_forward, + find_backward, + till_forward, + till_backward, delete_char, replace_char, repeat_last_change, @@ -310,6 +314,10 @@ define_runtime_action_bindings! { motion_word_end, motion_line_start, motion_line_end, + motion_find_forward, + motion_find_backward, + motion_till_forward, + motion_till_backward, select_inner_text_object, select_around_text_object, cancel, diff --git a/codex-rs/tui/src/keymap/chords.rs b/codex-rs/tui/src/keymap/chords.rs index 35e4105dd9..61749ac6d2 100644 --- a/codex-rs/tui/src/keymap/chords.rs +++ b/codex-rs/tui/src/keymap/chords.rs @@ -25,7 +25,7 @@ use crossterm::event::KeyModifiers; use std::time::Duration; use tokio::time::Instant; -const FIRST_DISPATCH_FUNCTION_KEY: u8 = 128; +const FIRST_DISPATCH_FUNCTION_KEY: u8 = codex_config::types::MAX_FUNCTION_KEY + 1; const LAST_DISPATCH_FUNCTION_KEY: u8 = u8::MAX; const LIST_RESERVED_BINDINGS: &[(&str, KeyBinding)] = &[ ("cancel", ctrl(KeyCode::Char('c'))), diff --git a/codex-rs/tui/src/keymap_setup/actions.rs b/codex-rs/tui/src/keymap_setup/actions.rs index 741a321348..b4adbc3c52 100644 --- a/codex-rs/tui/src/keymap_setup/actions.rs +++ b/codex-rs/tui/src/keymap_setup/actions.rs @@ -136,6 +136,10 @@ pub(super) const KEYMAP_ACTIONS: &[KeymapActionDescriptor] = &[ action("vim_normal", "Vim normal", "move_word_end", "Move to the end of the current or next word."), action("vim_normal", "Vim normal", "move_line_start", "Move to the start of the line."), action("vim_normal", "Vim normal", "move_line_end", "Move to the end of the line."), + action("vim_normal", "Vim normal", "find_forward", "Find the next character on the current line."), + action("vim_normal", "Vim normal", "find_backward", "Find the previous character on the current line."), + action("vim_normal", "Vim normal", "till_forward", "Stop before the next character on the current line."), + action("vim_normal", "Vim normal", "till_backward", "Stop after the previous character on the current line."), action("vim_normal", "Vim normal", "delete_char", "Delete the character under the cursor."), action("vim_normal", "Vim normal", "replace_char", "Replace the character under the cursor."), action("vim_normal", "Vim normal", "repeat_last_change", "Repeat the last complete edit."), @@ -159,6 +163,10 @@ pub(super) const KEYMAP_ACTIONS: &[KeymapActionDescriptor] = &[ action("vim_operator", "Vim operator", "motion_word_end", "Operator motion to end of word."), action("vim_operator", "Vim operator", "motion_line_start", "Operator motion to line start."), action("vim_operator", "Vim operator", "motion_line_end", "Operator motion to line end."), + action("vim_operator", "Vim operator", "motion_find_forward", "Operator motion to the next character on the current line."), + action("vim_operator", "Vim operator", "motion_find_backward", "Operator motion to the previous character on the current line."), + action("vim_operator", "Vim operator", "motion_till_forward", "Stop before the next character on the current line."), + action("vim_operator", "Vim operator", "motion_till_backward", "Stop after the previous character on the current line."), action("vim_operator", "Vim operator", "select_inner_text_object", "Select an inner text object."), action("vim_operator", "Vim operator", "select_around_text_object", "Select an around text object."), action("vim_operator", "Vim operator", "cancel", "Cancel the pending operator."), @@ -290,6 +298,10 @@ pub(super) fn binding_slot<'a>( ("vim_normal", "move_word_end") => Some(&mut keymap.vim_normal.move_word_end), ("vim_normal", "move_line_start") => Some(&mut keymap.vim_normal.move_line_start), ("vim_normal", "move_line_end") => Some(&mut keymap.vim_normal.move_line_end), + ("vim_normal", "find_forward") => Some(&mut keymap.vim_normal.find_forward), + ("vim_normal", "find_backward") => Some(&mut keymap.vim_normal.find_backward), + ("vim_normal", "till_forward") => Some(&mut keymap.vim_normal.till_forward), + ("vim_normal", "till_backward") => Some(&mut keymap.vim_normal.till_backward), ("vim_normal", "delete_char") => Some(&mut keymap.vim_normal.delete_char), ("vim_normal", "replace_char") => Some(&mut keymap.vim_normal.replace_char), ("vim_normal", "repeat_last_change") => Some(&mut keymap.vim_normal.repeat_last_change), @@ -313,6 +325,10 @@ pub(super) fn binding_slot<'a>( ("vim_operator", "motion_word_end") => Some(&mut keymap.vim_operator.motion_word_end), ("vim_operator", "motion_line_start") => Some(&mut keymap.vim_operator.motion_line_start), ("vim_operator", "motion_line_end") => Some(&mut keymap.vim_operator.motion_line_end), + ("vim_operator", "motion_find_forward") => Some(&mut keymap.vim_operator.motion_find_forward), + ("vim_operator", "motion_find_backward") => Some(&mut keymap.vim_operator.motion_find_backward), + ("vim_operator", "motion_till_forward") => Some(&mut keymap.vim_operator.motion_till_forward), + ("vim_operator", "motion_till_backward") => Some(&mut keymap.vim_operator.motion_till_backward), ("vim_operator", "select_inner_text_object") => Some(&mut keymap.vim_operator.select_inner_text_object), ("vim_operator", "select_around_text_object") => Some(&mut keymap.vim_operator.select_around_text_object), ("vim_operator", "cancel") => Some(&mut keymap.vim_operator.cancel), 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 bb447df146..fbc49b6b84 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. - 119 actions, 1 customized, 4 unbound. + 127 actions, 1 customized, 4 unbound. [All] Common Customized (1) Unbound (4) 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 c9ee971bda..b04c78abd4 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. - 120 actions, 0 customized, 5 unbound. + 128 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_first_actions.snap b/codex-rs/tui/src/snapshots/codex_tui__keymap_setup__tests__keymap_picker_first_actions.snap index 56d5133a95..a63f05b418 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 (119 selectable) +tab: All (127 selectable) tab: Common (20 selectable) tab: Customized (0) (0 selectable) tab: Unbound (4) (4 selectable) tab: App (14 selectable) tab: Composer (5 selectable) tab: Editor (17 selectable) -tab: Vim (50 selectable) +tab: Vim (58 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 d09404301d..9ef42945c9 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. - 119 actions, 1 customized, 4 unbound. + 127 actions, 1 customized, 4 unbound. [All] Common Customized (1) Unbound (4) 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 43c77e183d..0f060d2baa 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. - 119 actions, 0 customized, 4 unbound. + 127 actions, 0 customized, 4 unbound. [All] Common Customized (0) Unbound (4) App Composer Editor Vim Navigation Agents 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 33eba8b0ee..127df665a2 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. - 119 actions, 0 customized, 4 unbound. + 127 actions, 0 customized, 4 unbound. [All] Common Customized (0) Unbound (4) App Composer Editor Vim Navigation Agents Approval Debug