Fixed a bug where the command palette wouldn't check the keymap context when showing available actions

Mikayla Maki and Max created

Fixed a bug where context menus wouldn't show action keystrokes
WIP Fixing a bug where tooltips won't show action keystrokes

Co-Authored-By: Max <max@zed.dev>

Change summary

crates/context_menu/src/context_menu.rs          | 10 ++
crates/gpui/src/app.rs                           | 74 ++++++++++-------
crates/gpui/src/elements/keystroke_label.rs      | 11 ++
crates/gpui/src/elements/tooltip.rs              | 16 +++
crates/gpui/src/keymap_matcher/binding.rs        |  2 
crates/gpui/src/keymap_matcher/keymap_context.rs |  2 
crates/gpui/src/presenter.rs                     |  9 --
7 files changed, 80 insertions(+), 44 deletions(-)

Detailed changes

crates/context_menu/src/context_menu.rs 🔗

@@ -63,6 +63,7 @@ pub struct ContextMenu {
     visible: bool,
     previously_focused_view_id: Option<usize>,
     clicked: bool,
+    parent_view_id: usize,
     _actions_observation: Subscription,
 }
 
@@ -114,6 +115,8 @@ impl View for ContextMenu {
 
 impl ContextMenu {
     pub fn new(cx: &mut ViewContext<Self>) -> Self {
+        let parent_view_id = cx.parent().unwrap();
+
         Self {
             show_count: 0,
             anchor_position: Default::default(),
@@ -123,6 +126,7 @@ impl ContextMenu {
             visible: Default::default(),
             previously_focused_view_id: Default::default(),
             clicked: false,
+            parent_view_id,
             _actions_observation: cx.observe_actions(Self::action_dispatched),
         }
     }
@@ -251,6 +255,7 @@ impl ContextMenu {
     }
 
     fn render_menu_for_measurement(&self, cx: &mut RenderContext<Self>) -> impl Element {
+        let window_id = cx.window_id();
         let style = cx.global::<Settings>().theme.context_menu.clone();
         Flex::row()
             .with_child(
@@ -289,6 +294,8 @@ impl ContextMenu {
                                     Some(ix) == self.selected_index,
                                 );
                                 KeystrokeLabel::new(
+                                    window_id,
+                                    self.parent_view_id,
                                     action.boxed_clone(),
                                     style.keystroke.container,
                                     style.keystroke.text.clone(),
@@ -318,6 +325,7 @@ impl ContextMenu {
 
         let style = cx.global::<Settings>().theme.context_menu.clone();
 
+        let window_id = cx.window_id();
         MouseEventHandler::<Menu>::new(0, cx, |_, cx| {
             Flex::column()
                 .with_children(self.items.iter().enumerate().map(|(ix, item)| {
@@ -337,6 +345,8 @@ impl ContextMenu {
                                     )
                                     .with_child({
                                         KeystrokeLabel::new(
+                                            window_id,
+                                            self.parent_view_id,
                                             action.boxed_clone(),
                                             style.keystroke.container,
                                             style.keystroke.text.clone(),

crates/gpui/src/app.rs 🔗

@@ -1333,6 +1333,31 @@ impl MutableAppContext {
         self.action_deserializers.keys().copied()
     }
 
+    /// Return keystrokes that would dispatch the given action on the given view.
+    pub(crate) fn keystrokes_for_action(
+        &mut self,
+        window_id: usize,
+        view_id: usize,
+        action: &dyn Action,
+    ) -> Option<SmallVec<[Keystroke; 2]>> {
+        let mut contexts = Vec::new();
+        for view_id in self.ancestors(window_id, view_id) {
+            if let Some(view) = self.views.get(&(window_id, view_id)) {
+                contexts.push(view.keymap_context(self));
+            }
+        }
+
+        self.keystroke_matcher
+            .bindings_for_action_type(action.as_any().type_id())
+            .find_map(|b| {
+                if b.match_context(&contexts) {
+                    b.keystrokes().map(|s| s.into())
+                } else {
+                    None
+                }
+            })
+    }
+
     pub fn available_actions(
         &self,
         window_id: usize,
@@ -1340,8 +1365,10 @@ impl MutableAppContext {
     ) -> impl Iterator<Item = (&'static str, Box<dyn Action>, SmallVec<[&Binding; 1]>)> {
         let mut action_types: HashSet<_> = self.global_actions.keys().copied().collect();
 
+        let mut contexts = Vec::new();
         for view_id in self.ancestors(window_id, view_id) {
             if let Some(view) = self.views.get(&(window_id, view_id)) {
+                contexts.push(view.keymap_context(self));
                 let view_type = view.as_any().type_id();
                 if let Some(actions) = self.actions.get(&view_type) {
                     action_types.extend(actions.keys().copied());
@@ -1358,6 +1385,7 @@ impl MutableAppContext {
                         deserialize("{}").ok()?,
                         self.keystroke_matcher
                             .bindings_for_action_type(*type_id)
+                            .filter(|b| b.match_context(&contexts))
                             .collect(),
                     ))
                 } else {
@@ -1385,34 +1413,6 @@ impl MutableAppContext {
         self.global_actions.contains_key(&action_type)
     }
 
-    /// Return keystrokes that would dispatch the given action closest to the focused view, if there are any.
-    pub(crate) fn keystrokes_for_action(
-        &mut self,
-        window_id: usize,
-        view_stack: &[usize],
-        action: &dyn Action,
-    ) -> Option<SmallVec<[Keystroke; 2]>> {
-        self.keystroke_matcher.contexts.clear();
-        for view_id in view_stack.iter().rev() {
-            let view = self
-                .cx
-                .views
-                .get(&(window_id, *view_id))
-                .expect("view in responder chain does not exist");
-            self.keystroke_matcher
-                .contexts
-                .push(view.keymap_context(self.as_ref()));
-            let keystrokes = self
-                .keystroke_matcher
-                .keystrokes_for_action(action, &self.keystroke_matcher.contexts);
-            if keystrokes.is_some() {
-                return keystrokes;
-            }
-        }
-
-        None
-    }
-
     // Traverses the parent tree. Walks down the tree toward the passed
     // view calling visit with true. Then walks back up the tree calling visit with false.
     // If `visit` returns false this function will immediately return.
@@ -1916,10 +1916,11 @@ impl MutableAppContext {
     {
         self.update(|this| {
             let view_id = post_inc(&mut this.next_entity_id);
+            // Make sure we can tell child views about their parent
+            this.cx.parents.insert((window_id, view_id), parent_id);
             let mut cx = ViewContext::new(this, window_id, view_id);
             let handle = if let Some(view) = build_view(&mut cx) {
                 this.cx.views.insert((window_id, view_id), Box::new(view));
-                this.cx.parents.insert((window_id, view_id), parent_id);
                 if let Some(window) = this.cx.windows.get_mut(&window_id) {
                     window
                         .invalidation
@@ -1929,6 +1930,7 @@ impl MutableAppContext {
                 }
                 Some(ViewHandle::new(window_id, view_id, &this.cx.ref_counts))
             } else {
+                this.cx.parents.remove(&(window_id, view_id));
                 None
             };
             handle
@@ -2810,6 +2812,16 @@ impl AppContext {
             }))
     }
 
+    /// Returns the id of the parent of the given view, or none if the given
+    /// view is the root.
+    fn parent(&self, window_id: usize, view_id: usize) -> Option<usize> {
+        if let Some(ParentId::View(view_id)) = self.parents.get(&(window_id, view_id)) {
+            Some(*view_id)
+        } else {
+            None
+        }
+    }
+
     pub fn is_child_focused(&self, view: impl Into<AnyViewHandle>) -> bool {
         let view = view.into();
         if let Some(focused_view_id) = self.focused_view_id(view.window_id) {
@@ -3853,6 +3865,10 @@ impl<'a, T: View> ViewContext<'a, T> {
             .build_and_insert_view(self.window_id, ParentId::View(self.view_id), build_view)
     }
 
+    pub fn parent(&mut self) -> Option<usize> {
+        self.cx.parent(self.window_id, self.view_id)
+    }
+
     pub fn reparent(&mut self, view_handle: impl Into<AnyViewHandle>) {
         let view_handle = view_handle.into();
         if self.window_id != view_handle.window_id {

crates/gpui/src/elements/keystroke_label.rs 🔗

@@ -12,15 +12,21 @@ pub struct KeystrokeLabel {
     action: Box<dyn Action>,
     container_style: ContainerStyle,
     text_style: TextStyle,
+    window_id: usize,
+    view_id: usize,
 }
 
 impl KeystrokeLabel {
     pub fn new(
+        window_id: usize,
+        view_id: usize,
         action: Box<dyn Action>,
         container_style: ContainerStyle,
         text_style: TextStyle,
     ) -> Self {
         Self {
+            window_id,
+            view_id,
             action,
             container_style,
             text_style,
@@ -37,7 +43,10 @@ impl Element for KeystrokeLabel {
         constraint: SizeConstraint,
         cx: &mut LayoutContext,
     ) -> (Vector2F, ElementBox) {
-        let mut element = if let Some(keystrokes) = cx.keystrokes_for_action(self.action.as_ref()) {
+        let mut element = if let Some(keystrokes) =
+            cx.app
+                .keystrokes_for_action(self.window_id, self.view_id, self.action.as_ref())
+        {
             Flex::row()
                 .with_children(keystrokes.iter().map(|keystroke| {
                     Label::new(keystroke.to_string(), self.text_style.clone())

crates/gpui/src/elements/tooltip.rs 🔗

@@ -61,11 +61,14 @@ impl Tooltip {
     ) -> Self {
         struct ElementState<Tag>(Tag);
         struct MouseEventHandlerState<Tag>(Tag);
+        let focused_view_id = cx.focused_view_id(cx.window_id).unwrap();
 
         let state_handle = cx.default_element_state::<ElementState<Tag>, Rc<TooltipState>>(id);
         let state = state_handle.read(cx).clone();
         let tooltip = if state.visible.get() {
             let mut collapsed_tooltip = Self::render_tooltip(
+                cx.window_id,
+                focused_view_id,
                 text.clone(),
                 style.clone(),
                 action.as_ref().map(|a| a.boxed_clone()),
@@ -74,7 +77,7 @@ impl Tooltip {
             .boxed();
             Some(
                 Overlay::new(
-                    Self::render_tooltip(text, style, action, false)
+                    Self::render_tooltip(cx.window_id, focused_view_id, text, style, action, false)
                         .constrained()
                         .dynamically(move |constraint, cx| {
                             SizeConstraint::strict_along(
@@ -128,6 +131,8 @@ impl Tooltip {
     }
 
     pub fn render_tooltip(
+        window_id: usize,
+        focused_view_id: usize,
         text: String,
         style: TooltipStyle,
         action: Option<Box<dyn Action>>,
@@ -145,8 +150,13 @@ impl Tooltip {
                 }
             })
             .with_children(action.map(|action| {
-                let keystroke_label =
-                    KeystrokeLabel::new(action, style.keystroke.container, style.keystroke.text);
+                let keystroke_label = KeystrokeLabel::new(
+                    window_id,
+                    focused_view_id,
+                    action,
+                    style.keystroke.container,
+                    style.keystroke.text,
+                );
                 if measure {
                     keystroke_label.boxed()
                 } else {

crates/gpui/src/keymap_matcher/binding.rs 🔗

@@ -41,7 +41,7 @@ impl Binding {
         })
     }
 
-    fn match_context(&self, contexts: &[KeymapContext]) -> bool {
+    pub fn match_context(&self, contexts: &[KeymapContext]) -> bool {
         self.context_predicate
             .as_ref()
             .map(|predicate| predicate.eval(contexts))

crates/gpui/src/keymap_matcher/keymap_context.rs 🔗

@@ -43,7 +43,7 @@ impl KeymapContextPredicate {
     pub fn eval(&self, contexts: &[KeymapContext]) -> bool {
         let Some(context) = contexts.first() else { return false };
         match self {
-            Self::Identifier(name) => context.set.contains(name.as_str()),
+            Self::Identifier(name) => (&context.set).contains(name.as_str()),
             Self::Equal(left, right) => context
                 .map
                 .get(left)

crates/gpui/src/presenter.rs 🔗

@@ -4,7 +4,6 @@ use crate::{
     font_cache::FontCache,
     geometry::rect::RectF,
     json::{self, ToJson},
-    keymap_matcher::Keystroke,
     platform::{CursorStyle, Event},
     scene::{
         CursorRegion, MouseClick, MouseDown, MouseDownOut, MouseDrag, MouseEvent, MouseHover,
@@ -604,14 +603,6 @@ pub struct LayoutContext<'a> {
 }
 
 impl<'a> LayoutContext<'a> {
-    pub(crate) fn keystrokes_for_action(
-        &mut self,
-        action: &dyn Action,
-    ) -> Option<SmallVec<[Keystroke; 2]>> {
-        self.app
-            .keystrokes_for_action(self.window_id, &self.view_stack, action)
-    }
-
     fn layout(&mut self, view_id: usize, constraint: SizeConstraint) -> Vector2F {
         let print_error = |view_id| {
             format!(