From 4f7e4c6c56d434579aeacf382ee7b24561764ad3 Mon Sep 17 00:00:00 2001 From: Josh McKinney Date: Mon, 9 Feb 2026 23:12:38 -0800 Subject: [PATCH] tui: keep history recall cursor at line end Move chat composer history recall to place the cursor at end-of-line after rehydrating a history entry so repeated Up/Down behaves like shell history navigation. Update history navigation gating to allow traversal when recalled text is at either line boundary (start or end), which preserves multiline cursor movement from interior positions. Add regression tests for cursor placement and boundary-based history gating, and document the contracts in chat composer module docs and the TUI composer narrative guide. --- codex-rs/tui/src/bottom_pane/chat_composer.rs | 68 ++++++++++++++++++- .../src/bottom_pane/chat_composer_history.rs | 36 ++++++++-- docs/tui-chat-composer.md | 5 ++ 3 files changed, 99 insertions(+), 10 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 8bf46bdc666..38376a5278e 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -25,6 +25,8 @@ //! //! When recalling a local entry, the composer rehydrates text elements and image attachments. //! When recalling a persistent entry, only the text is restored. +//! Recalled entries move the cursor to end-of-line so repeated Up/Down presses keep shell-like +//! history traversal semantics instead of dropping to column 0. //! //! # Submission and Prompt Expansion //! @@ -532,9 +534,12 @@ impl ChatComposer { self.history.set_metadata(log_id, entry_count); } - /// Integrate an asynchronous response to an on-demand history lookup. If - /// the entry is present and the offset matches the current cursor we - /// immediately populate the textarea. + /// Integrate an asynchronous response to an on-demand history lookup. + /// + /// If the entry is present and the offset still matches the active history cursor, the + /// composer rehydrates the entry immediately. This path intentionally routes through + /// [`Self::apply_history_entry`] so cursor placement remains aligned with keyboard history + /// recall semantics. pub(crate) fn on_history_entry_response( &mut self, log_id: u64, @@ -783,6 +788,10 @@ impl ChatComposer { /// draft; if callers restore only text and images, mentions can appear /// intact to users while resolving to the wrong target or dropping on /// retry. + /// + /// This helper intentionally places the cursor at the start of the restored text. Callers + /// that need end-of-line restore behavior (for example shell-style history recall) should call + /// [`Self::move_cursor_to_end`] after this method. pub(crate) fn set_text_content_with_mention_bindings( &mut self, text: String, @@ -857,6 +866,13 @@ impl ChatComposer { self.textarea.text().to_string() } + /// Rehydrate a history entry into the composer with shell-like cursor placement. + /// + /// This path restores text, elements, images, mention bindings, and pending paste payloads, + /// then moves the cursor to end-of-line. If a caller reused + /// [`Self::set_text_content_with_mention_bindings`] directly for history recall and forgot the + /// final cursor move, repeated Up/Down would stop navigating history because cursor-gating + /// treats interior positions as normal editing mode. fn apply_history_entry(&mut self, entry: HistoryEntry) { let HistoryEntry { text, @@ -872,6 +888,7 @@ impl ChatComposer { mention_bindings, ); self.set_pending_pastes(pending_pastes); + self.move_cursor_to_end(); } pub(crate) fn text_elements(&self) -> Vec { @@ -5973,6 +5990,51 @@ mod tests { assert_eq!(text_elements.len(), 1); assert_eq!(text_elements[0].placeholder(&text), Some("[Image #1]")); assert_eq!(composer.local_image_paths(), vec![path]); + assert_eq!(composer.textarea.cursor(), composer.textarea.text().len()); + } + + #[test] + fn history_navigation_leaves_cursor_at_end_of_line() { + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + composer.set_steer_enabled(true); + + type_chars_humanlike(&mut composer, &['f', 'i', 'r', 's', 't']); + let (result, _needs_redraw) = + composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + assert!(matches!(result, InputResult::Submitted { .. })); + + type_chars_humanlike(&mut composer, &['s', 'e', 'c', 'o', 'n', 'd']); + let (result, _needs_redraw) = + composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + assert!(matches!(result, InputResult::Submitted { .. })); + + let (_result, _needs_redraw) = + composer.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE)); + assert_eq!(composer.textarea.text(), "second"); + assert_eq!(composer.textarea.cursor(), composer.textarea.text().len()); + + let (_result, _needs_redraw) = + composer.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE)); + assert_eq!(composer.textarea.text(), "first"); + assert_eq!(composer.textarea.cursor(), composer.textarea.text().len()); + + let (_result, _needs_redraw) = + composer.handle_key_event(KeyEvent::new(KeyCode::Down, KeyModifiers::NONE)); + assert_eq!(composer.textarea.text(), "second"); + assert_eq!(composer.textarea.cursor(), composer.textarea.text().len()); + + let (_result, _needs_redraw) = + composer.handle_key_event(KeyEvent::new(KeyCode::Down, KeyModifiers::NONE)); + assert!(composer.textarea.is_empty()); + assert_eq!(composer.textarea.cursor(), composer.textarea.text().len()); } #[test] diff --git a/codex-rs/tui/src/bottom_pane/chat_composer_history.rs b/codex-rs/tui/src/bottom_pane/chat_composer_history.rs index d080b79369c..e1c1d32ca1f 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer_history.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer_history.rs @@ -82,7 +82,8 @@ pub(crate) struct ChatComposerHistory { /// The text that was last inserted into the composer as a result of /// history navigation. Used to decide if further Up/Down presses should be - /// treated as navigation versus normal cursor movement. + /// treated as navigation versus normal cursor movement, together with the + /// "cursor at line boundary" check in [`Self::should_handle_navigation`]. last_history_text: Option, } @@ -136,8 +137,16 @@ impl ChatComposerHistory { self.last_history_text = None; } - /// Should Up/Down key presses be interpreted as history navigation given - /// the current content and cursor position of `textarea`? + /// Returns whether Up/Down should navigate history for the current textarea state. + /// + /// Empty text always enables history traversal. For non-empty text, this requires both: + /// + /// - the current text exactly matching the last recalled history entry, and + /// - the cursor being at a line boundary (start or end). + /// + /// This boundary gate keeps multiline cursor movement usable while preserving shell-like + /// history recall. If callers moved the cursor into the middle of a recalled entry and still + /// forced navigation, users would lose normal vertical movement within the draft. pub fn should_handle_navigation(&self, text: &str, cursor: usize) -> bool { if self.history_entry_count == 0 && self.local_history.is_empty() { return false; @@ -147,10 +156,11 @@ impl ChatComposerHistory { return true; } - // Textarea is not empty – only navigate when cursor is at start and - // text matches last recalled history entry so regular editing is not - // hijacked. - if cursor != 0 { + // Textarea is not empty – only navigate when text matches the last + // recalled history entry and the cursor is at a line boundary. This + // keeps shell-like Up/Down recall working while still allowing normal + // multiline cursor movement from interior positions. + if cursor != 0 && cursor != text.len() { return false; } @@ -378,4 +388,16 @@ mod tests { history.navigate_up(&tx) ); } + + #[test] + fn should_handle_navigation_when_cursor_is_at_line_boundaries() { + let mut history = ChatComposerHistory::new(); + history.record_local_submission(HistoryEntry::new("hello".to_string())); + history.last_history_text = Some("hello".to_string()); + + assert!(history.should_handle_navigation("hello", 0)); + assert!(history.should_handle_navigation("hello", "hello".len())); + assert!(!history.should_handle_navigation("hello", 1)); + assert!(!history.should_handle_navigation("other", 0)); + } } diff --git a/docs/tui-chat-composer.md b/docs/tui-chat-composer.md index 1e84a1b13b3..e4cf0c1b0c3 100644 --- a/docs/tui-chat-composer.md +++ b/docs/tui-chat-composer.md @@ -144,6 +144,10 @@ Local history entries capture: Persistent history entries only restore text. They intentionally do **not** rehydrate attachments or pending paste payloads. +For non-empty drafts, Up/Down navigation is only treated as history recall when the current text +matches the last recalled history entry and the cursor is at a boundary (start or end of the +line). This keeps multiline cursor movement intact while preserving shell-like history traversal. + ### Draft recovery (Ctrl+C) Ctrl+C clears the composer but stashes the full draft state (text elements, image paths, and @@ -157,6 +161,7 @@ ranges and local image paths. Pending paste payloads are cleared during submissi placeholders are expanded into their full text before being recorded. This means: - Up/Down recall of a submitted message restores image placeholders and their local paths. +- Recalled entries place the cursor at end-of-line to match typical shell history editing. - Large-paste placeholders are not expected in recalled submitted history; the text is the expanded paste content.