From 295fa004d4fcbbcaaa17c065d7f71e45c8b481da Mon Sep 17 00:00:00 2001 From: felipe Date: Wed, 7 Oct 2026 19:51:03 -0300 Subject: [PATCH] refactor: centralize matches_key in keybindings and make char matching case-insensitive --- src/config/keybindings.rs | 47 +++++++++++++++++++++++++++ src/config/mod.rs | 2 +- src/gui/controller/branch_commits.rs | 3 +- src/gui/controller/branches.rs | 12 ++----- src/gui/controller/commit_files.rs | 16 ++------- src/gui/controller/commits.rs | 10 +----- src/gui/controller/diff_mode.rs | 15 ++------- src/gui/controller/files.rs | 14 ++------ src/gui/controller/reflog.rs | 10 +----- src/gui/controller/remote_branches.rs | 10 +----- src/gui/controller/remotes.rs | 10 +----- src/gui/controller/stash.rs | 10 +----- src/gui/controller/status.rs | 10 +----- src/gui/controller/tree.rs | 18 +--------- src/gui/mod.rs | 10 +----- 15 files changed, 69 insertions(+), 128 deletions(-) diff --git a/src/config/keybindings.rs b/src/config/keybindings.rs index f2983a2..8294411 100644 --- a/src/config/keybindings.rs +++ b/src/config/keybindings.rs @@ -467,6 +467,34 @@ pub fn parse_key(s: &str) -> Option { None } +/// Check if a `KeyEvent` matches the configured keybinding string. +/// Character keys are matched case-insensitively while preserving modifier equality. +/// Shifted punctuation arriving with SHIFT modifier is normalized when the expected binding has no modifiers. +pub fn matches_key(mut key: KeyEvent, binding: &str) -> bool { + if let Some(expected) = parse_key(binding) { + if expected.modifiers.is_empty() + && matches!(expected.code, KeyCode::Char(c) if c.is_ascii_punctuation()) + { + key.modifiers.remove(KeyModifiers::SHIFT); + } + let code_match = match (key.code, expected.code) { + (KeyCode::Char(c1), KeyCode::Char(c2)) => { + c1.to_ascii_lowercase() == c2.to_ascii_lowercase() + } + (code1, code2) => code1 == code2, + }; + code_match && key.modifiers == expected.modifiers + } else { + false + } +} + +impl KeybindingConfig { + pub fn matches_key(&self, key: KeyEvent, binding: &str) -> bool { + matches_key(key, binding) + } +} + #[cfg(test)] mod tests { use super::*; @@ -542,4 +570,23 @@ universal: assert_eq!(parsed.universal.tree_prev_sibling, "h"); assert_eq!(parsed.universal.tree_next_sibling, "l"); } + + #[test] + fn test_matches_key_case_insensitive() { + let key_upper = KeyEvent::new(KeyCode::Char('L'), KeyModifiers::CONTROL); + let key_lower = KeyEvent::new(KeyCode::Char('l'), KeyModifiers::CONTROL); + assert!(matches_key(key_upper, "")); + assert!(matches_key(key_lower, "")); + assert!(matches_key(key_upper, "")); + assert!(matches_key(key_lower, "")); + + let key_shift_d = KeyEvent::new(KeyCode::Char('D'), KeyModifiers::SHIFT); + let key_lower_d = KeyEvent::new(KeyCode::Char('d'), KeyModifiers::NONE); + assert!(matches_key(key_shift_d, "D")); + assert!(matches_key(key_lower_d, "d")); + assert!(!matches_key(key_lower_d, "D")); + + // Different modifiers must NOT match + assert!(!matches_key(key_lower, "l")); + } } diff --git a/src/config/mod.rs b/src/config/mod.rs index abfdd5b..c35c8f5 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -8,7 +8,7 @@ use std::path::PathBuf; use anyhow::Result; pub use app_state::AppState; -pub use keybindings::KeybindingConfig; +pub use keybindings::{KeybindingConfig, matches_key}; pub use theme::{COLOR_THEMES, ColorTheme, Theme, ThemeAppearance}; pub use user_config::UserConfig; diff --git a/src/gui/controller/branch_commits.rs b/src/gui/controller/branch_commits.rs index 86371d5..7aa194f 100644 --- a/src/gui/controller/branch_commits.rs +++ b/src/gui/controller/branch_commits.rs @@ -2,11 +2,12 @@ use anyhow::Result; use crossterm::event::{KeyCode, KeyEvent}; use crate::config::KeybindingConfig; +use crate::config::matches_key; use crate::gui::Gui; use crate::gui::context::ContextId; pub fn handle_key(gui: &mut Gui, key: KeyEvent, keybindings: &KeybindingConfig) -> Result<()> { - if super::commits::matches_key(key, &keybindings.commits.open_log_menu) { + if matches_key(key, &keybindings.commits.open_log_menu) { return super::commits::show_filtering_menu(gui); } diff --git a/src/gui/controller/branches.rs b/src/gui/controller/branches.rs index b7ae85c..91fe796 100644 --- a/src/gui/controller/branches.rs +++ b/src/gui/controller/branches.rs @@ -2,13 +2,13 @@ use anyhow::Result; use crossterm::event::{KeyCode, KeyEvent}; use crate::config::KeybindingConfig; -use crate::config::keybindings::parse_key; +use crate::config::matches_key; use crate::gui::Gui; use crate::gui::popup::{MenuItem, MessageKind, PopupState, make_textarea}; use crate::os::platform::Platform; pub fn handle_key(gui: &mut Gui, key: KeyEvent, keybindings: &KeybindingConfig) -> Result<()> { - if super::commits::matches_key(key, &keybindings.commits.open_log_menu) { + if matches_key(key, &keybindings.commits.open_log_menu) { return super::commits::show_filtering_menu(gui); } @@ -768,11 +768,3 @@ fn open_in_browser_menu(gui: &mut Gui) -> Result<()> { } Ok(()) } - -fn matches_key(key: KeyEvent, binding: &str) -> bool { - if let Some(expected) = parse_key(binding) { - key.code == expected.code && key.modifiers == expected.modifiers - } else { - false - } -} diff --git a/src/gui/controller/commit_files.rs b/src/gui/controller/commit_files.rs index bb0d35c..149b7d1 100644 --- a/src/gui/controller/commit_files.rs +++ b/src/gui/controller/commit_files.rs @@ -2,7 +2,7 @@ use anyhow::Result; use crossterm::event::{KeyCode, KeyEvent}; use crate::config::KeybindingConfig; -use crate::config::keybindings::parse_key; +use crate::config::matches_key; use crate::gui::Gui; use crate::gui::context::ContextId; use crate::gui::popup::{MenuItem, PopupState}; @@ -13,7 +13,7 @@ pub fn handle_key(gui: &mut Gui, key: KeyEvent, keybindings: &KeybindingConfig) if super::diff_grep::is_diff_grep_key(key) { return super::diff_grep::open_diff_grep_picker(gui); } - if super::commits::matches_key(key, &keybindings.commits.open_log_menu) { + if matches_key(key, &keybindings.commits.open_log_menu) { let selected = gui.context_mgr.selected_active(); let selected_path = if gui.show_commit_file_tree { gui.commit_file_tree_nodes @@ -48,9 +48,7 @@ pub fn handle_key(gui: &mut Gui, key: KeyEvent, keybindings: &KeybindingConfig) } // Folding is explicit; Enter focuses file or combined directory diffs. - if gui.show_commit_file_tree - && super::tree::matches_key(key, &keybindings.universal.fold_directory) - { + if gui.show_commit_file_tree && matches_key(key, &keybindings.universal.fold_directory) { if let Some(node) = gui .commit_file_tree_nodes .get(gui.context_mgr.selected_active()) @@ -316,11 +314,3 @@ pub fn update_commit_file_tree_state(gui: &mut Gui) { gui.context_mgr.commit_files_list_len_override = None; } } - -fn matches_key(key: KeyEvent, binding: &str) -> bool { - if let Some(expected) = parse_key(binding) { - key.code == expected.code && key.modifiers == expected.modifiers - } else { - false - } -} diff --git a/src/gui/controller/commits.rs b/src/gui/controller/commits.rs index b4f8f41..01ce93b 100644 --- a/src/gui/controller/commits.rs +++ b/src/gui/controller/commits.rs @@ -5,7 +5,7 @@ use anyhow::Result; use crossterm::event::KeyEvent; use crate::config::KeybindingConfig; -use crate::config::keybindings::parse_key; +use crate::config::matches_key; use crate::git::rebase::RebaseAction; use crate::gui::Gui; use crate::gui::popup::{ @@ -1501,14 +1501,6 @@ fn enter_interactive_rebase(gui: &mut Gui) -> Result<()> { Ok(()) } -pub(super) fn matches_key(key: KeyEvent, binding: &str) -> bool { - if let Some(expected) = parse_key(binding) { - key.code == expected.code && key.modifiers == expected.modifiers - } else { - false - } -} - #[cfg(test)] mod tests { use super::{branches_at_commit, checkout_branch_key, nonempty, reset_ref_display}; diff --git a/src/gui/controller/diff_mode.rs b/src/gui/controller/diff_mode.rs index c3a806d..40179cb 100644 --- a/src/gui/controller/diff_mode.rs +++ b/src/gui/controller/diff_mode.rs @@ -4,7 +4,7 @@ use std::sync::atomic::Ordering; use anyhow::Result; use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; -use crate::config::keybindings::parse_key; +use crate::config::matches_key; use crate::gui::modes::diff_mode::{CompareDiffSource, DiffModeFocus, DiffModeSelector}; use crate::gui::popup::{CommandEntry, CommandSection, MenuItem, PopupState}; use crate::gui::{DiffPayload, Gui, textarea_input}; @@ -419,9 +419,7 @@ fn handle_commit_files_key(gui: &mut Gui, key: KeyEvent) -> Result<()> { return Ok(()); } - if gui.diff_mode.show_tree - && super::tree::matches_key(key, &keybindings.universal.fold_directory) - { + if gui.diff_mode.show_tree && matches_key(key, &keybindings.universal.fold_directory) { if let Some(node) = gui .diff_mode .tree_nodes @@ -1197,17 +1195,10 @@ fn show_diff_mode_command_palette(gui: &mut Gui) { }; } -fn matches_key(key: KeyEvent, binding: &str) -> bool { - if let Some(expected) = parse_key(binding) { - key.code == expected.code && key.modifiers == expected.modifiers - } else { - false - } -} - #[cfg(test)] mod tests { use super::*; + use crate::config::keybindings::parse_key; use crate::config::{AppConfig, AppState, UserConfig}; use crate::git::GitCommands; use std::path::PathBuf; diff --git a/src/gui/controller/files.rs b/src/gui/controller/files.rs index e88ae89..96d4e6a 100644 --- a/src/gui/controller/files.rs +++ b/src/gui/controller/files.rs @@ -2,7 +2,7 @@ use anyhow::Result; use crossterm::event::{KeyCode, KeyEvent}; use crate::config::KeybindingConfig; -use crate::config::keybindings::parse_key; +use crate::config::matches_key; use crate::gui::Gui; use crate::gui::popup::{ CommitInputFocus, CommitInputKind, MenuItem, PopupState, make_commit_body_textarea, @@ -15,12 +15,12 @@ pub fn handle_key(gui: &mut Gui, key: KeyEvent, keybindings: &KeybindingConfig) if super::diff_grep::is_diff_grep_key(key) { return super::diff_grep::open_diff_grep_picker(gui); } - if super::commits::matches_key(key, &keybindings.commits.open_log_menu) { + if matches_key(key, &keybindings.commits.open_log_menu) { return super::commits::show_files_filtering_menu(gui); } // Folding is explicit; Enter focuses file or combined directory diffs. - if gui.show_file_tree && super::tree::matches_key(key, &keybindings.universal.fold_directory) { + if gui.show_file_tree && matches_key(key, &keybindings.universal.fold_directory) { if let Some(node) = gui.file_tree_nodes.get(gui.context_mgr.selected_active()) { if node.is_dir { let path = node.path.clone(); @@ -947,11 +947,3 @@ fn commit_with_editor(gui: &mut Gui) -> Result<()> { }; Ok(()) } - -fn matches_key(key: KeyEvent, binding: &str) -> bool { - if let Some(expected) = parse_key(binding) { - key.code == expected.code && key.modifiers == expected.modifiers - } else { - false - } -} diff --git a/src/gui/controller/reflog.rs b/src/gui/controller/reflog.rs index fe8200d..a121270 100644 --- a/src/gui/controller/reflog.rs +++ b/src/gui/controller/reflog.rs @@ -2,7 +2,7 @@ use anyhow::Result; use crossterm::event::KeyEvent; use crate::config::KeybindingConfig; -use crate::config::keybindings::parse_key; +use crate::config::matches_key; use crate::gui::Gui; use crate::gui::popup::{MenuItem, PopupState}; use crate::os::platform::Platform; @@ -187,11 +187,3 @@ fn copy_to_clipboard_menu(gui: &mut Gui) -> Result<()> { } Ok(()) } - -fn matches_key(key: KeyEvent, binding: &str) -> bool { - if let Some(expected) = parse_key(binding) { - key.code == expected.code && key.modifiers == expected.modifiers - } else { - false - } -} diff --git a/src/gui/controller/remote_branches.rs b/src/gui/controller/remote_branches.rs index 8a36d9c..ec36d7b 100644 --- a/src/gui/controller/remote_branches.rs +++ b/src/gui/controller/remote_branches.rs @@ -2,7 +2,7 @@ use anyhow::Result; use crossterm::event::{KeyCode, KeyEvent}; use crate::config::KeybindingConfig; -use crate::config::keybindings::parse_key; +use crate::config::matches_key; use crate::gui::Gui; use crate::gui::context::ContextId; use crate::gui::popup::PopupState; @@ -160,11 +160,3 @@ fn delete_remote_branch(gui: &mut Gui) -> Result<()> { } Ok(()) } - -fn matches_key(key: KeyEvent, binding: &str) -> bool { - if let Some(expected) = parse_key(binding) { - key.code == expected.code && key.modifiers == expected.modifiers - } else { - false - } -} diff --git a/src/gui/controller/remotes.rs b/src/gui/controller/remotes.rs index 34f5f31..a9fc326 100644 --- a/src/gui/controller/remotes.rs +++ b/src/gui/controller/remotes.rs @@ -2,7 +2,7 @@ use anyhow::Result; use crossterm::event::{KeyCode, KeyEvent}; use crate::config::KeybindingConfig; -use crate::config::keybindings::parse_key; +use crate::config::matches_key; use crate::gui::Gui; use crate::gui::popup::{MenuItem, MessageKind, PopupState, make_textarea}; @@ -480,11 +480,3 @@ fn show_pull_menu(gui: &mut Gui) -> Result<()> { }; Ok(()) } - -fn matches_key(key: KeyEvent, binding: &str) -> bool { - if let Some(expected) = parse_key(binding) { - key.code == expected.code && key.modifiers == expected.modifiers - } else { - false - } -} diff --git a/src/gui/controller/stash.rs b/src/gui/controller/stash.rs index a37d98f..425529e 100644 --- a/src/gui/controller/stash.rs +++ b/src/gui/controller/stash.rs @@ -2,7 +2,7 @@ use anyhow::Result; use crossterm::event::{KeyCode, KeyEvent}; use crate::config::KeybindingConfig; -use crate::config::keybindings::parse_key; +use crate::config::matches_key; use crate::gui::Gui; use crate::gui::popup::{MenuItem, PopupState, make_textarea}; @@ -214,11 +214,3 @@ fn rename_stash(gui: &mut Gui) -> Result<()> { } Ok(()) } - -fn matches_key(key: KeyEvent, binding: &str) -> bool { - if let Some(expected) = parse_key(binding) { - key.code == expected.code && key.modifiers == expected.modifiers - } else { - false - } -} diff --git a/src/gui/controller/status.rs b/src/gui/controller/status.rs index d1a4e18..aedf727 100644 --- a/src/gui/controller/status.rs +++ b/src/gui/controller/status.rs @@ -2,7 +2,7 @@ use anyhow::Result; use crossterm::event::{KeyCode, KeyEvent}; use crate::config::KeybindingConfig; -use crate::config::keybindings::parse_key; +use crate::config::matches_key; use crate::gui::Gui; use crate::gui::popup::{MenuItem, PopupState}; use crate::os::platform::Platform; @@ -156,11 +156,3 @@ fn open_menu(gui: &mut Gui) -> Result<()> { }; Ok(()) } - -fn matches_key(key: KeyEvent, binding: &str) -> bool { - if let Some(expected) = parse_key(binding) { - key.code == expected.code && key.modifiers == expected.modifiers - } else { - false - } -} diff --git a/src/gui/controller/tree.rs b/src/gui/controller/tree.rs index adeed34..de59dd3 100644 --- a/src/gui/controller/tree.rs +++ b/src/gui/controller/tree.rs @@ -1,25 +1,9 @@ //! Shared tree shortcuts for sidebar and diff-focused navigation. use crate::config::KeybindingConfig; -use crate::config::keybindings::parse_key; +pub use crate::config::matches_key; use crate::model::file_tree::{self, NavigableTreeNode}; use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; -/// Outer Option means the key is claimed; inner Option means a destination exists. -/// Boundary keys are consumed too, so they cannot fall through to unrelated actions. -pub fn matches_key(mut key: KeyEvent, binding: &str) -> bool { - let Some(expected) = parse_key(binding) else { - return false; - }; - // Shifted punctuation may arrive with SHIFT on real terminals. - // Do not strip CTRL/ALT or SHIFT for letters / explicit modifier bindings. - if expected.modifiers.is_empty() - && matches!(expected.code, KeyCode::Char(c) if c.is_ascii_punctuation()) - { - key.modifiers.remove(KeyModifiers::SHIFT); - } - key.code == expected.code && key.modifiers == expected.modifiers -} - pub fn destination( key: KeyEvent, kb: &KeybindingConfig, diff --git a/src/gui/mod.rs b/src/gui/mod.rs index cf297b9..da2778d 100644 --- a/src/gui/mod.rs +++ b/src/gui/mod.rs @@ -24,6 +24,7 @@ use ratatui::backend::CrosstermBackend; use ratatui::layout::Rect; use crate::config::keybindings::parse_key; +use crate::config::matches_key; use crate::config::{AppConfig, AppState}; use crate::git::{DEFAULT_COMMIT_LIMIT, GitCommands, HeadState, MODEL_PART_COUNT, ModelPart}; use crate::model::Model; @@ -9257,15 +9258,6 @@ fn read_clipboard() -> Option { .map(|o| String::from_utf8_lossy(&o.stdout).trim().to_string()) } -fn matches_key(key: KeyEvent, binding: &str) -> bool { - if let Some(expected) = parse_key(binding) { - // Compare code and modifiers, ignore kind/state - key.code == expected.code && key.modifiers == expected.modifiers - } else { - false - } -} - fn rect_contains(r: ratatui::layout::Rect, col: u16, row: u16) -> bool { col >= r.x && col < r.x + r.width && row >= r.y && row < r.y + r.height }