Merge pull request #2207 from zed-industries/project-panel-actions

Fix project panel actions

assets/keymaps/default.json

@@ -457,7 +457,7 @@
         }
     },
     {
-        "context": "Dock",
+        "context": "Pane && docked",
         "bindings": {
             "shift-escape": "dock::HideDock",
             "cmd-escape": "dock::RemoveTabFromDock"

crates/collab_ui/src/contact_list.rs

@@ -1294,7 +1294,7 @@ impl View for ContactList {
 
     fn keymap_context(&self, _: &AppContext) -> KeymapContext {
         let mut cx = Self::default_keymap_context();
-        cx.set.insert("menu".into());
+        cx.add_identifier("menu");
         cx
     }
 

crates/context_menu/src/context_menu.rs

@@ -82,7 +82,7 @@ impl View for ContextMenu {
 
     fn keymap_context(&self, _: &AppContext) -> KeymapContext {
         let mut cx = Self::default_keymap_context();
-        cx.set.insert("menu".into());
+        cx.add_identifier("menu");
         cx
     }
 

crates/editor/src/editor.rs

@@ -6433,17 +6433,13 @@ impl View for Editor {
             EditorMode::AutoHeight { .. } => "auto_height",
             EditorMode::Full => "full",
         };
-        context.map.insert("mode".into(), mode.into());
+        context.add_key("mode", mode);
         if self.pending_rename.is_some() {
-            context.set.insert("renaming".into());
+            context.add_identifier("renaming");
         }
         match self.context_menu.as_ref() {
-            Some(ContextMenu::Completions(_)) => {
+            Some(ContextMenu::Completions(_)) => context.add_identifier("showing_completions"),
-                context.set.insert("showing_completions".into());
+            Some(ContextMenu::CodeActions(_)) => context.add_identifier("showing_code_actions"),
-            }
-            Some(ContextMenu::CodeActions(_)) => {
-                context.set.insert("showing_code_actions".into());
-            }
             None => {}
         }
 

crates/gpui/src/app.rs

@@ -86,7 +86,7 @@ pub trait View: Entity + Sized {
     }
     fn default_keymap_context() -> keymap_matcher::KeymapContext {
         let mut cx = keymap_matcher::KeymapContext::default();
-        cx.set.insert(Self::ui_name().into());
+        cx.add_identifier(Self::ui_name());
         cx
     }
     fn debug_json(&self, _: &AppContext) -> serde_json::Value {
@@ -6639,12 +6639,12 @@ mod tests {
         let mut view_1 = View::new(1);
         let mut view_2 = View::new(2);
         let mut view_3 = View::new(3);
-        view_1.keymap_context.set.insert("a".into());
+        view_1.keymap_context.add_identifier("a");
-        view_2.keymap_context.set.insert("a".into());
+        view_2.keymap_context.add_identifier("a");
-        view_2.keymap_context.set.insert("b".into());
+        view_2.keymap_context.add_identifier("b");
-        view_3.keymap_context.set.insert("a".into());
+        view_3.keymap_context.add_identifier("a");
-        view_3.keymap_context.set.insert("b".into());
+        view_3.keymap_context.add_identifier("b");
-        view_3.keymap_context.set.insert("c".into());
+        view_3.keymap_context.add_identifier("c");
 
         let (window_id, view_1) = cx.add_window(Default::default(), |_| view_1);
         let view_2 = cx.add_view(&view_1, |_| view_2);

crates/gpui/src/app/action.rs

@@ -16,6 +16,14 @@ pub trait Action: 'static {
         Self: Sized;
 }
 
+impl std::fmt::Debug for dyn Action {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("dyn Action")
+            .field("namespace", &self.namespace())
+            .field("name", &self.name())
+            .finish()
+    }
+}
 /// Define a set of unit struct types that all implement the `Action` trait.
 ///
 /// The first argument is a namespace that will be associated with each of

crates/gpui/src/keymap_matcher.rs

@@ -5,7 +5,7 @@ mod keystroke;
 
 use std::{any::TypeId, fmt::Debug};
 
-use collections::{BTreeMap, HashMap};
+use collections::HashMap;
 use smallvec::SmallVec;
 
 use crate::Action;
@@ -68,8 +68,8 @@ impl KeymapMatcher {
     ///         There exist bindings which are still waiting for more keys.
     ///     MatchResult::Complete(matches) =>
     ///         1 or more bindings have recieved the necessary key presses.
-    ///         The order of the matched actions is by order in the keymap file first and
+    ///         The order of the matched actions is by position of the matching first,
-    ///         position of the matching view second.
+    //          and order in the keymap second.
     pub fn push_keystroke(
         &mut self,
         keystroke: Keystroke,
@@ -80,8 +80,7 @@ impl KeymapMatcher {
         // and then the order the binding matched in the view tree second.
         // The key is the reverse position of the binding in the bindings list so that later bindings
         // match before earlier ones in the user's config
-        let mut matched_bindings: BTreeMap<usize, Vec<(usize, Box<dyn Action>)>> =
+        let mut matched_bindings: Vec<(usize, Box<dyn Action>)> = Default::default();
-            Default::default();
 
         let first_keystroke = self.pending_keystrokes.is_empty();
         self.pending_keystrokes.push(keystroke.clone());
@@ -105,14 +104,11 @@ impl KeymapMatcher {
                 }
             }
 
-            for (order, binding) in self.keymap.bindings().iter().rev().enumerate() {
+            for binding in self.keymap.bindings().iter().rev() {
                 match binding.match_keys_and_context(&self.pending_keystrokes, &self.contexts[i..])
                 {
                     BindingMatchResult::Complete(action) => {
-                        matched_bindings
+                        matched_bindings.push((*view_id, action));
-                            .entry(order)
-                            .or_default()
-                            .push((*view_id, action));
                     }
                     BindingMatchResult::Partial => {
                         self.pending_views
@@ -131,7 +127,7 @@ impl KeymapMatcher {
         if !matched_bindings.is_empty() {
             // Collect the sorted matched bindings into the final vec for ease of use
             // Matched bindings are in order by precedence
-            MatchResult::Matches(matched_bindings.into_values().flatten().collect())
+            MatchResult::Matches(matched_bindings)
         } else if any_pending {
             MatchResult::Pending
         } else {
@@ -225,15 +221,47 @@ mod tests {
 
     use super::*;
 
+    #[test]
+    fn test_keymap_and_view_ordering() -> Result<()> {
+        actions!(test, [EditorAction, ProjectPanelAction]);
+
+        let mut editor = KeymapContext::default();
+        editor.add_identifier("Editor");
+
+        let mut project_panel = KeymapContext::default();
+        project_panel.add_identifier("ProjectPanel");
+
+        // Editor 'deeper' in than project panel
+        let dispatch_path = vec![(2, editor), (1, project_panel)];
+
+        // But editor actions 'higher' up in keymap
+        let keymap = Keymap::new(vec![
+            Binding::new("left", EditorAction, Some("Editor")),
+            Binding::new("left", ProjectPanelAction, Some("ProjectPanel")),
+        ]);
+
+        let mut matcher = KeymapMatcher::new(keymap);
+
+        assert_eq!(
+            matcher.push_keystroke(Keystroke::parse("left")?, dispatch_path.clone()),
+            MatchResult::Matches(vec![
+                (2, Box::new(EditorAction)),
+                (1, Box::new(ProjectPanelAction)),
+            ]),
+        );
+
+        Ok(())
+    }
+
     #[test]
     fn test_push_keystroke() -> Result<()> {
         actions!(test, [B, AB, C, D, DA, E, EF]);
 
         let mut context1 = KeymapContext::default();
-        context1.set.insert("1".into());
+        context1.add_identifier("1");
 
         let mut context2 = KeymapContext::default();
-        context2.set.insert("2".into());
+        context2.add_identifier("2");
 
         let dispatch_path = vec![(2, context2), (1, context1)];
 
@@ -367,22 +395,22 @@ mod tests {
         let predicate = KeymapContextPredicate::parse("a && b || c == d").unwrap();
 
         let mut context = KeymapContext::default();
-        context.set.insert("a".into());
+        context.add_identifier("a");
         assert!(!predicate.eval(&[context]));
 
         let mut context = KeymapContext::default();
-        context.set.insert("a".into());
+        context.add_identifier("a");
-        context.set.insert("b".into());
+        context.add_identifier("b");
         assert!(predicate.eval(&[context]));
 
         let mut context = KeymapContext::default();
-        context.set.insert("a".into());
+        context.add_identifier("a");
-        context.map.insert("c".into(), "x".into());
+        context.add_key("c", "x");
         assert!(!predicate.eval(&[context]));
 
         let mut context = KeymapContext::default();
-        context.set.insert("a".into());
+        context.add_identifier("a");
-        context.map.insert("c".into(), "d".into());
+        context.add_key("c", "d");
         assert!(predicate.eval(&[context]));
 
         let predicate = KeymapContextPredicate::parse("!a").unwrap();
@@ -422,10 +450,11 @@ mod tests {
         assert!(!predicate.eval(&contexts[6..]));
 
         fn context_set(names: &[&str]) -> KeymapContext {
-            KeymapContext {
+            let mut keymap = KeymapContext::new();
-                set: names.iter().copied().map(str::to_string).collect(),
+            names
-                ..Default::default()
+                .iter()
-            }
+                .for_each(|name| keymap.add_identifier(name.to_string()));
+            keymap
         }
     }
 
@@ -448,10 +477,10 @@ mod tests {
         ]);
 
         let mut context_a = KeymapContext::default();
-        context_a.set.insert("a".into());
+        context_a.add_identifier("a");
 
         let mut context_b = KeymapContext::default();
-        context_b.set.insert("b".into());
+        context_b.add_identifier("b");
 
         let mut matcher = KeymapMatcher::new(keymap);
 
@@ -496,7 +525,7 @@ mod tests {
         matcher.clear_pending();
 
         let mut context_c = KeymapContext::default();
-        context_c.set.insert("c".into());
+        context_c.add_identifier("c");
 
         // Pending keystrokes are maintained per-view
         assert_eq!(

crates/gpui/src/keymap_matcher/keymap_context.rs

@@ -1,13 +1,22 @@
+use std::borrow::Cow;
+
 use anyhow::{anyhow, Result};
 use collections::{HashMap, HashSet};
 
 #[derive(Clone, Debug, Default, Eq, PartialEq)]
 pub struct KeymapContext {
-    pub set: HashSet<String>,
+    set: HashSet<Cow<'static, str>>,
-    pub map: HashMap<String, String>,
+    map: HashMap<Cow<'static, str>, Cow<'static, str>>,
 }
 
 impl KeymapContext {
+    pub fn new() -> Self {
+        KeymapContext {
+            set: HashSet::default(),
+            map: HashMap::default(),
+        }
+    }
+
     pub fn extend(&mut self, other: &Self) {
         for v in &other.set {
             self.set.insert(v.clone());
@@ -16,6 +25,18 @@ impl KeymapContext {
             self.map.insert(k.clone(), v.clone());
         }
     }
+
+    pub fn add_identifier<I: Into<Cow<'static, str>>>(&mut self, identifier: I) {
+        self.set.insert(identifier.into());
+    }
+
+    pub fn add_key<S1: Into<Cow<'static, str>>, S2: Into<Cow<'static, str>>>(
+        &mut self,
+        key: S1,
+        value: S2,
+    ) {
+        self.map.insert(key.into(), value.into());
+    }
 }
 
 #[derive(Debug, Eq, PartialEq)]
@@ -46,12 +67,12 @@ impl KeymapContextPredicate {
             Self::Identifier(name) => (&context.set).contains(name.as_str()),
             Self::Equal(left, right) => context
                 .map
-                .get(left)
+                .get(left.as_str())
                 .map(|value| value == right)
                 .unwrap_or(false),
             Self::NotEqual(left, right) => context
                 .map
-                .get(left)
+                .get(left.as_str())
                 .map(|value| value != right)
                 .unwrap_or(true),
             Self::Not(pred) => !pred.eval(contexts),

crates/picker/src/picker.rs

@@ -126,7 +126,7 @@ impl<D: PickerDelegate> View for Picker<D> {
 
     fn keymap_context(&self, _: &AppContext) -> KeymapContext {
         let mut cx = Self::default_keymap_context();
-        cx.set.insert("menu".into());
+        cx.add_identifier("menu");
         cx
     }
 

crates/project_panel/src/project_panel.rs

@@ -1314,7 +1314,7 @@ impl View for ProjectPanel {
 
     fn keymap_context(&self, _: &AppContext) -> KeymapContext {
         let mut cx = Self::default_keymap_context();
-        cx.set.insert("menu".into());
+        cx.add_identifier("menu");
         cx
     }
 }

crates/terminal_view/src/terminal_view.rs

@@ -469,53 +469,50 @@ impl View for TerminalView {
         let mut context = Self::default_keymap_context();
 
         let mode = self.terminal.read(cx).last_content.mode;
-        context.map.insert(
+        context.add_key(
-            "screen".to_string(),
+            "screen",
-            (if mode.contains(TermMode::ALT_SCREEN) {
+            if mode.contains(TermMode::ALT_SCREEN) {
                 "alt"
             } else {
                 "normal"
-            })
+            },
-            .to_string(),
         );
 
         if mode.contains(TermMode::APP_CURSOR) {
-            context.set.insert("DECCKM".to_string());
+            context.add_identifier("DECCKM");
         }
         if mode.contains(TermMode::APP_KEYPAD) {
-            context.set.insert("DECPAM".to_string());
+            context.add_identifier("DECPAM");
-        }
+        } else {
-        //Note the ! here
+            context.add_identifier("DECPNM");
-        if !mode.contains(TermMode::APP_KEYPAD) {
-            context.set.insert("DECPNM".to_string());
         }
         if mode.contains(TermMode::SHOW_CURSOR) {
-            context.set.insert("DECTCEM".to_string());
+            context.add_identifier("DECTCEM");
         }
         if mode.contains(TermMode::LINE_WRAP) {
-            context.set.insert("DECAWM".to_string());
+            context.add_identifier("DECAWM");
         }
         if mode.contains(TermMode::ORIGIN) {
-            context.set.insert("DECOM".to_string());
+            context.add_identifier("DECOM");
         }
         if mode.contains(TermMode::INSERT) {
-            context.set.insert("IRM".to_string());
+            context.add_identifier("IRM");
         }
         //LNM is apparently the name for this. https://vt100.net/docs/vt510-rm/LNM.html
         if mode.contains(TermMode::LINE_FEED_NEW_LINE) {
-            context.set.insert("LNM".to_string());
+            context.add_identifier("LNM");
         }
         if mode.contains(TermMode::FOCUS_IN_OUT) {
-            context.set.insert("report_focus".to_string());
+            context.add_identifier("report_focus");
         }
         if mode.contains(TermMode::ALTERNATE_SCROLL) {
-            context.set.insert("alternate_scroll".to_string());
+            context.add_identifier("alternate_scroll");
         }
         if mode.contains(TermMode::BRACKETED_PASTE) {
-            context.set.insert("bracketed_paste".to_string());
+            context.add_identifier("bracketed_paste");
         }
         if mode.intersects(TermMode::MOUSE_MODE) {
-            context.set.insert("any_mouse_reporting".to_string());
+            context.add_identifier("any_mouse_reporting");
         }
         {
             let mouse_reporting = if mode.contains(TermMode::MOUSE_REPORT_CLICK) {
@@ -527,9 +524,7 @@ impl View for TerminalView {
             } else {
                 "off"
             };
-            context
+            context.add_key("mouse_reporting", mouse_reporting);
-                .map
-                .insert("mouse_reporting".to_string(), mouse_reporting.to_string());
         }
         {
             let format = if mode.contains(TermMode::SGR_MOUSE) {
@@ -539,9 +534,7 @@ impl View for TerminalView {
             } else {
                 "normal"
             };
-            context
+            context.add_key("mouse_format", format);
-                .map
-                .insert("mouse_format".to_string(), format.to_string());
         }
         context
     }

crates/vim/src/state.rs

@@ -73,34 +73,30 @@ impl VimState {
 
     pub fn keymap_context_layer(&self) -> KeymapContext {
         let mut context = KeymapContext::default();
-        context.map.insert(
+        context.add_key(
-            "vim_mode".to_string(),
+            "vim_mode",
             match self.mode {
                 Mode::Normal => "normal",
                 Mode::Visual { .. } => "visual",
                 Mode::Insert => "insert",
-            }
+            },
-            .to_string(),
         );
 
         if self.vim_controlled() {
-            context.set.insert("VimControl".to_string());
+            context.add_identifier("VimControl");
         }
 
         let active_operator = self.operator_stack.last();
 
         if let Some(active_operator) = active_operator {
             for context_flag in active_operator.context_flags().into_iter() {
-                context.set.insert(context_flag.to_string());
+                context.add_identifier(*context_flag);
             }
         }
 
-        context.map.insert(
+        context.add_key(
-            "vim_operator".to_string(),
+            "vim_operator",
-            active_operator
+            active_operator.map(|op| op.id()).unwrap_or_else(|| "none"),
-                .map(|op| op.id())
-                .unwrap_or_else(|| "none")
-                .to_string(),
         );
 
         context

crates/workspace/src/pane.rs

@@ -21,6 +21,7 @@ use gpui::{
         vector::{vec2f, Vector2F},
     },
     impl_actions, impl_internal_actions,
+    keymap_matcher::KeymapContext,
     platform::{CursorStyle, NavigationDirection},
     Action, AnyViewHandle, AnyWeakViewHandle, AppContext, AsyncAppContext, Entity, EventContext,
     ModelHandle, MouseButton, MutableAppContext, PromptLevel, Quad, RenderContext, Task, View,
@@ -1550,6 +1551,14 @@ impl View for Pane {
             }
         }
     }
+
+    fn keymap_context(&self, _: &AppContext) -> KeymapContext {
+        let mut keymap = Self::default_keymap_context();
+        if self.docked.is_some() {
+            keymap.add_identifier("docked");
+        }
+        keymap
+    }
 }
 
 fn tab_bar_button<A: Action>(

crates/workspace/src/workspace.rs

@@ -2716,11 +2716,7 @@ impl View for Workspace {
     }
 
     fn keymap_context(&self, _: &AppContext) -> KeymapContext {
-        let mut keymap = Self::default_keymap_context();
+        Self::default_keymap_context()
-        if self.active_pane() == self.dock_pane() {
-            keymap.set.insert("Dock".into());
-        }
-        keymap
     }
 }