Fix cmd+k in terminal and fix sporadic keybind misses (#7388)

This fixes `cmd+k` in the terminal taking 1s to have an effect. It is
now immediate.

It also fixes #7270 by ensuring that we don't set a bad state when
matching keybindings.

It matches keybindings per context and if it finds a match on a lower
context it doesn't keep pending keystrokes. If it finds two matches on
the same context level, requiring more keystrokes, then it waits.



Release Notes:

- Fixed `cmd-k` in terminal taking 1s to have an effect. Also fixed
sporadic non-matching of keybindings if there are overlapping
keybindings.
([#7270](https://github.com/zed-industries/zed/issues/7270)).

---------

Co-authored-by: Conrad <conrad@zed.dev>
Co-authored-by: Conrad Irwin <conrad.irwin@gmail.com>

assets/keymaps/default.json

@@ -415,7 +415,15 @@
       "cmd-?": "assistant::ToggleFocus",
       "cmd-alt-s": "workspace::SaveAll",
       "cmd-k m": "language_selector::Toggle",
-      "escape": "workspace::Unfollow"
+      "escape": "workspace::Unfollow",
+      "cmd-k cmd-left": ["workspace::ActivatePaneInDirection", "Left"],
+      "cmd-k cmd-right": ["workspace::ActivatePaneInDirection", "Right"],
+      "cmd-k cmd-up": ["workspace::ActivatePaneInDirection", "Up"],
+      "cmd-k cmd-down": ["workspace::ActivatePaneInDirection", "Down"],
+      "cmd-k shift-left": ["workspace::SwapPaneInDirection", "Left"],
+      "cmd-k shift-right": ["workspace::SwapPaneInDirection", "Right"],
+      "cmd-k shift-up": ["workspace::SwapPaneInDirection", "Up"],
+      "cmd-k shift-down": ["workspace::SwapPaneInDirection", "Down"]
     }
   },
   // Bindings from Sublime Text
@@ -441,18 +449,6 @@
       "ctrl-alt-shift-f": "editor::SelectToNextSubwordEnd"
     }
   },
-  {
-    "bindings": {
-      "cmd-k cmd-left": ["workspace::ActivatePaneInDirection", "Left"],
-      "cmd-k cmd-right": ["workspace::ActivatePaneInDirection", "Right"],
-      "cmd-k cmd-up": ["workspace::ActivatePaneInDirection", "Up"],
-      "cmd-k cmd-down": ["workspace::ActivatePaneInDirection", "Down"],
-      "cmd-k shift-left": ["workspace::SwapPaneInDirection", "Left"],
-      "cmd-k shift-right": ["workspace::SwapPaneInDirection", "Right"],
-      "cmd-k shift-up": ["workspace::SwapPaneInDirection", "Up"],
-      "cmd-k shift-down": ["workspace::SwapPaneInDirection", "Down"]
-    }
-  },
   // Bindings from Atom
   {
     "context": "Pane",

crates/collab/src/tests/integration_tests.rs

@@ -5967,6 +5967,6 @@ async fn test_cmd_k_left(cx: &mut TestAppContext) {
     cx.executor().advance_clock(Duration::from_secs(2));
     cx.simulate_keystrokes("left");
     workspace.update(cx, |workspace, cx| {
-        assert!(workspace.items(cx).collect::<Vec<_>>().len() == 3);
+        assert!(workspace.items(cx).collect::<Vec<_>>().len() == 2);
     });
 }

crates/gpui/src/key_dispatch.rs

@@ -62,16 +62,6 @@ use std::{
     rc::Rc,
 };
 
-/// KeymatchMode controls how keybindings are resolved in the case of conflicting pending keystrokes.
-/// When `Sequenced`, gpui will wait for 1s for sequences to complete.
-/// When `Immediate`, gpui will immediately resolve the keybinding.
-#[derive(Default, PartialEq)]
-pub enum KeymatchMode {
-    #[default]
-    Sequenced,
-    Immediate,
-}
-
 #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)]
 pub(crate) struct DispatchNodeId(usize);
 
@@ -84,7 +74,6 @@ pub(crate) struct DispatchTree {
     keystroke_matchers: FxHashMap<SmallVec<[KeyContext; 4]>, KeystrokeMatcher>,
     keymap: Rc<RefCell<Keymap>>,
     action_registry: Rc<ActionRegistry>,
-    pub(crate) keymatch_mode: KeymatchMode,
 }
 
 #[derive(Default)]
@@ -116,7 +105,6 @@ impl DispatchTree {
             keystroke_matchers: FxHashMap::default(),
             keymap,
             action_registry,
-            keymatch_mode: KeymatchMode::Sequenced,
         }
     }
 
