gpui: Improve displayed keybinds shown in macOS application menus (#28440)

Closes #28164

This PR adresses inproper keybinds being shown in MacOS application
menus. The issue arises because the keybinds shown in MacOS application
menus are unaware of keybind contexts (they are only ever updated [on a
keymap-change](6d1dd109f5/crates/zed/src/zed.rs (L1421))).
Thus, using the keybind that was added last in the keymap can result in
incorrect keybindings being shown quite frequently, as they might belong
to a different context not generally available (applies the same for the
default keymap as well as for user-keymaps).

For example, the linked issue arises because the keybind found last in
the iterator is
6d1dd109f5/assets/keymaps/vim.json (L759),
which is not even available in most contexts (and, additionally, the `e`
of `escape` is rendered here as a keybind which seems to be a seperate
issue).

Additionally, this would result in inconsistent behavior with some
Vim-keybinds. A vim-keybind would be used only when available but
otherwise the default binding would be shown (see `Undo` and `Redo` as
an example below), which seems inconsistent.

This PR fixes this by instead using the first keybind found in keymaps,
which is expected to be the keybind available in most contexts.
Additionally, this allows rendering some more keybinds for actions which
vim-keybind cannot be displayed (Find In Project for example) .This
seems to be more reasonable until [this related
comment](6d1dd109f5/crates/gpui/src/keymap.rs (L199-L204))
is resolved.

This includes a revert of #25878 as well. With this change, the change
made in #25878 becomes obsolete and would also regress the behavior back
to the state prior to that PR.

|  | `main` | This PR |
| --- | --- | --- |
| Edit-menu | <img width="220" alt="main_edit"
src="https://github.com/user-attachments/assets/9f793b64-80b6-4a5b-b7e5-628f0d552166"
/> | <img width="220" alt="PR_edit"
src="https://github.com/user-attachments/assets/bccb444c-7a49-41d5-9377-d90b1639a3ed"
/> |
| View-menu | <img width="214" alt="main_view"
src="https://github.com/user-attachments/assets/0e6a6632-df02-4883-9f5a-facb4d0263b5"
/> | <img width="214" alt="PR_view"
src="https://github.com/user-attachments/assets/14600ece-fcaa-447a-94ef-4fa350eca49c"
/> |


Release Notes:

- Improved keybinds displayed for actions in MacOS application menus.

assets/keymaps/default-macos.json

@@ -1,15 +1,4 @@
 [
-  // Moved before Standard macOS bindings so that `cmd-w` is not the last binding for
-  // `workspace::CloseWindow` and displayed/intercepted by macOS
-  {
-    "context": "PromptLibrary",
-    "use_key_equivalents": true,
-    "bindings": {
-      "cmd-n": "rules_library::NewRule",
-      "cmd-shift-s": "rules_library::ToggleDefaultRule",
-      "cmd-w": "workspace::CloseWindow"
-    }
-  },
   // Standard macOS bindings
   {
     "use_key_equivalents": true,
@@ -380,6 +369,15 @@
       "shift-backspace": "agent::RemoveSelectedThread"
     }
   },
+  {
+    "context": "PromptLibrary",
+    "use_key_equivalents": true,
+    "bindings": {
+      "cmd-n": "rules_library::NewRule",
+      "cmd-shift-s": "rules_library::ToggleDefaultRule",
+      "cmd-w": "workspace::CloseWindow"
+    }
+  },
   {
     "context": "BufferSearchBar",
     "use_key_equivalents": true,

assets/keymaps/vim.json

@@ -846,13 +846,5 @@
       // and Windows.
       "alt-l": "editor::AcceptEditPrediction"
     }
-  },
-  {
-    // Fixes https://github.com/zed-industries/zed/issues/29095 by ensuring that
-    // the last binding for editor::ToggleComments is not ctrl-c.
-    "context": "hack_to_fix_ctrl-c",
-    "bindings": {
-      "g c": "editor::ToggleComments"
-    }
   }
 ]

crates/gpui/src/keymap.rs

@@ -206,12 +206,15 @@ impl Keymap {
         bindings.pop()
     }
 
