From 8e78f929d8f0306a5c68518969e90274c44cb1f0 Mon Sep 17 00:00:00 2001 From: Can Celik Date: Tue, 29 Sep 2026 20:34:45 +0300 Subject: fix: keep escape-prefixed terminal key bindings intact (#4759) * fix: keep escape-prefixed terminal key bindings intact refs #4751 * test: cover escape-prefixed option+backspace binding refs #4751 (cherry picked from commit 898f733f92dbbe53c428845534b03adb573f92be) --- src/client/input.rs | 278 ++++++++++++++++++++++++++++++++++++++++++++++------ src/raw_input.rs | 160 ++++++++++++------------------ 2 files changed, 312 insertions(+), 126 deletions(-) diff --git a/src/client/input.rs b/src/client/input.rs index ab46e788..08254155 100644 --- a/src/client/input.rs +++ b/src/client/input.rs @@ -143,8 +143,7 @@ fn unix_stdin_reader_loop( return; } if held_escape - && stdin_read_ready(&reader, crate::raw_input::RAW_INPUT_IDLE_FLUSH_TIMEOUT_MS) - == Some(false) + && stdin_read_ready(&reader, framer.held_input_flush_timeout_ms()) == Some(false) && !send_unix_input_chunks( framer.flush_timeout(), &event_tx, @@ -253,10 +252,8 @@ fn unix_stdin_reader_loop( return; } if held_escape - && stdin_read_ready( - &reader, - crate::raw_input::RAW_INPUT_IDLE_FLUSH_TIMEOUT_MS, - ) == Some(false) + && stdin_read_ready(&reader, framer.held_input_flush_timeout_ms()) + == Some(false) { let chunks = framer.flush_timeout(); if !framer.has_pending_input() { @@ -379,14 +376,14 @@ fn idle_flush_timeout_ms( framer: &crate::raw_input::RawInputByteFramer, host_mouse_capture_active: bool, ) -> i32 { - // A mouse report split after ESC[ is still ambiguous with legacy Alt+[. - // Give it the mouse-active grace period without changing timeout decoding. - if host_mouse_capture_active - && (framer.has_pending_lone_escape() - || framer.has_pending_csi_introducer() - || framer.has_pending_incomplete_mouse_sequence()) - { + if !host_mouse_capture_active { + return crate::raw_input::RAW_INPUT_IDLE_FLUSH_TIMEOUT_MS; + } + if framer.has_pending_lone_escape() || framer.has_pending_incomplete_mouse_sequence() { crate::raw_input::MOUSE_ACTIVE_ESCAPE_SEQUENCE_FLUSH_TIMEOUT_MS + } else if framer.has_pending_csi_introducer() { + // A mouse report split after ESC[ is still ambiguous with legacy Alt+[. + crate::raw_input::MOUSE_ACTIVE_CSI_INTRODUCER_FLUSH_TIMEOUT_MS } else { crate::raw_input::RAW_INPUT_IDLE_FLUSH_TIMEOUT_MS } @@ -799,19 +796,16 @@ mod tests { assert_eq!(framer.flush_timeout().len(), 1); } - #[test] - fn captured_mouse_report_split_after_csi_survives_idle_gap() { - let mut framer = crate::raw_input::RawInputByteFramer::for_host_input(); - framer.enable_host_appearance_query_on_focus(); - assert_eq!(framer.push(b"\x1b[I"), vec![b"\x1b[I".to_vec()]); - assert!(framer.push(b"\x1b[").is_empty()); - - // #4630 input.bin offsets 10465/10467: ESC[ and its continuation - // arrived 32.937 ms apart. Apply the Unix reader's two idle waits - // without sleeping, using its production timeout selector. - let gap = std::time::Duration::from_micros(32_937); + /// Applies the Unix reader's idle waits for a gap between two host writes + /// without sleeping, using its production timeout selector. + fn reader_chunks_across_gap( + framer: &mut crate::raw_input::RawInputByteFramer, + mouse_capture: bool, + gap: std::time::Duration, + next: &[u8], + ) -> Vec> { let first_wait = - std::time::Duration::from_millis(idle_flush_timeout_ms(&framer, true) as u64); + std::time::Duration::from_millis(idle_flush_timeout_ms(framer, mouse_capture) as u64); let mut chunks = Vec::new(); if gap >= first_wait { chunks.extend(framer.flush_timeout()); @@ -819,13 +813,237 @@ mod tests { && gap >= first_wait + std::time::Duration::from_millis( - crate::raw_input::RAW_INPUT_IDLE_FLUSH_TIMEOUT_MS as u64, + framer.held_input_flush_timeout_ms() as u64 ) { chunks.extend(framer.flush_timeout()); } } - chunks.extend(framer.push(b"<35;64;37M\x1b[<35;65;36M\x1b[<35;64;36M")); + chunks.extend(framer.push(next)); + chunks + } + + fn confirmed_disambiguation_framer() -> crate::raw_input::RawInputByteFramer { + let mut framer = crate::raw_input::RawInputByteFramer::for_host_input(); + framer.set_host_escape_disambiguation_active(true); + framer + } + + #[test] + fn confirmed_disambiguation_keeps_mouse_report_split_by_delayed_tail() { + // #3480: the tail of a click arrived 350 ms after its ESC. + let gap = std::time::Duration::from_millis(350); + for (prefix, tail) in [ + (b"\x1b".as_slice(), b"[<0;5;5M".as_slice()), + (b"\x1b[", b"<0;5;5M"), + (b"\x1b[<0;", b"5;5M"), + ] { + let mut framer = confirmed_disambiguation_framer(); + assert!(framer.push(prefix).is_empty()); + + assert_eq!( + reader_chunks_across_gap(&mut framer, true, gap, tail), + vec![b"\x1b[<0;5;5M".to_vec()], + "prefix {prefix:?} must rejoin its mouse tail" + ); + } + } + + #[test] + fn confirmed_disambiguation_keeps_escape_prefixed_bindings_intact() { + // Ghostty's macOS Alt+Left/Right and Alacritty Shift+Enter bindings (#4751). + // iTerm2's Natural Text Editing preset also sends ESC DEL for Option+Backspace. + for binding in [b"\x1bb".as_slice(), b"\x1bf", b"\x1b\r", b"\x1b\x7f"] { + let mut framer = confirmed_disambiguation_framer(); + + assert_eq!(framer.push(binding), vec![binding.to_vec()]); + } + } + + #[test] + fn confirmed_disambiguation_releases_escape_prefixed_bindings_after_bounded_wait() { + // Terminal text bindings can send ESC, Alt+[ or Alt+O alone (#4751). + let gap = std::time::Duration::from_secs(1); + for binding in [b"\x1b".as_slice(), b"\x1b[", b"\x1bO"] { + let mut framer = confirmed_disambiguation_framer(); + assert!(framer.push(binding).is_empty()); + + assert_eq!( + reader_chunks_across_gap(&mut framer, true, gap, b"x"), + vec![binding.to_vec(), b"x".to_vec()], + "binding {binding:?} must not be held or dropped" + ); + } + } + + #[test] + fn confirmed_disambiguation_separates_escape_prefixed_binding_from_fast_next_key() { + // Typing right after an Alt+O / Alt+[ / ESC text binding must not glue + // the next key onto it while Herdr is watching for a delayed mouse tail. + for (binding, next, gap_ms) in [ + (b"\x1bO".as_slice(), b"q".as_slice(), 80), + (b"\x1b[", b"A", 80), + (b"\x1b", b"x", 200), + ] { + let mut framer = confirmed_disambiguation_framer(); + assert!(framer.push(binding).is_empty()); + + assert_eq!( + reader_chunks_across_gap( + &mut framer, + true, + std::time::Duration::from_millis(gap_ms), + next + ), + vec![binding.to_vec(), next.to_vec()], + "binding {binding:?} followed by {next:?} after {gap_ms} ms" + ); + } + } + + #[test] + fn confirmed_disambiguation_releases_binding_while_host_reply_is_pending() { + for (binding, next) in [(b"\x1b[".as_slice(), b"A".as_slice()), (b"\x1b", b"x")] { + let mut framer = confirmed_disambiguation_framer(); + framer.host_cell_size_query_sent(); + framer.host_color_query_sent(); + assert!(framer.push(binding).is_empty()); + + assert_eq!( + reader_chunks_across_gap( + &mut framer, + true, + std::time::Duration::from_secs(1), + next + ), + vec![binding.to_vec(), next.to_vec()], + "binding {binding:?} must not stay held behind a pending host reply" + ); + } + } + + #[test] + fn confirmed_disambiguation_keeps_slow_host_reply_and_paste_whole() { + let paste = b"200~hello\n\x1b[201~"; + for (tail, gap_ms) in [(b"6;21;10t".as_slice(), 55), (paste.as_slice(), 80)] { + let mut framer = confirmed_disambiguation_framer(); + framer.host_cell_size_query_sent(); + assert!(framer.push(b"\x1b[").is_empty()); + + let mut whole = b"\x1b[".to_vec(); + whole.extend_from_slice(tail); + assert_eq!( + reader_chunks_across_gap( + &mut framer, + true, + std::time::Duration::from_millis(gap_ms), + tail + ), + vec![whole], + "tail {tail:?} after {gap_ms} ms must stay attached to ESC[" + ); + } + } + + #[test] + fn confirmed_disambiguation_keeps_osc_reply_split_after_escape_whole() { + let mut framer = confirmed_disambiguation_framer(); + framer.host_color_query_sent(); + assert!(framer.push(b"\x1b").is_empty()); + + let tail = b"]11;rgb:1111/2222/3333\x07"; + let mut whole = b"\x1b".to_vec(); + whole.extend_from_slice(tail); + assert_eq!( + reader_chunks_across_gap( + &mut framer, + true, + std::time::Duration::from_millis(155), + tail + ), + vec![whole] + ); + } + + #[test] + fn confirmed_disambiguation_mouse_wait_does_not_consume_later_reply_hold() { + let mut framer = confirmed_disambiguation_framer(); + framer.enable_host_appearance_query_on_focus(); + assert_eq!(framer.push(b"\x1b[I"), vec![b"\x1b[I".to_vec()]); + assert!(framer.push(b"\x1b[<0;").is_empty()); + + assert!(reader_chunks_across_gap( + &mut framer, + true, + std::time::Duration::from_millis(650), + b"\x1b[?997;" + ) + .is_empty()); + assert_eq!( + reader_chunks_across_gap( + &mut framer, + true, + std::time::Duration::from_millis(15), + b"2n" + ), + vec![b"\x1b[?997;2n".to_vec()] + ); + } + + #[test] + fn confirmed_disambiguation_keeps_kitty_escape_immediate() { + let mut framer = confirmed_disambiguation_framer(); + + assert_eq!(framer.push(b"\x1b[27u"), vec![b"\x1b[27u".to_vec()]); + } + + #[test] + fn confirmed_disambiguation_joins_escape_binding_split_before_wait_ends() { + let mut framer = confirmed_disambiguation_framer(); + assert!(framer.push(b"\x1b").is_empty()); + + assert_eq!( + reader_chunks_across_gap( + &mut framer, + true, + std::time::Duration::from_millis(40), + b"\r" + ), + vec![b"\x1b\r".to_vec()] + ); + } + + #[test] + fn legacy_alt_bracket_is_not_glued_to_following_key() { + let mut framer = crate::raw_input::RawInputByteFramer::for_host_input(); + assert!(framer.push(b"\x1b[").is_empty()); + + assert_eq!( + reader_chunks_across_gap( + &mut framer, + true, + std::time::Duration::from_millis(80), + b"a" + ), + vec![b"\x1b[".to_vec(), b"a".to_vec()] + ); + } + + #[test] + fn captured_mouse_report_split_after_csi_survives_idle_gap() { + let mut framer = crate::raw_input::RawInputByteFramer::for_host_input(); + framer.enable_host_appearance_query_on_focus(); + assert_eq!(framer.push(b"\x1b[I"), vec![b"\x1b[I".to_vec()]); + assert!(framer.push(b"\x1b[").is_empty()); + + // #4630 input.bin offsets 10465/10467: ESC[ and its continuation + // arrived 32.937 ms apart. + let chunks = reader_chunks_across_gap( + &mut framer, + true, + std::time::Duration::from_micros(32_937), + b"<35;64;37M\x1b[<35;65;36M\x1b[<35;64;36M", + ); assert_eq!( chunks, vec![ @@ -858,12 +1076,16 @@ mod tests { crate::raw_input::RAW_INPUT_IDLE_FLUSH_TIMEOUT_MS ); } - for framer in [&escape, &csi, &sgr_mouse, &default_mouse] { + for framer in [&escape, &sgr_mouse, &default_mouse] { assert_eq!( idle_flush_timeout_ms(framer, true), crate::raw_input::MOUSE_ACTIVE_ESCAPE_SEQUENCE_FLUSH_TIMEOUT_MS ); } + assert_eq!( + idle_flush_timeout_ms(&csi, true), + crate::raw_input::MOUSE_ACTIVE_CSI_INTRODUCER_FLUSH_TIMEOUT_MS + ); assert_eq!( idle_flush_timeout_ms(&unrelated, true), crate::raw_input::RAW_INPUT_IDLE_FLUSH_TIMEOUT_MS diff --git a/src/raw_input.rs b/src/raw_input.rs index 1af4e894..c66d7584 100644 --- a/src/raw_input.rs +++ b/src/raw_input.rs @@ -25,6 +25,12 @@ const ESC: u8 = 0x1b; pub(crate) const RAW_INPUT_IDLE_FLUSH_TIMEOUT_MS: i32 = 10; #[cfg(unix)] pub(crate) const MOUSE_ACTIVE_ESCAPE_SEQUENCE_FLUSH_TIMEOUT_MS: i32 = 150; +/// Covers the 33 ms split in #4630 without gluing legacy Alt+[ to the next key. +#[cfg(unix)] +pub(crate) const MOUSE_ACTIVE_CSI_INTRODUCER_FLUSH_TIMEOUT_MS: i32 = 50; +/// Covers the 350 ms mouse tail delay in #3480. Other input ends it early (#4751). +#[cfg(unix)] +const DISAMBIGUATED_MOUSE_TAIL_FLUSH_TIMEOUT_MS: i32 = 500; pub(crate) const GHOSTTY_COLOR_SCHEME_DARK_REPORT: &[u8] = b"\x1b[?997;1n"; pub(crate) const GHOSTTY_COLOR_SCHEME_LIGHT_REPORT: &[u8] = b"\x1b[?997;2n"; const BRACKETED_PASTE_START: &[u8] = b"\x1b[200~"; @@ -192,7 +198,10 @@ pub(crate) struct RawInputByteFramer { host_color_scheme_change_tracking: bool, host_appearance_query_on_focus: bool, split_coalesced_escape: bool, + #[cfg(unix)] host_escape_disambiguation_active: bool, + #[cfg(unix)] + awaiting_mouse_tail_after: Option, } const HOST_COLOR_QUERY_REPLIES: u16 = 258; @@ -216,6 +225,15 @@ impl RawInputByteFramer { pub(crate) fn push(&mut self, data: &[u8]) -> Vec> { self.buffer.extend_from_slice(data); + #[cfg(unix)] + if let Some(prefix_len) = self.awaiting_mouse_tail_after.take() { + if !continues_escape_sequence(&self.buffer) { + // The prefix already outlived keyboard timing; it was a key. + let mut chunks = vec![self.buffer.drain(..prefix_len).collect()]; + chunks.extend(self.drain_available_chunks()); + return chunks; + } + } self.drain_available_chunks() } @@ -281,11 +299,21 @@ impl RawInputByteFramer { !self.buffer.is_empty() } - #[cfg(any(unix, test))] + #[cfg(unix)] pub(crate) fn set_host_escape_disambiguation_active(&mut self, active: bool) { self.host_escape_disambiguation_active = active; } + /// How long to keep holding input after a first idle flush held it. + #[cfg(unix)] + pub(crate) fn held_input_flush_timeout_ms(&self) -> i32 { + if self.awaiting_mouse_tail_after.is_some() { + DISAMBIGUATED_MOUSE_TAIL_FLUSH_TIMEOUT_MS + } else { + RAW_INPUT_IDLE_FLUSH_TIMEOUT_MS + } + } + #[cfg(unix)] pub(crate) fn has_pending_lone_escape(&self) -> bool { self.buffer.as_slice() == [ESC] @@ -354,13 +382,21 @@ impl RawInputByteFramer { return chunks; } - if self.host_escape_disambiguation_active - && starts_with_bounded_incomplete_escape_sequence(&self.buffer) + // A confirmed host sends Escape as `CSI 27u`, so a mouse report prefix + // that outlives keyboard timing may still get a delayed tail (#3480). + // Keep it briefly; `push` releases it if other input follows. + // A finished mouse wait also counts as this prefix's one-flush host + // reply hold, so the two holds never stack. + #[cfg(unix)] + let mouse_wait_served = self.awaiting_mouse_tail_after.take().is_some(); + #[cfg(not(unix))] + let mouse_wait_served = false; + #[cfg(unix)] + if !mouse_wait_served + && self.host_escape_disambiguation_active + && could_continue_as_mouse_report(&self.buffer) { - tracing::trace!( - len = self.buffer.len(), - "holding incomplete host escape sequence with disambiguation active" - ); + self.awaiting_mouse_tail_after = Some(self.buffer.len()); return chunks; } @@ -433,7 +469,7 @@ impl RawInputByteFramer { if (self.host_cell_size_replies_awaited > 0 || self.host_appearance_reply_awaited) && self.buffer.as_slice() == b"\x1b[" { - if !self.held_pending_host_reply_esc { + if !self.held_pending_host_reply_esc && !mouse_wait_served { self.held_pending_host_reply_esc = true; tracing::trace!("holding incomplete host CSI reply one flush"); return chunks; @@ -500,7 +536,8 @@ impl RawInputByteFramer { // Native console reply fragments can arrive across several idle polls. return chunks; } - if self.awaiting_host_reply() && !self.held_pending_host_reply_esc { + if self.awaiting_host_reply() && !self.held_pending_host_reply_esc && !mouse_wait_served + { self.held_pending_host_reply_esc = true; tracing::trace!("holding lone escape one flush while awaiting host reply"); return chunks; @@ -657,15 +694,6 @@ impl RawInputByteFramer { continue; } - if self.host_escape_disambiguation_active - && self.buffer.first() == Some(&ESC) - && self.buffer.len() > 1 - && !starts_with_known_escape_introducer(&self.buffer) - { - self.buffer.drain(..1); - continue; - } - let Some((event, consumed)) = extract_one_event(&self.buffer) else { break; }; @@ -1072,23 +1100,24 @@ fn starts_with_incomplete_sgr_mouse_sequence(buffer: &[u8]) -> bool { .all(|byte| byte.is_ascii_digit() || *byte == b';') } -fn starts_with_known_escape_introducer(buffer: &[u8]) -> bool { - buffer - .get(1) - .is_some_and(|byte| matches!(*byte, b'[' | b'O' | b']' | b'P' | b'_' | b'^' | b'X' | ESC)) +/// Whether bytes after a held ESC or ESC[ still read as one escape sequence, +/// such as a mouse report, host reply, or bracketed paste, rather than a key. +#[cfg(unix)] +fn continues_escape_sequence(buffer: &[u8]) -> bool { + match buffer { + [ESC] | [ESC, b'['] => true, + // OSC, DCS and APC host replies can also split after their ESC. + [ESC, b']' | b'P' | b'_', ..] => true, + [ESC, b'[', next, ..] => *next == b'M' || matches!(*next, 0x20..=0x3f), + _ => false, + } } -fn starts_with_bounded_incomplete_escape_sequence(buffer: &[u8]) -> bool { - if buffer.len() >= MAX_DISCARDED_CONTROL_TAIL_BYTES { - return false; - } - if buffer == [ESC] || buffer == b"\x1bO" { - return true; - } - let Some(body) = buffer.strip_prefix(b"\x1b[") else { - return false; - }; - body.iter().all(|byte| matches!(*byte, 0x20..=0x3f)) +#[cfg(unix)] +fn could_continue_as_mouse_report(buffer: &[u8]) -> bool { + [b"\x1b[<".as_slice(), b"\x1b[M"] + .iter() + .any(|prefix| buffer.starts_with(prefix) || prefix.starts_with(buffer)) } #[cfg(any(unix, windows, test))] @@ -2276,71 +2305,6 @@ mod tests { assert!(framer.timed_out_mouse_prefix.is_none()); } - #[test] - fn confirmed_host_disambiguation_retains_split_sgr_mouse_without_escape() { - for (prefix, tail) in [ - (b"\x1b".as_slice(), b"[<0;5;5M".as_slice()), - (b"\x1b[".as_slice(), b"<0;5;5M".as_slice()), - (b"\x1b[<0;".as_slice(), b"5;5M".as_slice()), - ] { - let mut framer = RawInputFramer::default(); - framer - .byte_framer - .set_host_escape_disambiguation_active(true); - - assert!(framer.push(prefix).is_empty()); - assert!(framer.flush_timeout().is_empty()); - assert!(framer.flush_timeout().is_empty()); - let events = framer.push(tail); - - assert!(matches!( - events.as_slice(), - [RawInputEvent::Mouse(MouseEvent { - kind: MouseEventKind::Down(MouseButton::Left), - column: 4, - row: 4, - .. - })] - )); - } - } - - #[test] - fn confirmed_host_disambiguation_drops_stale_escape_before_plain_input() { - let mut framer = RawInputFramer::default(); - framer - .byte_framer - .set_host_escape_disambiguation_active(true); - - assert!(framer.push(b"\x1b").is_empty()); - assert!(framer.flush_timeout().is_empty()); - let events = framer.push(b"x"); - - assert_eq!(events.len(), 1); - assert_raw_key( - events.into_iter().next().unwrap(), - KeyCode::Char('x'), - KeyModifiers::empty(), - ); - } - - #[test] - fn confirmed_host_disambiguation_keeps_kitty_escape_immediate() { - let mut framer = RawInputFramer::default(); - framer - .byte_framer - .set_host_escape_disambiguation_active(true); - - let events = framer.push(b"\x1b[27u"); - - assert_eq!(events.len(), 1); - assert_raw_key( - events.into_iter().next().unwrap(), - KeyCode::Esc, - KeyModifiers::empty(), - ); - } - #[test] fn sgr_mouse_tail_after_lone_escape_timeout_is_discarded() { let mut framer = RawInputFramer::default(); -- cgit v1.2.3