diff --git a/crates/gpui/src/keymap.rs b/crates/gpui/src/keymap.rs index 174dbc80f02e9c1a16d563bfceff65c2bbb34e64..69700e64dc879fe73f7f79c73a5aa6f5fad2f46f 100644 --- a/crates/gpui/src/keymap.rs +++ b/crates/gpui/src/keymap.rs @@ -5,7 +5,7 @@ pub use binding::*; pub use context::*; use crate::{Action, Keystroke, is_no_action}; -use collections::HashMap; +use collections::{HashMap, HashSet}; use smallvec::SmallVec; use std::any::TypeId; @@ -167,76 +167,49 @@ impl Keymap { input: &[Keystroke], context_stack: &[KeyContext], ) -> (SmallVec<[(BindingIndex, KeyBinding); 1]>, bool) { - let mut possibilities = self - .bindings() - .enumerate() - .rev() - .filter_map(|(ix, binding)| { - let depth = self.binding_enabled(binding, &context_stack)?; - let pending = binding.match_keystrokes(input)?; - Some((depth, BindingIndex(ix), binding, pending)) - }) - .collect::>(); - possibilities.sort_by(|(depth_a, ix_a, _, _), (depth_b, ix_b, _, _)| { - depth_b.cmp(depth_a).then(ix_b.cmp(ix_a)) - }); + let mut bindings: SmallVec<[(usize, BindingIndex, &KeyBinding); 1]> = SmallVec::new(); + let mut pending_bindings: SmallVec<[(BindingIndex, &KeyBinding); 1]> = SmallVec::new(); - let mut bindings: SmallVec<[(BindingIndex, KeyBinding, usize); 1]> = SmallVec::new(); - - // (pending, is_no_action, depth, keystrokes) - let mut pending_info_opt: Option<(bool, bool, usize, &[Keystroke])> = None; - - 'outer: for (depth, binding_index, binding, pending) in possibilities { - let is_no_action = is_no_action(&*binding.action); - // We only want to consider a binding pending if it has an action - // This, however, means that if we have both a NoAction binding and a binding - // with an action at the same depth, we should still set is_pending to true. - if let Some(pending_info) = pending_info_opt.as_mut() { - let (already_pending, pending_is_no_action, pending_depth, pending_keystrokes) = - *pending_info; - - // We only want to change the pending status if it's not already pending AND if - // the existing pending status was set by a NoAction binding. This avoids a NoAction - // binding erroneously setting the pending status to true when a binding with an action - // already set it to false - // - // We also want to change the pending status if the keystrokes don't match, - // meaning it's different keystrokes than the NoAction that set pending to false - if pending - && !already_pending - && pending_is_no_action - && (pending_depth == depth || pending_keystrokes != binding.keystrokes()) - { - pending_info.0 = !is_no_action; - } - } else { - pending_info_opt = Some(( - pending && !is_no_action, - is_no_action, - depth, - binding.keystrokes(), - )); - } + for (ix, binding) in self.bindings().enumerate().rev() { + let Some(depth) = self.binding_enabled(binding, &context_stack) else { + continue; + }; + let Some(pending) = binding.match_keystrokes(input) else { + continue; + }; if !pending { - bindings.push((binding_index, binding.clone(), depth)); - continue 'outer; + bindings.push((depth, BindingIndex(ix), binding)) + } else { + pending_bindings.push((BindingIndex(ix), binding)) } } - // sort by descending depth - bindings.sort_by(|a, b| a.2.cmp(&b.2).reverse()); - let bindings = bindings + + 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() - .map_while(|(binding_index, binding, _)| { - if is_no_action(&*binding.action) { - None - } else { - Some((binding_index, binding)) - } - }) + .take_while(|(_, _, binding)| !is_no_action(&*binding.action)) + .map(|(_, ix, binding)| (ix, binding.clone())) .collect(); - (bindings, pending_info_opt.unwrap_or_default().0) + let mut pending = HashSet::default(); + for (ix, binding) in pending_bindings.into_iter().rev() { + if let Some((binding_ix, _)) = bindings.first() + && *binding_ix > ix + { + continue; + } + if is_no_action(&*binding.action) { + pending.remove(&&binding.keystrokes); + continue; + } + pending.insert(&binding.keystrokes); + } + + (bindings, !pending.is_empty()) } /// Check if the given binding is enabled, given a certain key context. @@ -302,6 +275,30 @@ mod tests { ); } + #[test] + fn test_depth_precedence() { + let bindings = [ + KeyBinding::new("ctrl-a", ActionBeta {}, Some("pane")), + KeyBinding::new("ctrl-a", ActionGamma {}, Some("editor")), + ]; + + let mut keymap = Keymap::default(); + keymap.add_bindings(bindings.clone()); + + let (result, pending) = keymap.bindings_for_input( + &[Keystroke::parse("ctrl-a").unwrap()], + &[ + KeyContext::parse("pane").unwrap(), + KeyContext::parse("editor").unwrap(), + ], + ); + + assert!(!pending); + assert_eq!(result.len(), 2); + assert!(result[0].action.partial_eq(&ActionGamma {})); + assert!(result[1].action.partial_eq(&ActionBeta {})); + } + #[test] fn test_keymap_disabled() { let bindings = [ @@ -453,6 +450,193 @@ mod tests { assert_eq!(space_editor.1, true); } + #[test] + fn test_override_multikey() { + let bindings = [ + KeyBinding::new("ctrl-w left", ActionAlpha {}, Some("editor")), + KeyBinding::new("ctrl-w", NoAction {}, Some("editor")), + ]; + + let mut keymap = Keymap::default(); + keymap.add_bindings(bindings.clone()); + + // Ensure `space` results in pending input on the workspace, but not editor + let (result, pending) = keymap.bindings_for_input( + &[Keystroke::parse("ctrl-w").unwrap()], + &[KeyContext::parse("editor").unwrap()], + ); + assert!(result.is_empty()); + assert_eq!(pending, true); + + let bindings = [ + KeyBinding::new("ctrl-w left", ActionAlpha {}, Some("editor")), + KeyBinding::new("ctrl-w", ActionBeta {}, Some("editor")), + ]; + + let mut keymap = Keymap::default(); + keymap.add_bindings(bindings.clone()); + + // Ensure `space` results in pending input on the workspace, but not editor + let (result, pending) = keymap.bindings_for_input( + &[Keystroke::parse("ctrl-w").unwrap()], + &[KeyContext::parse("editor").unwrap()], + ); + assert_eq!(result.len(), 1); + assert_eq!(pending, false); + } + + #[test] + fn test_simple_disable() { + let bindings = [ + KeyBinding::new("ctrl-x", ActionAlpha {}, Some("editor")), + KeyBinding::new("ctrl-x", NoAction {}, Some("editor")), + ]; + + let mut keymap = Keymap::default(); + keymap.add_bindings(bindings.clone()); + + // Ensure `space` results in pending input on the workspace, but not editor + let (result, pending) = keymap.bindings_for_input( + &[Keystroke::parse("ctrl-x").unwrap()], + &[KeyContext::parse("editor").unwrap()], + ); + assert!(result.is_empty()); + assert_eq!(pending, false); + } + + #[test] + fn test_fail_to_disable() { + // disabled at the wrong level + let bindings = [ + KeyBinding::new("ctrl-x", ActionAlpha {}, Some("editor")), + KeyBinding::new("ctrl-x", NoAction {}, Some("workspace")), + ]; + + let mut keymap = Keymap::default(); + keymap.add_bindings(bindings.clone()); + + // Ensure `space` results in pending input on the workspace, but not editor + let (result, pending) = keymap.bindings_for_input( + &[Keystroke::parse("ctrl-x").unwrap()], + &[ + KeyContext::parse("workspace").unwrap(), + KeyContext::parse("editor").unwrap(), + ], + ); + assert_eq!(result.len(), 1); + assert_eq!(pending, false); + } + + #[test] + fn test_disable_deeper() { + let bindings = [ + KeyBinding::new("ctrl-x", ActionAlpha {}, Some("workspace")), + KeyBinding::new("ctrl-x", NoAction {}, Some("editor")), + ]; + + let mut keymap = Keymap::default(); + keymap.add_bindings(bindings.clone()); + + // Ensure `space` results in pending input on the workspace, but not editor + let (result, pending) = keymap.bindings_for_input( + &[Keystroke::parse("ctrl-x").unwrap()], + &[ + KeyContext::parse("workspace").unwrap(), + KeyContext::parse("editor").unwrap(), + ], + ); + assert_eq!(result.len(), 0); + assert_eq!(pending, false); + } + + #[test] + fn test_pending_match_enabled() { + let bindings = [ + KeyBinding::new("ctrl-x", ActionBeta, Some("vim_mode == normal")), + KeyBinding::new("ctrl-x 0", ActionAlpha, Some("Workspace")), + ]; + let mut keymap = Keymap::default(); + keymap.add_bindings(bindings.clone()); + + let matched = keymap.bindings_for_input( + &[Keystroke::parse("ctrl-x")].map(Result::unwrap), + &[ + KeyContext::parse("Workspace"), + KeyContext::parse("Pane"), + KeyContext::parse("Editor vim_mode=normal"), + ] + .map(Result::unwrap), + ); + assert_eq!(matched.0.len(), 1); + assert!(matched.0[0].action.partial_eq(&ActionBeta)); + assert!(matched.1); + } + + #[test] + fn test_pending_match_enabled_extended() { + let bindings = [ + KeyBinding::new("ctrl-x", ActionBeta, Some("vim_mode == normal")), + KeyBinding::new("ctrl-x 0", NoAction, Some("Workspace")), + ]; + let mut keymap = Keymap::default(); + keymap.add_bindings(bindings.clone()); + + let matched = keymap.bindings_for_input( + &[Keystroke::parse("ctrl-x")].map(Result::unwrap), + &[ + KeyContext::parse("Workspace"), + KeyContext::parse("Pane"), + KeyContext::parse("Editor vim_mode=normal"), + ] + .map(Result::unwrap), + ); + assert_eq!(matched.0.len(), 1); + assert!(matched.0[0].action.partial_eq(&ActionBeta)); + assert!(!matched.1); + let bindings = [ + KeyBinding::new("ctrl-x", ActionBeta, Some("Workspace")), + KeyBinding::new("ctrl-x 0", NoAction, Some("vim_mode == normal")), + ]; + let mut keymap = Keymap::default(); + keymap.add_bindings(bindings.clone()); + + let matched = keymap.bindings_for_input( + &[Keystroke::parse("ctrl-x")].map(Result::unwrap), + &[ + KeyContext::parse("Workspace"), + KeyContext::parse("Pane"), + KeyContext::parse("Editor vim_mode=normal"), + ] + .map(Result::unwrap), + ); + assert_eq!(matched.0.len(), 1); + assert!(matched.0[0].action.partial_eq(&ActionBeta)); + assert!(!matched.1); + } + + #[test] + fn test_overriding_prefix() { + let bindings = [ + KeyBinding::new("ctrl-x 0", ActionAlpha, Some("Workspace")), + KeyBinding::new("ctrl-x", ActionBeta, Some("vim_mode == normal")), + ]; + let mut keymap = Keymap::default(); + keymap.add_bindings(bindings.clone()); + + let matched = keymap.bindings_for_input( + &[Keystroke::parse("ctrl-x")].map(Result::unwrap), + &[ + KeyContext::parse("Workspace"), + KeyContext::parse("Pane"), + KeyContext::parse("Editor vim_mode=normal"), + ] + .map(Result::unwrap), + ); + assert_eq!(matched.0.len(), 1); + assert!(matched.0[0].action.partial_eq(&ActionBeta)); + assert!(!matched.1); + } + #[test] fn test_bindings_for_action() { let bindings = [