Take a `Keymap` when setting app menus

Antonio Scandurra and Marshall created 2 years ago

For a brief period on this branch, we were taking a `DispatchTree`. Doing so
resulted in more accurate key bindings but it meant that we would have had to
recompute the app menus every time the key context changed.

We decided to err on the side of keeping things simple and work in the same
way they worked back in zed1.

Co-Authored-By: Marshall <[e-mail]>

Change summary

crates/gpui2/src/app.rs                    | 11 +----------
crates/gpui2/src/platform.rs               |  6 +++---
crates/gpui2/src/platform/mac/platform.rs  | 23 ++++++++++-------------
crates/gpui2/src/platform/test/platform.rs |  7 +++----
4 files changed, 17 insertions(+), 30 deletions(-)

Detailed changes

crates/gpui2/src/app.rs

@@ -1052,16 +1052,7 @@ impl AppContext {
     }
 
     pub fn set_menus(&mut self, menus: Vec<Menu>) {
-        if let Some(active_window) = self.active_window() {
-            active_window
-                .update(self, |_, cx| {
-                    cx.platform
-                        .set_menus(menus, Some(&cx.window.current_frame.dispatch_tree));
-                })
-                .ok();
-        } else {
-            self.platform.set_menus(menus, None);
-        }
+        self.platform.set_menus(menus, &self.keymap.lock());
     }
 
     pub fn dispatch_action(&mut self, action: &dyn Action) {

crates/gpui2/src/platform.rs

@@ -6,8 +6,8 @@ mod mac;
 mod test;
 
 use crate::{
-    point, size, Action, AnyWindowHandle, BackgroundExecutor, Bounds, DevicePixels, DispatchTree,
-    Font, FontId, FontMetrics, FontRun, ForegroundExecutor, GlobalPixels, GlyphId, InputEvent,
+    point, size, Action, AnyWindowHandle, BackgroundExecutor, Bounds, DevicePixels, Font, FontId,
+    FontMetrics, FontRun, ForegroundExecutor, GlobalPixels, GlyphId, InputEvent, Keymap,
     LineLayout, Pixels, Point, RenderGlyphParams, RenderImageParams, RenderSvgParams, Result,
     Scene, SharedString, Size, TaskLabel,
 };
@@ -92,7 +92,7 @@ pub(crate) trait Platform: 'static {
     fn on_reopen(&self, callback: Box<dyn FnMut()>);
     fn on_event(&self, callback: Box<dyn FnMut(InputEvent) -> bool>);
 
-    fn set_menus(&self, menus: Vec<Menu>, dispatch_tree: Option<&DispatchTree>);
+    fn set_menus(&self, menus: Vec<Menu>, keymap: &Keymap);
     fn on_app_menu_action(&self, callback: Box<dyn FnMut(&dyn Action)>);
     fn on_will_open_app_menu(&self, callback: Box<dyn FnMut()>);
     fn on_validate_app_menu_command(&self, callback: Box<dyn FnMut(&dyn Action) -> bool>);

crates/gpui2/src/platform/mac/platform.rs

@@ -1,7 +1,7 @@
 use super::{events::key_to_native, BoolExt};
 use crate::{
-    Action, AnyWindowHandle, BackgroundExecutor, ClipboardItem, CursorStyle, DispatchTree,
-    DisplayId, ForegroundExecutor, InputEvent, MacDispatcher, MacDisplay, MacDisplayLinker,
+    Action, AnyWindowHandle, BackgroundExecutor, ClipboardItem, CursorStyle, DisplayId,
+    ForegroundExecutor, InputEvent, Keymap, MacDispatcher, MacDisplay, MacDisplayLinker,
     MacTextSystem, MacWindow, Menu, MenuItem, PathPromptOptions, Platform, PlatformDisplay,
     PlatformTextSystem, PlatformWindow, Result, SemanticVersion, VideoTimestamp, WindowOptions,
 };
@@ -206,7 +206,7 @@ impl MacPlatform {
         menus: Vec<Menu>,
         delegate: id,
         actions: &mut Vec<Box<dyn Action>>,
-        dispatch_tree: Option<&DispatchTree>,
+        keymap: &Keymap,
     ) -> id {
         let application_menu = NSMenu::new(nil).autorelease();
         application_menu.setDelegate_(delegate);
@@ -217,7 +217,7 @@ impl MacPlatform {
             menu.setDelegate_(delegate);
 
             for item_config in menu_config.items {
-                menu.addItem_(self.create_menu_item(item_config, delegate, actions, dispatch_tree));
+                menu.addItem_(self.create_menu_item(item_config, delegate, actions, keymap));
             }
 
             let menu_item = NSMenuItem::new(nil).autorelease();
@@ -238,7 +238,7 @@ impl MacPlatform {
         item: MenuItem,
         delegate: id,
         actions: &mut Vec<Box<dyn Action>>,
-        dispatch_tree: Option<&DispatchTree>,
+        keymap: &Keymap,
     ) -> id {
         match item {
             MenuItem::Separator => NSMenuItem::separatorItem(nil),
@@ -247,11 +247,8 @@ impl MacPlatform {
                 action,
                 os_action,
             } => {
-                let bindings = dispatch_tree
-                    .map(|tree| tree.bindings_for_action(action.as_ref(), &tree.context_stack))
-                    .unwrap_or_default();
-                let keystrokes = bindings
-                    .iter()
+                let keystrokes = keymap
+                    .bindings_for_action(action.type_id())
                     .find(|binding| binding.action().partial_eq(action.as_ref()))
                     .map(|binding| binding.keystrokes());
 
@@ -343,7 +340,7 @@ impl MacPlatform {
                 let submenu = NSMenu::new(nil).autorelease();
                 submenu.setDelegate_(delegate);
                 for item in items {
-                    submenu.addItem_(self.create_menu_item(item, delegate, actions, dispatch_tree));
+                    submenu.addItem_(self.create_menu_item(item, delegate, actions, keymap));
                 }
                 item.setSubmenu_(submenu);
                 item.setTitle_(ns_string(name));
@@ -691,12 +688,12 @@ impl Platform for MacPlatform {
         }
     }
 
-    fn set_menus(&self, menus: Vec<Menu>, dispatch_tree: Option<&DispatchTree>) {
+    fn set_menus(&self, menus: Vec<Menu>, keymap: &Keymap) {
         unsafe {
             let app: id = msg_send![APP_CLASS, sharedApplication];
             let mut state = self.0.lock();
             let actions = &mut state.menu_actions;
-            app.setMainMenu_(self.create_menu_bar(menus, app.delegate(), actions, dispatch_tree));
+            app.setMainMenu_(self.create_menu_bar(menus, app.delegate(), actions, keymap));
         }
     }
 

crates/gpui2/src/platform/test/platform.rs

@@ -1,7 +1,6 @@
 use crate::{
-    AnyWindowHandle, BackgroundExecutor, ClipboardItem, CursorStyle, DispatchTree, DisplayId,
-    ForegroundExecutor, Platform, PlatformDisplay, PlatformTextSystem, TestDisplay, TestWindow,
-    WindowOptions,
+    AnyWindowHandle, BackgroundExecutor, ClipboardItem, CursorStyle, DisplayId, ForegroundExecutor,
+    Keymap, Platform, PlatformDisplay, PlatformTextSystem, TestDisplay, TestWindow, WindowOptions,
 };
 use anyhow::{anyhow, Result};
 use collections::VecDeque;
@@ -213,7 +212,7 @@ impl Platform for TestPlatform {
         unimplemented!()
     }
 
-    fn set_menus(&self, _menus: Vec<crate::Menu>, _dispatch_tree: Option<&DispatchTree>) {
+    fn set_menus(&self, _menus: Vec<crate::Menu>, _keymap: &Keymap) {
         unimplemented!()
     }