Improve doc comments about keybinding order (#23014)

Michael Sloan created

Release Notes:

- N/A

Change summary

crates/gpui/src/key_dispatch.rs        |  2 ++
crates/gpui/src/keymap.rs              | 26 +++++++++++++-------------
crates/gpui/src/window.rs              | 14 ++++++++++----
crates/ui/src/components/keybinding.rs |  5 +++--
4 files changed, 28 insertions(+), 19 deletions(-)

Detailed changes

crates/gpui/src/key_dispatch.rs 🔗

@@ -393,6 +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).
     pub fn bindings_for_action(
         &self,
         action: &dyn Action,

crates/gpui/src/keymap.rs 🔗

@@ -108,8 +108,8 @@ impl Keymap {
         })
     }
 
-    /// all bindings for input returns all bindings that might match the input
-    /// (without checking context)
+    /// Returns all bindings that might match the input without checking context. The bindings
+    /// returned in precedence order (reverse of the order they were added to the keymap).
     pub fn all_bindings_for_input(&self, input: &[Keystroke]) -> Vec<KeyBinding> {
         self.bindings()
             .rev()
@@ -120,20 +120,20 @@ impl Keymap {
             .collect()
     }
 
-    /// bindings_for_input returns a list of bindings that match the given input,
-    /// and a boolean indicating whether or not more bindings might match if
-    /// the input was longer.
+    /// Returns a list of bindings that match the given input, and a boolean indicating whether or
+    /// not more bindings might match if the input was longer. Bindings are returned in precedence
+    /// order.
     ///
-    /// Precedence is defined by the depth in the tree (matches on the Editor take
-    /// precedence over matches on the Pane, then the Workspace, etc.). Bindings with
-    /// no context are treated as the same as the deepest context.
+    /// Precedence is defined by the depth in the tree (matches on the Editor take precedence over
+    /// matches on the Pane, then the Workspace, etc.). Bindings with no context are treated as the
+    /// same as the deepest context.
     ///
-    /// In the case of multiple bindings at the same depth, the ones defined later in the
-    /// keymap take precedence (so user bindings take precedence over built-in bindings).
+    /// In the case of multiple bindings at the same depth, the ones added to the keymap later take
+    /// precedence. User bindings are added after built-in bindings so that they take precedence.
     ///
-    /// If a user has disabled a binding with `"x": null` it will not be returned. Disabled
-    /// bindings are evaluated with the same precedence rules so you can disable a rule in
-    /// a given context only.
+    /// If a user has disabled a binding with `"x": null` it will not be returned. Disabled bindings
+    /// are evaluated with the same precedence rules so you can disable a rule in a given context
+    /// only.
     pub fn bindings_for_input(
         &self,
         input: &[Keystroke],

crates/gpui/src/window.rs 🔗

@@ -3077,7 +3077,8 @@ impl<'a> WindowContext<'a> {
         false
     }
 
-    /// Represent this action as a key binding string, to display in the UI.
+    /// Return a key binding string for an action, to display in the UI. Uses the highest precedence
+    /// 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()
@@ -3734,7 +3735,8 @@ impl<'a> WindowContext<'a> {
         actions
     }
 
-    /// Returns key bindings that invoke the given action on the currently focused element.
+    /// 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).
     pub fn bindings_for_action(&self, action: &dyn Action) -> Vec<KeyBinding> {
         self.window
             .rendered_frame
@@ -3745,12 +3747,16 @@ impl<'a> WindowContext<'a> {
             )
     }
 
-    /// Returns key bindings that invoke the given action on the currently focused element.
+    /// 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).
     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 the given action on the given focus handle if it were focused.
+    /// 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).
     pub fn bindings_for_action_in(
         &self,
         action: &dyn Action,

crates/ui/src/components/keybinding.rs 🔗

@@ -16,13 +16,14 @@ pub struct KeyBinding {
 }
 
 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()?;
         Some(Self::new(key_binding))
     }
 
-    // like for_action(), but lets you specify the context from which keybindings
-    // are matched.
+    /// Like `for_action`, but lets you specify the context from which keybindings are matched.
     pub fn for_action_in(
         action: &dyn Action,
         focus: &FocusHandle,