checkpoint

crates/gpui/src/platform/keyboard.rs

@@ -13,3 +13,11 @@ pub trait PlatformKeyboardMapper {
     /// Map a key equivalent to its platform-specific representation
     fn map_key_equivalent(&self, keystroke: Keystroke) -> KeybindingKeystroke;
 }
+
+pub(crate) struct DummyKeyboardMapper;
+
+impl PlatformKeyboardMapper for DummyKeyboardMapper {
+    fn map_key_equivalent(&self, keystroke: Keystroke) -> KeybindingKeystroke {
+        KeybindingKeystroke::new(keystroke)
+    }
+}

crates/gpui/src/platform/keystroke.rs

@@ -301,10 +301,12 @@ impl Keystroke {
 impl KeybindingKeystroke {
     /// Create a new keybinding keystroke from the given keystroke
     pub fn new(inner: Keystroke) -> Self {
+        let key = inner.key.clone();
+        let modifiers = inner.modifiers;
         KeybindingKeystroke {
             inner,
-            modifiers: inner.modifiers,
-            key: inner.key.clone(),
+            modifiers,
+            key,
         }
     }
 }

crates/gpui/src/platform/test/platform.rs

@@ -1,8 +1,9 @@
 use crate::{
     AnyWindowHandle, BackgroundExecutor, ClipboardItem, CursorStyle, DevicePixels,
-    ForegroundExecutor, Keymap, NoopTextSystem, Platform, PlatformDisplay, PlatformKeyboardLayout,
-    PlatformTextSystem, PromptButton, ScreenCaptureFrame, ScreenCaptureSource, ScreenCaptureStream,
-    SourceMetadata, Task, TestDisplay, TestWindow, WindowAppearance, WindowParams, size,
+    DummyKeyboardMapper, ForegroundExecutor, Keymap, NoopTextSystem, Platform, PlatformDisplay,
+    PlatformKeyboardLayout, PlatformKeyboardMapper, PlatformTextSystem, PromptButton,
+    ScreenCaptureFrame, ScreenCaptureSource, ScreenCaptureStream, SourceMetadata, Task,
+    TestDisplay, TestWindow, WindowAppearance, WindowParams, size,
 };
 use anyhow::Result;
 use collections::VecDeque;
@@ -237,6 +238,10 @@ impl Platform for TestPlatform {
         Box::new(TestKeyboardLayout)
     }
 
+    fn keyboard_mapper(&self) -> Rc<dyn PlatformKeyboardMapper> {
+        Rc::new(DummyKeyboardMapper)
+    }
+
     fn on_keyboard_layout_change(&self, _: Box<dyn FnMut()>) {}
 
     fn run(&self, _on_finish_launching: Box<dyn FnOnce()>) {

crates/gpui/src/platform/windows/keyboard.rs

@@ -1,5 +1,3 @@
-use std::borrow::Cow;
-
 use anyhow::Result;
 use windows::Win32::UI::{
     Input::KeyboardAndMouse::{
@@ -35,17 +33,19 @@ impl PlatformKeyboardLayout for WindowsKeyboardLayout {
 
 impl PlatformKeyboardMapper for WindowsKeyboardMapper {
     fn map_key_equivalent(&self, mut keystroke: Keystroke) -> KeybindingKeystroke {
-        let Some((vkey, shift)) = key_needs_processing(&keystroke.key) else {
+        let Some((vkey, shifted_key)) = key_needs_processing(&keystroke.key) else {
             return KeybindingKeystroke::new(keystroke);
         };
-        if shift && keystroke.modifiers.shift {
+        if shifted_key && keystroke.modifiers.shift {
             log::warn!(
                 "Keystroke '{}' has both shift and a shifted key, this is likely a bug",
                 keystroke.key
             );
-            keystroke.modifiers.shift = false;
         }
-        // translate to unshifted key first
+
+        let shift = shifted_key || keystroke.modifiers.shift;
+        keystroke.modifiers.shift = false;
+
         let Some(key) = get_key_from_vkey(vkey) else {
             log::error!(
                 "Failed to map key equivalent '{:?}' to a valid key",
@@ -53,13 +53,6 @@ impl PlatformKeyboardMapper for WindowsKeyboardMapper {
             );
             return KeybindingKeystroke::new(keystroke);
         };
-        let modifiers = Modifiers {
-            control: keystroke.modifiers.control,
-            alt: keystroke.modifiers.alt,
-            shift,
-            platform: keystroke.modifiers.platform,
-            function: keystroke.modifiers.function,
-        };
 
         keystroke.key = if shift {
             let scan_code = unsafe { MapVirtualKeyW(vkey.0 as u32, MAPVK_VK_TO_VSC) };
@@ -84,6 +77,11 @@ impl PlatformKeyboardMapper for WindowsKeyboardMapper {
             key.clone()
         };
 
+        let modifiers = Modifiers {
+            shift,
+            ..keystroke.modifiers
+        };
+
         KeybindingKeystroke {
             inner: keystroke,
             modifiers,
@@ -259,3 +257,58 @@ fn key_needs_processing(key: &str) -> Option<(VIRTUAL_KEY, bool)> {
         _ => None,
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::{Keystroke, Modifiers, PlatformKeyboardMapper, WindowsKeyboardMapper};
+
+    #[test]
+    fn test_keyboard_mapper() {
+        let mapper = WindowsKeyboardMapper::new();
+
+        // Normal case
+        let keystroke = Keystroke {
+            modifiers: Modifiers::control(),
+            key: "a".to_string(),
+            key_char: None,
+        };
+        let mapped = mapper.map_key_equivalent(keystroke.clone());
+        assert_eq!(mapped.inner, keystroke);
+        assert_eq!(mapped.key, "a");
+        assert_eq!(mapped.modifiers, Modifiers::control());
+
+        // Shifted case, ctrl-$
+        let keystroke = Keystroke {
+            modifiers: Modifiers::control(),
+            key: "$".to_string(),
+            key_char: None,
+        };
+        let mapped = mapper.map_key_equivalent(keystroke.clone());
+        assert_eq!(mapped.inner, keystroke);
+        assert_eq!(mapped.key, "4");
+        assert_eq!(mapped.modifiers, Modifiers::control_shift());
+
+        // Shifted case, but shift is true
+        let keystroke = Keystroke {
+            modifiers: Modifiers::control_shift(),
+            key: "$".to_string(),
+            key_char: None,
+        };
+        let mapped = mapper.map_key_equivalent(keystroke.clone());
+        assert_eq!(mapped.inner.modifiers, Modifiers::control());
+        assert_eq!(mapped.key, "4");
+        assert_eq!(mapped.modifiers, Modifiers::control_shift());
+
+        // Windows style
+        let keystroke = Keystroke {
+            modifiers: Modifiers::control_shift(),
+            key: "4".to_string(),
+            key_char: None,
+        };
+        let mapped = mapper.map_key_equivalent(keystroke.clone());
+        assert_eq!(mapped.inner.modifiers, Modifiers::control());
+        assert_eq!(mapped.inner.key, "$");
+        assert_eq!(mapped.key, "4");
+        assert_eq!(mapped.modifiers, Modifiers::control_shift());
+    }
+}