mirror of
https://github.com/herdrdev/herdr.git
synced 2026-11-06 00:06:18 +00:00
fix: wait for split input sequences instead of typing their fragments
This commit is contained in:
Ogulcan Celik
+17
-23
@@ -376,17 +376,7 @@ fn idle_flush_timeout_ms(
|
||||
framer: &crate::raw_input::RawInputByteFramer,
|
||||
host_mouse_capture_active: bool,
|
||||
) -> i32 {
|
||||
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
|
||||
}
|
||||
framer.idle_flush_timeout_ms(host_mouse_capture_active)
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
@@ -1058,7 +1048,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mouse_active_escape_sequences_get_longer_reassembly_window() {
|
||||
fn only_ambiguous_escape_prefixes_get_the_short_keyboard_window() {
|
||||
let mut escape = crate::raw_input::RawInputByteFramer::default();
|
||||
assert!(escape.push(b"\x1b").is_empty());
|
||||
let mut csi = crate::raw_input::RawInputByteFramer::default();
|
||||
@@ -1070,26 +1060,30 @@ mod tests {
|
||||
let mut unrelated = crate::raw_input::RawInputByteFramer::default();
|
||||
assert!(unrelated.push(b"\x1b[49:33;2:").is_empty());
|
||||
|
||||
for framer in [&escape, &csi, &sgr_mouse, &default_mouse, &unrelated] {
|
||||
// A lone ESC or ESC[ may still be a key (Escape, Alt+[).
|
||||
for framer in [&escape, &csi] {
|
||||
assert_eq!(
|
||||
idle_flush_timeout_ms(framer, false),
|
||||
crate::raw_input::RAW_INPUT_IDLE_FLUSH_TIMEOUT_MS
|
||||
);
|
||||
}
|
||||
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(&escape, 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
|
||||
);
|
||||
// Anything further inside a sequence cannot be a key: wait for the rest.
|
||||
for framer in [&sgr_mouse, &default_mouse, &unrelated] {
|
||||
for mouse_capture in [false, true] {
|
||||
assert_eq!(
|
||||
idle_flush_timeout_ms(framer, mouse_capture),
|
||||
crate::raw_input::INCOMPLETE_SEQUENCE_FLUSH_TIMEOUT_MS
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
let mouse_timeout_ms =
|
||||
std::hint::black_box(crate::raw_input::MOUSE_ACTIVE_ESCAPE_SEQUENCE_FLUSH_TIMEOUT_MS);
|
||||
|
||||
@@ -596,10 +596,12 @@ fn print_report(report: &Report) {
|
||||
|
||||
// Ratchet: failure counts may only go down. Lower them when a change fixes
|
||||
// cases; a rise means a regression in Herdr's input transparency.
|
||||
const KEYBOARD_FAILURES_BASELINE: usize = 120;
|
||||
// Remaining: Alt+], Alt+Shift+P/X, Alt+^, Alt+_ behind legacy hosts. Their bytes
|
||||
// also start host replies, so they are never forwarded (#344).
|
||||
const KEYBOARD_FAILURES_BASELINE: usize = 50;
|
||||
const MOUSE_FAILURES_BASELINE: usize = 96;
|
||||
const SPLIT_IDLE_MISMATCH_BASELINE: usize = 6_730;
|
||||
const REPLY_IDLE_MISMATCH_BASELINE: usize = 77;
|
||||
// Remaining: legacy hosts split right after ESC[, which is also Alt+[.
|
||||
const SPLIT_IDLE_MISMATCH_BASELINE: usize = 207;
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Mouse
|
||||
@@ -961,14 +963,20 @@ const HOST_REPLIES: &[&[u8]] = &[
|
||||
b"\x1b[?997;2n",
|
||||
];
|
||||
|
||||
fn framed_events(host: HostProfile, pieces: &[&[u8]], idle_between: bool) -> String {
|
||||
framed_events_with(host, pieces, idle_between, false)
|
||||
/// A network hiccup between two reads of one sequence (SSH, slow links).
|
||||
const SPLIT_PAUSE_MS: i32 = 50;
|
||||
|
||||
fn framed_events(host: HostProfile, pieces: &[&[u8]], pause_ms: Option<i32>) -> String {
|
||||
framed_events_with(host, pieces, pause_ms, false)
|
||||
}
|
||||
|
||||
/// Feed `pieces` as separate reads. With `pause_ms`, the reads are that far
|
||||
/// apart: like the client loop, the framer is flushed only when the pause
|
||||
/// outlasts the wait it asked for.
|
||||
fn framed_events_with(
|
||||
host: HostProfile,
|
||||
pieces: &[&[u8]],
|
||||
idle_between: bool,
|
||||
pause_ms: Option<i32>,
|
||||
awaiting_replies: bool,
|
||||
) -> String {
|
||||
let mut framer = HerdrPath::fresh_framer(host);
|
||||
@@ -980,9 +988,18 @@ fn framed_events_with(
|
||||
framer.host_cell_size_query_sent();
|
||||
}
|
||||
let mut chunks = Vec::new();
|
||||
for piece in pieces {
|
||||
for (index, piece) in pieces.iter().enumerate() {
|
||||
chunks.extend(framer.push(piece));
|
||||
if idle_between {
|
||||
let Some(pause_ms) = pause_ms.filter(|_| index + 1 < pieces.len()) else {
|
||||
continue;
|
||||
};
|
||||
let mut waited = 0;
|
||||
for _ in 0..3 {
|
||||
let wait = framer.idle_flush_timeout_ms(true);
|
||||
if !framer.has_pending_input() || waited + wait > pause_ms {
|
||||
break;
|
||||
}
|
||||
waited += wait;
|
||||
chunks.extend(framer.flush_timeout());
|
||||
}
|
||||
}
|
||||
@@ -1006,11 +1023,11 @@ fn split_read_robustness() {
|
||||
let mut burst_mismatch = Vec::new();
|
||||
let mut idle_mismatch = Vec::new();
|
||||
for (host, bytes) in &corpus {
|
||||
let whole = framed_events(*host, &[bytes], false);
|
||||
let whole = framed_events(*host, &[bytes], None);
|
||||
for cut in 1..bytes.len() {
|
||||
splits += 1;
|
||||
let pieces = [&bytes[..cut], &bytes[cut..]];
|
||||
if framed_events(*host, &pieces, false) != whole {
|
||||
if framed_events(*host, &pieces, None) != whole {
|
||||
burst_mismatch.push(format!(
|
||||
"{}\t{}|{}",
|
||||
host.name(),
|
||||
@@ -1018,7 +1035,7 @@ fn split_read_robustness() {
|
||||
show(pieces[1])
|
||||
));
|
||||
}
|
||||
if framed_events(*host, &pieces, true) != whole {
|
||||
if framed_events(*host, &pieces, Some(SPLIT_PAUSE_MS)) != whole {
|
||||
idle_mismatch.push(format!(
|
||||
"{}\t{}|{}",
|
||||
host.name(),
|
||||
@@ -1032,21 +1049,22 @@ fn split_read_robustness() {
|
||||
let mut reply_burst_mismatch = Vec::new();
|
||||
let mut reply_idle_mismatch = Vec::new();
|
||||
for reply in HOST_REPLIES {
|
||||
let whole = framed_events_with(HostProfile::Kitty, &[reply], false, true);
|
||||
let whole = framed_events_with(HostProfile::Kitty, &[reply], None, true);
|
||||
for cut in 1..reply.len() {
|
||||
reply_splits += 1;
|
||||
let pieces = [&reply[..cut], &reply[cut..]];
|
||||
let line = format!("reply\t{}|{}", show(pieces[0]), show(pieces[1]));
|
||||
if framed_events_with(HostProfile::Kitty, &pieces, false, true) != whole {
|
||||
if framed_events_with(HostProfile::Kitty, &pieces, None, true) != whole {
|
||||
reply_burst_mismatch.push(line.clone());
|
||||
}
|
||||
if framed_events_with(HostProfile::Kitty, &pieces, true, true) != whole {
|
||||
if framed_events_with(HostProfile::Kitty, &pieces, Some(SPLIT_PAUSE_MS), true) != whole
|
||||
{
|
||||
reply_idle_mismatch.push(line);
|
||||
}
|
||||
}
|
||||
}
|
||||
println!(
|
||||
"\nhost replies: {} replies, {} splits\n burst (no pause): {} differ\n pause between: {} differ",
|
||||
"\nhost replies: {} replies, {} splits\n burst (no pause): {} differ\n {SPLIT_PAUSE_MS}ms pause: {} differ",
|
||||
HOST_REPLIES.len(),
|
||||
reply_splits,
|
||||
reply_burst_mismatch.len(),
|
||||
@@ -1055,7 +1073,7 @@ fn split_read_robustness() {
|
||||
|
||||
let pct = |bad: usize| 100.0 * (splits - bad) as f64 / splits.max(1) as f64;
|
||||
println!(
|
||||
"\nsplit reads: {} sequences, {} splits\n burst (no pause): {:.1}% identical ({} differ)\n pause between: {:.1}% identical ({} differ)",
|
||||
"\nsplit reads: {} sequences, {} splits\n burst (no pause): {:.1}% identical ({} differ)\n {SPLIT_PAUSE_MS}ms pause: {:.1}% identical ({} differ)",
|
||||
corpus.len(),
|
||||
splits,
|
||||
pct(burst_mismatch.len()),
|
||||
@@ -1087,9 +1105,8 @@ fn split_read_robustness() {
|
||||
idle_mismatch.len()
|
||||
);
|
||||
assert!(
|
||||
reply_idle_mismatch.len() <= REPLY_IDLE_MISMATCH_BASELINE,
|
||||
"host-reply split robustness regressed: {} > {REPLY_IDLE_MISMATCH_BASELINE}",
|
||||
reply_idle_mismatch.len()
|
||||
reply_idle_mismatch.is_empty(),
|
||||
"split host replies changed after a {SPLIT_PAUSE_MS}ms pause: {reply_idle_mismatch:?}"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
+109
-19
@@ -21,16 +21,20 @@ use crate::terminal_theme::{
|
||||
};
|
||||
|
||||
const ESC: u8 = 0x1b;
|
||||
#[cfg(unix)]
|
||||
#[cfg(any(unix, test))]
|
||||
pub(crate) const RAW_INPUT_IDLE_FLUSH_TIMEOUT_MS: i32 = 10;
|
||||
#[cfg(unix)]
|
||||
#[cfg(any(unix, test))]
|
||||
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)]
|
||||
#[cfg(any(unix, test))]
|
||||
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;
|
||||
/// Bytes already inside a sequence cannot be a key, so wait long enough for a
|
||||
/// split read over a slow link before giving up on them.
|
||||
#[cfg(any(unix, test))]
|
||||
pub(crate) const INCOMPLETE_SEQUENCE_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~";
|
||||
@@ -166,6 +170,12 @@ impl RawInputFramer {
|
||||
.with_vt_bytes(chunk),
|
||||
));
|
||||
}
|
||||
// The byte framer released this introducer as a key on its own.
|
||||
if is_alt_key_introducer(&chunk) {
|
||||
let text = std::str::from_utf8(&chunk).ok()?;
|
||||
return parse_terminal_key_sequence(text)
|
||||
.map(|key| RawInputEvent::Key(key.with_vt_bytes(chunk.clone())));
|
||||
}
|
||||
extract_one_event(&chunk).map(|(event, _consumed)| {
|
||||
tracing::debug!(raw_bytes = ?chunk, event = ?event, "raw input event parsed");
|
||||
event
|
||||
@@ -304,6 +314,44 @@ impl RawInputByteFramer {
|
||||
self.host_escape_disambiguation_active = active;
|
||||
}
|
||||
|
||||
/// Pending bytes that could still be a key on their own: a lone ESC or ESC
|
||||
/// plus one introducer. Legacy hosts send those for Escape and Alt+key, and
|
||||
/// host text bindings (e.g. Ghostty `text:\x1bO`) send them even when the
|
||||
/// host disambiguates escapes.
|
||||
#[cfg(any(unix, test))]
|
||||
fn pending_could_be_escape_key(&self) -> bool {
|
||||
is_escape_key_prefix(&self.buffer)
|
||||
}
|
||||
|
||||
/// How long to wait for more input before `flush_timeout`. Only bytes that
|
||||
/// could still be a key get the short keyboard window; anything already
|
||||
/// inside a sequence waits long enough for split reads over slow links.
|
||||
#[cfg(any(unix, test))]
|
||||
pub(crate) fn idle_flush_timeout_ms(&self, host_mouse_capture_active: bool) -> i32 {
|
||||
if self.buffer.is_empty() {
|
||||
return RAW_INPUT_IDLE_FLUSH_TIMEOUT_MS;
|
||||
}
|
||||
// A reply Herdr asked for can split right after its introducer.
|
||||
if self.awaiting_host_reply() && matches!(self.buffer.as_slice(), b"\x1b[" | b"\x1b]") {
|
||||
return INCOMPLETE_SEQUENCE_FLUSH_TIMEOUT_MS;
|
||||
}
|
||||
if self.pending_could_be_escape_key() {
|
||||
if !host_mouse_capture_active {
|
||||
return RAW_INPUT_IDLE_FLUSH_TIMEOUT_MS;
|
||||
}
|
||||
return match self.buffer.as_slice() {
|
||||
[ESC] => MOUSE_ACTIVE_ESCAPE_SEQUENCE_FLUSH_TIMEOUT_MS,
|
||||
// A mouse report split after ESC[ is still ambiguous with legacy Alt+[.
|
||||
b"\x1b[" => MOUSE_ACTIVE_CSI_INTRODUCER_FLUSH_TIMEOUT_MS,
|
||||
_ => RAW_INPUT_IDLE_FLUSH_TIMEOUT_MS,
|
||||
};
|
||||
}
|
||||
if self.buffer.first() == Some(&ESC) || starts_with_incomplete_utf8_char(&self.buffer) {
|
||||
return INCOMPLETE_SEQUENCE_FLUSH_TIMEOUT_MS;
|
||||
}
|
||||
RAW_INPUT_IDLE_FLUSH_TIMEOUT_MS
|
||||
}
|
||||
|
||||
/// 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 {
|
||||
@@ -314,22 +362,6 @@ impl RawInputByteFramer {
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
pub(crate) fn has_pending_lone_escape(&self) -> bool {
|
||||
self.buffer.as_slice() == [ESC]
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
pub(crate) fn has_pending_csi_introducer(&self) -> bool {
|
||||
self.buffer.as_slice() == b"\x1b["
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
pub(crate) fn has_pending_incomplete_mouse_sequence(&self) -> bool {
|
||||
starts_with_incomplete_sgr_mouse_sequence(&self.buffer)
|
||||
|| starts_with_incomplete_default_mouse_sequence(&self.buffer)
|
||||
}
|
||||
|
||||
#[cfg(any(windows, test))]
|
||||
pub(crate) fn has_pending_bracketed_paste(&self) -> bool {
|
||||
self.buffer.starts_with(BRACKETED_PASTE_START)
|
||||
@@ -392,8 +424,11 @@ impl RawInputByteFramer {
|
||||
#[cfg(not(unix))]
|
||||
let mouse_wait_served = false;
|
||||
#[cfg(unix)]
|
||||
// Prefixes already inside a sequence waited the long window; only the
|
||||
// ambiguous ESC / ESC[ prefixes need this extra mouse-tail hold.
|
||||
if !mouse_wait_served
|
||||
&& self.host_escape_disambiguation_active
|
||||
&& self.pending_could_be_escape_key()
|
||||
&& could_continue_as_mouse_report(&self.buffer)
|
||||
{
|
||||
self.awaiting_mouse_tail_after = Some(self.buffer.len());
|
||||
@@ -515,6 +550,26 @@ impl RawInputByteFramer {
|
||||
return chunks;
|
||||
}
|
||||
|
||||
// A legacy host sends Alt+[ and Alt+O as ESC plus that byte. When nothing
|
||||
// followed and no reply is expected, it was the key. String introducers
|
||||
// (OSC, DCS, APC, PM, SOS) also start host replies, so they keep the
|
||||
// control-string path and are never forwarded as Alt keys (#344).
|
||||
if is_alt_key_introducer(&self.buffer) && !self.awaiting_host_reply() {
|
||||
chunks.push(std::mem::take(&mut self.buffer));
|
||||
return chunks;
|
||||
}
|
||||
// A lone string introducer with no reply expected is a legacy Alt chord
|
||||
// that cannot be forwarded safely. Drop just these bytes; entering
|
||||
// control-string discard would swallow the user's next keystrokes.
|
||||
if is_escape_key_prefix(&self.buffer)
|
||||
&& self.buffer.len() == 2
|
||||
&& !self.awaiting_host_reply()
|
||||
{
|
||||
tracing::debug!(bytes = ?self.buffer, "dropping lone string introducer");
|
||||
self.buffer.clear();
|
||||
return chunks;
|
||||
}
|
||||
|
||||
if let Some(ControlString::Incomplete { family }) = control_string(&self.buffer) {
|
||||
tracing::debug!(
|
||||
len = self.buffer.len(),
|
||||
@@ -991,6 +1046,18 @@ fn starts_with_incomplete_host_cell_size_report(buffer: &[u8]) -> bool {
|
||||
&& !(height.is_some_and(<[u8]>::is_empty) && width.is_some())
|
||||
}
|
||||
|
||||
fn is_alt_key_introducer(buffer: &[u8]) -> bool {
|
||||
matches!(buffer, [ESC, b'[' | b'O'])
|
||||
}
|
||||
|
||||
fn is_escape_key_prefix(buffer: &[u8]) -> bool {
|
||||
match buffer {
|
||||
[ESC] => true,
|
||||
[ESC, introducer] => matches!(introducer, b'[' | b'O' | b'P' | b']' | b'X' | b'^' | b'_'),
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
fn control_string(buffer: &[u8]) -> Option<ControlString> {
|
||||
let family = match buffer.get(..2)? {
|
||||
b"\x1b]" => ControlStringFamily::Osc,
|
||||
@@ -1121,6 +1188,7 @@ fn could_continue_as_mouse_report(buffer: &[u8]) -> bool {
|
||||
}
|
||||
|
||||
#[cfg(any(unix, windows, test))]
|
||||
#[cfg(any(windows, test))]
|
||||
fn starts_with_incomplete_default_mouse_sequence(buffer: &[u8]) -> bool {
|
||||
buffer.starts_with(b"\x1b[M") && buffer.len() < 6
|
||||
}
|
||||
@@ -2632,6 +2700,7 @@ mod tests {
|
||||
#[test]
|
||||
fn raw_input_byte_framer_discards_split_control_string_after_timeout() {
|
||||
let mut framer = RawInputByteFramer::default();
|
||||
framer.host_color_query_sent();
|
||||
|
||||
assert!(framer.push(b"\x1b]").is_empty());
|
||||
assert!(framer.flush_timeout().is_empty());
|
||||
@@ -2642,6 +2711,7 @@ mod tests {
|
||||
#[test]
|
||||
fn raw_input_byte_framer_keeps_discarding_tail_across_timeout() {
|
||||
let mut framer = RawInputByteFramer::default();
|
||||
framer.host_color_query_sent();
|
||||
|
||||
assert!(framer.push(b"\x1b]").is_empty());
|
||||
assert!(framer.flush_timeout().is_empty());
|
||||
@@ -2654,6 +2724,7 @@ mod tests {
|
||||
#[test]
|
||||
fn raw_input_byte_framer_releases_discard_on_implausible_tail() {
|
||||
let mut framer = RawInputByteFramer::default();
|
||||
framer.host_color_query_sent();
|
||||
|
||||
assert!(framer.push(b"\x1b]").is_empty());
|
||||
assert!(framer.flush_timeout().is_empty());
|
||||
@@ -2662,6 +2733,25 @@ mod tests {
|
||||
assert_eq!(framer.push(b"b"), vec![b"b".to_vec()]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn csi_and_ss3_introducers_without_expected_reply_are_alt_keys() {
|
||||
for (bytes, code) in [
|
||||
(&b"\x1b["[..], KeyCode::Char('[')),
|
||||
(b"\x1bO", KeyCode::Char('O')),
|
||||
] {
|
||||
let mut framer = RawInputFramer::default();
|
||||
assert!(framer.push(bytes).is_empty());
|
||||
let events = framer.flush_timeout();
|
||||
assert!(
|
||||
matches!(&events[..], [RawInputEvent::Key(key)]
|
||||
if key.code == code && key.modifiers.contains(KeyModifiers::ALT)),
|
||||
"{bytes:?}: {events:?}"
|
||||
);
|
||||
// The next keystroke is not swallowed as a sequence tail.
|
||||
assert_eq!(framer.push(b"a").len(), 1, "{bytes:?}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_raw_input_bytes_sync_does_not_parse_incomplete_strings_as_alt_keys() {
|
||||
for bytes in [
|
||||
|
||||
Reference in New Issue
Block a user