action dispatch target (#3494)

- Ensure the candidate keybinding matches the correct context
- Fix context key matching
- I was soooo close
- Dispatch actions on focused node

[[PR Description]]

Release Notes:

- (Added|Fixed|Improved) ...
([#<public_issue_number_if_exists>](https://github.com/zed-industries/community/issues/<public_issue_number_if_exists>)).

crates/command_palette2/src/command_palette.rs

@@ -311,7 +311,11 @@ impl PickerDelegate for CommandPaletteDelegate {
                         command.name.clone(),
                         r#match.positions.clone(),
                     ))
-                    .children(KeyBinding::for_action(&*command.action, cx)),
+                    .children(KeyBinding::for_action_in(
+                        &*command.action,
+                        &self.previous_focus_handle,
+                        cx,
+                    )),
             ),
         )
     }

crates/copilot_button2/src/copilot_button.rs

@@ -201,9 +201,8 @@ impl CopilotButton {
                     url: COPILOT_SETTINGS_URL.to_string(),
                 }
                 .boxed_clone(),
-                cx,
             )
-            .action("Sign Out", SignOut.boxed_clone(), cx)
+            .action("Sign Out", SignOut.boxed_clone())
         });
     }
 

crates/editor2/src/mouse_context_menu.rs

@@ -37,19 +37,18 @@ pub fn deploy_context_menu(
     });
 
     let context_menu = ui::ContextMenu::build(cx, |menu, cx| {
-        menu.action("Rename Symbol", Box::new(Rename), cx)
+        menu.action("Rename Symbol", Box::new(Rename))
-            .action("Go to Definition", Box::new(GoToDefinition), cx)
+            .action("Go to Definition", Box::new(GoToDefinition))
-            .action("Go to Type Definition", Box::new(GoToTypeDefinition), cx)
+            .action("Go to Type Definition", Box::new(GoToTypeDefinition))
-            .action("Find All References", Box::new(FindAllReferences), cx)
+            .action("Find All References", Box::new(FindAllReferences))
             .action(
                 "Code Actions",
                 Box::new(ToggleCodeActions {
                     deployed_from_indicator: false,
                 }),
-                cx,
             )
             .separator()
-            .action("Reveal in Finder", Box::new(RevealInFinder), cx)
+            .action("Reveal in Finder", Box::new(RevealInFinder))
     });
     let context_menu_focus = context_menu.focus_handle(cx);
     cx.focus(&context_menu_focus);

crates/gpui2/src/elements/div.rs