-    /// Like `bindings_to_display_from_bindings` but takes a `DoubleEndedIterator` and returns a
-    /// reference.
-    pub fn binding_to_display_from_bindings_iterator<'a>(
-        mut bindings: impl DoubleEndedIterator<Item = &'a KeyBinding>,
+    /// Returns the first binding present in the iterator, which tends to be the
+    /// default binding without any key context. This is useful for cases where no
+    /// key context is available on binding display. Otherwise, bindings with a
+    /// more specific key context would take precedence and result in a
+    /// potentially invalid keybind being returned.
+    pub fn default_binding_from_bindings_iterator<'a>(
+        mut bindings: impl Iterator<Item = &'a KeyBinding>,
     ) -> Option<&'a KeyBinding> {
-        bindings.next_back()
+        bindings.next()
     }
 }
 

crates/gpui/src/platform/mac/events.rs

@@ -30,6 +30,7 @@ pub fn key_to_native(key: &str) -> Cow<str> {
     let code = match key {
         "space" => SPACE_KEY,
         "backspace" => BACKSPACE_KEY,
+        "escape" => ESCAPE_KEY,
         "up" => NSUpArrowFunctionKey,
         "down" => NSDownArrowFunctionKey,
         "left" => NSLeftArrowFunctionKey,

crates/gpui/src/platform/mac/platform.rs

@@ -6,8 +6,8 @@ use super::{
 };
 use crate::{
     Action, AnyWindowHandle, BackgroundExecutor, ClipboardEntry, ClipboardItem, ClipboardString,
-    CursorStyle, ForegroundExecutor, Image, ImageFormat, Keymap, MacDispatcher, MacDisplay,
-    MacWindow, Menu, MenuItem, PathPromptOptions, Platform, PlatformDisplay,
+    CursorStyle, ForegroundExecutor, Image, ImageFormat, KeyContext, Keymap, MacDispatcher,
+    MacDisplay, MacWindow, Menu, MenuItem, PathPromptOptions, Platform, PlatformDisplay,
     PlatformKeyboardLayout, PlatformTextSystem, PlatformWindow, Result, ScreenCaptureSource,
     SemanticVersion, Task, WindowAppearance, WindowParams, hash,
 };
@@ -36,6 +36,7 @@ use core_foundation::{
 };
 use ctor::ctor;
 use futures::channel::oneshot;
+use itertools::Itertools;
 use objc::{
     class,
     declare::ClassDecl,
@@ -46,7 +47,7 @@ use objc::{
 use parking_lot::Mutex;
 use ptr::null_mut;
 use std::{
-    cell::Cell,
+    cell::{Cell, LazyCell},
     convert::TryInto,
     ffi::{CStr, OsStr, c_void},
     os::{raw::c_char, unix::ffi::OsStrExt},
@@ -293,6 +294,19 @@ impl MacPlatform {
         actions: &mut Vec<Box<dyn Action>>,
         keymap: &Keymap,
     ) -> id {
+        const DEFAULT_CONTEXT: LazyCell<Vec<KeyContext>> = LazyCell::new(|| {
+            let mut workspace_context = KeyContext::new_with_defaults();
+            workspace_context.add("Workspace");
+            let mut pane_context = KeyContext::new_with_defaults();
+            pane_context.add("Pane");
+            let mut editor_context = KeyContext::new_with_defaults();
+            editor_context.add("Editor");
+
+            pane_context.extend(&editor_context);
+            workspace_context.extend(&pane_context);
+            vec![workspace_context]
+        });
+
         unsafe {
             match item {
                 MenuItem::Separator => NSMenuItem::separatorItem(nil),
@@ -301,10 +315,14 @@ impl MacPlatform {
                     action,
                     os_action,
                 } => {
-                    let keystrokes = crate::Keymap::binding_to_display_from_bindings_iterator(
-                        keymap.bindings_for_action(action.as_ref()),
-                    )
-                    .map(|binding| binding.keystrokes());
+                    let keystrokes = keymap
+                        .bindings_for_action(action.as_ref())
+                        .find_or_first(|binding| {
+                            binding
+                                .predicate()
+                                .is_none_or(|predicate| predicate.eval(&DEFAULT_CONTEXT))
+                        })
+                        .map(|binding| binding.keystrokes());
 
                     let selector = match os_action {
                         Some(crate::OsAction::Cut) => selector("cut:"),