keymap_editor: Fix incorrect keystroke being reported (#36998)

张小白 created

This PR fixes two bugs and also changes one behavior in the **Keymap
Editor**.

As shown in the video, when I press `ctrl-shift-2` in the Keymap Editor,
the first keystroke is displayed as `ctrl-shift-@`, which is incorrect.
On macOS and Linux, it should be `ctrl-@`.



https://github.com/user-attachments/assets/69cfcfa0-b422-45d6-8e69-80f8608180fd



Also, after pressing `ctrl-shift-2` and then releasing `2` and `ctrl`, a
`shift` keystroke was incorrectly added.



https://github.com/user-attachments/assets/892124fd-847d-4fde-9b20-a27ba49ac934



Now, when you enter a sequence like `+ctrl+alt-alt+f` in the Keymap
Editor, it will output `ctrl-f` instead of `ctrl-alt-f`, matching VS
Code’s behavior.


Release Notes:

- Fixed incorrect keystroke reporting in the Keymap Editor.

Change summary

crates/gpui/src/window.rs                               |   7 
crates/settings_ui/src/ui_components/keystroke_input.rs | 149 +++++++++-
2 files changed, 137 insertions(+), 19 deletions(-)

Detailed changes

crates/gpui/src/window.rs 🔗

@@ -4578,6 +4578,13 @@ impl Window {
         }
         None
     }
+
+    /// For testing: set the current modifier keys state.
+    /// This does not generate any events.
+    #[cfg(any(test, feature = "test-support"))]
+    pub fn set_modifiers(&mut self, modifiers: Modifiers) {
+        self.modifiers = modifiers;
+    }
 }
 
 // #[derive(Clone, Copy, Eq, PartialEq, Hash)]

crates/settings_ui/src/ui_components/keystroke_input.rs 🔗

@@ -303,17 +303,12 @@ impl KeystrokeInput {
             return;
         }
 
-        let mut keystroke =
+        let keystroke =
             KeybindingKeystroke::new(keystroke.clone(), false, cx.keyboard_mapper().as_ref());
         if let Some(last) = self.keystrokes.last()
             && last.display_key.is_empty()
             && (!self.search || self.previous_modifiers.modified())
         {
-            let display_key = keystroke.display_key.clone();
-            let inner_key = keystroke.inner.key.clone();
-            keystroke = last.clone();
-            keystroke.display_key = display_key;
-            keystroke.inner.key = inner_key;
             self.keystrokes.pop();
         }
 
@@ -329,18 +324,19 @@ impl KeystrokeInput {
             return;
         }
 
-        self.keystrokes.push(keystroke.clone());
+        self.keystrokes.push(keystroke);
         self.keystrokes_changed(cx);
 
+        // The reason we use the real modifiers from the window instead of the keystroke's modifiers
+        // is that for keystrokes like `ctrl-$` the modifiers reported by keystroke is `ctrl` which
+        // is wrong, it should be `ctrl-shift`. The window's modifiers are always correct.
+        let real_modifiers = window.modifiers();
         if self.search {
-            self.previous_modifiers = keystroke.display_modifiers;
+            self.previous_modifiers = real_modifiers;
             return;
         }
-        if self.keystrokes.len() < Self::KEYSTROKE_COUNT_MAX
-            && keystroke.display_modifiers.modified()
-        {
-            self.keystrokes
-                .push(Self::dummy(keystroke.display_modifiers));
+        if self.keystrokes.len() < Self::KEYSTROKE_COUNT_MAX && real_modifiers.modified() {
+            self.keystrokes.push(Self::dummy(real_modifiers));
         }
     }
 
@@ -718,8 +714,11 @@ mod tests {
 
             // Combine current modifiers with keystroke modifiers
             keystroke.modifiers |= self.current_modifiers;
+            let real_modifiers = keystroke.modifiers;
+            keystroke = to_gpui_keystroke(keystroke);
 
             self.update_input(|input, window, cx| {
+                window.set_modifiers(real_modifiers);
                 input.handle_keystroke(&keystroke, window, cx);
             });
 
@@ -747,6 +746,7 @@ mod tests {
             };
 
             self.update_input(|input, window, cx| {
+                window.set_modifiers(new_modifiers);
                 input.on_modifiers_changed(&event, window, cx);
             });
 
@@ -954,6 +954,100 @@ mod tests {
         }
     }
 
