From a1bef28da322bdb6b0ce54046acdebf089a80645 Mon Sep 17 00:00:00 2001 From: Ben Kunkle Date: Mon, 31 Mar 2025 18:31:01 -0400 Subject: [PATCH] keymap: Allow upper-case keys in keybinds (#27813) Reverts the error behavior introduced in #27558. Upper-case keys in keybindings no longer generate errors, instead they are transformed into `shift-{KEY}` e.g. `ctrl-N` becomes `ctrl-shift-n` The behavior introduced in #27558 where "special" keys such as function keys, `control`, `shift`, etc. Are parsed case-insensitively is preserved. Release Notes: - Improved how upper-case characters are handled in keybinds. "special" keys such as the function keys, `control`, `shift`, etc. are now parsed case-insensitively, so for example `F8`, `CTRL`, `SHIFT` are now acceptable alternatives to `f8`, `ctrl`, and `shift` when declaring keybindings. Additionally, upper-case (ascii) characters will now be converted explicitly to `shift` + the lowercase version of the character, to match the Vim behavior. NOTE: Release notes above should replace the release notes from #27558 --- crates/editor/src/test/editor_test_context.rs | 2 +- crates/gpui/src/app/test_context.rs | 8 +--- crates/gpui/src/platform/keystroke.rs | 40 +++---------------- crates/terminal/src/mappings/keys.rs | 4 +- 4 files changed, 11 insertions(+), 43 deletions(-) diff --git a/crates/editor/src/test/editor_test_context.rs b/crates/editor/src/test/editor_test_context.rs index 2a2712bf30..109f76a5e0 100644 --- a/crates/editor/src/test/editor_test_context.rs +++ b/crates/editor/src/test/editor_test_context.rs @@ -240,7 +240,7 @@ impl EditorTestContext { // unlike cx.simulate_keystrokes(), this does not run_until_parked // so you can use it to test detailed timing pub fn simulate_keystroke(&mut self, keystroke_text: &str) { - let keystroke = Keystroke::parse_case_insensitive(keystroke_text).unwrap(); + let keystroke = Keystroke::parse(keystroke_text).unwrap(); self.cx.dispatch_keystroke(self.window, keystroke); } diff --git a/crates/gpui/src/app/test_context.rs b/crates/gpui/src/app/test_context.rs index e639da7654..be36324c0f 100644 --- a/crates/gpui/src/app/test_context.rs +++ b/crates/gpui/src/app/test_context.rs @@ -399,7 +399,7 @@ impl TestAppContext { pub fn simulate_keystrokes(&mut self, window: AnyWindowHandle, keystrokes: &str) { for keystroke in keystrokes .split(' ') - .map(Keystroke::parse_case_insensitive) + .map(Keystroke::parse) .map(Result::unwrap) { self.dispatch_keystroke(window, keystroke); @@ -413,11 +413,7 @@ impl TestAppContext { /// will type abc into your current editor /// This will also run the background executor until it's parked. pub fn simulate_input(&mut self, window: AnyWindowHandle, input: &str) { - for keystroke in input - .split("") - .map(Keystroke::parse_case_insensitive) - .map(Result::unwrap) - { + for keystroke in input.split("").map(Keystroke::parse).map(Result::unwrap) { self.dispatch_keystroke(window, keystroke); } diff --git a/crates/gpui/src/platform/keystroke.rs b/crates/gpui/src/platform/keystroke.rs index 67c8b64c14..e2e977539f 100644 --- a/crates/gpui/src/platform/keystroke.rs +++ b/crates/gpui/src/platform/keystroke.rs @@ -42,9 +42,9 @@ impl Display for InvalidKeystrokeError { } /// Sentence explaining what keystroke parser expects, starting with "Expected ..." -pub const KEYSTROKE_PARSE_EXPECTED_MESSAGE: &str = "Expected a sequence of lowercase modifiers \ +pub const KEYSTROKE_PARSE_EXPECTED_MESSAGE: &str = "Expected a sequence of modifiers \ (`ctrl`, `alt`, `shift`, `fn`, `cmd`, `super`, or `win`) \ - followed by a lowercase key, separated by `-`."; + followed by a key, separated by `-`."; impl Keystroke { /// When matching a key we cannot know whether the user intended to type @@ -81,28 +81,6 @@ impl Keystroke { /// secondary means "cmd" on macOS and "ctrl" on other platforms /// when matching a key with an key_char set will be matched without it. pub fn parse(source: &str) -> std::result::Result { - return Self::parse_impl(source, true); - } - - /// Parse a keystroke case-insensitively. This means - /// keystrokes like `ctrl-T` will not be rejected. - /// Useful in tests to allow more concise keystroke inputs, - /// e.g., `simulate_keystrokes("ctrl-T")` instead of `simulate_keystrokes("ctrl-shift-t")`. - /// This also allows `simulate_input` style functions to support capital letters, - /// e.g., `simulate_input("Title Case")` can work by just parsing each character as a keystroke - /// and dispatching it, instead of needing to parse something like - /// `simulate_input("shift-title shift-case")`. - #[cfg(any(test, feature = "test-support"))] - pub fn parse_case_insensitive( - source: &str, - ) -> std::result::Result { - return Self::parse_impl(source, false); - } - - fn parse_impl( - source: &str, - case_sensitive: bool, - ) -> std::result::Result { let mut control = false; let mut alt = false; let mut shift = false; @@ -166,16 +144,10 @@ impl Keystroke { } if component.len() == 1 && component.as_bytes()[0].is_ascii_uppercase() { - if case_sensitive { - return Err(InvalidKeystrokeError { - keystroke: source.to_owned(), - }); - } else { - // Convert to shift + lowercase char if parsing case insensitively - shift = true; - key_str.make_ascii_lowercase(); - } - } else if case_sensitive { + // Convert to shift + lowercase char + shift = true; + key_str.make_ascii_lowercase(); + } else { // convert ascii chars to lowercase so that named keys like "tab" and "enter" // are accepted case insensitively and stored how we expect so they are matched properly key_str.make_ascii_lowercase() diff --git a/crates/terminal/src/mappings/keys.rs b/crates/terminal/src/mappings/keys.rs index ef1b3464af..785cfaeacc 100644 --- a/crates/terminal/src/mappings/keys.rs +++ b/crates/terminal/src/mappings/keys.rs @@ -398,7 +398,7 @@ mod test { false ), to_esc_str( - &Keystroke::parse_case_insensitive(&format!("ctrl-{}", upper)).unwrap(), + &Keystroke::parse(&format!("ctrl-{}", upper)).unwrap(), &mode, false ), @@ -415,7 +415,7 @@ mod test { for character in ascii_printable { assert_eq!( to_esc_str( - &Keystroke::parse_case_insensitive(&format!("alt-{}", character)).unwrap(), + &Keystroke::parse(&format!("alt-{}", character)).unwrap(), &TermMode::NONE, true )