zed-industries
keymap: Allow modifiers as keys
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
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());
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 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.
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 I'm back if you want to find a time: https://calendly.com/conradirwin/pairing
@DAlperin It looks like we introduced some real-seeming test failures. Would you be able to see if we actually broke the features or if we just need to update the tests?
Hi, after bisecting it seems this PR breaks some vim commands like Ctrl-w q. Instead Ctrl-w closes the current tab, so any commands starting with Ctrl-w don't work