+    /// For GPUI, when you press `ctrl-shift-2`, it produces `ctrl-@` without the shift modifier.
+    fn to_gpui_keystroke(mut keystroke: Keystroke) -> Keystroke {
+        if keystroke.modifiers.shift {
+            match keystroke.key.as_str() {
+                "`" => {
+                    keystroke.key = "~".into();
+                    keystroke.modifiers.shift = false;
+                }
+                "1" => {
+                    keystroke.key = "!".into();
+                    keystroke.modifiers.shift = false;
+                }
+                "2" => {
+                    keystroke.key = "@".into();
+                    keystroke.modifiers.shift = false;
+                }
+                "3" => {
+                    keystroke.key = "#".into();
+                    keystroke.modifiers.shift = false;
+                }
+                "4" => {
+                    keystroke.key = "$".into();
+                    keystroke.modifiers.shift = false;
+                }
+                "5" => {
+                    keystroke.key = "%".into();
+                    keystroke.modifiers.shift = false;
+                }
+                "6" => {
+                    keystroke.key = "^".into();
+                    keystroke.modifiers.shift = false;
+                }
+                "7" => {
+                    keystroke.key = "&".into();
+                    keystroke.modifiers.shift = false;
+                }
+                "8" => {
+                    keystroke.key = "*".into();
+                    keystroke.modifiers.shift = false;
+                }
+                "9" => {
+                    keystroke.key = "(".into();
+                    keystroke.modifiers.shift = false;
+                }
+                "0" => {
+                    keystroke.key = ")".into();
+                    keystroke.modifiers.shift = false;
+                }
+                "-" => {
+                    keystroke.key = "_".into();
+                    keystroke.modifiers.shift = false;
+                }
+                "=" => {
+                    keystroke.key = "+".into();
+                    keystroke.modifiers.shift = false;
+                }
+                "[" => {
+                    keystroke.key = "{".into();
+                    keystroke.modifiers.shift = false;
+                }
+                "]" => {
+                    keystroke.key = "}".into();
+                    keystroke.modifiers.shift = false;
+                }
+                "\\" => {
+                    keystroke.key = "|".into();
+                    keystroke.modifiers.shift = false;
+                }
+                ";" => {
+                    keystroke.key = ":".into();
+                    keystroke.modifiers.shift = false;
+                }
+                "'" => {
+                    keystroke.key = "\"".into();
+                    keystroke.modifiers.shift = false;
+                }
+                "," => {
+                    keystroke.key = "<".into();
+                    keystroke.modifiers.shift = false;
+                }
+                "." => {
+                    keystroke.key = ">".into();
+                    keystroke.modifiers.shift = false;
+                }
+                "/" => {
+                    keystroke.key = "?".into();
+                    keystroke.modifiers.shift = false;
+                }
+                _ => {}
+            }
+        }
+        keystroke
+    }
+
     struct KeystrokeUpdateTracker {
         initial_keystrokes: Vec<KeybindingKeystroke>,
         _subscription: Subscription,
@@ -1057,7 +1151,15 @@ mod tests {
             .send_events(&["+cmd", "shift-f", "-cmd"])
             // In search mode, when completing a modifier-only keystroke with a key,
             // only the original modifiers are preserved, not the keystroke's modifiers
-            .expect_keystrokes(&["cmd-f"]);
+            //
+            // Update:
+            // This behavior was changed to preserve all modifiers in search mode, this is now reflected in the expected keystrokes.
+            // Specifically, considering the sequence: `+cmd +shift -shift 2`, we expect it to produce the same result as `+cmd +shift 2`
+            // which is `cmd-@`. But in the case of `+cmd +shift -shift 2`, the keystroke we receive is `cmd-2`, which means that
+            // we need to dynamically map the key from `2` to `@` when the shift modifier is not present, which is not possible.
+            // Therefore, we now preserve all modifiers in search mode to ensure consistent behavior.
+            // And also, VSCode seems to preserve all modifiers in search mode as well.
+            .expect_keystrokes(&["cmd-shift-f"]);
     }
 
     #[gpui::test]
@@ -1234,7 +1336,7 @@ mod tests {
             .await
             .with_search_mode(true)
             .send_events(&["+ctrl", "+shift", "-shift", "a", "-ctrl"])
-            .expect_keystrokes(&["ctrl-shift-a"]);
+            .expect_keystrokes(&["ctrl-a"]);
     }
 
     #[gpui::test]
@@ -1342,7 +1444,7 @@ mod tests {
             .await
             .with_search_mode(true)
             .send_events(&["+ctrl+alt", "-ctrl", "j"])
-            .expect_keystrokes(&["ctrl-alt-j"]);
+            .expect_keystrokes(&["alt-j"]);
     }
 
     #[gpui::test]
@@ -1364,11 +1466,11 @@ mod tests {
             .send_events(&["+ctrl+alt", "-ctrl", "+shift"])
             .expect_keystrokes(&["ctrl-shift-alt-"])
             .send_keystroke("j")
-            .expect_keystrokes(&["ctrl-shift-alt-j"])
+            .expect_keystrokes(&["shift-alt-j"])
             .send_keystroke("i")
-            .expect_keystrokes(&["ctrl-shift-alt-j", "shift-alt-i"])
+            .expect_keystrokes(&["shift-alt-j", "shift-alt-i"])
             .send_events(&["-shift-alt", "+cmd"])
-            .expect_keystrokes(&["ctrl-shift-alt-j", "shift-alt-i", "cmd-"]);
+            .expect_keystrokes(&["shift-alt-j", "shift-alt-i", "cmd-"]);
     }
 
     #[gpui::test]
@@ -1401,4 +1503,13 @@ mod tests {
             .send_events(&["+ctrl", "-ctrl", "+alt", "-alt", "+shift", "-shift"])
             .expect_empty();
     }
+
+    #[gpui::test]
+    async fn test_not_search_shifted_keys(cx: &mut TestAppContext) {
+        init_test(cx)
+            .await
+            .with_search_mode(false)
+            .send_events(&["+ctrl", "+shift", "4", "-all"])
+            .expect_keystrokes(&["ctrl-$"]);
+    }
 }