@@ -221,20 +221,6 @@ pub trait InteractiveElement: Sized + Element {
 
     /// Add a listener for the given action, fires during the bubble event phase
     fn on_action<A: Action>(mut self, listener: impl Fn(&A, &mut WindowContext) + 'static) -> Self {
-        // NOTE: this debug assert has the side-effect of working around
-        // a bug where a crate consisting only of action definitions does
-        // not register the actions in debug builds:
-        //
-        // https://github.com/rust-lang/rust/issues/47384
-        // https://github.com/mmastrac/rust-ctor/issues/280
-        //
-        // if we are relying on this side-effect still, removing the debug_assert!
-        // likely breaks the command_palette tests.
-        // debug_assert!(
-        //     A::is_registered(),
-        //     "{:?} is not registered as an action",
-        //     A::qualified_name()
-        // );
         self.interactivity().action_listeners.push((
             TypeId::of::<A>(),
             Box::new(move |action, phase, cx| {
@@ -247,6 +233,23 @@ pub trait InteractiveElement: Sized + Element {
         self
     }
 
+    fn on_boxed_action(
+        mut self,
+        action: &Box<dyn Action>,
+        listener: impl Fn(&Box<dyn Action>, &mut WindowContext) + 'static,
+    ) -> Self {
+        let action = action.boxed_clone();
+        self.interactivity().action_listeners.push((
+            (*action).type_id(),
+            Box::new(move |_, phase, cx| {
+                if phase == DispatchPhase::Bubble {
+                    (listener)(&action, cx)
+                }
+            }),
+        ));
+        self
+    }
+
     fn on_key_down(
         mut self,
         listener: impl Fn(&KeyDownEvent, &mut WindowContext) + 'static,

crates/gpui2/src/key_dispatch.rs

@@ -16,7 +16,7 @@ pub struct DispatchNodeId(usize);
 
 pub(crate) struct DispatchTree {
     node_stack: Vec<DispatchNodeId>,
-    context_stack: Vec<KeyContext>,
+    pub(crate) context_stack: Vec<KeyContext>,
     nodes: Vec<DispatchNode>,
     focusable_node_ids: HashMap<FocusId, DispatchNodeId>,
     keystroke_matchers: HashMap<SmallVec<[KeyContext; 4]>, KeystrokeMatcher>,
@@ -163,11 +163,25 @@ impl DispatchTree {
         actions
     }
 
-    pub fn bindings_for_action(&self, action: &dyn Action) -> Vec<KeyBinding> {
+    pub fn bindings_for_action(
+        &self,
+        action: &dyn Action,
+        context_stack: &Vec<KeyContext>,
+    ) -> Vec<KeyBinding> {
         self.keymap
             .lock()
             .bindings_for_action(action.type_id())
-            .filter(|candidate| candidate.action.partial_eq(action))
+            .filter(|candidate| {
+                if !candidate.action.partial_eq(action) {
+                    return false;
+                }
+                for i in 1..context_stack.len() {
+                    if candidate.matches_context(&context_stack[0..=i]) {
+                        return true;
+                    }
+                }
+                return false;
+            })
             .cloned()
             .collect()
     }

crates/gpui2/src/window.rs

@@ -1348,6 +1348,8 @@ impl<'a> WindowContext<'a> {
                 .dispatch_tree
                 .dispatch_path(node_id);
 
+            let mut actions: Vec<Box<dyn Action>> = Vec::new();
+
             // Capture phase
             let mut context_stack: SmallVec<[KeyContext; 16]> = SmallVec::new();
             self.propagate_event = true;
@@ -1382,22 +1384,26 @@ impl<'a> WindowContext<'a> {
                 let node = self.window.current_frame.dispatch_tree.node(*node_id);
                 if !node.context.is_empty() {
                     if let Some(key_down_event) = event.downcast_ref::<KeyDownEvent>() {
-                        if let Some(action) = self
+                        if let Some(found) = self
                             .window
                             .current_frame
                             .dispatch_tree
                             .dispatch_key(&key_down_event.keystroke, &context_stack)
                         {
-                            self.dispatch_action_on_node(*node_id, action);
+                            actions.push(found.boxed_clone())
-                            if !self.propagate_event {
-                                return;
-                            }
                         }
                     }
 
                     context_stack.pop();
                 }
             }
+
+            for action in actions {
+                self.dispatch_action_on_node(node_id, action);
+                if !self.propagate_event {
+                    return;
+                }
+            }
         }
     }
 
@@ -1425,7 +1431,6 @@ impl<'a> WindowContext<'a> {
                 }
             }
         }
-
         // Bubble phase
         for node_id in dispatch_path.iter().rev() {
             let node = self.window.current_frame.dispatch_tree.node(*node_id);
@@ -1492,10 +1497,28 @@ impl<'a> WindowContext<'a> {
     }
 
     pub fn bindings_for_action(&self, action: &dyn Action) -> Vec<KeyBinding> {
-        self.window
+        self.window.current_frame.dispatch_tree.bindings_for_action(
-            .current_frame
+            action,
-            .dispatch_tree
+            &self.window.current_frame.dispatch_tree.context_stack,
-            .bindings_for_action(action)
+        )
+    }
+
+    pub fn bindings_for_action_in(
+        &self,
+        action: &dyn Action,
+        focus_handle: &FocusHandle,
+    ) -> Vec<KeyBinding> {
+        let dispatch_tree = &self.window.previous_frame.dispatch_tree;
+
+        let Some(node_id) = dispatch_tree.focusable_node_id(focus_handle.id) else {
+            return vec![];
+        };
+        let context_stack = dispatch_tree
+            .dispatch_path(node_id)
+            .into_iter()
+            .map(|node_id| dispatch_tree.node(node_id).context.clone())
+            .collect();
+        dispatch_tree.bindings_for_action(action, &context_stack)
     }
 
     pub fn listener_for<V: Render, E>(

crates/project_panel2/src/project_panel.rs

@@ -397,7 +397,6 @@ impl ProjectPanel {
                     menu = menu.action(
                         "Add Folder to Project",
                         Box::new(workspace::AddFolderToProject),
-                        cx,
                     );
                     if is_root {
                         menu = menu.entry(
@@ -412,35 +411,35 @@ impl ProjectPanel {
                 }
 
                 menu = menu
-                    .action("New File", Box::new(NewFile), cx)
+                    .action("New File", Box::new(NewFile))
-                    .action("New Folder", Box::new(NewDirectory), cx)
+                    .action("New Folder", Box::new(NewDirectory))
                     .separator()
-                    .action("Cut", Box::new(Cut), cx)
+                    .action("Cut", Box::new(Cut))
-                    .action("Copy", Box::new(Copy), cx);
+                    .action("Copy", Box::new(Copy));
 
                 if let Some(clipboard_entry) = self.clipboard_entry {
                     if clipboard_entry.worktree_id() == worktree_id {
-                        menu = menu.action("Paste", Box::new(Paste), cx);
+                        menu = menu.action("Paste", Box::new(Paste));
                     }
                 }
 
                 menu = menu
                     .separator()
-                    .action("Copy Path", Box::new(CopyPath), cx)
+                    .action("Copy Path", Box::new(CopyPath))
-                    .action("Copy Relative Path", Box::new(CopyRelativePath), cx)
+                    .action("Copy Relative Path", Box::new(CopyRelativePath))
                     .separator()
-                    .action("Reveal in Finder", Box::new(RevealInFinder), cx);
+                    .action("Reveal in Finder", Box::new(RevealInFinder));
 
                 if is_dir {
                     menu = menu
-                        .action("Open in Terminal", Box::new(OpenInTerminal), cx)
+                        .action("Open in Terminal", Box::new(OpenInTerminal))
-                        .action("Search Inside", Box::new(NewSearchInDirectory), cx)
+                        .action("Search Inside", Box::new(NewSearchInDirectory))
                 }
 
-                menu = menu.separator().action("Rename", Box::new(Rename), cx);
+                menu = menu.separator().action("Rename", Box::new(Rename));
 
                 if !is_root {
-                    menu = menu.action("Delete", Box::new(Delete), cx);
+                    menu = menu.action("Delete", Box::new(Delete));
                 }
 
                 menu

crates/terminal_view2/src/terminal_view.rs

@@ -299,11 +299,8 @@ impl TerminalView {
         cx: &mut ViewContext<Self>,
     ) {
         self.context_menu = Some(ContextMenu::build(cx, |menu, cx| {
-            menu.action("Clear", Box::new(Clear), cx).action(
+            menu.action("Clear", Box::new(Clear))
-                "Close",
+                .action("Close", Box::new(CloseActiveItem { save_intent: None }))
-                Box::new(CloseActiveItem { save_intent: None }),
-                cx,
-            )
         }));
         dbg!(&position);
         // todo!()

crates/ui2/src/components/context_menu.rs

@@ -7,7 +7,7 @@ use gpui::{
     IntoElement, Render, View, VisualContext,
 };
 use menu::{SelectFirst, SelectLast, SelectNext, SelectPrev};
-use std::rc::Rc;
+use std::{rc::Rc, time::Duration};
 
 pub enum ContextMenuItem {
     Separator,
@@ -16,7 +16,7 @@ pub enum ContextMenuItem {
         label: SharedString,
         icon: Option<Icon>,
         handler: Rc<dyn Fn(&mut WindowContext)>,
-        key_binding: Option<KeyBinding>,
+        action: Option<Box<dyn Action>>,
     },
 }
 
@@ -70,36 +70,26 @@ impl ContextMenu {
         self.items.push(ContextMenuItem::Entry {
             label: label.into(),
             handler: Rc::new(on_click),
-            key_binding: None,
             icon: None,
+            action: None,
         });
         self
     }
 
-    pub fn action(
+    pub fn action(mut self, label: impl Into<SharedString>, action: Box<dyn Action>) -> Self {
-        mut self,
-        label: impl Into<SharedString>,
-        action: Box<dyn Action>,
-        cx: &mut WindowContext,
-    ) -> Self {
         self.items.push(ContextMenuItem::Entry {
             label: label.into(),
-            key_binding: KeyBinding::for_action(&*action, cx),
+            action: Some(action.boxed_clone()),
             handler: Rc::new(move |cx| cx.dispatch_action(action.boxed_clone())),
             icon: None,
         });
         self
     }
 
-    pub fn link(
+    pub fn link(mut self, label: impl Into<SharedString>, action: Box<dyn Action>) -> Self {
-        mut self,
-        label: impl Into<SharedString>,
-        action: Box<dyn Action>,
-        cx: &mut WindowContext,
-    ) -> Self {
         self.items.push(ContextMenuItem::Entry {
             label: label.into(),
-            key_binding: KeyBinding::for_action(&*action, cx),
+            action: Some(action.boxed_clone()),
             handler: Rc::new(move |cx| cx.dispatch_action(action.boxed_clone())),
             icon: Some(Icon::Link),
         });
@@ -161,6 +151,36 @@ impl ContextMenu {
             self.select_last(&Default::default(), cx);
         }
     }
+
+    pub fn on_action_dispatch(&mut self, dispatched: &Box<dyn Action>, cx: &mut ViewContext<Self>) {
+        if let Some(ix) = self.items.iter().position(|item| {
+            if let ContextMenuItem::Entry {
+                action: Some(action),
+                ..
+            } = item
+            {
+                action.partial_eq(&**dispatched)
+            } else {
+                false
+            }
+        }) {
+            self.selected_index = Some(ix);
+            cx.notify();
+            let action = dispatched.boxed_clone();
+            cx.spawn(|this, mut cx| async move {
+                cx.background_executor()
+                    .timer(Duration::from_millis(50))
+                    .await;
+                this.update(&mut cx, |this, cx| {
+                    cx.dispatch_action(action);
+                    this.cancel(&Default::default(), cx)
+                })
+            })
+            .detach_and_log_err(cx);
+        } else {
+            cx.propagate()
+        }
+    }
 }
 
 impl ContextMenuItem {
@@ -185,6 +205,22 @@ impl Render for ContextMenu {
                 .on_action(cx.listener(ContextMenu::select_prev))
                 .on_action(cx.listener(ContextMenu::confirm))
                 .on_action(cx.listener(ContextMenu::cancel))
+                .map(|mut el| {
+                    for item in self.items.iter() {
+                        if let ContextMenuItem::Entry {
+                            action: Some(action),
+                            ..
+                        } = item
+                        {
+                            el = el.on_boxed_action(
+                                action,
+                                cx.listener(ContextMenu::on_action_dispatch),
+                            );
+                        }
+                    }
+                    el
+                })
+                .on_blur(cx.listener(|this, _, cx| this.cancel(&Default::default(), cx)))
                 .flex_none()
                 .child(
                     List::new().children(self.items.iter().enumerate().map(
@@ -196,8 +232,8 @@ impl Render for ContextMenu {
                             ContextMenuItem::Entry {
                                 label,
                                 handler,
-                                key_binding,
                                 icon,
+                                action,
                             } => {
                                 let handler = handler.clone();
                                 let dismiss = cx.listener(|_, _, cx| cx.emit(DismissEvent));
@@ -218,11 +254,10 @@ impl Render for ContextMenu {
                                             .w_full()
                                             .justify_between()
                                             .child(label_element)
-                                            .children(
+                                            .children(action.as_ref().and_then(|action| {
-                                                key_binding
+                                                KeyBinding::for_action(&**action, cx)
-                                                    .clone()
+                                                    .map(|binding| div().ml_1().child(binding))
-                                                    .map(|binding| div().ml_1().child(binding)),
+                                            })),
-                                            ),
                                     )
                                     .selected(Some(ix) == self.selected_index)
                                     .on_click(move |event, cx| {

crates/ui2/src/components/keybinding.rs

@@ -1,5 +1,5 @@
 use crate::{h_stack, prelude::*, Icon, IconElement, IconSize};
-use gpui::{relative, rems, Action, Div, IntoElement, Keystroke};
+use gpui::{relative, rems, Action, Div, FocusHandle, IntoElement, Keystroke};
 
 #[derive(IntoElement, Clone)]
 pub struct KeyBinding {
@@ -49,12 +49,21 @@ impl RenderOnce for KeyBinding {
 
 impl KeyBinding {
     pub fn for_action(action: &dyn Action, cx: &mut WindowContext) -> Option<Self> {
-        // todo! this last is arbitrary, we want to prefer users key bindings over defaults,
-        // and vim over normal (in vim mode), etc.
         let key_binding = cx.bindings_for_action(action).last().cloned()?;
         Some(Self::new(key_binding))
     }
 
+    // like for_action(), but lets you specify the context from which keybindings
+    // are matched.
+    pub fn for_action_in(
+        action: &dyn Action,
+        focus: &FocusHandle,
+        cx: &mut WindowContext,
+    ) -> Option<Self> {
+        let key_binding = cx.bindings_for_action_in(action, focus).last().cloned()?;
+        Some(Self::new(key_binding))
+    }
+
     fn icon_for_key(keystroke: &Keystroke) -> Option<Icon> {
         let mut icon: Option<Icon> = None;
 

crates/workspace2/src/pane.rs

@@ -1531,24 +1531,17 @@ impl Pane {
                 menu.action(
                     "Close Active Item",
                     CloseActiveItem { save_intent: None }.boxed_clone(),
-                    cx,
-                )
-                .action("Close Inactive Items", CloseInactiveItems.boxed_clone(), cx)
-                .action("Close Clean Items", CloseCleanItems.boxed_clone(), cx)
-                .action(
-                    "Close Items To The Left",
-                    CloseItemsToTheLeft.boxed_clone(),
-                    cx,
                 )
+                .action("Close Inactive Items", CloseInactiveItems.boxed_clone())
+                .action("Close Clean Items", CloseCleanItems.boxed_clone())
+                .action("Close Items To The Left", CloseItemsToTheLeft.boxed_clone())
                 .action(
                     "Close Items To The Right",
                     CloseItemsToTheRight.boxed_clone(),
-                    cx,
                 )
                 .action(
                     "Close All Items",
                     CloseAllItems { save_intent: None }.boxed_clone(),
-                    cx,
                 )
             })
         })
@@ -1627,17 +1620,12 @@ impl Pane {
                                     .child(IconButton::new("plus", Icon::Plus).on_click(
                                         cx.listener(|this, _, cx| {
                                             let menu = ContextMenu::build(cx, |menu, cx| {
-                                                menu.action("New File", NewFile.boxed_clone(), cx)
+                                                menu.action("New File", NewFile.boxed_clone())
                                                     .action(
                                                         "New Terminal",
                                                         NewCenterTerminal.boxed_clone(),
-                                                        cx,
-                                                    )
-                                                    .action(
-                                                        "New Search",
-                                                        NewSearch.boxed_clone(),
-                                                        cx,
                                                     )
+                                                    .action("New Search", NewSearch.boxed_clone())
                                             });
                                             cx.subscribe(
                                                 &menu,
@@ -1661,14 +1649,10 @@ impl Pane {
                                     .child(IconButton::new("split", Icon::Split).on_click(
                                         cx.listener(|this, _, cx| {
                                             let menu = ContextMenu::build(cx, |menu, cx| {
-                                                menu.action(
+                                                menu.action("Split Right", SplitRight.boxed_clone())
-                                                    "Split Right",
+                                                    .action("Split Left", SplitLeft.boxed_clone())
-                                                    SplitRight.boxed_clone(),
+                                                    .action("Split Up", SplitUp.boxed_clone())
-                                                    cx,
+                                                    .action("Split Down", SplitDown.boxed_clone())
-                                                )
-                                                .action("Split Left", SplitLeft.boxed_clone(), cx)
-                                                .action("Split Up", SplitUp.boxed_clone(), cx)
-                                                .action("Split Down", SplitDown.boxed_clone(), cx)
                                             });
                                             cx.subscribe(
                                                 &menu,