X11: Don't emit keypress events for modifiers (#10271)

Kamal Ahmad created

This fixes certain shortcuts/motions on X11 like in vim mode "v i )",
where previously zed would interpret it as "v i SHIFT )" due to the x11
backend emitting key press events for modifier keys even though other
platforms like Wayland don't. This also adds support for
ModifiersChanged events to X11

Release Notes:

- Fixed vim motions like "v i )" not working on X11
([#10199](https://github.com/zed-industries/zed/issues/10199)).

Change summary

crates/gpui/src/platform/linux/platform.rs       | 18 +++++++
crates/gpui/src/platform/linux/wayland/client.rs | 15 -----
crates/gpui/src/platform/linux/x11/client.rs     | 46 +++++++++++++++++
3 files changed, 63 insertions(+), 16 deletions(-)

Detailed changes

crates/gpui/src/platform/linux/platform.rs 🔗

@@ -624,6 +624,24 @@ impl Keystroke {
     }
 }
 
+impl Modifiers {
+    pub(super) fn from_xkb(keymap_state: &State) -> Self {
+        let shift = keymap_state.mod_name_is_active(xkb::MOD_NAME_SHIFT, xkb::STATE_MODS_EFFECTIVE);
+        let alt = keymap_state.mod_name_is_active(xkb::MOD_NAME_ALT, xkb::STATE_MODS_EFFECTIVE);
+        let control =
+            keymap_state.mod_name_is_active(xkb::MOD_NAME_CTRL, xkb::STATE_MODS_EFFECTIVE);
+        let platform =
+            keymap_state.mod_name_is_active(xkb::MOD_NAME_LOGO, xkb::STATE_MODS_EFFECTIVE);
+        Modifiers {
+            shift,
+            alt,
+            control,
+            platform,
+            function: false,
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

crates/gpui/src/platform/linux/wayland/client.rs 🔗

@@ -589,20 +589,7 @@ impl Dispatch<wl_keyboard::WlKeyboard, ()> for WaylandClient {
 
                 let keymap_state = state.keymap_state.as_mut().unwrap();
                 keymap_state.update_mask(mods_depressed, mods_latched, mods_locked, 0, 0, group);
-
-                let shift =
-                    keymap_state.mod_name_is_active(xkb::MOD_NAME_SHIFT, xkb::STATE_MODS_EFFECTIVE);
-                let alt =
-                    keymap_state.mod_name_is_active(xkb::MOD_NAME_ALT, xkb::STATE_MODS_EFFECTIVE);
-                let control =
-                    keymap_state.mod_name_is_active(xkb::MOD_NAME_CTRL, xkb::STATE_MODS_EFFECTIVE);
-                let command =
-                    keymap_state.mod_name_is_active(xkb::MOD_NAME_LOGO, xkb::STATE_MODS_EFFECTIVE);
-
-                state.modifiers.shift = shift;
-                state.modifiers.alt = alt;
-                state.modifiers.control = control;
-                state.modifiers.platform = command;
+                state.modifiers = Modifiers::from_xkb(keymap_state);
 
                 let input = PlatformInput::ModifiersChanged(ModifiersChangedEvent {
                     modifiers: state.modifiers,

crates/gpui/src/platform/linux/x11/client.rs 🔗

@@ -22,8 +22,8 @@ use xkbcommon::xkb as xkbc;
 use crate::platform::linux::LinuxClient;
 use crate::platform::{LinuxCommon, PlatformWindow};
 use crate::{
-    px, AnyWindowHandle, Bounds, CursorStyle, DisplayId, Pixels, PlatformDisplay, PlatformInput,
-    Point, ScrollDelta, Size, TouchPhase, WindowParams,
+    px, AnyWindowHandle, Bounds, CursorStyle, DisplayId, Modifiers, ModifiersChangedEvent, Pixels,
+    PlatformDisplay, PlatformInput, Point, ScrollDelta, Size, TouchPhase, WindowParams,
 };
 
 use super::{super::SCROLL_LINES, X11Display, X11Window, XcbAtoms};
@@ -60,6 +60,7 @@ pub struct X11ClientState {
     pub(crate) x_root_index: usize,
     pub(crate) atoms: XcbAtoms,
     pub(crate) windows: HashMap<xproto::Window, WindowRef>,
+    pub(crate) focused_window: Option<xproto::Window>,
     pub(crate) xkb: xkbc::State,
 
     pub(crate) common: LinuxCommon,
@@ -99,6 +100,17 @@ impl X11Client {
 
         let atoms = atoms.reply().unwrap();
         let xkb = xkb.reply().unwrap();
+        let events = xkb::EventType::STATE_NOTIFY;
+        xcb_connection
+            .xkb_select_events(
+                xkb::ID::USE_CORE_KBD.into(),
+                0u8.into(),
+                events,
+                0u8.into(),
+                0u8.into(),
+                &xkb::SelectEventsAux::new(),
+            )
+            .unwrap();
         assert!(xkb.supported);
 
         let xkb_state = {
@@ -152,6 +164,7 @@ impl X11Client {
             x_root_index,
             atoms,
             windows: HashMap::default(),
+            focused_window: None,
             xkb: xkb_state,
             clipboard,
             primary,
@@ -205,10 +218,31 @@ impl X11Client {
             Event::FocusIn(event) => {
                 let window = self.get_window(event.event)?;
                 window.set_focused(true);
+                self.0.borrow_mut().focused_window = Some(event.event);
             }
             Event::FocusOut(event) => {
                 let window = self.get_window(event.event)?;
                 window.set_focused(false);
+                self.0.borrow_mut().focused_window = None;
+            }
+            Event::XkbStateNotify(event) => {
+                let mut state = self.0.borrow_mut();
+                state.xkb.update_mask(
+                    event.base_mods.into(),
+                    event.latched_mods.into(),
+                    event.locked_mods.into(),
+                    0,
+                    0,
+                    event.locked_group.into(),
+                );
+                let modifiers = Modifiers::from_xkb(&state.xkb);
+                let focused_window_id = state.focused_window?;
+                drop(state);
+
+                let focused_window = self.get_window(focused_window_id)?;
+                focused_window.handle_input(PlatformInput::ModifiersChanged(
+                    ModifiersChangedEvent { modifiers },
+                ));
             }
             Event::KeyPress(event) => {
                 let window = self.get_window(event.event)?;
@@ -219,6 +253,10 @@ impl X11Client {
                     let code = event.detail.into();
                     let keystroke = crate::Keystroke::from_xkb(&state.xkb, modifiers, code);
                     state.xkb.update_key(code, xkbc::KeyDirection::Down);
+                    let keysym = state.xkb.key_get_one_sym(code);
+                    if keysym.is_modifier_key() {
+                        return Some(());
+                    }
                     keystroke
                 };
 
@@ -237,6 +275,10 @@ impl X11Client {
                     let code = event.detail.into();
                     let keystroke = crate::Keystroke::from_xkb(&state.xkb, modifiers, code);
                     state.xkb.update_key(code, xkbc::KeyDirection::Up);
+                    let keysym = state.xkb.key_get_one_sym(code);
+                    if keysym.is_modifier_key() {
+                        return Some(());
+                    }
                     keystroke
                 };
                 drop(state);