mirror of
https://github.com/openai/codex.git
synced 2026-11-03 19:53:25 +00:00
Fix startup draft recovery for pending Vim Replace keys (#53148)
## Why Startup recovery treated a buffered Vim Replace key as an insertion, so the recovered unsent text could differ from the eventual edit. Replacing the first character of `abcd` with `X` should recover `Xbcd`, not `Xabcd`. ## What changed Preserve the distinction between held typed keys and buffered pastes when building recovery snapshots. Apply Vim Replace semantics to held keys, including mouse selections and the bash-mode prefix offset, without consuming live pending input. Keep buffered pastes literal so the live paste path retains responsibility for large-paste and image classification. ## Testing Add regression tests for recovery after startup draft handoff and replacement of mouse-selected text with and without the bash-mode prefix. Verify that taking a recovery snapshot leaves live input unchanged and that flushing the held key produces the same text. GitOrigin-RevId: 97fada635ad9f0f01042b8e96020372d6df9d951
This commit is contained in:
@@ -267,6 +267,9 @@
|
||||
//! from UI ticks to integrate a pending burst via [`ChatComposer::apply_paste`] or insert a
|
||||
//! held single character. Explicit terminal paste events use [`ChatComposer::handle_paste`] to
|
||||
//! classify input first; buffered keys have already been classified.
|
||||
//! - Startup recovery previews held keys with the live Vim Replace policy without consuming them.
|
||||
//! Buffered pastes remain literal in recovery so large-paste and image classification stays with
|
||||
//! the live paste path.
|
||||
//!
|
||||
//! # Input Disabled Mode
|
||||
//!
|
||||
|
||||
@@ -56,6 +56,36 @@ fn composer_mouse_edits_flush_typing_and_reconcile_placeholders() {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn recovery_keeps_unselected_text_after_a_held_replace_key() {
|
||||
for (text, expected) in [("abcd", "aXd"), ("!abcd", "!aXd")] {
|
||||
let (mut composer, _rx) = new_test_composer();
|
||||
composer.set_text_content(text.into(), Vec::new(), Vec::new());
|
||||
composer.draft.textarea.set_cursor(/*pos*/ 0);
|
||||
composer.set_vim_enabled(/*enabled*/ true);
|
||||
composer.handle_key_event(KeyCode::Char('R').into());
|
||||
let area = Rect::new(
|
||||
/*x*/ 0, /*y*/ 0, /*width*/ 80, /*height*/ 4,
|
||||
);
|
||||
composer.render(area, &mut Buffer::empty(area));
|
||||
let (x, y) = composer.cursor_pos(area).unwrap();
|
||||
for (kind, column) in [
|
||||
(Down(Left), x + 1),
|
||||
(MouseEventKind::Drag(Left), x + 3),
|
||||
(Up(Left), x + 3),
|
||||
] {
|
||||
mouse(&mut composer, area, kind, column, y);
|
||||
}
|
||||
let now = Instant::now();
|
||||
composer.handle_input_basic_with_time(KeyCode::Char('X').into(), now);
|
||||
let before = composer.draft_snapshot();
|
||||
assert_eq!(composer.recovery_snapshot().text, expected);
|
||||
assert_eq!(composer.draft_snapshot(), before);
|
||||
assert!(composer.handle_paste_burst_flush(now + PasteBurst::recommended_flush_delay()));
|
||||
assert_eq!(composer.current_text(), expected);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn selected_text_navigation_does_not_recall_history_or_select_images() {
|
||||
for (key, remote) in [
|
||||
|
||||
@@ -38,13 +38,31 @@ impl ChatComposer {
|
||||
/// Include accepted but unflushed keys without changing live paste detection.
|
||||
pub(crate) fn recovery_snapshot(&self) -> ComposerDraftSnapshot {
|
||||
let mut snapshot = self.draft_snapshot();
|
||||
if let Some(pending) = self.draft.paste_burst.clone().flush_before_modified_input() {
|
||||
let mut textarea = TextArea::new();
|
||||
textarea.set_text_with_elements(&snapshot.text, &snapshot.text_elements);
|
||||
textarea.insert_str_at(snapshot.cursor, &pending);
|
||||
snapshot.text = textarea.text().to_owned();
|
||||
snapshot.text_elements = textarea.text_elements();
|
||||
let pending = self.draft.paste_burst.clone().flush_pending();
|
||||
if matches!(pending, FlushResult::None) {
|
||||
return snapshot;
|
||||
}
|
||||
let mut textarea = TextArea::new();
|
||||
textarea.set_text_with_elements(&snapshot.text, &snapshot.text_elements);
|
||||
match pending {
|
||||
FlushResult::Typed(ch) if self.draft.textarea.is_vim_replace_mode() => {
|
||||
textarea.set_cursor(snapshot.cursor);
|
||||
if let Some(selection) = self.draft.textarea.mouse_selection_range() {
|
||||
let offset = usize::from(self.draft.is_bash_mode);
|
||||
textarea.replace_range(
|
||||
selection.start + offset..selection.end + offset,
|
||||
&ch.to_string(),
|
||||
);
|
||||
} else {
|
||||
textarea.replace_vim_text(&ch.to_string());
|
||||
}
|
||||
}
|
||||
FlushResult::Typed(ch) => textarea.insert_str_at(snapshot.cursor, &ch.to_string()),
|
||||
FlushResult::Paste(pasted) => textarea.insert_str_at(snapshot.cursor, &pasted),
|
||||
FlushResult::None => {}
|
||||
}
|
||||
snapshot.text = textarea.text().to_owned();
|
||||
snapshot.text_elements = textarea.text_elements();
|
||||
snapshot
|
||||
}
|
||||
|
||||
|
||||
@@ -304,18 +304,21 @@ impl PasteBurst {
|
||||
let timed_out = self
|
||||
.last_plain_char_time
|
||||
.is_some_and(|t| now.duration_since(t) > timeout);
|
||||
if timed_out && self.is_active_internal() {
|
||||
if timed_out {
|
||||
self.flush_pending()
|
||||
} else {
|
||||
FlushResult::None
|
||||
}
|
||||
}
|
||||
|
||||
/// Flush without waiting, retaining the typed-key versus paste distinction.
|
||||
pub(crate) fn flush_pending(&mut self) -> FlushResult {
|
||||
if self.is_active_internal() {
|
||||
self.active = false;
|
||||
let out = std::mem::take(&mut self.buffer);
|
||||
FlushResult::Paste(out)
|
||||
} else if timed_out {
|
||||
// If we were saving a single fast char and no burst followed,
|
||||
// flush it as normal typed input.
|
||||
if let Some((ch, _at)) = self.pending_first_char.take() {
|
||||
FlushResult::Typed(ch)
|
||||
} else {
|
||||
FlushResult::None
|
||||
}
|
||||
} else if let Some((ch, _at)) = self.pending_first_char.take() {
|
||||
FlushResult::Typed(ch)
|
||||
} else {
|
||||
FlushResult::None
|
||||
}
|
||||
|
||||
@@ -119,7 +119,7 @@ impl TextArea {
|
||||
self.vim_commands.replace_steps.clear();
|
||||
}
|
||||
|
||||
pub(super) fn replace_vim_text(&mut self, text: &str) {
|
||||
pub(in crate::bottom_pane) fn replace_vim_text(&mut self, text: &str) {
|
||||
for grapheme in text.graphemes(/*is_extended*/ true) {
|
||||
let cursor_before = self.cursor_pos;
|
||||
while let Some(element) = self
|
||||
|
||||
@@ -48,6 +48,32 @@ async fn recovery_keeps_edits_and_pastes_after_chatwidget_handoff() {
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test(start_paused = true)]
|
||||
async fn recovery_after_handoff_preserves_pending_vim_replace_key() {
|
||||
scope(async {
|
||||
let (mut chat, _sender, _rx, _op_rx) =
|
||||
crate::chatwidget::tests::make_chatwidget_manual_with_sender().await;
|
||||
let mut draft = snapshot("abcd");
|
||||
draft.cursor = 0;
|
||||
remember(draft.clone());
|
||||
let mut pending = Some(draft);
|
||||
let mut confirmed = false;
|
||||
chat.restore_startup_input_when_ready(&mut pending, &mut confirmed);
|
||||
chat.toggle_vim_mode_and_notify();
|
||||
chat.handle_key_event(KeyEvent::from(KeyCode::Char('R')));
|
||||
chat.handle_key_event(KeyEvent::from(KeyCode::Char('X')));
|
||||
|
||||
// A failure now prints the eventual edit but must not consume the live pending key.
|
||||
assert_eq!(chat.composer_text_with_pending(), "abcd");
|
||||
insta::assert_snapshot!(take_unsent_text().unwrap(), @"Xbcd");
|
||||
assert_eq!(chat.composer_text_with_pending(), "abcd");
|
||||
tokio::time::advance(std::time::Duration::from_millis(100)).await;
|
||||
assert!(chat.handle_paste_burst_tick(crate::tui::FrameRequester::test_dummy()));
|
||||
assert_eq!(chat.composer_text_with_pending(), "Xbcd");
|
||||
})
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn recovery_refresh_stops_after_submission_or_startup_completion() {
|
||||
scope(async {
|
||||
|
||||
Reference in New Issue
Block a user