diff --git a/src/client/shell/tests/input.rs b/src/client/shell/tests/input.rs index aef2c9f3a..7fef78664 100644 --- a/src/client/shell/tests/input.rs +++ b/src/client/shell/tests/input.rs @@ -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())); diff --git a/src/client/shell/tests/input_conformance.rs b/src/client/shell/tests/input_conformance.rs index fad1a7a6c..b8fd82986 100644 --- a/src/client/shell/tests/input_conformance.rs +++ b/src/client/shell/tests/input_conformance.rs @@ -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() { diff --git a/src/client/terminal_setup.rs b/src/client/terminal_setup.rs index a5787c75e..9c2ddf0e0 100644 --- a/src/client/terminal_setup.rs +++ b/src/client/terminal_setup.rs @@ -217,16 +217,16 @@ fn query_host_escape_disambiguation() -> (HostTerminalProbe, Vec) { } } - ( - 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() { diff --git a/src/input/lease.rs b/src/input/lease.rs index d145b9805..cae306643 100644 --- a/src/input/lease.rs +++ b/src/input/lease.rs @@ -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> { - 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)] diff --git a/src/pane/terminal.rs b/src/pane/terminal.rs index e2f46ee5f..4a5f91758 100644 --- a/src/pane/terminal.rs +++ b/src/pane/terminal.rs @@ -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!(