From e536bd8bd99de975cb2d15bb384e6e35f88ef88e Mon Sep 17 00:00:00 2001 From: Can Celik Date: Sun, 26 Jul 2026 04:22:05 +0300 Subject: [PATCH] fix: support non-us shifted keybindings (#1876) * fix: support non-us shifted keybindings refs #1870 * docs: require current pull request base --- AGENTS.md | 2 + docs/next/CHANGELOG.md | 2 +- 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 ++++++++++++ 8 files changed, 204 insertions(+), 47 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 9c8ce3b7..72c926d2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -76,6 +76,8 @@ Commit on the task branch in that worktree. For substantive feature and bug-fix work, default to opening a pull request instead of pushing `master` directly. Small, low-risk changes and documentation-only updates can use a lighter workflow when Can prefers it. +Immediately before opening a pull request, fetch `origin` and make sure the task branch is based on the current `origin/master`; rebase it when behind, then rerun relevant validation before pushing. If `master` advances while the pull request is under review and GitHub marks it behind, update the branch and repeat checks and bot review on the new head. + After opening or updating a pull request, monitor all checks to completion with `gh pr checks --watch` or an equivalent command. Treat Greptile and CodeRabbit as part of CI: wait for both to review the latest pushed commit, not only for the build and test jobs to pass. Evaluate every actionable finding. Fix findings you agree with and reply with the fix; reply inline with a concise technical reason when you disagree. After any fix, wait for CI and both review bots again on the new head. When the current pull request head is green and both bot reviews are complete, report that it is ready and stop. Never merge a pull request; Can performs the final merge. diff --git a/docs/next/CHANGELOG.md b/docs/next/CHANGELOG.md index 2622c30a..1e44e91d 100644 --- a/docs/next/CHANGELOG.md +++ b/docs/next/CHANGELOG.md @@ -80,7 +80,7 @@ ### Fixed - Collapsed Agent sidebar rows now follow the same ordering and click targets as the expanded panel, and their shortcut numbers are assigned by visible list position instead of repeating across workspaces. (#1168, #1344) -- Shifted indexed bindings such as `prefix+shift+1..9` now match terminals that report the corresponding punctuation characters. (#1184) +- On Kitty-keyboard hosts, prefix and navigate modes now request layout-aware input, so shifted bindings and indexed ranges such as `prefix+shift+1..9` match non-US keys while retaining legacy US punctuation support. (#1184, #1870) - Plugin-driven tab renames now immediately refresh tab-bar geometry and labels. (#1111, #1179, thanks @kovalov) - New tabs, splits, layouts, and workspaces configured to follow the foreground directory now start from the focused pane's current working directory. (#1245) - Amp, Codex, and Claude Code detection now recognizes current active-turn UI variants, including reordered Codex title spinners and Claude `/btw` turns. (#1208, #1281, #1366) 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 binding in &kb.switch_tab { - if trigger_matches(binding) { - if let Some(idx) = binding.matched_index(key) { - return Some(NavigateAction::SwitchTab(idx)); + 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_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(