mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
Exclude shortcut-modified input from TUI paste bursts (#38675)
## What changed - Treat plain, Shift-modified, and Windows AltGr character events as text-producing input for paste-burst detection. - Exclude Super, Hyper, and Meta character events from paste bursts, flushing any pending text before handling them as shortcuts. ## Testing - Add coverage for plain and Shift-modified spaces, shortcut-modified spaces, pending burst flushes, and platform-specific AltGr input. GitOrigin-RevId: 3c43224966307fd014eb1586dce0c53ee56c75aa
This commit is contained in:
1 parent
1873e947f8
commit
233739e76a
5 files changed
+205
-22
No files matched your search
@@ -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]
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
+13
@@ -0,0 +1,13 @@
|
||||
---
|
||||
source: tui/src/bottom_pane/chat_composer.rs
|
||||
expression: terminal.backend()
|
||||
---
|
||||
" "
|
||||
"› a b "
|
||||
" "
|
||||
" "
|
||||
" "
|
||||
" "
|
||||
" "
|
||||
" "
|
||||
" 100% context left "
|
||||
+13
@@ -0,0 +1,13 @@
|
||||
---
|
||||
source: tui/src/bottom_pane/chat_composer.rs
|
||||
expression: terminal.backend()
|
||||
---
|
||||
" "
|
||||
"› a b "
|
||||
" "
|
||||
" "
|
||||
" "
|
||||
" "
|
||||
" "
|
||||
" "
|
||||
" 100% context left "
|
||||
+13
@@ -0,0 +1,13 @@
|
||||
---
|
||||
source: tui/src/bottom_pane/chat_composer.rs
|
||||
expression: terminal.backend()
|
||||
---
|
||||
" "
|
||||
"› ab "
|
||||
" "
|
||||
" "
|
||||
" "
|
||||
" "
|
||||
" "
|
||||
" "
|
||||
" 100% context left "
|
||||
Reference in new issue
Block a user