From 1a76a6b0bfc5145cf51355ff7777b0413063a868 Mon Sep 17 00:00:00 2001 From: Ben Kunkle Date: Tue, 22 Jul 2025 09:59:51 -0500 Subject: [PATCH] gpui: Simplify `bindings_for_action` API (#34857) Closes #ISSUE Simplifies the API to no longer have a variant that returns indices. The downside is that a few places that used to call `bindings_for_action_with_indices` now compare `Box` instead of indices, however the result is the removal of wrapper code and index handling that is largely unnecessary Release Notes: - N/A *or* Added/Fixed/Improved ... Co-authored-by: Conrad --- crates/gpui/src/key_dispatch.rs | 39 +++++++++--------------- crates/gpui/src/keymap.rs | 53 +++++++++++---------------------- 2 files changed, 31 insertions(+), 61 deletions(-) diff --git a/crates/gpui/src/key_dispatch.rs b/crates/gpui/src/key_dispatch.rs index a290a132c3..cc6ebb9b08 100644 --- a/crates/gpui/src/key_dispatch.rs +++ b/crates/gpui/src/key_dispatch.rs @@ -50,8 +50,8 @@ /// KeyBinding::new("cmd-k left", pane::SplitLeft, Some("Pane")) /// use crate::{ - Action, ActionRegistry, App, BindingIndex, DispatchPhase, EntityId, FocusId, KeyBinding, - KeyContext, Keymap, Keystroke, ModifiersChangedEvent, Window, + Action, ActionRegistry, App, DispatchPhase, EntityId, FocusId, KeyBinding, KeyContext, Keymap, + Keystroke, ModifiersChangedEvent, Window, }; use collections::FxHashMap; use smallvec::SmallVec; @@ -406,16 +406,11 @@ impl DispatchTree { // methods, but this can't be done very cleanly since keymap must be borrowed. let keymap = self.keymap.borrow(); keymap - .bindings_for_action_with_indices(action) - .filter(|(binding_index, binding)| { - Self::binding_matches_predicate_and_not_shadowed( - &keymap, - *binding_index, - &binding.keystrokes, - context_stack, - ) + .bindings_for_action(action) + .filter(|binding| { + Self::binding_matches_predicate_and_not_shadowed(&keymap, &binding, context_stack) }) - .map(|(_, binding)| binding.clone()) + .cloned() .collect() } @@ -428,28 +423,22 @@ impl DispatchTree { ) -> Option { let keymap = self.keymap.borrow(); keymap - .bindings_for_action_with_indices(action) + .bindings_for_action(action) .rev() - .find_map(|(binding_index, binding)| { - let found = Self::binding_matches_predicate_and_not_shadowed( - &keymap, - binding_index, - &binding.keystrokes, - context_stack, - ); - if found { Some(binding.clone()) } else { None } + .find(|binding| { + Self::binding_matches_predicate_and_not_shadowed(&keymap, &binding, context_stack) }) + .cloned() } fn binding_matches_predicate_and_not_shadowed( keymap: &Keymap, - binding_index: BindingIndex, - keystrokes: &[Keystroke], + binding: &KeyBinding, context_stack: &[KeyContext], ) -> bool { - let (bindings, _) = keymap.bindings_for_input_with_indices(&keystrokes, context_stack); - if let Some((highest_precedence_index, _)) = bindings.iter().next() { - binding_index == *highest_precedence_index + let (bindings, _) = keymap.bindings_for_input(&binding.keystrokes, context_stack); + if let Some(found) = bindings.iter().next() { + found.action.partial_eq(binding.action.as_ref()) } else { false } diff --git a/crates/gpui/src/keymap.rs b/crates/gpui/src/keymap.rs index 69700e64dc..83d7479a04 100644 --- a/crates/gpui/src/keymap.rs +++ b/crates/gpui/src/keymap.rs @@ -77,15 +77,6 @@ impl Keymap { &'a self, action: &'a dyn Action, ) -> impl 'a + DoubleEndedIterator { - self.bindings_for_action_with_indices(action) - .map(|(_, binding)| binding) - } - - /// Like `bindings_for_action_with_indices`, but also returns the binding indices. - pub fn bindings_for_action_with_indices<'a>( - &'a self, - action: &'a dyn Action, - ) -> impl 'a + DoubleEndedIterator { let action_id = action.type_id(); let binding_indices = self .binding_indices_by_action_id @@ -118,7 +109,7 @@ impl Keymap { } } - Some((BindingIndex(*ix), binding)) + Some(binding) }) } @@ -153,22 +144,8 @@ impl Keymap { input: &[Keystroke], context_stack: &[KeyContext], ) -> (SmallVec<[KeyBinding; 1]>, bool) { - let (bindings, pending) = self.bindings_for_input_with_indices(input, context_stack); - let bindings = bindings - .into_iter() - .map(|(_, binding)| binding) - .collect::>(); - (bindings, pending) - } - - /// Like `bindings_for_input`, but also returns the binding indices. - pub fn bindings_for_input_with_indices( - &self, - input: &[Keystroke], - context_stack: &[KeyContext], - ) -> (SmallVec<[(BindingIndex, KeyBinding); 1]>, bool) { - let mut bindings: SmallVec<[(usize, BindingIndex, &KeyBinding); 1]> = SmallVec::new(); - let mut pending_bindings: SmallVec<[(BindingIndex, &KeyBinding); 1]> = SmallVec::new(); + let mut matched_bindings = SmallVec::<[(usize, BindingIndex, &KeyBinding); 1]>::new(); + let mut pending_bindings = SmallVec::<[(BindingIndex, &KeyBinding); 1]>::new(); for (ix, binding) in self.bindings().enumerate().rev() { let Some(depth) = self.binding_enabled(binding, &context_stack) else { @@ -179,26 +156,30 @@ impl Keymap { }; if !pending { - bindings.push((depth, BindingIndex(ix), binding)) + matched_bindings.push((depth, BindingIndex(ix), binding)); } else { - pending_bindings.push((BindingIndex(ix), binding)) + pending_bindings.push((BindingIndex(ix), binding)); } } - bindings.sort_by(|(depth_a, ix_a, _), (depth_b, ix_b, _)| { + matched_bindings.sort_by(|(depth_a, ix_a, _), (depth_b, ix_b, _)| { depth_b.cmp(depth_a).then(ix_b.cmp(ix_a)) }); - let bindings: SmallVec<[_; 1]> = bindings - .into_iter() - .take_while(|(_, _, binding)| !is_no_action(&*binding.action)) - .map(|(_, ix, binding)| (ix, binding.clone())) - .collect(); + let mut bindings: SmallVec<[_; 1]> = SmallVec::new(); + let mut first_binding_index = None; + for (_, ix, binding) in matched_bindings { + if is_no_action(&*binding.action) { + break; + } + bindings.push(binding.clone()); + first_binding_index.get_or_insert(ix); + } let mut pending = HashSet::default(); for (ix, binding) in pending_bindings.into_iter().rev() { - if let Some((binding_ix, _)) = bindings.first() - && *binding_ix > ix + if let Some(binding_ix) = first_binding_index + && binding_ix > ix { continue; }