From 4151ba13a138cef88fabb053119a507c15d49271 Mon Sep 17 00:00:00 2001 From: Kamal Ahmad Date: Wed, 10 Apr 2024 06:46:23 +0500 Subject: [PATCH] X11: Don't emit keypress events for modifiers (#10271) 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)). --- crates/gpui/src/platform/linux/platform.rs | 18 ++++++++ .../gpui/src/platform/linux/wayland/client.rs | 15 +----- crates/gpui/src/platform/linux/x11/client.rs | 46 ++++++++++++++++++- 3 files changed, 63 insertions(+), 16 deletions(-) diff --git a/crates/gpui/src/platform/linux/platform.rs b/crates/gpui/src/platform/linux/platform.rs index 79a2e8432f..5f09849a89 100644 --- a/crates/gpui/src/platform/linux/platform.rs +++ b/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::*; diff --git a/crates/gpui/src/platform/linux/wayland/client.rs b/crates/gpui/src/platform/linux/wayland/client.rs index fcf56518c4..2b6e1cbfba 100644 --- a/crates/gpui/src/platform/linux/wayland/client.rs +++ b/crates/gpui/src/platform/linux/wayland/client.rs @@ -589,20 +589,7 @@ impl Dispatch 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, diff --git a/crates/gpui/src/platform/linux/x11/client.rs b/crates/gpui/src/platform/linux/x11/client.rs index 210a0e4853..908d2ea750 100644 --- a/crates/gpui/src/platform/linux/x11/client.rs +++ b/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, + pub(crate) focused_window: Option, 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);