Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Refactor key dispatch (#14942)

Browse source 
Simplify key dispatch code.

Previously we would maintain a cache of key matchers for each context
that
would store the pending input. For the last while we've also stored the
typed prefix on the window. This is redundant, we only need one copy, so
now
it's just stored on the window, which lets us avoid the boilerplate of
keeping
all the matchers in sync.

This stops us from losing multikey bindings when the context on a node
changes
(#11009) (though we still interrupt multikey bindings if the focus
changes).

While in the code, I fixed up a few other things with multi-key bindings
that
were causing problems:

Previously we assumed that all multi-key bindings took precedence over
any
single-key binding, now this is done such that if a user binds a
single-key
binding, it will take precedence over all system-defined multi-key
bindings
(irrespective of the depth in the context tree). This was a common cause
of
confusion for new users trying to bind to `cmd-k` or `ctrl-w` in vim
mode
(#13543).

Previously after a pending multi-key keystroke failed to match, we would
drop
the prefix if it was an input event. Now we correctly replay it
(#14725).

Release Notes:

- Fixed multi-key shortcuts not working across completion menu changes
([#11009](https://github.com/zed-industries/zed/issues/11009))
- Fixed multi-key shortcuts discarding earlier input
([#14445](https://github.com/zed-industries/zed/pull/14445))
- vim: Fixed `jk` binding preventing you from repeating `j`
([#14725](https://github.com/zed-industries/zed/issues/14725))
- vim: Fixed `escape` in normal mode to also clear the selected
register.
- Fixed key maps so user-defined mappings take precedence over builtin
multi-key mappings
([#13543](https://github.com/zed-industries/zed/issues/13543))
- Fixed a bug where overridden shortcuts would still show in the Command
Palette
This commit is contained in:
Conrad Irwin 2024-07-22 10:46:16 -06:00 • committed by GitHub
parent 865904a0c9
commit 2e23527e09
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
13 changed files with 462 additions and 422 deletions
Download patch file Download diff file Expand all files Collapse all files

137
crates/gpui/src/keymap.rs
View file

@ -1,13 +1,11 @@
mod binding;
mod context;
mod matcher;
pub use binding::*;
pub use context::*;
pub(crate) use matcher::*;
use crate::{Action, Keystroke, NoAction};
use collections::{HashMap, HashSet};
use collections::HashMap;
use smallvec::SmallVec;
use std::any::{Any, TypeId};
@ -21,8 +19,6 @@ pub struct KeymapVersion(usize);
pub struct Keymap {
bindings: Vec<KeyBinding>,
binding_indices_by_action_id: HashMap<TypeId, SmallVec<[usize; 3]>>,
disabled_keystrokes:
HashMap<SmallVec<[Keystroke; 2]>, HashSet<Option<KeyBindingContextPredicate>>>,
version: KeymapVersion,
}
@ -41,22 +37,13 @@ impl Keymap {
/// Add more bindings to the keymap.
pub fn add_bindings<T: IntoIterator<Item = KeyBinding>>(&mut self, bindings: T) {
let no_action_id = (NoAction {}).type_id();
for binding in bindings {
let action_id = binding.action().as_any().type_id();
if action_id == no_action_id {
self.disabled_keystrokes
.entry(binding.keystrokes)
.or_default()
.insert(binding.context_predicate);
} else {
self.binding_indices_by_action_id
.entry(action_id)
.or_default()
.push(self.bindings.len());
self.bindings.push(binding);
}
self.binding_indices_by_action_id
.entry(action_id)
.or_default()
.push(self.bindings.len());
self.bindings.push(binding);
}
self.version.0 += 1;
@ -66,7 +53,6 @@ impl Keymap {
pub fn clear(&mut self) {
self.bindings.clear();
self.binding_indices_by_action_id.clear();
self.disabled_keystrokes.clear();
self.version.0 += 1;
}
@ -89,8 +75,66 @@ impl Keymap {
.filter(move |binding| binding.action().partial_eq(action))
}
/// bindings_for_input returns a list of bindings that match the given input,
/// and a boolean indicating whether or not more bindings might match if
/// the input was longer.
///
/// Precedence is defined by the depth in the tree (matches on the Editor take
/// precedence over matches on the Pane, then the Workspace, etc.). Bindings with
/// no context are treated as the same as the deepest context.
///
/// In the case of multiple bindings at the same depth, the ones defined later in the
/// keymap take precedence (so user bindings take precedence over built-in bindings).
///
/// If a user has disabled a binding with `"x": null` it will not be returned. Disabled
/// bindings are evaluated with the same precedence rules so you can disable a rule in
/// a given context only.
///
/// In the case of multi-key bindings, the
pub fn bindings_for_input(
&self,
input: &[Keystroke],
context_stack: &[KeyContext],
) -> (SmallVec<[KeyBinding; 1]>, bool) {
let possibilities = self.bindings().rev().filter_map(|binding| {
binding
.match_keystrokes(input)
.map(|pending| (binding, pending))
});
let mut bindings: SmallVec<[(KeyBinding, usize); 1]> = SmallVec::new();
let mut is_pending = None;
'outer: for (binding, pending) in possibilities {
for depth in (0..=context_stack.len()).rev() {
if self.binding_enabled(binding, &context_stack[0..depth]) {
if is_pending.is_none() {
is_pending = Some(pending);
}
if !pending {
bindings.push((binding.clone(), depth));
continue 'outer;
}
}
}
}
bindings.sort_by(|a, b| a.1.cmp(&b.1).reverse());
let bindings = bindings
.into_iter()
.map_while(|(binding, _)| {
if binding.action.as_any().type_id() == (NoAction {}).type_id() {
None
} else {
Some(binding)
}
})
.collect();
return (bindings, is_pending.unwrap_or_default());
}
/// Check if the given binding is enabled, given a certain key context.
pub fn binding_enabled(&self, binding: &KeyBinding, context: &[KeyContext]) -> bool {
fn binding_enabled(&self, binding: &KeyBinding, context: &[KeyContext]) -> bool {
// If binding has a context predicate, it must match the current context,
if let Some(predicate) = &binding.context_predicate {
if !predicate.eval(context) {
@ -98,22 +142,6 @@ impl Keymap {
}
}
if let Some(disabled_predicates) = self.disabled_keystrokes.get(&binding.keystrokes) {
for disabled_predicate in disabled_predicates {
match disabled_predicate {
// The binding must not be globally disabled.
None => return false,
// The binding must not be disabled in the current context.
Some(predicate) => {
if predicate.eval(context) {
return false;
}
}
}
}
}
true
}
}
@ -168,16 +196,37 @@ mod tests {
keymap.add_bindings(bindings.clone());
// binding is only enabled in a specific context
assert!(!keymap.binding_enabled(&bindings[0], &[KeyContext::parse("barf").unwrap()]));
assert!(keymap.binding_enabled(&bindings[0], &[KeyContext::parse("editor").unwrap()]));
assert!(keymap
.bindings_for_input(
&[Keystroke::parse("ctrl-a").unwrap()],
&[KeyContext::parse("barf").unwrap()],
)
.0
.is_empty());
assert!(!keymap
.bindings_for_input(
&[Keystroke::parse("ctrl-a").unwrap()],
&[KeyContext::parse("editor").unwrap()],
)
.0
.is_empty());
// binding is disabled in a more specific context
assert!(!keymap.binding_enabled(
&bindings[0],
&[KeyContext::parse("editor mode=full").unwrap()]
));
assert!(keymap
.bindings_for_input(
&[Keystroke::parse("ctrl-a").unwrap()],
&[KeyContext::parse("editor mode=full").unwrap()],
)
.0
.is_empty());
// binding is globally disabled
assert!(!keymap.binding_enabled(&bindings[1], &[KeyContext::parse("barf").unwrap()]));
assert!(keymap
.bindings_for_input(
&[Keystroke::parse("ctrl-b").unwrap()],
&[KeyContext::parse("barf").unwrap()],
)
.0
.is_empty());
}
}