Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 65 additions & 3 deletions codex-rs/tui/src/bottom_pane/chat_composer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
//!
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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<TextElement> {
Expand Down Expand Up @@ -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::<AppEvent>();
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]
Expand Down
36 changes: 29 additions & 7 deletions codex-rs/tui/src/bottom_pane/chat_composer_history.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,
}

Expand Down Expand Up @@ -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;
Expand All @@ -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;
}

Expand Down Expand Up @@ -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));
}
}
5 changes: 5 additions & 0 deletions docs/tui-chat-composer.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.

Expand Down
Loading