Prefer later bindings in keymap section for display in UI (#23378)

Closes #23015

Release Notes:

- Improved which keybindings are selected for display. Now later entries
within `bindings` will take precedence. The default keymaps have been
updated accordingly.

crates/docs_preprocessor/src/docs_preprocessor.rs

@@ -32,8 +32,9 @@ impl PreprocessorContext {
             _ => return None,
         };
 
-        keymap.sections().find_map(|section| {
-            section.bindings().find_map(|(keystroke, a)| {
+        // Find the binding in reverse order, as the last binding takes precedence.
+        keymap.sections().rev().find_map(|section| {
+            section.bindings().rev().find_map(|(keystroke, a)| {
                 if a.to_string() == action {
                     Some(keystroke.to_string())
                 } else {

crates/gpui/src/key_dispatch.rs

@@ -393,8 +393,8 @@ impl DispatchTree {
         false
     }
 
-    /// Returns key bindings that invoke an action on the currently focused element, in precedence
-    /// order (reverse of the order they were added to the keymap).
+    /// Returns key bindings that invoke an action on the currently focused element. Bindings are
+    /// returned in the order they were added. For display, the last binding should take precedence.
     pub fn bindings_for_action(
         &self,
         action: &dyn Action,

crates/gpui/src/keymap.rs

@@ -67,7 +67,8 @@ impl Keymap {
         self.bindings.iter()
     }
 
-    /// Iterate over all bindings for the given action, in the order they were added.
+    /// Iterate over all bindings for the given action, in the order they were added. For display,
+    /// the last binding should take precedence.
     pub fn bindings_for_action<'a>(
         &'a self,
         action: &'a dyn Action,

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

@@ -292,6 +292,7 @@ impl MacPlatform {
             } => {
                 let keystrokes = keymap
                     .bindings_for_action(action.as_ref())
+                    .rev()
                     .next()
                     .map(|binding| binding.keystrokes());
 

crates/gpui/src/window.rs

@@ -3090,8 +3090,7 @@ impl<'a> WindowContext<'a> {
     /// binding for the action (last binding added to the keymap).
     pub fn keystroke_text_for(&self, action: &dyn Action) -> String {
         self.bindings_for_action(action)
-            .into_iter()
-            .next()
+            .last()
             .map(|binding| {
                 binding
                     .keystrokes()
@@ -3744,8 +3743,8 @@ impl<'a> WindowContext<'a> {
         actions
     }
 
-    /// Returns key bindings that invoke an action on the currently focused element, in precedence
-    /// order (reverse of the order they were added to the keymap).
+    /// Returns key bindings that invoke an action on the currently focused element. Bindings are
+    /// returned in the order they were added. For display, the last binding should take precedence.
     pub fn bindings_for_action(&self, action: &dyn Action) -> Vec<KeyBinding> {
         self.window
             .rendered_frame
@@ -3757,15 +3756,15 @@ impl<'a> WindowContext<'a> {
     }
 
     /// Returns key bindings that invoke the given action on the currently focused element, without
-    /// checking context. Bindings are returned returned in precedence order (reverse of the order
-    /// they were added to the keymap).
+    /// checking context. Bindings are returned in the order they were added. For display, the last
+    /// binding should take precedence.
     pub fn all_bindings_for_input(&self, input: &[Keystroke]) -> Vec<KeyBinding> {
         RefCell::borrow(&self.keymap).all_bindings_for_input(input)
     }
 
     /// Returns any bindings that would invoke an action on the given focus handle if it were
-    /// focused. Bindings are returned returned in precedence order (reverse of the order
-    /// they were added to the keymap).
+    /// focused. Bindings are returned in the order they were added. For display, the last binding
+    /// should take precedence.
     pub fn bindings_for_action_in(
         &self,
         action: &dyn Action,

crates/settings/src/keymap_file.rs

@@ -2,7 +2,7 @@ use std::rc::Rc;
 
 use crate::{settings_store::parse_json_with_comments, SettingsAssets};
 use anyhow::anyhow;
-use collections::{BTreeMap, HashMap, IndexMap};
+use collections::{HashMap, IndexMap};
 use gpui::{
     Action, ActionBuildError, AppContext, InvalidKeystrokeError, KeyBinding,
     KeyBindingContextPredicate, NoAction, SharedString, KEYSTROKE_PARSE_EXPECTED_MESSAGE,
@@ -50,9 +50,12 @@ pub struct KeymapSection {
     /// This keymap section's bindings, as a JSON object mapping keystrokes to actions. The
     /// keystrokes key is a string representing a sequence of keystrokes to type, where the
     /// keystrokes are separated by whitespace. Each keystroke is a sequence of modifiers (`ctrl`,
-    /// `alt`, `shift`, `fn`, `cmd`, `super`, or `win`) followed by a key, separated by `-`.
+    /// `alt`, `shift`, `fn`, `cmd`, `super`, or `win`) followed by a key, separated by `-`. The
+    /// order of bindings does matter. When the same keystrokes are bound at the same context depth,
+    /// the binding that occurs later in the file is preferred. For displaying keystrokes in the UI,
+    /// the later binding for the same action is preferred.
     #[serde(default)]
-    bindings: Option<BTreeMap<String, KeymapAction>>,
+    bindings: Option<IndexMap<String, KeymapAction>>,
     #[serde(flatten)]
     unrecognized_fields: IndexMap<String, Value>,
     // This struct intentionally uses permissive types for its fields, rather than validating during
@@ -64,7 +67,7 @@ pub struct KeymapSection {
 }
 
 impl KeymapSection {
-    pub fn bindings(&self) -> impl Iterator<Item = (&String, &KeymapAction)> {
+    pub fn bindings(&self) -> impl DoubleEndedIterator<Item = (&String, &KeymapAction)> {
         self.bindings.iter().flatten()
     }
 }
@@ -235,8 +238,7 @@ impl KeymapFile {
             }
 
             if let Some(bindings) = bindings {
-                for binding in bindings {
-                    let (keystrokes, action) = binding;
+                for (keystrokes, action) in bindings {
                     let result = Self::load_keybinding(
                         keystrokes,
                         action,
@@ -548,7 +550,7 @@ impl KeymapFile {
         serde_json::to_value(root_schema).unwrap()
     }
 
-    pub fn sections(&self) -> impl Iterator<Item = &KeymapSection> {
+    pub fn sections(&self) -> impl DoubleEndedIterator<Item = &KeymapSection> {
         self.0.iter()
     }
 }

crates/ui/src/components/keybinding.rs

@@ -19,7 +19,7 @@ impl KeyBinding {
     /// Returns the highest precedence keybinding for an action. This is the last binding added to
     /// the keymap. User bindings are added after built-in bindings so that they take precedence.
     pub fn for_action(action: &dyn Action, cx: &mut WindowContext) -> Option<Self> {
-        let key_binding = cx.bindings_for_action(action).last().cloned()?;
+        let key_binding = cx.bindings_for_action(action).into_iter().rev().next()?;
         Some(Self::new(key_binding))
     }
 
@@ -29,7 +29,11 @@ impl KeyBinding {
         focus: &FocusHandle,
         cx: &mut WindowContext,
     ) -> Option<Self> {
-        let key_binding = cx.bindings_for_action_in(action, focus).last().cloned()?;
+        let key_binding = cx
+            .bindings_for_action_in(action, focus)
+            .into_iter()
+            .rev()
+            .next()?;
         Some(Self::new(key_binding))
     }
 
@@ -198,7 +202,8 @@ impl KeyIcon {
 
 /// Returns a textual representation of the key binding for the given [`Action`].
 pub fn text_for_action(action: &dyn Action, cx: &WindowContext) -> Option<String> {
-    let key_binding = cx.bindings_for_action(action).last().cloned()?;
+    let bindings = cx.bindings_for_action(action);
+    let key_binding = bindings.last()?;
     Some(text_for_key_binding(key_binding, PlatformStyle::platform()))
 }
 
@@ -209,13 +214,14 @@ pub fn text_for_action_in(
     focus: &FocusHandle,
     cx: &mut WindowContext,
 ) -> Option<String> {
-    let key_binding = cx.bindings_for_action_in(action, focus).last().cloned()?;
+    let bindings = cx.bindings_for_action_in(action, focus);
+    let key_binding = bindings.last()?;
     Some(text_for_key_binding(key_binding, PlatformStyle::platform()))
 }
 
 /// Returns a textual representation of the given key binding for the specified platform.
 pub fn text_for_key_binding(
-    key_binding: gpui::KeyBinding,
+    key_binding: &gpui::KeyBinding,
     platform_style: PlatformStyle,
 ) -> String {
     key_binding