zed icon indicating copy to clipboard operation
zed copied to clipboard

keymap: Allow modifiers as keys

Open DAlperin opened this issue 9 months ago • 4 comments

It is sometimes desirable to allow modifers to serve as keys themselves for the purposes of keybinds. For example, the popular keybind in jetbrains IDEs shift shift which opens the file finder.

This change treats modifers in the keymaps as keys themselves if they are not accompanied by a key they are modifying.

Further this change wires up they key dispatcher to treat modifer change events as key presses which are considered for matching against keybinds.

Release Notes:

  • Fixes #6460

DAlperin avatar May 20 '24 04:05 DAlperin

A weird behavior currently is that there is no timeout for multi "chorded" bindings. So if you press shift, then wait 30 minutes, come back and press shift again, the shift shift binding will fire. This is currently true for all multi part binds. I don't know if it's intended behavior. This can be trivially "fixed" with something like

diff --git a/crates/gpui/src/keymap/matcher.rs b/crates/gpui/src/keymap/matcher.rs
index c2dec94a5..cd69347ce 100644
--- a/crates/gpui/src/keymap/matcher.rs
+++ b/crates/gpui/src/keymap/matcher.rs
@@ -1,9 +1,10 @@
 use crate::{KeyBinding, KeyContext, Keymap, KeymapVersion, Keystroke};
 use smallvec::SmallVec;
-use std::{cell::RefCell, rc::Rc};
+use std::{cell::RefCell, rc::Rc, time::SystemTime};

 pub(crate) struct KeystrokeMatcher {
     pending_keystrokes: Vec<Keystroke>,
+    last_timestamp: u64,
     keymap: Rc<RefCell<Keymap>>,
     keymap_version: KeymapVersion,
 }
@@ -18,6 +19,7 @@ impl KeystrokeMatcher {
         let keymap_version = keymap.borrow().version();
         Self {
             pending_keystrokes: Vec::new(),
+            last_timestamp: 0,
             keymap_version,
             keymap,
         }
@@ -52,6 +54,16 @@ impl KeystrokeMatcher {
         let mut pending_key = None;
         let mut bindings = SmallVec::new();

+        let now = SystemTime::now()
+            .duration_since(SystemTime::UNIX_EPOCH)
+            .unwrap()
+            .as_millis() as u64;
+
+        //TODO: make configurable
+        if now - self.last_timestamp > 1000 {
+            self.pending_keystrokes.clear();
+        }
+
         for binding in keymap.bindings().rev() {
             if !keymap.binding_enabled(binding, context_stack) {
                 continue;
@@ -59,6 +71,7 @@ impl KeystrokeMatcher {

             for candidate in keystroke.match_candidates() {
                 self.pending_keystrokes.push(candidate.clone());
+                self.last_timestamp = now;
                 match binding.match_keystrokes(&self.pending_keystrokes) {
                     KeyMatch::Matched => {
                         bindings.push(binding.clone());

DAlperin avatar May 20 '24 16:05 DAlperin

For now I have adopted the behavior of vscode: for chords that contain just modifiers, use a 300ms timeout. Currently this works by storing and checking a timestamp in the KeystrokeMatcher but this should potentially happen async to head off any performance hits. I haven't fully wrapped my head around gpui async yet though.

DAlperin avatar May 20 '24 16:05 DAlperin

@DAlperin This is super cool!

Unfortunately, after this is merged it's no longer possible to trigger cmd-k cmd-s if you release the cmd key between the two (because now there's an additional cmd key event sent by the modifier change code).

Does jet brains open the command palette on key-down or key-up of the second shift?

Regardless, for the first shift, I think we need to:

  • On modifier-key-down set a flag that says "maybe just a modifier key"
  • On modifier-key-up if the flag is set, trigger the key event for that modifier key
  • If any other key down happens, we clear the flag.

If we want to handle key-up on the second shift, then we can do the same there too. If we want to handle key-down on the second shift, then we need another flag that says "in single key modifier mode". (In general I'm in favor of doing stuff on key-down because it feels much faster, but there may be reasons they chose not to).

From looking at the code, I'd love to re-use the existing pending timer in the window, instead of having another thing in the key matcher that does something similar.

There are also a few places where we render keys that need to support this too: https://github.com/zed-industries/zed/blob/ac892ab47a8d7edfbd54bc1684151093affd6bcf/crates/ui/src/components/keybinding.rs#L33-L34, https://github.com/zed-industries/zed/blob/ac892ab47a8d7edfbd54bc1684151093affd6bcf/crates/gpui/src/platform/keystroke.rs#L193-L194

I'd love to help you get this over the line, but I'll be out next week. If you'd like to pair on it when I'm back, https://calendly.com/conradirwin/pairing.

ConradIrwin avatar May 21 '24 02:05 ConradIrwin

Ah that's a good point I stupidly glazed over in my first pass.

To your point, jetbrains opens on key up. Let me think about the tradeoffs a bit, though I tend to agree that keydown events feel faster.

I'd love to pair with you @ConradIrwin but that link has no times even after next week :)

DAlperin avatar May 21 '24 02:05 DAlperin