mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
fix: Don't trigger keybindings view on input burst (#7980)
Human TL;DR - in some situations, pasting/rapidly inputting text will currently cause `?` characters to be stripped from the input message content, and display the key bindings helper. For instance, writing "Where is X defined? Can we do Y?" results in "Where is X defined Can we do Y" being added to the message draft area. This is mildly annoying. The fix was a simple one line addition. Added a test, ran linters, and all looks good to me. I didn't create an issue to link to in this PR - I had submitted this bug as a report a while ago but can't seem to find it now. Let me know if it's an absolute must for the PR to be accepted. I have read the CLA Document and I hereby sign the CLA Below is Codex's summary. --- # `?` characters toggling shortcuts / being dropped ## Symptom On Termux (and potentially other terminal environments), composing text in the native input field and sending it to the TTY can cause: - The shortcuts overlay to appear (as if `?` was pressed on an empty prompt), and - All of the literal `?` characters in the text to be **missing** from the composer input, even when `?` is not the first character. This typically happens when the composer was previously empty and the terminal delivers the text as a rapid sequence of key events rather than a single bracketed paste event. ## Root cause The TUI has two relevant behaviors: 1. **Shortcut toggle on `?` when empty** - `ChatComposer::handle_shortcut_overlay_key` treats a plain `?` press as a toggle between the shortcut summary and the full shortcut overlay, but only when the composer is empty. - When it toggles, it consumes the key event (so `?` is *not* inserted into the text input). 2. **“Paste burst” buffering for fast key streams** - The TUI uses a heuristic to detect “paste-like” input bursts even when the terminal doesn’t send an explicit paste event. - During that burst detection, characters can be buffered (and the text area can remain empty temporarily) while the system decides whether to treat the stream as paste-like input. In Termux’s “send composed text all at once” mode, the input often arrives as a very fast stream of `KeyCode::Char(...)` events. While that stream is being buffered as a burst, the visible textarea can still be empty. If a `?` arrives during this window, it matches “empty composer” and is interpreted as “toggle shortcuts” instead of “insert literal `?`”, so the `?` is dropped. ## Fix Make the `?` toggle conditional on not being in any paste-burst transient state. Implementation: - `ChatComposer::handle_shortcut_overlay_key` now checks `!self.is_in_paste_burst()` in addition to `self.is_empty()` before toggling. - This ensures that when input is arriving as a fast burst (including the “pending first char” case), `?` is treated as normal text input rather than a UI toggle. ## Test coverage Added a test that simulates a Termux-like fast stream: - Sends `h i ? t h e r e` as immediate successive `KeyEvent::Char` events (no delays). - Asserts that a paste burst is active and the textarea is still empty while buffering. - Flushes the burst and verifies: - The final text contains the literal `?` (`"hi?there"`), and - The footer mode is not `ShortcutOverlay`. ## Notes This fix intentionally keeps the existing UX: - `?` still toggles shortcuts when the composer is genuinely empty and the user is not in the middle of entering text. - `?` typed while composing content (including IME/native-input fast streams) remains literal.
This commit is contained in:
committed by
GitHub
Unverified
parent
adbbcb0a15
commit
fc53411938
@@ -1511,7 +1511,8 @@ impl ChatComposer {
|
||||
|
||||
let toggles = matches!(key_event.code, KeyCode::Char('?'))
|
||||
&& !has_ctrl_or_alt(key_event.modifiers)
|
||||
&& self.is_empty();
|
||||
&& self.is_empty()
|
||||
&& !self.is_in_paste_burst();
|
||||
|
||||
if !toggles {
|
||||
return false;
|
||||
@@ -2171,6 +2172,35 @@ mod tests {
|
||||
assert_eq!(composer.footer_mode(), FooterMode::ContextOnly);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn question_mark_does_not_toggle_during_paste_burst() {
|
||||
use crossterm::event::KeyCode;
|
||||
use crossterm::event::KeyEvent;
|
||||
use crossterm::event::KeyModifiers;
|
||||
|
||||
let (tx, _rx) = unbounded_channel::<AppEvent>();
|
||||
let sender = AppEventSender::new(tx);
|
||||
let mut composer = ChatComposer::new(
|
||||
true,
|
||||
sender,
|
||||
false,
|
||||
"Ask Codex to do anything".to_string(),
|
||||
false,
|
||||
);
|
||||
|
||||
for ch in ['h', 'i', '?', 't', 'h', 'e', 'r', 'e'] {
|
||||
let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Char(ch), KeyModifiers::NONE));
|
||||
}
|
||||
assert!(composer.is_in_paste_burst());
|
||||
assert_eq!(composer.textarea.text(), "");
|
||||
|
||||
std::thread::sleep(ChatComposer::recommended_paste_flush_delay());
|
||||
let _ = composer.flush_paste_burst_if_due();
|
||||
|
||||
assert_eq!(composer.textarea.text(), "hi?there");
|
||||
assert_ne!(composer.footer_mode, FooterMode::ShortcutOverlay);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn shortcut_overlay_persists_while_task_running() {
|
||||
use crossterm::event::KeyCode;
|
||||
|
||||
Reference in New Issue
Block a user