gpui: Filter out NoAction bindings from pending input (#30260)

This prevents the 1 second delay happening on input when all of the
pending bindings are NoAction

Closes #30259

Release Notes:

- Fixed unnecessary delay when typing a multi-stroke binding that
doesn't match any non-null bindings

---------

Co-authored-by: Conrad Irwin <conrad.irwin@gmail.com>
This commit is contained in:
Joe Polny 2025-06-02 15:22:36 -04:00 committed by GitHub
parent 9d5fb3c3f3
commit 3f90bc81bd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -147,14 +147,49 @@ impl Keymap {
});
let mut bindings: SmallVec<[(KeyBinding, usize); 1]> = SmallVec::new();
let mut is_pending = None;
// (pending, is_no_action, depth, keystrokes)
let mut pending_info_opt: Option<(bool, bool, usize, &[Keystroke])> = 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);
let is_no_action = is_no_action(&*binding.action);
// We only want to consider a binding pending if it has an action
// This, however, means that if we have both a NoAction binding and a binding
// with an action at the same depth, we should still set is_pending to true.
if let Some(pending_info) = pending_info_opt.as_mut() {
let (
already_pending,
pending_is_no_action,
pending_depth,
pending_keystrokes,
) = *pending_info;
// We only want to change the pending status if it's not already pending AND if
// the existing pending status was set by a NoAction binding. This avoids a NoAction
// binding erroneously setting the pending status to true when a binding with an action
// already set it to false
//
// We also want to change the pending status if the keystrokes don't match,
// meaning it's different keystrokes than the NoAction that set pending to false
if pending
&& !already_pending
&& pending_is_no_action
&& (pending_depth == depth
|| pending_keystrokes != binding.keystrokes())
{
pending_info.0 = !is_no_action;
}
} else {
pending_info_opt = Some((
pending && !is_no_action,
is_no_action,
depth,
binding.keystrokes(),
));
}
if !pending {
bindings.push((binding.clone(), depth));
continue 'outer;
@ -174,7 +209,7 @@ impl Keymap {
})
.collect();
(bindings, is_pending.unwrap_or_default())
(bindings, pending_info_opt.unwrap_or_default().0)
}
/// Check if the given binding is enabled, given a certain key context.
@ -310,6 +345,102 @@ mod tests {
);
}
#[test]
/// Tests for https://github.com/zed-industries/zed/issues/30259
fn test_multiple_keystroke_binding_disabled() {
let bindings = [
KeyBinding::new("space w w", ActionAlpha {}, Some("workspace")),
KeyBinding::new("space w w", NoAction {}, Some("editor")),
];
let mut keymap = Keymap::default();
keymap.add_bindings(bindings.clone());
let space = || Keystroke::parse("space").unwrap();
let w = || Keystroke::parse("w").unwrap();
let space_w = [space(), w()];
let space_w_w = [space(), w(), w()];
let workspace_context = || [KeyContext::parse("workspace").unwrap()];
let editor_workspace_context = || {
[
KeyContext::parse("workspace").unwrap(),
KeyContext::parse("editor").unwrap(),
]
};
// Ensure `space` results in pending input on the workspace, but not editor
let space_workspace = keymap.bindings_for_input(&[space()], &workspace_context());
assert!(space_workspace.0.is_empty());
assert_eq!(space_workspace.1, true);
let space_editor = keymap.bindings_for_input(&[space()], &editor_workspace_context());
assert!(space_editor.0.is_empty());
assert_eq!(space_editor.1, false);
// Ensure `space w` results in pending input on the workspace, but not editor
let space_w_workspace = keymap.bindings_for_input(&space_w, &workspace_context());
assert!(space_w_workspace.0.is_empty());
assert_eq!(space_w_workspace.1, true);
let space_w_editor = keymap.bindings_for_input(&space_w, &editor_workspace_context());
assert!(space_w_editor.0.is_empty());
assert_eq!(space_w_editor.1, false);
// Ensure `space w w` results in the binding in the workspace, but not in the editor
let space_w_w_workspace = keymap.bindings_for_input(&space_w_w, &workspace_context());
assert!(!space_w_w_workspace.0.is_empty());
assert_eq!(space_w_w_workspace.1, false);
let space_w_w_editor = keymap.bindings_for_input(&space_w_w, &editor_workspace_context());
assert!(space_w_w_editor.0.is_empty());
assert_eq!(space_w_w_editor.1, false);
// Now test what happens if we have another binding defined AFTER the NoAction
// that should result in pending
let bindings = [
KeyBinding::new("space w w", ActionAlpha {}, Some("workspace")),
KeyBinding::new("space w w", NoAction {}, Some("editor")),
KeyBinding::new("space w x", ActionAlpha {}, Some("editor")),
];
let mut keymap = Keymap::default();
keymap.add_bindings(bindings.clone());
let space_editor = keymap.bindings_for_input(&[space()], &editor_workspace_context());
assert!(space_editor.0.is_empty());
assert_eq!(space_editor.1, true);
// Now test what happens if we have another binding defined BEFORE the NoAction
// that should result in pending
let bindings = [
KeyBinding::new("space w w", ActionAlpha {}, Some("workspace")),
KeyBinding::new("space w x", ActionAlpha {}, Some("editor")),
KeyBinding::new("space w w", NoAction {}, Some("editor")),
];
let mut keymap = Keymap::default();
keymap.add_bindings(bindings.clone());
let space_editor = keymap.bindings_for_input(&[space()], &editor_workspace_context());
assert!(space_editor.0.is_empty());
assert_eq!(space_editor.1, true);
// Now test what happens if we have another binding defined at a higher context
// that should result in pending
let bindings = [
KeyBinding::new("space w w", ActionAlpha {}, Some("workspace")),
KeyBinding::new("space w x", ActionAlpha {}, Some("workspace")),
KeyBinding::new("space w w", NoAction {}, Some("editor")),
];
let mut keymap = Keymap::default();
keymap.add_bindings(bindings.clone());
let space_editor = keymap.bindings_for_input(&[space()], &editor_workspace_context());
assert!(space_editor.0.is_empty());
assert_eq!(space_editor.1, true);
}
#[test]
fn test_bindings_for_action() {
let bindings = [