From 71c5dfbfc6fb40debf5ec49e3ad1d8c9ecc27cb6 Mon Sep 17 00:00:00 2001 From: Reid <61492567+reidliu41@users.noreply.github.com> Date: Fri, 8 May 2026 14:52:00 +0800 Subject: [PATCH] fix(tui): make composer arrows navigate input history (#1117) Make plain Up/Down navigate composer input history instead of scrolling the transcript from an empty composer. Keep menu overlays in control of arrow keys, preserve existing transcript scroll shortcuts, and support word-wise cursor movement with Ctrl or Alt/Option Left/Right. --- crates/tui/src/tui/app.rs | 90 ++++++++++++++++++++++------------ crates/tui/src/tui/ui.rs | 62 ++++++++++++++--------- crates/tui/src/tui/ui/tests.rs | 83 ++++++++++++++++++++++++------- 3 files changed, 165 insertions(+), 70 deletions(-) diff --git a/crates/tui/src/tui/app.rs b/crates/tui/src/tui/app.rs index 9301774c..2933733f 100644 --- a/crates/tui/src/tui/app.rs +++ b/crates/tui/src/tui/app.rs @@ -2965,34 +2965,9 @@ impl App { self.needs_redraw = true; } - // === Vim composer mode helpers === - - /// Move the cursor to the start of the current logical line (vim `0`). - pub fn vim_move_line_start(&mut self) { - let text = self.input.clone(); - let cursor_byte = byte_index_at_char(&text, self.cursor_position); - // Walk backward until we find a newline or the start of the string. - let line_start_byte = text[..cursor_byte].rfind('\n').map_or(0, |idx| idx + 1); - self.cursor_position = char_count(&text[..line_start_byte]); - self.needs_redraw = true; - } - - /// Move the cursor to the end of the current logical line (vim `$`). - pub fn vim_move_line_end(&mut self) { - let text = self.input.clone(); - let cursor_byte = byte_index_at_char(&text, self.cursor_position); - // Walk forward to the next newline or end-of-string. - let line_end_char = text[cursor_byte..].find('\n').map_or_else( - || char_count(&text), - |rel| char_count(&text[..cursor_byte + rel]), - ); - self.cursor_position = line_end_char; - self.needs_redraw = true; - } - - /// Move forward one word (vim `w`). Skips over the current word then any - /// trailing whitespace to land on the first character of the next word. - pub fn vim_move_word_forward(&mut self) { + /// Move forward one word. Skips over the current word then any trailing + /// whitespace to land on the first character of the next word. + pub fn move_cursor_word_forward(&mut self) { let text = self.input.clone(); let total = char_count(&text); let mut pos = self.cursor_position; @@ -3021,9 +2996,9 @@ impl App { self.needs_redraw = true; } - /// Move backward one word (vim `b`). Skips leading whitespace then the - /// preceding word to land on its first character. - pub fn vim_move_word_backward(&mut self) { + /// Move backward one word. Skips leading whitespace then the preceding + /// word to land on its first character. + pub fn move_cursor_word_backward(&mut self) { let text = self.input.clone(); let mut pos = self.cursor_position; if pos == 0 { @@ -3053,6 +3028,43 @@ impl App { self.needs_redraw = true; } + // === Vim composer mode helpers === + + /// Move the cursor to the start of the current logical line (vim `0`). + pub fn vim_move_line_start(&mut self) { + let text = self.input.clone(); + let cursor_byte = byte_index_at_char(&text, self.cursor_position); + // Walk backward until we find a newline or the start of the string. + let line_start_byte = text[..cursor_byte].rfind('\n').map_or(0, |idx| idx + 1); + self.cursor_position = char_count(&text[..line_start_byte]); + self.needs_redraw = true; + } + + /// Move the cursor to the end of the current logical line (vim `$`). + pub fn vim_move_line_end(&mut self) { + let text = self.input.clone(); + let cursor_byte = byte_index_at_char(&text, self.cursor_position); + // Walk forward to the next newline or end-of-string. + let line_end_char = text[cursor_byte..].find('\n').map_or_else( + || char_count(&text), + |rel| char_count(&text[..cursor_byte + rel]), + ); + self.cursor_position = line_end_char; + self.needs_redraw = true; + } + + /// Move forward one word (vim `w`). Skips over the current word then any + /// trailing whitespace to land on the first character of the next word. + pub fn vim_move_word_forward(&mut self) { + self.move_cursor_word_forward(); + } + + /// Move backward one word (vim `b`). Skips leading whitespace then the + /// preceding word to land on its first character. + pub fn vim_move_word_backward(&mut self) { + self.move_cursor_word_backward(); + } + /// Delete the character under the cursor (vim `x`). pub fn vim_delete_char_under_cursor(&mut self) { let total = char_count(&self.input); @@ -4296,6 +4308,22 @@ mod tests { assert!(app.history_index.is_none()); } + #[test] + fn word_cursor_helpers_move_by_whitespace_delimited_words() { + let mut app = App::new(test_options(false), &Config::default()); + app.input = "alpha beta gamma".to_string(); + app.cursor_position = 0; + + app.move_cursor_word_forward(); + assert_eq!(app.cursor_position, "alpha ".chars().count()); + + app.move_cursor_word_forward(); + assert_eq!(app.cursor_position, "alpha beta ".chars().count()); + + app.move_cursor_word_backward(); + assert_eq!(app.cursor_position, "alpha ".chars().count()); + } + #[test] fn editing_history_entry_leaves_navigation_mode() { let mut app = App::new(test_options(false), &Config::default()); diff --git a/crates/tui/src/tui/ui.rs b/crates/tui/src/tui/ui.rs index 874b1354..84cd432c 100644 --- a/crates/tui/src/tui/ui.rs +++ b/crates/tui/src/tui/ui.rs @@ -2677,9 +2677,15 @@ async fn run_event_loop( app.delete_char_forward(); } KeyCode::Delete => {} + KeyCode::Left if is_word_cursor_modifier(key.modifiers) => { + app.move_cursor_word_backward(); + } KeyCode::Left => { app.move_cursor_left(); } + KeyCode::Right if is_word_cursor_modifier(key.modifiers) => { + app.move_cursor_word_forward(); + } KeyCode::Right => { app.move_cursor_right(); } @@ -2746,22 +2752,12 @@ async fn run_event_loop( app.needs_redraw = true; } KeyCode::Up => { - if key.modifiers.contains(KeyModifiers::CONTROL) { - app.history_up(); - } else if should_scroll_with_arrows(app) { - app.scroll_up(1); - } else { - app.history_up(); - } + let _ = + handle_composer_history_arrow(app, key, slash_menu_open, mention_menu_open); } KeyCode::Down => { - if key.modifiers.contains(KeyModifiers::CONTROL) { - app.history_down(); - } else if should_scroll_with_arrows(app) { - app.scroll_down(1); - } else { - app.history_down(); - } + let _ = + handle_composer_history_arrow(app, key, slash_menu_open, mention_menu_open); } KeyCode::Char('u') if key.modifiers.contains(KeyModifiers::CONTROL) => { app.clear_input_recoverable(); @@ -3444,6 +3440,36 @@ fn next_escape_action(app: &App, slash_menu_open: bool) -> EscapeAction { } } +fn handle_composer_history_arrow( + app: &mut App, + key: KeyEvent, + slash_menu_open: bool, + mention_menu_open: bool, +) -> bool { + if slash_menu_open || mention_menu_open { + return false; + } + if key.modifiers.contains(KeyModifiers::ALT) || key.modifiers.contains(KeyModifiers::SUPER) { + return false; + } + + match key.code { + KeyCode::Up => { + app.history_up(); + true + } + KeyCode::Down => { + app.history_down(); + true + } + _ => false, + } +} + +fn is_word_cursor_modifier(modifiers: KeyModifiers) -> bool { + modifiers.contains(KeyModifiers::CONTROL) || modifiers.contains(KeyModifiers::ALT) +} + fn is_composer_newline_key(key: KeyEvent) -> bool { match key.code { KeyCode::Char('j') => key.modifiers.contains(KeyModifiers::CONTROL), @@ -8172,14 +8198,6 @@ fn is_ctrl_h_backspace(key: &KeyEvent) -> bool { && !key.modifiers.contains(KeyModifiers::SUPER) } -fn should_scroll_with_arrows(app: &App) -> bool { - // When the composer is empty (or only whitespace), Up/Down arrows - // scroll the transcript. When the composer has text, they navigate - // composer history so the user can recall previous prompts. - // Cmd+Up / Alt+Up always scroll regardless, handled upstream. - app.input.trim().is_empty() -} - fn extract_reasoning_header(text: &str) -> Option { let start = text.find("**")?; let rest = &text[start + 2..]; diff --git a/crates/tui/src/tui/ui/tests.rs b/crates/tui/src/tui/ui/tests.rs index d681452c..21787e96 100644 --- a/crates/tui/src/tui/ui/tests.rs +++ b/crates/tui/src/tui/ui/tests.rs @@ -79,6 +79,17 @@ fn composer_newline_shortcuts_do_not_steal_ctrl_enter() { ))); } +#[test] +fn word_cursor_modifier_accepts_control_and_alt() { + assert!(is_word_cursor_modifier(KeyModifiers::CONTROL)); + assert!(is_word_cursor_modifier(KeyModifiers::ALT)); + assert!(is_word_cursor_modifier( + KeyModifiers::CONTROL | KeyModifiers::SHIFT + )); + assert!(!is_word_cursor_modifier(KeyModifiers::NONE)); + assert!(!is_word_cursor_modifier(KeyModifiers::SHIFT)); +} + #[test] fn selection_point_from_position_ignores_top_padding() { let area = Rect { @@ -1688,6 +1699,24 @@ fn test_esc_closes_slash_menu_before_other_actions() { assert_eq!(next_escape_action(&app, true), EscapeAction::CloseSlashMenu); } +#[test] +fn history_arrow_does_not_steal_open_menus() { + let mut app = create_test_app(); + app.input_history.push("previous prompt".to_string()); + app.input = "/".to_string(); + app.cursor_position = 1; + + assert!(!handle_composer_history_arrow( + &mut app, + KeyEvent::new(KeyCode::Up, KeyModifiers::NONE), + true, + false, + )); + + assert_eq!(app.input, "/"); + assert!(app.history_index.is_none()); +} + #[test] fn test_ctrl_c_cancels_streaming_sets_status() { let mut app = create_test_app(); @@ -3795,35 +3824,55 @@ fn checklist_write_renders_dedicated_card() { ); } -// ---- scroll_with_arrows ---- +// ---- composer arrow history ---- #[test] -fn scroll_with_arrows_returns_true_when_input_empty() { - let app = create_test_app(); - assert!( - super::should_scroll_with_arrows(&app), - "empty composer: Up/Down should scroll transcript" - ); +fn history_arrow_handles_empty_input() { + let mut app = create_test_app(); + app.input_history.push("previous prompt".to_string()); + + assert!(handle_composer_history_arrow( + &mut app, + KeyEvent::new(KeyCode::Up, KeyModifiers::NONE), + false, + false, + )); + + assert_eq!(app.input, "previous prompt"); } #[test] -fn scroll_with_arrows_returns_true_when_input_only_whitespace() { +fn history_arrow_handles_whitespace_input() { let mut app = create_test_app(); app.input = " ".to_string(); - assert!( - super::should_scroll_with_arrows(&app), - "whitespace-only composer: Up/Down should scroll transcript" - ); + app.cursor_position = app.input.chars().count(); + app.input_history.push("previous prompt".to_string()); + + assert!(handle_composer_history_arrow( + &mut app, + KeyEvent::new(KeyCode::Up, KeyModifiers::NONE), + false, + false, + )); + + assert_eq!(app.input, "previous prompt"); } #[test] -fn scroll_with_arrows_returns_false_when_input_has_text() { +fn history_arrow_handles_nonempty_input() { let mut app = create_test_app(); app.input = "hello".to_string(); - assert!( - !super::should_scroll_with_arrows(&app), - "text in composer: Up/Down should navigate history" - ); + app.cursor_position = app.input.chars().count(); + app.input_history.push("previous prompt".to_string()); + + assert!(handle_composer_history_arrow( + &mut app, + KeyEvent::new(KeyCode::Up, KeyModifiers::NONE), + false, + false, + )); + + assert_eq!(app.input, "previous prompt"); } #[test]