mirror of
https://github.com/openai/codex.git
synced 2026-09-15 12:08:01 +00:00
Preserve tabs in non-bracketed paste bursts (#45454)
## Why When terminals deliver pasted text as individual key events, tabs can trigger completion, submission, or queuing instead of preserving indentation in the draft. ## What changed Capture unmodified `Tab` events during paste bursts before shortcut dispatch, including after short Unicode prefixes. Refresh the burst idle timeout when appending tabs or newlines, and flush expired bursts before handling manual `Tab` shortcuts. ## Testing Add regression tests for multiline tab preservation, ASCII and Unicode prefixes, idle timeout refresh, completion suppression, and normal submission and queue shortcuts. Add a snapshot for pasted indentation. GitOrigin-RevId: ba404cfe66c23f37da5a0db7cccfdd3d4c4af331
This commit is contained in:
@@ -167,7 +167,7 @@
|
||||
//! # Non-bracketed Paste Bursts
|
||||
//!
|
||||
//! On some terminals (especially on Windows), pastes arrive as a rapid sequence of
|
||||
//! `KeyCode::Char` and `KeyCode::Enter` key events instead of a single paste event.
|
||||
//! `KeyCode::Char`, `KeyCode::Enter`, and `KeyCode::Tab` key events instead of a single paste event.
|
||||
//!
|
||||
//! To avoid misinterpreting these bursts as real typing (and to prevent transient UI effects like
|
||||
//! shortcut overlays toggling on a pasted `?`), we feed text-producing character events (plain,
|
||||
@@ -198,6 +198,8 @@
|
||||
//! input to either buffer it or insert normally.
|
||||
//! - [`ChatComposer::handle_non_ascii_char`]: handles the non-ASCII/IME path without holding the
|
||||
//! first char, while still allowing paste detection via retro-capture.
|
||||
//! - Unmodified Tab joins detected bursts, including short Unicode prefixes, before popup dispatch.
|
||||
//! Expired bursts are flushed first so manual Tab keeps its normal shortcut behavior.
|
||||
//! - [`ChatComposer::flush_paste_burst_if_due`]/[`ChatComposer::handle_paste_burst_flush`]: called
|
||||
//! from UI ticks to turn a pending burst into either an explicit paste (`handle_paste`) or a
|
||||
//! normal typed character.
|
||||
@@ -313,6 +315,7 @@ mod draft_state;
|
||||
mod footer_state;
|
||||
mod history_search;
|
||||
mod inline_input;
|
||||
mod paste_input;
|
||||
mod popup_state;
|
||||
mod reconnect;
|
||||
mod slash_input;
|
||||
@@ -2046,6 +2049,10 @@ impl ChatComposer {
|
||||
return self.begin_history_search();
|
||||
}
|
||||
|
||||
if self.handle_paste_tab(key_event, Instant::now()) {
|
||||
return (InputResult::None, true);
|
||||
}
|
||||
|
||||
let result = match &mut self.popups.active {
|
||||
ActivePopup::Command(_) => self.handle_key_event_with_slash_popup(key_event),
|
||||
ActivePopup::File(_) => self.handle_key_event_with_file_popup(key_event),
|
||||
@@ -3701,7 +3708,10 @@ impl ChatComposer {
|
||||
if matches!(input.code, KeyCode::Enter)
|
||||
&& !self.draft.disable_paste_burst
|
||||
&& self.draft.paste_burst.is_active()
|
||||
&& self.draft.paste_burst.append_newline_if_active(now)
|
||||
&& self
|
||||
.draft
|
||||
.paste_burst
|
||||
.append_control_char_if_active('\n', now)
|
||||
{
|
||||
return (InputResult::None, true);
|
||||
}
|
||||
@@ -5079,6 +5089,10 @@ mod effort_tests;
|
||||
#[path = "chat_composer/embedded_input_tests.rs"]
|
||||
mod embedded_input_tests;
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "chat_composer/paste_tests.rs"]
|
||||
mod paste_tests;
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "chat_composer/snapshot_tests.rs"]
|
||||
mod snapshot_tests;
|
||||
|
||||
41
codex-rs/tui/src/bottom_pane/chat_composer/paste_input.rs
Normal file
41
codex-rs/tui/src/bottom_pane/chat_composer/paste_input.rs
Normal file
@@ -0,0 +1,41 @@
|
||||
//! Capture raw paste tabs before completion and submission shortcuts see them.
|
||||
|
||||
use super::*;
|
||||
|
||||
impl ChatComposer {
|
||||
pub(super) fn handle_paste_tab(&mut self, key: KeyEvent, now: Instant) -> bool {
|
||||
if key.code != KeyCode::Tab
|
||||
|| !key.modifiers.is_empty()
|
||||
|| self.draft.disable_paste_burst
|
||||
|| !self.draft.textarea.allows_paste_burst()
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
self.handle_paste_burst_flush(now);
|
||||
if self
|
||||
.draft
|
||||
.paste_burst
|
||||
.append_control_char_if_active('\t', now)
|
||||
{
|
||||
return true;
|
||||
}
|
||||
|
||||
// Short non-ASCII prefixes are inserted directly, without a held first character.
|
||||
if self
|
||||
.draft
|
||||
.paste_burst
|
||||
.direct_insert_newline_should_insert(now)
|
||||
{
|
||||
self.draft
|
||||
.paste_burst
|
||||
.begin_with_retro_grabbed(String::new(), now);
|
||||
return self
|
||||
.draft
|
||||
.paste_burst
|
||||
.append_control_char_if_active('\t', now);
|
||||
}
|
||||
|
||||
false
|
||||
}
|
||||
}
|
||||
147
codex-rs/tui/src/bottom_pane/chat_composer/paste_tests.rs
Normal file
147
codex-rs/tui/src/bottom_pane/chat_composer/paste_tests.rs
Normal file
@@ -0,0 +1,147 @@
|
||||
//! Raw paste tabs remain draft text until the user explicitly submits or queues it.
|
||||
|
||||
use super::tests::new_test_composer;
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
use ratatui::Terminal;
|
||||
use ratatui::backend::TestBackend;
|
||||
|
||||
#[test]
|
||||
fn paste_burst_tabs_preserve_multiline_draft() {
|
||||
for running in [false, true] {
|
||||
for first_line in ["x", "界", "first line\n"] {
|
||||
let (mut composer, _rx) = new_test_composer();
|
||||
composer.set_task_running(running);
|
||||
let mut now = Instant::now();
|
||||
let payload = format!("{first_line}\t\tsecond line\n\tthird line\n");
|
||||
|
||||
for ch in payload.chars() {
|
||||
let (result, _) = match ch {
|
||||
'\t' => {
|
||||
assert!(composer.handle_paste_tab(KeyEvent::from(KeyCode::Tab), now));
|
||||
(InputResult::None, true)
|
||||
}
|
||||
'\n' => composer.handle_submission_with_time(/*should_queue*/ false, now),
|
||||
ch => composer
|
||||
.handle_input_basic_with_time(KeyEvent::from(KeyCode::Char(ch)), now),
|
||||
};
|
||||
assert_eq!(result, InputResult::None);
|
||||
now += Duration::from_millis(/*millis*/ 1);
|
||||
}
|
||||
|
||||
composer.handle_paste_burst_flush(now + PasteBurst::recommended_active_flush_delay());
|
||||
assert_eq!(composer.current_text(), payload);
|
||||
|
||||
if running && first_line == "first line\n" {
|
||||
let mut terminal = Terminal::new(TestBackend::new(60, 8)).unwrap();
|
||||
terminal
|
||||
.draw(|frame| composer.render(frame.area(), frame.buffer_mut()))
|
||||
.unwrap();
|
||||
insta::assert_snapshot!("paste_burst_tab_indentation", terminal.backend());
|
||||
}
|
||||
|
||||
let (result, _) = composer.handle_key_event(KeyEvent::from(KeyCode::Tab));
|
||||
let expected = if running {
|
||||
InputResult::Queued {
|
||||
text: payload.trim().to_string(),
|
||||
text_elements: Vec::new(),
|
||||
action: QueuedInputAction::Plain,
|
||||
pending_pastes: Vec::new(),
|
||||
}
|
||||
} else {
|
||||
InputResult::Submitted {
|
||||
text: payload.trim().to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}
|
||||
};
|
||||
assert_eq!(result, expected);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn paste_burst_tabs_refresh_idle_timeout() {
|
||||
for first_char in ['x', '界'] {
|
||||
let (mut composer, _rx) = new_test_composer();
|
||||
let mut now = Instant::now();
|
||||
composer.handle_input_basic_with_time(KeyEvent::from(KeyCode::Char(first_char)), now);
|
||||
assert!(composer.handle_paste_tab(KeyEvent::from(KeyCode::Tab), now));
|
||||
|
||||
for _ in 0..3 {
|
||||
now += PasteBurst::recommended_active_flush_delay() / 2;
|
||||
assert!(composer.handle_paste_tab(KeyEvent::from(KeyCode::Tab), now));
|
||||
}
|
||||
|
||||
composer.handle_paste_burst_flush(now + PasteBurst::recommended_active_flush_delay());
|
||||
assert_eq!(composer.current_text(), format!("{first_char}\t\t\t\t"));
|
||||
assert!(!composer.handle_paste_tab(
|
||||
KeyEvent::from(KeyCode::Tab),
|
||||
now + PasteBurst::recommended_active_flush_delay()
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn paste_burst_tab_does_not_accept_a_completion() {
|
||||
let (mut composer, _rx) = new_test_composer();
|
||||
composer.insert_str("/");
|
||||
assert!(matches!(composer.popups.active, ActivePopup::Command(_)));
|
||||
composer
|
||||
.draft
|
||||
.paste_burst
|
||||
.begin_with_retro_grabbed("review this".to_string(), Instant::now());
|
||||
|
||||
let (result, _) = composer.handle_key_event(KeyEvent::from(KeyCode::Tab));
|
||||
|
||||
assert_eq!(result, InputResult::None);
|
||||
let pasted = composer
|
||||
.draft
|
||||
.paste_burst
|
||||
.flush_before_modified_input()
|
||||
.unwrap();
|
||||
composer.handle_paste(pasted);
|
||||
assert_eq!(composer.current_text(), "/review this\t");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn paste_burst_expired_before_tab_still_queues() {
|
||||
let (mut composer, _rx) = new_test_composer();
|
||||
composer.set_task_running(/*running*/ true);
|
||||
composer.handle_input_basic_with_time(
|
||||
KeyEvent::from(KeyCode::Char('x')),
|
||||
Instant::now() - Duration::from_secs(/*secs*/ 1),
|
||||
);
|
||||
|
||||
let (result, _) = composer.handle_key_event(KeyEvent::from(KeyCode::Tab));
|
||||
|
||||
assert_eq!(
|
||||
result,
|
||||
InputResult::Queued {
|
||||
text: "x".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
action: QueuedInputAction::Plain,
|
||||
pending_pastes: Vec::new(),
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn paste_burst_modified_queue_binding_still_dispatches() {
|
||||
let (mut composer, _rx) = new_test_composer();
|
||||
composer.set_task_running(/*running*/ true);
|
||||
composer.queue_keys = vec![key_hint::ctrl(KeyCode::Char('q'))];
|
||||
composer.handle_input_basic_with_time(KeyEvent::from(KeyCode::Char('x')), Instant::now());
|
||||
|
||||
let (result, _) =
|
||||
composer.handle_key_event(KeyEvent::new(KeyCode::Char('q'), KeyModifiers::CONTROL));
|
||||
|
||||
assert_eq!(
|
||||
result,
|
||||
InputResult::Queued {
|
||||
text: "x".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
action: QueuedInputAction::Plain,
|
||||
pending_pastes: Vec::new(),
|
||||
}
|
||||
);
|
||||
}
|
||||
@@ -20,7 +20,10 @@ impl ChatComposer {
|
||||
if !self.draft.disable_paste_burst
|
||||
&& self.draft.paste_burst.is_active()
|
||||
&& !in_slash_context
|
||||
&& self.draft.paste_burst.append_newline_if_active(now)
|
||||
&& self
|
||||
.draft
|
||||
.paste_burst
|
||||
.append_control_char_if_active('\n', now)
|
||||
{
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,13 @@
|
||||
---
|
||||
source: tui/src/bottom_pane/chat_composer/paste_tests.rs
|
||||
assertion_line: 38
|
||||
expression: terminal.backend()
|
||||
---
|
||||
" "
|
||||
"› first line "
|
||||
" second line "
|
||||
" third line "
|
||||
" "
|
||||
" "
|
||||
" "
|
||||
" tab to queue message 100% context left "
|
||||
@@ -1,7 +1,7 @@
|
||||
//! Paste-burst detection for terminals without bracketed paste.
|
||||
//!
|
||||
//! On some platforms (notably Windows), pastes often arrive as a rapid stream of
|
||||
//! `KeyCode::Char` and `KeyCode::Enter` key events rather than as a single "paste" event.
|
||||
//! `KeyCode::Char`, `KeyCode::Enter`, and `KeyCode::Tab` key events rather than as a single "paste" event.
|
||||
//! In that mode, the composer needs to:
|
||||
//!
|
||||
//! - Prevent transient UI side effects (e.g. toggles bound to `?`) from triggering on pasted text.
|
||||
@@ -26,8 +26,8 @@
|
||||
//! [`PasteBurst::on_plain_char_no_hold`] (non-ASCII/IME).
|
||||
//! - If the decision indicates buffering, the caller appends to `PasteBurst.buffer` via
|
||||
//! [`PasteBurst::append_char_to_buffer`].
|
||||
//! - On Enter, [`PasteBurst::append_newline_if_active`] promotes a held first character into
|
||||
//! the active buffer before appending the newline.
|
||||
//! - On Enter or Tab, [`PasteBurst::append_control_char_if_active`] promotes a held first character
|
||||
//! into the active buffer before appending the newline or tab and refreshing the idle timeout.
|
||||
//! - On a UI tick, call [`PasteBurst::flush_if_due`]. If it returns [`FlushResult::Typed`], insert
|
||||
//! that char as normal typing. If it returns [`FlushResult::Paste`], treat the returned string as
|
||||
//! an explicit paste.
|
||||
@@ -50,7 +50,7 @@
|
||||
//! A non-empty buffer is treated as "in burst context" even if `active` has been cleared.
|
||||
//! - `pending_first_char`: a single held ASCII char used for flicker suppression. The caller must
|
||||
//! not render this char until it joins a burst through `BeginBufferFromPending` or
|
||||
//! `append_newline_if_active`, or flushes as a normal typed char (`FlushResult::Typed`).
|
||||
//! `append_control_char_if_active`, or flushes as a normal typed char (`FlushResult::Typed`).
|
||||
//! - `last_plain_char_time`/`consecutive_plain_char_burst`: the timing/count heuristic for
|
||||
//! "paste-like" streams.
|
||||
//! - `burst_window_until`: the Enter suppression window ("Enter inserts newline") that outlives the
|
||||
@@ -321,18 +321,19 @@ impl PasteBurst {
|
||||
}
|
||||
}
|
||||
|
||||
/// While bursting: accumulate a newline into the buffer instead of
|
||||
/// submitting the textarea.
|
||||
/// Accumulate a newline or tab into the burst instead of invoking a shortcut.
|
||||
/// A held first character is promoted into the buffer before the control character.
|
||||
///
|
||||
/// Returns true if a newline was appended (we are in a burst context),
|
||||
/// Returns true if the character was appended (we are in a burst context),
|
||||
/// false otherwise.
|
||||
pub fn append_newline_if_active(&mut self, now: Instant) -> bool {
|
||||
pub fn append_control_char_if_active(&mut self, ch: char, now: Instant) -> bool {
|
||||
if self.is_active() {
|
||||
if let Some((held, _)) = self.pending_first_char.take() {
|
||||
self.buffer.push(held);
|
||||
}
|
||||
self.active = true;
|
||||
self.buffer.push('\n');
|
||||
self.buffer.push(ch);
|
||||
self.last_plain_char_time = Some(now);
|
||||
self.burst_window_until = Some(now + PASTE_ENTER_SUPPRESS_WINDOW);
|
||||
true
|
||||
} else {
|
||||
|
||||
Reference in New Issue
Block a user