gpui: Filter out NoAction bindings from pending input

crates/gpui/src/keymap.rs

@@ -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.
@@ -307,6 +342,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 = [