mirror of
https://github.com/herdrdev/herdr.git
synced 2026-11-06 00:06:18 +00:00
fix: address review findings on super+enter, held key releases and the pixel mouse probe
This commit is contained in:
@@ -401,6 +401,28 @@ fn shifted_punctuation_release_after_shift_reaches_the_pane() {
|
||||
assert!(pane_key_events(&ambiguous).is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unmatched_release_never_takes_a_held_key_while_herdr_owns_another_press() {
|
||||
use crate::protocol::{ClientKeyCode, ClientKeyKind};
|
||||
|
||||
let mut state = ClientShellState::new(ClientShellConfig::from_config(&Config::default()));
|
||||
state.set_host_reports_key_releases(true);
|
||||
state.set_snapshot(Box::new(snapshot()));
|
||||
state.set_pane_surface(surface());
|
||||
|
||||
// Hold `a` in the pane, then press the prefix: Herdr owns that press.
|
||||
let _ = state.handle_input_bytes(b"a");
|
||||
let _ = state.handle_input_bytes(b"\x1b[98;5u");
|
||||
// A release the leases cannot name must not be charged to the held `a`.
|
||||
let stray = state.handle_input_bytes(b"\x1b[47;1:3u");
|
||||
assert!(pane_key_events(&stray).is_empty());
|
||||
let release = state.handle_input_bytes(b"\x1b[97;1:3u");
|
||||
assert_eq!(
|
||||
pane_key_events(&release),
|
||||
[(ClientKeyCode::Char('a'), ClientKeyKind::Release)]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn physical_release_uses_the_leased_press_code_with_current_modifiers() {
|
||||
let mut state = ClientShellState::new(ClientShellConfig::from_config(&Config::default()));
|
||||
|
||||
@@ -472,10 +472,11 @@ fn run_keyboard_conformance() -> Report {
|
||||
.fold(0, |acc, (_, (bit, _))| acc | bit);
|
||||
let host_bytes = host_oracle.keystroke(*def, mods);
|
||||
let (mut press, release) = direct_oracle.keystroke(*def, mods);
|
||||
if let Some(classic) = legacy_shell_exception(pane_mode, def.name, mods) {
|
||||
press = classic;
|
||||
} else if is_legacy_pane(pane_mode) && mods & ghostty::MOD_SUPER != 0 {
|
||||
if is_legacy_pane(pane_mode) && mods & ghostty::MOD_SUPER != 0 {
|
||||
press = Vec::new();
|
||||
} else if let Some(classic) = legacy_shell_exception(pane_mode, def.name, mods)
|
||||
{
|
||||
press = classic;
|
||||
}
|
||||
// A host that never reports releases gives nobody a release to forward.
|
||||
let expected = if host_bytes.1.is_empty() {
|
||||
|
||||
@@ -217,16 +217,16 @@ fn query_host_escape_disambiguation() -> (HostTerminalProbe, Vec<u8>) {
|
||||
}
|
||||
}
|
||||
|
||||
(
|
||||
HostTerminalProbe {
|
||||
escape_disambiguation: host_escape_disambiguation_confirmed(&responses),
|
||||
sgr_pixel_mouse: responses
|
||||
.primary_device_attributes
|
||||
.then_some(responses.sgr_pixel_mouse)
|
||||
.flatten(),
|
||||
},
|
||||
buffered_input,
|
||||
)
|
||||
(host_terminal_probe(&responses), buffered_input)
|
||||
}
|
||||
|
||||
#[cfg(not(windows))]
|
||||
fn host_terminal_probe(responses: &HostKeyboardProbeResponses) -> HostTerminalProbe {
|
||||
HostTerminalProbe {
|
||||
escape_disambiguation: host_escape_disambiguation_confirmed(responses),
|
||||
// An explicit mode answer stands even if the probe timed out later.
|
||||
sgr_pixel_mouse: responses.sgr_pixel_mouse,
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(windows))]
|
||||
@@ -934,6 +934,17 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(windows))]
|
||||
#[test]
|
||||
fn host_probe_keeps_a_pixel_mouse_answer_without_device_attributes() {
|
||||
let mut buffered = b"\x1b[?1016;0$y".to_vec();
|
||||
let mut responses = HostKeyboardProbeResponses::default();
|
||||
consume_host_keyboard_probe_responses(&mut buffered, &mut responses);
|
||||
|
||||
assert!(!responses.primary_device_attributes);
|
||||
assert_eq!(host_terminal_probe(&responses).sgr_pixel_mouse, Some(false));
|
||||
}
|
||||
|
||||
#[cfg(not(windows))]
|
||||
#[test]
|
||||
fn host_keyboard_probe_preserves_typed_input_before_responses() {
|
||||
|
||||
+15
-12
@@ -175,25 +175,28 @@ where
|
||||
allowed
|
||||
}
|
||||
|
||||
/// The forwarded text press from `source` when it is the only one held.
|
||||
/// The forwarded text press from `source` when it is the only key held.
|
||||
/// A text press ("?") names a character, not a key, so its release ("/"
|
||||
/// after Shift was let go) cannot be matched by identity. With a single
|
||||
/// candidate the match is certain; with several, nothing is taken.
|
||||
/// after Shift was let go) cannot be matched by identity. When it is the
|
||||
/// only tracked key the match is certain; with any other press held,
|
||||
/// forwarded or consumed by Herdr, nothing is taken.
|
||||
pub(crate) fn remove_sole_text_press(
|
||||
&mut self,
|
||||
source: Source,
|
||||
) -> Option<InputLease<Context, Target>> {
|
||||
let mut candidates = self.leases.iter().filter(|(lease_key, lease)| {
|
||||
lease_key.source == source
|
||||
&& matches!(lease, InputLease::Forwarded(lease)
|
||||
if lease.key.generated_text.is_some()
|
||||
&& lease.key.physical_key_id().is_none())
|
||||
});
|
||||
let (only, None) = (candidates.next(), candidates.next()) else {
|
||||
let mut held = self
|
||||
.leases
|
||||
.iter()
|
||||
.filter(|(lease_key, _)| lease_key.source == source);
|
||||
let (Some((lease_key, lease)), None) = (held.next(), held.next()) else {
|
||||
return None;
|
||||
};
|
||||
let lease_key = *only?.0;
|
||||
self.leases.remove(&lease_key)
|
||||
let is_text_press = matches!(lease, InputLease::Forwarded(lease)
|
||||
if lease.key.generated_text.is_some() && lease.key.physical_key_id().is_none());
|
||||
let lease_key = *lease_key;
|
||||
is_text_press
|
||||
.then(|| self.leases.remove(&lease_key))
|
||||
.flatten()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
+15
-8
@@ -2054,17 +2054,17 @@ impl GhosttyPaneTerminal {
|
||||
return Vec::new();
|
||||
};
|
||||
let negotiated_nothing = keyboard_negotiated_nothing(&core.terminal);
|
||||
drop(core);
|
||||
// Before the shell table: Super+Enter must not become a plain Enter.
|
||||
if negotiated_nothing && legacy_super_chord(&key) {
|
||||
debug!(code = ?key.code, "super chord in a pane without keyboard protocol; not forwarded");
|
||||
return Vec::new();
|
||||
}
|
||||
let key = if negotiated_nothing {
|
||||
legacy_shell_key(key)
|
||||
} else {
|
||||
key
|
||||
};
|
||||
|
||||
drop(core);
|
||||
if negotiated_nothing && legacy_super_chord(&key) {
|
||||
debug!(code = ?key.code, "super chord in a pane without keyboard protocol; not forwarded");
|
||||
return Vec::new();
|
||||
}
|
||||
let Some(event) = ghostty_key_event_from_terminal_key(&key, negotiated_nothing) else {
|
||||
debug!(code = ?key.code, modifiers = ?key.modifiers, "key has no libghostty equivalent; not forwarded");
|
||||
return Vec::new();
|
||||
@@ -4786,7 +4786,11 @@ mod tests {
|
||||
KeyModifiers::ALT | KeyModifiers::CONTROL | KeyModifiers::SUPER,
|
||||
] {
|
||||
let key = crate::input::TerminalKey::new(KeyCode::Enter, modifiers);
|
||||
let expected = if modifiers.contains(KeyModifiers::ALT) {
|
||||
// Super chords never reach a plain shell, so Cmd+Enter cannot run
|
||||
// the command line.
|
||||
let expected = if modifiers.contains(KeyModifiers::SUPER) {
|
||||
b"".as_slice()
|
||||
} else if modifiers.contains(KeyModifiers::ALT) {
|
||||
b"\x1b\r".as_slice()
|
||||
} else {
|
||||
b"\r".as_slice()
|
||||
@@ -4821,7 +4825,8 @@ mod tests {
|
||||
let pane = GhosttyPaneTerminal::new(terminal, tx.clone()).unwrap();
|
||||
let pane_id = PaneId::from_raw(1);
|
||||
// Like Ghostty, modifyOtherKeys level 1 behaves as if nothing was negotiated.
|
||||
let legacy = ["\r", "\r", "\r", "\x1b\r"];
|
||||
// Super+Enter is not forwarded to panes without a keyboard protocol.
|
||||
let legacy = ["\r", "\r", "", "\x1b\r"];
|
||||
let mode_two = [
|
||||
"\x1b[27;2;13~",
|
||||
"\x1b[27;5;13~",
|
||||
@@ -4977,9 +4982,11 @@ mod tests {
|
||||
#[test]
|
||||
fn super_chords_reach_only_panes_that_negotiated_a_keyboard_protocol() {
|
||||
// Super+Space from a Kitty host (#4356) and Cmd+C (#3710).
|
||||
// Super+Enter must not become a plain Enter that runs the command line.
|
||||
for (host_bytes, kitty) in [
|
||||
("\x1b[32;9u", &b"\x1b[32;9u"[..]),
|
||||
("\x1b[99;9u", b"\x1b[99;9u"),
|
||||
("\x1b[13;9u", b"\x1b[13;9u"),
|
||||
] {
|
||||
let key = crate::input::parse_terminal_key_sequence(host_bytes).expect("kitty key");
|
||||
assert!(
|
||||
|
||||
Reference in New Issue
Block a user