mirror of
https://github.com/hanzoai/dev.git
synced 2026-10-25 03:32:32 +00:00
tui: keep history recall cursor at line end (#11295)
## Summary - keep cursor at end-of-line after Up/Down history recall - allow continued history navigation when recalled text cursor is at start or end boundary - add regression tests and document the history cursor contract in composer docs ## Testing - just fmt - cargo test -p codex-tui --lib history_navigation_leaves_cursor_at_end_of_line - cargo test -p codex-tui --lib should_handle_navigation_when_cursor_is_at_line_boundaries - cargo test -p codex-tui *(fails in existing integration test `suite::no_panic_on_startup::malformed_rules_should_not_panic` because `target/debug/codex` is not present in this environment)*
This commit is contained in:
@@ -25,6 +25,8 @@
|
|||||||
//!
|
//!
|
||||||
//! When recalling a local entry, the composer rehydrates text elements and image attachments.
|
//! When recalling a local entry, the composer rehydrates text elements and image attachments.
|
||||||
//! When recalling a persistent entry, only the text is restored.
|
//! 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
|
//! # Submission and Prompt Expansion
|
||||||
//!
|
//!
|
||||||
@@ -532,9 +534,12 @@ impl ChatComposer {
|
|||||||
self.history.set_metadata(log_id, entry_count);
|
self.history.set_metadata(log_id, entry_count);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Integrate an asynchronous response to an on-demand history lookup. If
|
/// Integrate an asynchronous response to an on-demand history lookup.
|
||||||
/// the entry is present and the offset matches the current cursor we
|
///
|
||||||
/// immediately populate the textarea.
|
/// 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(
|
pub(crate) fn on_history_entry_response(
|
||||||
&mut self,
|
&mut self,
|
||||||
log_id: u64,
|
log_id: u64,
|
||||||
@@ -783,6 +788,10 @@ impl ChatComposer {
|
|||||||
/// draft; if callers restore only text and images, mentions can appear
|
/// draft; if callers restore only text and images, mentions can appear
|
||||||
/// intact to users while resolving to the wrong target or dropping on
|
/// intact to users while resolving to the wrong target or dropping on
|
||||||
/// retry.
|
/// 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(
|
pub(crate) fn set_text_content_with_mention_bindings(
|
||||||
&mut self,
|
&mut self,
|
||||||
text: String,
|
text: String,
|
||||||
@@ -857,6 +866,13 @@ impl ChatComposer {
|
|||||||
self.textarea.text().to_string()
|
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) {
|
fn apply_history_entry(&mut self, entry: HistoryEntry) {
|
||||||
let HistoryEntry {
|
let HistoryEntry {
|
||||||
text,
|
text,
|
||||||
@@ -872,6 +888,7 @@ impl ChatComposer {
|
|||||||
mention_bindings,
|
mention_bindings,
|
||||||
);
|
);
|
||||||
self.set_pending_pastes(pending_pastes);
|
self.set_pending_pastes(pending_pastes);
|
||||||
|
self.move_cursor_to_end();
|
||||||
}
|
}
|
||||||
|
|
||||||
pub(crate) fn text_elements(&self) -> Vec<TextElement> {
|
pub(crate) fn text_elements(&self) -> Vec<TextElement> {
|
||||||
@@ -5973,6 +5990,51 @@ mod tests {
|
|||||||
assert_eq!(text_elements.len(), 1);
|
assert_eq!(text_elements.len(), 1);
|
||||||
assert_eq!(text_elements[0].placeholder(&text), Some("[Image #1]"));
|
assert_eq!(text_elements[0].placeholder(&text), Some("[Image #1]"));
|
||||||
assert_eq!(composer.local_image_paths(), vec![path]);
|
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]
|
#[test]
|
||||||
|
|||||||
@@ -82,7 +82,8 @@ pub(crate) struct ChatComposerHistory {
|
|||||||
|
|
||||||
/// The text that was last inserted into the composer as a result of
|
/// 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
|
/// 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>,
|
last_history_text: Option<String>,
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -136,8 +137,16 @@ impl ChatComposerHistory {
|
|||||||
self.last_history_text = None;
|
self.last_history_text = None;
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Should Up/Down key presses be interpreted as history navigation given
|
/// Returns whether Up/Down should navigate history for the current textarea state.
|
||||||
/// the current content and cursor position of `textarea`?
|
///
|
||||||
|
/// 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 {
|
pub fn should_handle_navigation(&self, text: &str, cursor: usize) -> bool {
|
||||||
if self.history_entry_count == 0 && self.local_history.is_empty() {
|
if self.history_entry_count == 0 && self.local_history.is_empty() {
|
||||||
return false;
|
return false;
|
||||||
@@ -147,10 +156,11 @@ impl ChatComposerHistory {
|
|||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Textarea is not empty – only navigate when cursor is at start and
|
// Textarea is not empty – only navigate when text matches the last
|
||||||
// text matches last recalled history entry so regular editing is not
|
// recalled history entry and the cursor is at a line boundary. This
|
||||||
// hijacked.
|
// keeps shell-like Up/Down recall working while still allowing normal
|
||||||
if cursor != 0 {
|
// multiline cursor movement from interior positions.
|
||||||
|
if cursor != 0 && cursor != text.len() {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -378,4 +388,16 @@ mod tests {
|
|||||||
history.navigate_up(&tx)
|
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));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -144,6 +144,10 @@ Local history entries capture:
|
|||||||
Persistent history entries only restore text. They intentionally do **not** rehydrate attachments
|
Persistent history entries only restore text. They intentionally do **not** rehydrate attachments
|
||||||
or pending paste payloads.
|
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)
|
### Draft recovery (Ctrl+C)
|
||||||
|
|
||||||
Ctrl+C clears the composer but stashes the full draft state (text elements, image paths, and
|
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:
|
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.
|
- 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
|
- Large-paste placeholders are not expected in recalled submitted history; the text is the
|
||||||
expanded paste content.
|
expanded paste content.
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user