Make getting keybinding for display more efficient (#27046)

Michael Sloan created 9 months ago

No longer iterates all the matching bindings, and no longer clones the
result.

Release Notes:

- N/A

Change summary

crates/gpui/src/keymap.rs                | 12 +++++++++---
crates/gpui/src/platform/mac/platform.rs |  2 +-
crates/ui/src/components/keybinding.rs   |  8 +++-----
3 files changed, 13 insertions(+), 9 deletions(-)

Detailed changes

crates/gpui/src/keymap.rs

@@ -202,10 +202,16 @@ impl Keymap {
     /// couple times. The decision as of now is to pick a side and leave it
     /// as is, until we have a better way to decide which binding to display
     /// that is consistent and not confusing.
-    pub fn binding_to_display_from_bindings<'a>(
-        bindings: impl IntoIterator<Item = &'a KeyBinding>,
+    pub fn binding_to_display_from_bindings(mut bindings: Vec<KeyBinding>) -> Option<KeyBinding> {
+        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>,
     ) -> Option<&'a KeyBinding> {
-        return bindings.into_iter().last();
+        bindings.next_back()
     }
 }
 

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

@@ -290,7 +290,7 @@ impl MacPlatform {
                 action,
                 os_action,
             } => {
-                let keystrokes = crate::Keymap::binding_to_display_from_bindings(
+                let keystrokes = crate::Keymap::binding_to_display_from_bindings_iterator(
                     keymap.bindings_for_action(action.as_ref()),
                 )
                 .map(|binding| binding.keystrokes());

crates/ui/src/components/keybinding.rs

@@ -33,8 +33,7 @@ impl KeyBinding {
             return Self::for_action_in(action, &focused, window, cx);
         }
         let key_binding =
-            gpui::Keymap::binding_to_display_from_bindings(&window.bindings_for_action(action))
-                .cloned()?;
+            gpui::Keymap::binding_to_display_from_bindings(window.bindings_for_action(action))?;
         Some(Self::new(key_binding, cx))
     }
 
@@ -46,9 +45,8 @@ impl KeyBinding {
         cx: &App,
     ) -> Option<Self> {
         let key_binding = gpui::Keymap::binding_to_display_from_bindings(
-            &window.bindings_for_action_in(action, focus),
-        )
-        .cloned()?;
+            window.bindings_for_action_in(action, focus),
+        )?;
         Some(Self::new(key_binding, cx))
     }