fixed binding fallback

K Simmons created

Change summary

crates/gpui/src/app.rs    |  21 +++---
crates/gpui/src/keymap.rs | 116 ++++++++++++++++++----------------------
2 files changed, 64 insertions(+), 73 deletions(-)

Detailed changes

crates/gpui/src/app.rs 🔗

@@ -1551,17 +1551,18 @@ impl MutableAppContext {
             {
                 MatchResult::None => false,
                 MatchResult::Pending => true,
-                MatchResult::Match { view_id, action } => {
-                    if self.handle_dispatch_action_from_effect(
-                        window_id,
-                        Some(view_id),
-                        action.as_ref(),
-                    ) {
-                        self.keystroke_matcher.clear_pending();
-                        true
-                    } else {
-                        false
+                MatchResult::Matches(matches) => {
+                    for (view_id, action) in matches {
+                        if self.handle_dispatch_action_from_effect(
+                            window_id,
+                            Some(view_id),
+                            action.as_ref(),
+                        ) {
+                            self.keystroke_matcher.clear_pending();
+                            return true;
+                        }
                     }
+                    false
                 }
             }
         } else {

crates/gpui/src/keymap.rs 🔗

@@ -13,16 +13,11 @@ extern "C" {
 }
 
 pub struct Matcher {
-    pending: HashMap<usize, Pending>,
+    pending_views: HashMap<usize, Context>,
+    pending_keystrokes: Vec<Keystroke>,
     keymap: Keymap,
 }
 
-#[derive(Default)]
-struct Pending {
-    keystrokes: Vec<Keystroke>,
-    context: Option<Context>,
-}
-
 #[derive(Default)]
 pub struct Keymap {
     bindings: Vec<Binding>,
@@ -77,7 +72,7 @@ where
 pub enum MatchResult {
     None,
     Pending,
-    Match(Vec<(usize, Box<dyn Action>)>),
+    Matches(Vec<(usize, Box<dyn Action>)>),
 }
 
 impl Debug for MatchResult {
@@ -85,10 +80,13 @@ impl Debug for MatchResult {
         match self {
             MatchResult::None => f.debug_struct("MatchResult2::None").finish(),
             MatchResult::Pending => f.debug_struct("MatchResult2::Pending").finish(),
-            MatchResult::Match { view_id, action } => f
-                .debug_struct("MatchResult::Match")
-                .field("view_id", view_id)
-                .field("action", &action.name())
+            MatchResult::Matches(matches) => f
+                .debug_list()
+                .entries(
+                    matches
+                        .iter()
+                        .map(|(view_id, action)| format!("{view_id}, {}", action.name())),
+                )
                 .finish(),
         }
     }
@@ -99,13 +97,14 @@ impl PartialEq for MatchResult {
         match (self, other) {
             (MatchResult::None, MatchResult::None) => true,
             (MatchResult::Pending, MatchResult::Pending) => true,
-            (
-                MatchResult::Match { view_id, action },
-                MatchResult::Match {
-                    view_id: other_view_id,
-                    action: other_action,
-                },
-            ) => view_id == other_view_id && action.eq(other_action.as_ref()),
+            (MatchResult::Matches(matches), MatchResult::Matches(other_matches)) => {
+                matches.len() == other_matches.len()
+                    && matches.iter().zip(other_matches.iter()).all(
+                        |((view_id, action), (other_view_id, other_action))| {
+                            view_id == other_view_id && action.eq(other_action.as_ref())
+                        },
+                    )
+            }
             _ => false,
         }
     }
@@ -116,23 +115,24 @@ impl Eq for MatchResult {}
 impl Matcher {
     pub fn new(keymap: Keymap) -> Self {
         Self {
-            pending: HashMap::new(),
+            pending_views: HashMap::new(),
+            pending_keystrokes: Vec::new(),
             keymap,
         }
     }
 
     pub fn set_keymap(&mut self, keymap: Keymap) {
-        self.pending.clear();
+        self.clear_pending();
         self.keymap = keymap;
     }
 
     pub fn add_bindings<T: IntoIterator<Item = Binding>>(&mut self, bindings: T) {
-        self.pending.clear();
+        self.clear_pending();
         self.keymap.add_bindings(bindings);
     }
 
     pub fn clear_bindings(&mut self) {
-        self.pending.clear();
+        self.clear_pending();
         self.keymap.clear();
     }
 
@@ -141,11 +141,12 @@ impl Matcher {
     }
 
     pub fn clear_pending(&mut self) {
-        self.pending.clear();
+        self.pending_keystrokes.clear();
+        self.pending_views.clear();
     }
 
     pub fn has_pending_keystrokes(&self) -> bool {
-        !self.pending.is_empty()
+        !self.pending_keystrokes.is_empty()
     }
 
     pub fn push_keystroke(
@@ -156,54 +157,49 @@ impl Matcher {
         let mut any_pending = false;
         let mut matched_bindings = Vec::new();
 
-        let first_keystroke = self.pending.is_empty();
-        dbg!(&dispatch_path);
+        let first_keystroke = self.pending_keystrokes.is_empty();
+        self.pending_keystrokes.push(keystroke);
+
         for (view_id, context) in dispatch_path {
-            if !first_keystroke && !self.pending.contains_key(&view_id) {
+            // Don't require pending view entry if there are no pending keystrokes
+            if !first_keystroke && !self.pending_views.contains_key(&view_id) {
                 continue;
             }
 
-            let pending = self.pending.entry(view_id).or_default();
-
-            if let Some(pending_context) = pending.context.as_ref() {
-                if pending_context != &context {
-                    pending.keystrokes.clear();
+            // If there is a previous view context, invalidate that view if it
+            // has changed
+            if let Some(previous_view_context) = self.pending_views.remove(&view_id) {
+                if previous_view_context != context {
+                    continue;
                 }
             }
 
-            pending.keystrokes.push(keystroke.clone());
-
-            let mut retain_pending = false;
             for binding in self.keymap.bindings.iter().rev() {
-                if binding.keystrokes.starts_with(&pending.keystrokes)
+                if binding.keystrokes.starts_with(&self.pending_keystrokes)
                     && binding
                         .context_predicate
                         .as_ref()
                         .map(|c| c.eval(&context))
                         .unwrap_or(true)
                 {
-                    if binding.keystrokes.len() == pending.keystrokes.len() {
-                        self.pending.remove(&view_id);
+                    if binding.keystrokes.len() == self.pending_keystrokes.len() {
                         matched_bindings.push((view_id, binding.action.boxed_clone()));
                     } else {
-                        retain_pending = true;
-                        pending.context = Some(context.clone());
+                        self.pending_views.insert(view_id, context.clone());
+                        any_pending = true;
                     }
                 }
             }
-
-            if retain_pending {
-                any_pending = true;
-            } else {
-                self.pending.remove(&view_id);
-            }
         }
 
         if !matched_bindings.is_empty() {
-            MatchResult::Match(matched_bindings)
+            self.pending_views.clear();
+            self.pending_keystrokes.clear();
+            MatchResult::Matches(matched_bindings)
         } else if any_pending {
             MatchResult::Pending
         } else {
+            self.pending_keystrokes.clear();
             MatchResult::None
         }
     }
@@ -520,21 +516,15 @@ mod tests {
             matcher.push_keystroke(Keystroke::parse("a")?, dispatch_path.clone())
         );
         assert_eq!(
-            MatchResult::Match {
-                view_id: 1,
-                action: Box::new(AB)
-            },
+            MatchResult::Matches(vec![(1, Box::new(AB))]),
             matcher.push_keystroke(Keystroke::parse("b")?, dispatch_path.clone())
         );
-        assert!(matcher.pending.is_empty());
+        assert!(!matcher.has_pending_keystrokes());
         assert_eq!(
-            MatchResult::Match {
-                view_id: 2,
-                action: Box::new(B)
-            },
+            MatchResult::Matches(vec![(2, Box::new(B))]),
             matcher.push_keystroke(Keystroke::parse("b")?, dispatch_path.clone())
         );
-        assert!(matcher.pending.is_empty());
+        assert!(!matcher.has_pending_keystrokes());
         assert_eq!(
             MatchResult::Pending,
             matcher.push_keystroke(Keystroke::parse("a")?, dispatch_path.clone())
@@ -543,7 +533,7 @@ mod tests {
             MatchResult::None,
             matcher.push_keystroke(Keystroke::parse("c")?, dispatch_path.clone())
         );
-        assert!(matcher.pending.is_empty());
+        assert!(!matcher.has_pending_keystrokes());
 
         Ok(())
     }
@@ -717,15 +707,15 @@ mod tests {
     }
 
     impl Matcher {
-        fn test_keystroke(
-            &mut self,
+        fn test_keystroke<'a>(
+            &'a mut self,
             keystroke: &str,
             dispatch_path: Vec<(usize, Context)>,
         ) -> Option<Box<dyn Action>> {
-            if let MatchResult::Match { action, .. } =
+            if let MatchResult::Matches(matches) =
                 self.push_keystroke(Keystroke::parse(keystroke).unwrap(), dispatch_path)
             {
-                Some(action.boxed_clone())
+                Some(matches[0].1.boxed_clone())
             } else {
                 None
             }