From e536bd8bd99de975cb2d15bb384e6e35f88ef88e Mon Sep 17 00:00:00 2001 From: Can Celik Date: Sun, 26 Jul 2026 04:22:05 +0300 Subject: fix: support non-us shifted keybindings (#1876) * fix: support non-us shifted keybindings refs #1870 * docs: require current pull request base --- src/app/input/navigate.rs | 136 +++++++++++++++++++++++++++++++++++----------- src/app/input/terminal.rs | 6 ++ src/app/mod.rs | 6 +- src/config/keybinds.rs | 42 +++++++++----- src/input/parse.rs | 16 ++++++ src/server/headless.rs | 41 ++++++++++++++ 6 files changed, 201 insertions(+), 46 deletions(-) (limited to 'src') diff --git a/src/app/input/navigate.rs b/src/app/input/navigate.rs index a5300461..6b578091 100644 --- a/src/app/input/navigate.rs +++ b/src/app/input/navigate.rs @@ -1131,8 +1131,29 @@ pub(crate) fn command_for_key( .cloned() } +fn unmodified_digit_for_key(key: TerminalKey) -> Option { + ('1'..='9').find(|digit| { + crate::config::terminal_key_matches_combo( + key, + ( + KeyCode::Char(*digit), + crossterm::event::KeyModifiers::empty(), + ), + ) + }) +} + #[cfg(test)] pub(super) fn handle_navigate_reserved_key(state: &mut AppState, key: TerminalKey) -> bool { + if let Some(c) = unmodified_digit_for_key(key) { + let idx = (c as usize) - ('1' as usize); + if let Some(ws_idx) = state.workspace_at_visible_position(idx) { + state.switch_workspace(ws_idx); + leave_navigate_mode(state); + } + return true; + } + let (code, modifiers) = crate::config::normalize_key_combo((key.code, key.modifiers)); if modifiers.is_empty() { match code { @@ -1143,14 +1164,6 @@ pub(super) fn handle_navigate_reserved_key(state: &mut AppState, key: TerminalKe } return true; } - KeyCode::Char(c @ '1'..='9') => { - let idx = (c as usize) - ('1' as usize); - if let Some(ws_idx) = state.workspace_at_visible_position(idx) { - state.switch_workspace(ws_idx); - leave_navigate_mode(state); - } - return true; - } KeyCode::Tab => { state.cycle_pane(false); return true; @@ -1205,6 +1218,12 @@ pub(super) fn handle_navigate_reserved_key(state: &mut AppState, key: TerminalKe } fn navigate_reserved_action_for_key(state: &AppState, key: TerminalKey) -> Option { + if let Some(c) = unmodified_digit_for_key(key) { + return Some(NavigateAction::SwitchWorkspace( + (c as usize) - ('1' as usize), + )); + } + let (code, modifiers) = crate::config::normalize_key_combo((key.code, key.modifiers)); if modifiers.is_empty() { match code { @@ -1217,11 +1236,6 @@ fn navigate_reserved_action_for_key(state: &AppState, key: TerminalKey) -> Optio .unwrap_or(state.selected), )); } - KeyCode::Char(c @ '1'..='9') => { - return Some(NavigateAction::SwitchWorkspace( - (c as usize) - ('1' as usize), - )); - } KeyCode::Tab => return Some(NavigateAction::CyclePaneNext), KeyCode::BackTab => return Some(NavigateAction::CyclePanePrevious), KeyCode::Left => return Some(NavigateAction::FocusPaneLeft), @@ -1367,29 +1381,37 @@ fn indexed_navigation_action( dispatch: BindingDispatch, ) -> Option { let kb = &state.keybinds; - let trigger_matches = |binding: &crate::config::IndexedKeybind| match dispatch { - BindingDispatch::Direct => binding.trigger.is_direct(), - BindingDispatch::Prefix => binding.trigger.is_prefix(), - }; + let actual_modifiers = crate::config::normalize_key_combo((key.code, key.modifiers)).1; + + for exact_modifiers in [true, false] { + let trigger_matches = |binding: &crate::config::IndexedKeybind| { + let dispatch_matches = match dispatch { + BindingDispatch::Direct => binding.trigger.is_direct(), + BindingDispatch::Prefix => binding.trigger.is_prefix(), + }; + let expected_modifiers = crate::config::normalize_key_combo(binding.trigger.combo()).1; + dispatch_matches && (actual_modifiers == expected_modifiers) == exact_modifiers + }; - for binding in &kb.switch_tab { - if trigger_matches(binding) { - if let Some(idx) = binding.matched_index(key) { - return Some(NavigateAction::SwitchTab(idx)); + for binding in &kb.switch_tab { + if trigger_matches(binding) { + if let Some(idx) = binding.matched_index(key) { + return Some(NavigateAction::SwitchTab(idx)); + } } } - } - for binding in &kb.switch_workspace { - if trigger_matches(binding) { - if let Some(idx) = binding.matched_index(key) { - return Some(NavigateAction::SwitchWorkspace(idx)); + for binding in &kb.switch_workspace { + if trigger_matches(binding) { + if let Some(idx) = binding.matched_index(key) { + return Some(NavigateAction::SwitchWorkspace(idx)); + } } } - } - for binding in &kb.focus_agent { - if trigger_matches(binding) { - if let Some(idx) = binding.matched_index(key) { - return Some(NavigateAction::FocusAgent(idx)); + for binding in &kb.focus_agent { + if trigger_matches(binding) { + if let Some(idx) = binding.matched_index(key) { + return Some(NavigateAction::FocusAgent(idx)); + } } } } @@ -2564,7 +2586,7 @@ last_pane = "prefix+tab" } #[test] - fn prefix_shift_indexed_workspace_shortcut_maps_shifted_symbol_key() { + fn prefix_shift_indexed_workspace_shortcut_maps_legacy_us_symbol_key() { let mut state = state_with_workspaces(&["one", "two"]); let config: Config = toml::from_str("[keys]\nswitch_workspace = \"prefix+shift+1..9\"\n").unwrap(); @@ -2579,6 +2601,42 @@ last_pane = "prefix+tab" assert_eq!(action, Some(NavigateAction::SwitchWorkspace(1))); } + #[test] + fn prefix_shift_indexed_workspace_shortcut_maps_non_us_number_rows() { + let mut state = state_with_workspaces(&["one", "two"]); + let config: Config = + toml::from_str("[keys]\nswitch_workspace = \"prefix+shift+1..9\"\n").unwrap(); + state.keybinds.switch_workspace = config.keybinds().switch_workspace; + + for key in [ + TerminalKey::new(KeyCode::Char('2'), KeyModifiers::SHIFT) + .with_shifted_codepoint('"' as u32), + TerminalKey::new(KeyCode::Char('é'), KeyModifiers::SHIFT) + .with_shifted_codepoint('2' as u32), + ] { + assert_eq!( + action_for_key(&state, key, BindingDispatch::Prefix), + Some(NavigateAction::SwitchWorkspace(1)) + ); + } + } + + #[test] + fn prefix_unshifted_indexed_shortcut_maps_shifted_french_number_row() { + let mut state = state_with_workspaces(&["one"]); + let config: Config = toml::from_str("[keys]\nswitch_tab = \"prefix+1..9\"\n").unwrap(); + state.keybinds.switch_tab = config.keybinds().switch_tab; + + let action = action_for_key( + &state, + TerminalKey::new(KeyCode::Char('é'), KeyModifiers::SHIFT) + .with_shifted_codepoint('2' as u32), + BindingDispatch::Prefix, + ); + + assert_eq!(action, Some(NavigateAction::SwitchTab(1))); + } + #[test] fn literal_symbol_binding_takes_precedence_over_shifted_indexed_alias() { let mut state = state_with_workspaces(&["one", "two"]); @@ -2784,6 +2842,20 @@ command = "printf literal > '{}'" assert_eq!(app.state.mode, Mode::Navigate); } + #[test] + fn app_navigate_mode_maps_french_number_row_to_workspace() { + let mut app = app_with_test_workspaces(&["one", "two"]); + app.state.mode = Mode::Navigate; + + app.handle_navigate_key( + TerminalKey::new(KeyCode::Char('é'), KeyModifiers::SHIFT) + .with_shifted_codepoint('2' as u32), + ); + + assert_eq!(app.state.active, Some(1)); + assert_eq!(app.state.mode, Mode::Terminal); + } + #[test] fn app_navigate_mode_workspace_keys_are_configurable() { let mut app = app_with_test_workspaces(&["one", "two"]); diff --git a/src/app/input/terminal.rs b/src/app/input/terminal.rs index d862702f..a6c3abcb 100644 --- a/src/app/input/terminal.rs +++ b/src/app/input/terminal.rs @@ -264,6 +264,12 @@ impl App { } pub(crate) fn host_keyboard_report_all_requested(&self) -> bool { + if self.state.popup_pane.is_none() + && matches!(self.state.mode, Mode::Prefix | Mode::Navigate) + { + return true; + } + let runtime = if self.state.popup_pane.is_some() { self.popup_runtime() } else if self.state.mode == Mode::Terminal { diff --git a/src/app/mod.rs b/src/app/mod.rs index e9aa24b6..dc6de136 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -5064,7 +5064,7 @@ last_pane = "prefix+tab" } #[tokio::test] - async fn host_report_all_follows_the_focused_terminal_protocol() { + async fn host_report_all_follows_terminal_protocol_and_command_modes() { let mut app = test_app(); let mut workspace = Workspace::test_new("test"); let focused = workspace.focused_pane_id().unwrap(); @@ -5099,6 +5099,10 @@ last_pane = "prefix+tab" assert!(app.state.focus_pane_in_workspace(0, focused)); app.state.mode = Mode::Prefix; + assert!(app.host_keyboard_report_all_requested()); + app.state.mode = Mode::Navigate; + assert!(app.host_keyboard_report_all_requested()); + app.state.mode = Mode::RenameWorkspace; assert!(!app.host_keyboard_report_all_requested()); } diff --git a/src/config/keybinds.rs b/src/config/keybinds.rs index 29b53355..02046ee3 100644 --- a/src/config/keybinds.rs +++ b/src/config/keybinds.rs @@ -264,20 +264,15 @@ pub struct IndexedKeybind { impl IndexedKeybind { pub fn matched_index(&self, key: TerminalKey) -> Option { - let key_number = match key.code { - KeyCode::Char(c @ '1'..='9') => c, - KeyCode::Char(c) => { - let number = shifted_number_symbol(c)?; - if !indexed_shifted_number_matches(key, self.trigger.combo(), number) { - return None; - } - number - } - _ => return None, + let combo = self.trigger.combo(); + let (expected_code, _) = normalize_key_combo(combo); + let KeyCode::Char(key_number @ '1'..='9') = expected_code else { + return None; }; - let legacy_shifted_number = - matches!(key.code, KeyCode::Char(c) if shifted_number_symbol(c) == Some(key_number)); - if terminal_key_matches_combo(key, self.trigger.combo()) || legacy_shifted_number { + let legacy_shifted_number = matches!(key.code, KeyCode::Char(c) + if shifted_number_symbol(c) == Some(key_number) + && indexed_shifted_number_matches(key, combo, key_number)); + if terminal_key_matches_combo(key, combo) || legacy_shifted_number { Some((key_number as usize) - ('1' as usize)) } else { None @@ -1650,6 +1645,27 @@ close_tab = "X" )); } + #[test] + fn unicode_prefix_bindings_match_non_us_keys() { + for ch in ['ğ', 'ç', 'ş', 'ı', 'é', 'ø'] { + let bindings = ActionKeybinds::prefix(&ch.to_string()); + assert!(bindings + .matches_prefix_key(TerminalKey::new(KeyCode::Char(ch), KeyModifiers::empty(),))); + } + } + + #[test] + fn shifted_unicode_prefix_bindings_match_layout_aware_input() { + for (base, shifted) in [('ğ', 'Ğ'), ('ç', 'Ç'), ('ş', 'Ş'), ('ı', 'I'), ('ø', 'Ø')] + { + let bindings = ActionKeybinds::prefix(&format!("shift+{base}")); + assert!(bindings.matches_prefix_key( + TerminalKey::new(KeyCode::Char(base), KeyModifiers::SHIFT) + .with_shifted_codepoint(shifted as u32) + )); + } + } + #[test] fn shifted_letter_binding_matches_uppercase_key_event() { let bindings = ActionKeybinds::prefix("shift+n"); diff --git a/src/input/parse.rs b/src/input/parse.rs index 36de52e8..e4c554f8 100644 --- a/src/input/parse.rs +++ b/src/input/parse.rs @@ -600,6 +600,22 @@ mod tests { assert_eq!(key.shifted_codepoint, Some('L' as u32)); } + #[test] + fn parse_kitty_sequence_preserves_non_us_shift_pairs() { + for (sequence, base, shifted) in [ + ("\x1b[50:34;2:1u", '2', '"'), + ("\x1b[38:49;2:1u", '&', '1'), + ("\x1b[305:73;2:1u", 'ı', 'I'), + ("\x1b[287:286;2:1u", 'ğ', 'Ğ'), + ] { + let key = parse_terminal_key_sequence(sequence).unwrap(); + assert_eq!(key.code, KeyCode::Char(base)); + assert_eq!(key.modifiers, KeyModifiers::SHIFT); + assert_eq!(key.kind, crossterm::event::KeyEventKind::Press); + assert_eq!(key.shifted_codepoint, Some(shifted as u32)); + } + } + #[test] fn parse_kitty_sequence_with_associated_emoji_text() { let key = parse_terminal_key_sequence("\x1b[128512;1;128512u").unwrap(); diff --git a/src/server/headless.rs b/src/server/headless.rs index 0b25dd97..c1644dcd 100644 --- a/src/server/headless.rs +++ b/src/server/headless.rs @@ -8424,6 +8424,46 @@ next_tab = "" )); } + #[tokio::test] + async fn command_mode_updates_headless_client_keyboard_flags() { + let mut server = test_headless_server(); + let (client_tx, client_control_rx, _client_rx) = test_client_writer(); + server.clients.insert( + 1, + ClientConnection::new( + (80, 24), + crate::kitty_graphics::HostCellSize::default(), + crate::terminal_theme::TerminalTheme::default(), + None, + 1, + RenderEncoding::SemanticFrame, + Some(client_tx), + ), + ); + + server.app.state.mode = crate::app::Mode::Prefix; + server.stream_host_keyboard_enhancement_flags(); + assert!(matches!( + read_server_message( + client_control_rx + .recv_timeout(Duration::from_millis(100)) + .expect("command-mode keyboard enhancement message") + ), + ServerMessage::KittyKeyboardReportAll { enabled: true } + )); + + server.app.state.mode = crate::app::Mode::Terminal; + server.stream_host_keyboard_enhancement_flags(); + assert!(matches!( + read_server_message( + client_control_rx + .recv_timeout(Duration::from_millis(100)) + .expect("IME-compatible keyboard enhancement message") + ), + ServerMessage::KittyKeyboardReportAll { enabled: false } + )); + } + #[tokio::test] async fn focused_report_all_pane_updates_headless_client_keyboard_flags() { let mut server = test_headless_server(); @@ -8456,6 +8496,7 @@ next_tab = "" )); assert!(server.app.close_popup_pane()); + server.app.state.mode = crate::app::Mode::Terminal; server.stream_host_keyboard_enhancement_flags(); assert!(matches!( read_server_message( -- cgit v1.2.3