@@ -127,7 +115,6 @@ impl DispatchTree {
         self.focusable_node_ids.clear();
         self.view_node_ids.clear();
         self.keystroke_matchers.clear();
-        self.keymatch_mode = KeymatchMode::Sequenced;
     }
 
     pub fn push_node(
@@ -335,7 +322,7 @@ impl DispatchTree {
             .collect()
     }
 
-    // dispatch_key pushses the next keystroke into any key binding matchers.
+    // dispatch_key pushes the next keystroke into any key binding matchers.
     // any matching bindings are returned in the order that they should be dispatched:
     // * First by length of binding (so if you have a binding for "b" and "ab", the "ab" binding fires first)
     // * Secondly by depth in the tree (so if Editor has a binding for "b" and workspace a
@@ -364,6 +351,11 @@ impl DispatchTree {
                 .or_insert_with(|| KeystrokeMatcher::new(self.keymap.clone()));
 
             let result = keystroke_matcher.match_keystroke(keystroke, &context_stack);
+            if result.pending && !pending && !bindings.is_empty() {
+                context_stack.pop();
+                continue;
+            }
+
             pending = result.pending || pending;
             for new_binding in result.bindings {
                 match bindings

crates/gpui/src/window.rs

@@ -2,12 +2,12 @@ use crate::{
     px, size, transparent_black, Action, AnyDrag, AnyView, AppContext, Arena, AsyncWindowContext,
     AvailableSpace, Bounds, Context, Corners, CursorStyle, DispatchActionListener, DispatchNodeId,
     DispatchTree, DisplayId, Edges, Effect, Entity, EntityId, EventEmitter, FileDropEvent, Flatten,
-    Global, GlobalElementId, Hsla, KeyBinding, KeyContext, KeyDownEvent, KeyMatch, KeymatchMode,
+    Global, GlobalElementId, Hsla, KeyBinding, KeyContext, KeyDownEvent, KeyMatch, KeymatchResult,
-    KeymatchResult, Keystroke, KeystrokeEvent, Model, ModelContext, Modifiers, MouseButton,
+    Keystroke, KeystrokeEvent, Model, ModelContext, Modifiers, MouseButton, MouseMoveEvent,
-    MouseMoveEvent, MouseUpEvent, Pixels, PlatformAtlas, PlatformDisplay, PlatformInput,
+    MouseUpEvent, Pixels, PlatformAtlas, PlatformDisplay, PlatformInput, PlatformWindow, Point,
-    PlatformWindow, Point, PromptLevel, Render, ScaledPixels, SharedString, Size, SubscriberSet,
+    PromptLevel, Render, ScaledPixels, SharedString, Size, SubscriberSet, Subscription,
-    Subscription, TaffyLayoutEngine, Task, View, VisualContext, WeakView, WindowAppearance,
+    TaffyLayoutEngine, Task, View, VisualContext, WeakView, WindowAppearance, WindowBounds,
-    WindowBounds, WindowOptions, WindowTextSystem,
+    WindowOptions, WindowTextSystem,
 };
 use anyhow::{anyhow, Context as _, Result};
 use collections::FxHashSet;
@@ -291,10 +291,6 @@ struct PendingInput {
 }
 
 impl PendingInput {
-    fn is_noop(&self) -> bool {
-        self.bindings.is_empty() && (self.keystrokes.iter().all(|k| k.ime_key.is_none()))
-    }
-
     fn input(&self) -> String {
         self.keystrokes
             .iter()
@@ -1282,21 +1278,12 @@ impl<'a> WindowContext<'a> {
             .dispatch_path(node_id);
 
         if let Some(key_down_event) = event.downcast_ref::<KeyDownEvent>() {
-            let KeymatchResult {
+            let KeymatchResult { bindings, pending } = self
-                bindings,
-                mut pending,
-            } = self
                 .window
                 .rendered_frame
                 .dispatch_tree
                 .dispatch_key(&key_down_event.keystroke, &dispatch_path);
 
-            if self.window.rendered_frame.dispatch_tree.keymatch_mode == KeymatchMode::Immediate
-                && !bindings.is_empty()
-            {
-                pending = false;
-            }
-
             if pending {
                 let mut currently_pending = self.window.pending_input.take().unwrap_or_default();
                 if currently_pending.focus.is_some() && currently_pending.focus != self.window.focus
@@ -1311,22 +1298,17 @@ impl<'a> WindowContext<'a> {
                     currently_pending.bindings.push(binding);
                 }
 
-                // for vim compatibility, we also should check "is input handler enabled"
+                currently_pending.timer = Some(self.spawn(|mut cx| async move {
-                if !currently_pending.is_noop() {
+                    cx.background_executor.timer(Duration::from_secs(1)).await;
-                    currently_pending.timer = Some(self.spawn(|mut cx| async move {
+                    cx.update(move |cx| {
-                        cx.background_executor.timer(Duration::from_secs(1)).await;
+                        cx.clear_pending_keystrokes();
-                        cx.update(move |cx| {
+                        let Some(currently_pending) = cx.window.pending_input.take() else {
-                            cx.clear_pending_keystrokes();
+                            return;
-                            let Some(currently_pending) = cx.window.pending_input.take() else {
+                        };
-                                return;
+                        cx.replay_pending_input(currently_pending)
-                            };
+                    })
-                            cx.replay_pending_input(currently_pending)
+                    .log_err();
-                        })
+                }));
-                        .log_err();
-                    }));
-                } else {
-                    currently_pending.timer = None;
-                }
                 self.window.pending_input = Some(currently_pending);
 
                 self.propagate_event = false;
@@ -1354,8 +1336,21 @@ impl<'a> WindowContext<'a> {
             }
         }
 
+        self.dispatch_key_down_up_event(event, &dispatch_path);
+        if !self.propagate_event {
+            return;
+        }
+
+        self.dispatch_keystroke_observers(event, None);
+    }
+
+    fn dispatch_key_down_up_event(
+        &mut self,
+        event: &dyn Any,
+        dispatch_path: &SmallVec<[DispatchNodeId; 32]>,
+    ) {
         // Capture phase
-        for node_id in &dispatch_path {
+        for node_id in dispatch_path {
             let node = self.window.rendered_frame.dispatch_tree.node(*node_id);
 
             for key_listener in node.key_listeners.clone() {
@@ -1381,8 +1376,6 @@ impl<'a> WindowContext<'a> {
                 }
             }
         }
-
-        self.dispatch_keystroke_observers(event, None);
     }
 
     /// Determine whether a potential multi-stroke key binding is in progress on this window.
@@ -1419,6 +1412,24 @@ impl<'a> WindowContext<'a> {
             }
         }
 
+        let dispatch_path = self
+            .window
+            .rendered_frame
+            .dispatch_tree
+            .dispatch_path(node_id);
+
+        for keystroke in currently_pending.keystrokes {
+            let event = KeyDownEvent {
+                keystroke,
+                is_held: false,
+            };
+
+            self.dispatch_key_down_up_event(&event, &dispatch_path);
+            if !self.propagate_event {
+                return;
+            }
+        }
+
         if !input.is_empty() {
             if let Some(mut input_handler) = self.window.platform_window.take_input_handler() {
                 input_handler.flush_pending_input(&input, self);

crates/gpui/src/window/element_cx.rs

@@ -31,11 +31,11 @@ use crate::{
     prelude::*, size, AnyTooltip, AppContext, AvailableSpace, Bounds, BoxShadow, ContentMask,
     Corners, CursorStyle, DevicePixels, DispatchPhase, DispatchTree, ElementId, ElementStateBox,
     EntityId, FocusHandle, FocusId, FontId, GlobalElementId, GlyphId, Hsla, ImageData,
-    InputHandler, IsZero, KeyContext, KeyEvent, KeymatchMode, LayoutId, MonochromeSprite,
+    InputHandler, IsZero, KeyContext, KeyEvent, LayoutId, MonochromeSprite, MouseEvent, PaintQuad,
-    MouseEvent, PaintQuad, Path, Pixels, PlatformInputHandler, Point, PolychromeSprite, Quad,
+    Path, Pixels, PlatformInputHandler, Point, PolychromeSprite, Quad, RenderGlyphParams,
-    RenderGlyphParams, RenderImageParams, RenderSvgParams, Scene, Shadow, SharedString, Size,
+    RenderImageParams, RenderSvgParams, Scene, Shadow, SharedString, Size, StackingContext,
-    StackingContext, StackingOrder, Style, Surface, TextStyleRefinement, Underline, UnderlineStyle,
+    StackingOrder, Style, Surface, TextStyleRefinement, Underline, UnderlineStyle, Window,
-    Window, WindowContext, SUBPIXEL_VARIANTS,
+    WindowContext, SUBPIXEL_VARIANTS,
 };
 
 type AnyMouseListener = Box<dyn FnMut(&dyn Any, DispatchPhase, &mut ElementContext) + 'static>;
@@ -1143,15 +1143,6 @@ impl<'a> ElementContext<'a> {
         }
     }
 
-    /// keymatch mode immediate instructs GPUI to prefer shorter action bindings.
-    /// In the case that you have a keybinding of `"cmd-k": "terminal::Clear"` and
-    /// `"cmd-k left": "workspace::MoveLeft"`, GPUI will by default wait for 1s after
-    /// you type cmd-k to see if you're going to type left.
-    /// This is problematic in the terminal
-    pub fn keymatch_mode_immediate(&mut self) {
-        self.window.next_frame.dispatch_tree.keymatch_mode = KeymatchMode::Immediate;
-    }
-
     /// Register a mouse event listener on the window for the next frame. The type of event
     /// is determined by the first parameter of the given listener. When the next frame is rendered
     /// the listener will be cleared.

crates/terminal_view/src/terminal_element.rs

@@ -776,7 +776,6 @@ impl Element for TerminalElement {
         self.interactivity
             .paint(bounds, bounds.size, state, cx, |_, _, cx| {
                 cx.handle_input(&self.focus, terminal_input_handler);
-                cx.keymatch_mode_immediate();
 
                 cx.on_key_event({
                     let this = self.terminal.clone();