Iterate over keymap then dispatch path when matching keybindings to make precedence more intuitive

Kay Simmons created 2 years ago

Rename action which adds the active tab to the dock to be more intuitive
Add action which moves the active tab out of the dock and bind it to the same keybinding

Change summary

assets/keymaps/default.json       |  9 +++---
crates/gpui/src/keymap_matcher.rs | 33 ++++++++++++++-----------
crates/workspace/src/dock.rs      | 42 +++++++++++++++++++++++++++++++-
3 files changed, 63 insertions(+), 21 deletions(-)

Detailed changes

assets/keymaps/default.json

@@ -450,15 +450,16 @@
         }
     },
     {
-        "context": "Dock",
+        "context": "Pane",
         "bindings": {
-            "shift-escape": "dock::HideDock"
+            "cmd-escape": "dock::AddTabToDock"
         }
     },
     {
-        "context": "Pane",
+        "context": "Dock",
         "bindings": {
-            "cmd-escape": "dock::MoveActiveItemToDock"
+            "shift-escape": "dock::HideDock",
+            "cmd-escape": "dock::RemoveTabFromDock"
         }
     },
     {

crates/gpui/src/keymap_matcher.rs

@@ -75,29 +75,32 @@ impl KeymapMatcher {
         self.contexts
             .extend(dispatch_path.iter_mut().map(|e| std::mem::take(&mut e.1)));
 
-        for (i, (view_id, _)) in dispatch_path.into_iter().enumerate() {
-            // Don't require pending view entry if there are no pending keystrokes
-            if !first_keystroke && !self.pending_views.contains_key(&view_id) {
-                continue;
-            }
-
-            // If there is a previous view context, invalidate that view if it
-            // has changed
-            if let Some(previous_view_context) = self.pending_views.remove(&view_id) {
-                if previous_view_context != self.contexts[i] {
+        // Find the bindings which map the pending keystrokes and current context
+        // Iterate over the bindings in precedence order before the dispatch path so that
+        // users have more control over precedence rules
+        for binding in self.keymap.bindings().iter().rev() {
+            for (i, (view_id, _)) in dispatch_path.iter().enumerate() {
+                // Don't require pending view entry if there are no pending keystrokes
+                if !first_keystroke && !self.pending_views.contains_key(view_id) {
                     continue;
                 }
-            }
 
-            // Find the bindings which map the pending keystrokes and current context
-            for binding in self.keymap.bindings().iter().rev() {
+                // If there is a previous view context, invalidate that view if it
+                // has changed
+                if let Some(previous_view_context) = self.pending_views.remove(view_id) {
+                    if previous_view_context != self.contexts[i] {
+                        continue;
+                    }
+                }
+
                 match binding.match_keys_and_context(&self.pending_keystrokes, &self.contexts[i..])
                 {
                     BindingMatchResult::Complete(action) => {
-                        matched_bindings.push((view_id, action))
+                        matched_bindings.push((*view_id, action))
                     }
                     BindingMatchResult::Partial => {
-                        self.pending_views.insert(view_id, self.contexts[i].clone());
+                        self.pending_views
+                            .insert(*view_id, self.contexts[i].clone());
                         any_pending = true;
                     }
                     _ => {}

crates/workspace/src/dock.rs

@@ -30,7 +30,8 @@ actions!(
         AnchorDockRight,
         AnchorDockBottom,
         ExpandDock,
-        MoveActiveItemToDock,
+        AddTabToDock,
+        RemoveTabFromDock,
     ]
 );
 impl_internal_actions!(dock, [MoveDock, AddDefaultItemToDock]);
@@ -55,7 +56,8 @@ pub fn init(cx: &mut MutableAppContext) {
         },
     );
     cx.add_action(
-        |workspace: &mut Workspace, _: &MoveActiveItemToDock, cx: &mut ViewContext<Workspace>| {
+        |workspace: &mut Workspace, _: &AddTabToDock, cx: &mut ViewContext<Workspace>| {
+            eprintln!("Add tab to dock");
             if let Some(active_item) = workspace.active_item(cx) {
                 let item_id = active_item.id();
 
@@ -67,6 +69,42 @@ pub fn init(cx: &mut MutableAppContext) {
 
                 let destination_index = to.read(cx).items_len() + 1;
 
+                Pane::move_item(
+                    workspace,
+                    from.clone(),
+                    to.clone(),
+                    item_id,
+                    destination_index,
+                    cx,
+                );
+            }
+        },
+    );
+    cx.add_action(
+        |workspace: &mut Workspace, _: &RemoveTabFromDock, cx: &mut ViewContext<Workspace>| {
+            eprintln!("Removing tab from dock");
+            if let Some(active_item) = workspace.active_item(cx) {
+                let item_id = active_item.id();
+
+                let from = workspace.dock_pane();
+                let to = workspace
+                    .last_active_center_pane
+                    .as_ref()
+                    .and_then(|pane| pane.upgrade(cx))
+                    .unwrap_or_else(|| {
+                        workspace
+                            .panes
+                            .first()
+                            .expect("There must be a pane")
+                            .clone()
+                    });
+
+                if from.id() == to.id() {
+                    return;
+                }
+
+                let destination_index = to.read(cx).items_len() + 1;
+
                 Pane::move_item(
                     workspace,
                     from.clone(),