diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index cdf84f64ab..63b5bd4cdf 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -144,7 +144,8 @@ //! `KeyCode::Char` and `KeyCode::Enter` 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 "plain" character events into +//! shortcut overlays toggling on a pasted `?`), we feed text-producing character events (plain, +//! Shift, or Windows AltGr) into //! [`PasteBurst`](super::paste_burst::PasteBurst), which buffers bursts and later flushes them //! through [`ChatComposer::handle_paste`]. //! @@ -3514,8 +3515,8 @@ impl ChatComposer { /// /// - Always flush any *due* paste burst first so buffered text does not lag behind unrelated /// edits. - /// - Then handle the incoming key, intercepting only "plain" (no Ctrl/Alt) char input. - /// - For non-plain keys, flush via `flush_before_modified_input()` before applying the key; + /// - Then handle the incoming key, intercepting only text-producing character input. + /// - For non-text keys, flush via `flush_before_modified_input()` before applying the key; /// otherwise `clear_window_after_non_char()` can leave buffered text waiting without a /// timestamp to time out against. fn handle_input_basic(&mut self, input: KeyEvent) -> (InputResult, bool) { @@ -3550,19 +3551,21 @@ impl ChatComposer { return (InputResult::None, true); } - // Intercept plain Char inputs to optionally accumulate into a burst buffer. + let has_non_text_modifier = has_ctrl_or_alt(input.modifiers) + || input + .modifiers + .intersects(KeyModifiers::SUPER | KeyModifiers::HYPER | KeyModifiers::META); + + // Intercept text-producing Char inputs to optionally accumulate into a burst buffer. // - // This is intentionally limited to "plain" (no Ctrl/Alt) chars so shortcuts keep their - // normal semantics, and so we can aggressively flush/clear any burst state when non-char - // keys are pressed. + // This preserves plain, Shift, and Windows AltGr input while keeping shortcut modifiers + // out of the burst detector and flushing any in-flight text before a non-text key. if let KeyEvent { code: KeyCode::Char(ch), - modifiers, .. } = input { - let has_ctrl_or_alt = has_ctrl_or_alt(modifiers); - if !has_ctrl_or_alt + if !has_non_text_modifier && !self.draft.disable_paste_burst && self.draft.textarea.allows_paste_burst() { @@ -3650,14 +3653,10 @@ impl ChatComposer { self.reconcile_deleted_elements(elements_before); } - // Update paste-burst heuristic for plain Char (no Ctrl/Alt) events. - let crossterm::event::KeyEvent { - code, modifiers, .. - } = input; - match code { + // Update the paste-burst heuristic for text, shortcut, and non-char events. + match input.code { KeyCode::Char(_) => { - let has_ctrl_or_alt = has_ctrl_or_alt(modifiers); - if has_ctrl_or_alt { + if has_non_text_modifier { self.draft.paste_burst.clear_window_after_non_char(); } } @@ -11906,6 +11905,148 @@ mod tests { } } + #[test] + fn shortcut_modified_input_does_not_enter_paste_bursts() { + snapshot_composer_state( + "shortcut_modified_space_is_not_rendered", + /*enhanced_keys_supported*/ false, + |composer| { + let now = Instant::now(); + for input in [ + KeyEvent::new(KeyCode::Char('a'), KeyModifiers::NONE), + KeyEvent::new(KeyCode::Char(' '), KeyModifiers::SUPER), + KeyEvent::new(KeyCode::Char('b'), KeyModifiers::NONE), + ] { + composer.handle_input_basic_with_time(input, now); + } + + assert!( + composer.handle_paste_burst_flush( + now + PasteBurst::recommended_active_flush_delay() + ) + ); + }, + ); + + for modifiers in [ + KeyModifiers::SUPER, + KeyModifiers::SHIFT | KeyModifiers::SUPER, + KeyModifiers::HYPER, + KeyModifiers::META, + ] { + let (mut composer, _rx) = new_test_composer(); + let now = Instant::now(); + + composer + .handle_input_basic_with_time(KeyEvent::new(KeyCode::Char(' '), modifiers), now); + + assert!( + !composer.is_in_paste_burst(), + "shortcut modifiers {modifiers:?} must not start a paste burst" + ); + assert!( + !composer.handle_paste_burst_flush(now + PasteBurst::recommended_flush_delay()) + ); + assert_eq!(composer.draft.textarea.text(), ""); + } + } + + #[test] + fn plain_and_shift_spaces_insert_normally() { + for modifiers in [KeyModifiers::NONE, KeyModifiers::SHIFT] { + let (mut composer, _rx) = new_test_composer(); + let now = Instant::now(); + + composer + .handle_input_basic_with_time(KeyEvent::new(KeyCode::Char(' '), modifiers), now); + + assert!(composer.is_in_paste_burst()); + assert_eq!(composer.draft.textarea.text(), ""); + assert!(composer.handle_paste_burst_flush(now + PasteBurst::recommended_flush_delay())); + assert_eq!(composer.draft.textarea.text(), " "); + assert!(!composer.is_in_paste_burst()); + + let snapshot_name = if modifiers == KeyModifiers::NONE { + "plain_space_is_rendered" + } else { + "shift_space_is_rendered" + }; + snapshot_composer_state( + snapshot_name, + /*enhanced_keys_supported*/ false, + |composer| { + let now = Instant::now(); + for input in [ + KeyEvent::new(KeyCode::Char('a'), KeyModifiers::NONE), + KeyEvent::new(KeyCode::Char(' '), modifiers), + KeyEvent::new(KeyCode::Char('b'), KeyModifiers::NONE), + ] { + composer.handle_input_basic_with_time(input, now); + } + + assert!(composer.handle_paste_burst_flush( + now + PasteBurst::recommended_active_flush_delay() + )); + }, + ); + } + } + + #[test] + fn non_text_input_flushes_pending_paste_without_inserting_text() { + let (mut composer, _rx) = new_test_composer(); + let now = Instant::now(); + + composer.handle_input_basic_with_time( + KeyEvent::new(KeyCode::Char('a'), KeyModifiers::NONE), + now, + ); + composer.handle_input_basic_with_time( + KeyEvent::new(KeyCode::Char('b'), KeyModifiers::NONE), + now, + ); + + assert!(composer.is_in_paste_burst()); + assert_eq!(composer.draft.textarea.text(), ""); + + composer.handle_input_basic_with_time( + KeyEvent::new(KeyCode::Char(' '), KeyModifiers::SUPER), + now, + ); + + assert_eq!(composer.draft.textarea.text(), "ab"); + assert!(!composer.is_in_paste_burst()); + assert!(!composer.handle_paste_burst_flush(now + PasteBurst::recommended_flush_delay())); + } + + #[test] + fn platform_specific_altgr_input_remains_text() { + for modifiers in [ + KeyModifiers::CONTROL | KeyModifiers::ALT, + KeyModifiers::SHIFT | KeyModifiers::CONTROL | KeyModifiers::ALT, + ] { + let (mut composer, _rx) = new_test_composer(); + let now = Instant::now(); + + composer + .handle_input_basic_with_time(KeyEvent::new(KeyCode::Char('@'), modifiers), now); + + if cfg!(windows) { + assert!(composer.is_in_paste_burst()); + assert!( + composer.handle_paste_burst_flush(now + PasteBurst::recommended_flush_delay()) + ); + assert_eq!(composer.draft.textarea.text(), "@"); + } else { + assert!(!composer.is_in_paste_burst()); + assert!( + !composer.handle_paste_burst_flush(now + PasteBurst::recommended_flush_delay()) + ); + assert_eq!(composer.draft.textarea.text(), ""); + } + } + } + /// Behavior: the first fast ASCII character is held briefly to avoid flicker; if no burst /// follows, it should eventually flush as normal typed input (not as a paste). #[test] diff --git a/codex-rs/tui/src/bottom_pane/paste_burst.rs b/codex-rs/tui/src/bottom_pane/paste_burst.rs index 89d8ca73c8..d134a3d882 100644 --- a/codex-rs/tui/src/bottom_pane/paste_burst.rs +++ b/codex-rs/tui/src/bottom_pane/paste_burst.rs @@ -9,8 +9,9 @@ //! - Avoid flicker caused by inserting a typed prefix and then immediately reclassifying it as //! paste once enough chars have arrived. //! -//! This module provides the `PasteBurst` state machine. `ChatComposer` feeds it only "plain" -//! character events (no Ctrl/Alt) and uses the full buffering decisions to either: +//! This module provides the `PasteBurst` state machine. `ChatComposer` feeds it only +//! text-producing character events (plain, Shift, or Windows AltGr) and uses the full buffering +//! decisions to either: //! //! - briefly hold a first ASCII char (flicker suppression), //! - buffer a burst as a single pasted string, or @@ -21,14 +22,15 @@ //! `PasteBurst` is a pure state machine: it never mutates the textarea directly. The caller feeds //! it events and then applies the chosen action: //! -//! - For each plain `KeyCode::Char`, call [`PasteBurst::on_plain_char`] (ASCII) or +//! - For each text-producing `KeyCode::Char`, call [`PasteBurst::on_plain_char`] (ASCII) or //! [`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 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. -//! - Before applying non-char input (arrow keys, Ctrl/Alt modifiers, etc.), use +//! - Before applying non-text input (arrow keys; Ctrl/Alt shortcuts other than Windows AltGr; +//! or Super, Hyper, and Meta modifiers), use //! [`PasteBurst::flush_before_modified_input`] to avoid leaving buffered text "stuck", and then //! [`PasteBurst::clear_window_after_non_char`] so subsequent typing does not get grouped into a //! previous burst. @@ -142,7 +144,8 @@ //! - [`FlushResult::Typed`]: insert that single char as normal typing. //! - [`FlushResult::Paste`]: treat the returned string as an explicit paste. //! -//! - When a non-plain key is pressed (Ctrl/Alt-modified input, arrows, etc.), callers should use +//! - When non-text input is pressed (Ctrl/Alt shortcuts other than Windows AltGr; Super, Hyper, +//! and Meta modifiers; arrows; etc.), callers should use //! [`PasteBurst::clear_window_after_non_char`] to prevent the next keystroke from being //! incorrectly grouped into a previous burst. diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__plain_space_is_rendered.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__plain_space_is_rendered.snap new file mode 100644 index 0000000000..a2df963f2b --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__plain_space_is_rendered.snap @@ -0,0 +1,13 @@ +--- +source: tui/src/bottom_pane/chat_composer.rs +expression: terminal.backend() +--- +" " +"› a b " +" " +" " +" " +" " +" " +" " +" 100% context left " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__shift_space_is_rendered.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__shift_space_is_rendered.snap new file mode 100644 index 0000000000..a2df963f2b --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__shift_space_is_rendered.snap @@ -0,0 +1,13 @@ +--- +source: tui/src/bottom_pane/chat_composer.rs +expression: terminal.backend() +--- +" " +"› a b " +" " +" " +" " +" " +" " +" " +" 100% context left " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__shortcut_modified_space_is_not_rendered.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__shortcut_modified_space_is_not_rendered.snap new file mode 100644 index 0000000000..04765b6550 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__shortcut_modified_space_is_not_rendered.snap @@ -0,0 +1,13 @@ +--- +source: tui/src/bottom_pane/chat_composer.rs +expression: terminal.backend() +--- +" " +"› ab " +" " +" " +" " +" " +" " +" " +" 100% context left "