From 7a48b95c6c399eb7986fb08b947fa2d45cb40370 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Mon, 14 Sep 2026 15:18:47 +0000 Subject: [PATCH] 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 --- codex-rs/tui/src/bottom_pane/chat_composer.rs | 18 ++- .../bottom_pane/chat_composer/paste_input.rs | 41 +++++ .../bottom_pane/chat_composer/paste_tests.rs | 147 ++++++++++++++++++ .../bottom_pane/chat_composer/reconnect.rs | 5 +- ...te_tests__paste_burst_tab_indentation.snap | 13 ++ codex-rs/tui/src/bottom_pane/paste_burst.rs | 19 +-- 6 files changed, 231 insertions(+), 12 deletions(-) create mode 100644 codex-rs/tui/src/bottom_pane/chat_composer/paste_input.rs create mode 100644 codex-rs/tui/src/bottom_pane/chat_composer/paste_tests.rs create mode 100644 codex-rs/tui/src/bottom_pane/chat_composer/snapshots/codex_tui__bottom_pane__chat_composer__paste_tests__paste_burst_tab_indentation.snap diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 2ffcafade3..7a6eeb5461 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -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; diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/paste_input.rs b/codex-rs/tui/src/bottom_pane/chat_composer/paste_input.rs new file mode 100644 index 0000000000..faf6d54fa9 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/chat_composer/paste_input.rs @@ -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 + } +} diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/paste_tests.rs b/codex-rs/tui/src/bottom_pane/chat_composer/paste_tests.rs new file mode 100644 index 0000000000..de8002709f --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/chat_composer/paste_tests.rs @@ -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(), + } + ); +} diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/reconnect.rs b/codex-rs/tui/src/bottom_pane/chat_composer/reconnect.rs index 5dfeaa8dfa..b0166f1fbf 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer/reconnect.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer/reconnect.rs @@ -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; } diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/snapshots/codex_tui__bottom_pane__chat_composer__paste_tests__paste_burst_tab_indentation.snap b/codex-rs/tui/src/bottom_pane/chat_composer/snapshots/codex_tui__bottom_pane__chat_composer__paste_tests__paste_burst_tab_indentation.snap new file mode 100644 index 0000000000..f6d632ac8b --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/chat_composer/snapshots/codex_tui__bottom_pane__chat_composer__paste_tests__paste_burst_tab_indentation.snap @@ -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 " diff --git a/codex-rs/tui/src/bottom_pane/paste_burst.rs b/codex-rs/tui/src/bottom_pane/paste_burst.rs index c5dee98c24..5fcd6615f0 100644 --- a/codex-rs/tui/src/bottom_pane/paste_burst.rs +++ b/codex-rs/tui/src/bottom_pane/paste_burst.rs @@ -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 {