Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Update keymap context binding behavior of > and ! (#34664)

Browse source 
Now ! means "no ancestors matches this", and > means "any descendent"
not "any child".

Updates #34570

Co-authored-by: Ben Kunkle <ben@zed.dev>

Release Notes:

- *Breaking change*. The context predicates in the keymap file now
handle ! and > differently. Before this change ! meant "this node does
not match", now it means "none of these nodes match". Before this change
> meant "child of", now it means "descendent of". We do not expect these
changes to break many keymaps, but they may cause subtle changes for
complex context queries.

---------

Co-authored-by: Ben Kunkle <ben@zed.dev>
This commit is contained in:
Conrad Irwin 2025-07-18 09:25:18 -06:00 • committed by GitHub
parent f461290ac3
commit e421fc7a2d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 261 additions and 90 deletions
Download patch file Download diff file Expand all files Collapse all files

136
crates/gpui/src/keymap.rs
View file

@ -24,7 +24,7 @@ pub struct Keymap {
}
/// Index of a binding within a keymap.
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
#[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd)]
pub struct BindingIndex(usize);
impl Keymap {
@ -167,65 +167,60 @@ impl Keymap {
input: &[Keystroke],
context_stack: &[KeyContext],
) -> (SmallVec<[(BindingIndex, KeyBinding); 1]>, bool) {
let possibilities = self
let mut possibilities = self
.bindings()
.enumerate()
.rev()
.filter_map(|(ix, binding)| {
binding
.match_keystrokes(input)
.map(|pending| (BindingIndex(ix), binding, pending))
});
let depth = self.binding_enabled(binding, &context_stack)?;
let pending = binding.match_keystrokes(input)?;
Some((depth, BindingIndex(ix), binding, pending))
})
.collect::<Vec<_>>();
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<[(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 (binding_index, binding, pending) in possibilities {
for depth in (0..=context_stack.len()).rev() {
if self.binding_enabled(binding, &context_stack[0..depth]) {
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;
'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(),
));
}
if !pending {
bindings.push((binding_index, binding.clone(), depth));
continue 'outer;
}
// 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(),
));
}
if !pending {
bindings.push((binding_index, binding.clone(), depth));
continue 'outer;
}
}
// sort by descending depth
@ -245,15 +240,13 @@ impl Keymap {
}
/// Check if the given binding is enabled, given a certain key context.
fn binding_enabled(&self, binding: &KeyBinding, context: &[KeyContext]) -> bool {
// If binding has a context predicate, it must match the current context,
/// Returns the deepest depth at which the binding matches, or None if it doesn't match.
fn binding_enabled(&self, binding: &KeyBinding, contexts: &[KeyContext]) -> Option<usize> {
if let Some(predicate) = &binding.context_predicate {
if !predicate.eval(context) {
return false;
}
predicate.depth_of(contexts)
} else {
Some(contexts.len())
}
true
}
}
@ -280,18 +273,33 @@ mod tests {
keymap.add_bindings(bindings.clone());
// global bindings are enabled in all contexts
assert!(keymap.binding_enabled(&bindings[0], &[]));
assert!(keymap.binding_enabled(&bindings[0], &[KeyContext::parse("terminal").unwrap()]));
assert_eq!(keymap.binding_enabled(&bindings[0], &[]), Some(0));
assert_eq!(
keymap.binding_enabled(&bindings[0], &[KeyContext::parse("terminal").unwrap()]),
Some(1)
);
// contextual bindings are enabled in contexts that match their predicate
assert!(!keymap.binding_enabled(&bindings[1], &[KeyContext::parse("barf x=y").unwrap()]));
assert!(keymap.binding_enabled(&bindings[1], &[KeyContext::parse("pane x=y").unwrap()]));
assert_eq!(
keymap.binding_enabled(&bindings[1], &[KeyContext::parse("barf x=y").unwrap()]),
None
);
assert_eq!(
keymap.binding_enabled(&bindings[1], &[KeyContext::parse("pane x=y").unwrap()]),
Some(1)
);
assert!(!keymap.binding_enabled(&bindings[2], &[KeyContext::parse("editor").unwrap()]));
assert!(keymap.binding_enabled(
&bindings[2],
&[KeyContext::parse("editor mode=full").unwrap()]
));
assert_eq!(
keymap.binding_enabled(&bindings[2], &[KeyContext::parse("editor").unwrap()]),
None
);
assert_eq!(
keymap.binding_enabled(
&bindings[2],
&[KeyContext::parse("editor mode=full").unwrap()]
),
Some(1)
);
}
#[test]

181
crates/gpui/src/keymap/context.rs
View file

@ -178,7 +178,7 @@ pub enum KeyBindingContextPredicate {
NotEqual(SharedString, SharedString),
/// A predicate that will match a given predicate appearing below another predicate.
/// in the element tree
Child(
Descendant(
Box<KeyBindingContextPredicate>,
Box<KeyBindingContextPredicate>,
),
@ -203,7 +203,7 @@ impl fmt::Display for KeyBindingContextPredicate {
Self::Equal(left, right) => write!(f, "{} == {}", left, right),
Self::NotEqual(left, right) => write!(f, "{} != {}", left, right),
Self::Not(pred) => write!(f, "!{}", pred),
Self::Child(parent, child) => write!(f, "{} > {}", parent, child),
Self::Descendant(parent, child) => write!(f, "{} > {}", parent, child),
Self::And(left, right) => write!(f, "({} && {})", left, right),
Self::Or(left, right) => write!(f, "({} || {})", left, right),
}
@ -249,8 +249,25 @@ impl KeyBindingContextPredicate {
}
}
/// Find the deepest depth at which the predicate matches.
pub fn depth_of(&self, contexts: &[KeyContext]) -> Option<usize> {
for depth in (0..=contexts.len()).rev() {
let context_slice = &contexts[0..depth];
if self.eval_inner(context_slice, contexts) {
return Some(depth);
}
}
None
}
/// Eval a predicate against a set of contexts, arranged from lowest to highest.
pub fn eval(&self, contexts: &[KeyContext]) -> bool {
#[allow(unused)]
pub(crate) fn eval(&self, contexts: &[KeyContext]) -> bool {
self.eval_inner(contexts, contexts)
}
/// Eval a predicate against a set of contexts, arranged from lowest to highest.
pub fn eval_inner(&self, contexts: &[KeyContext], all_contexts: &[KeyContext]) -> bool {
let Some(context) = contexts.last() else {
return false;
};
@ -264,12 +281,38 @@ impl KeyBindingContextPredicate {
.get(left)
.map(|value| value != right)
.unwrap_or(true),
Self::Not(pred) => !pred.eval(contexts),
Self::Child(parent, child) => {
parent.eval(&contexts[..contexts.len() - 1]) && child.eval(contexts)
Self::Not(pred) => {
for i in 0..all_contexts.len() {
if pred.eval_inner(&all_contexts[..=i], all_contexts) {
return false;
}
}
return true;
}
// Workspace > Pane > Editor
//
// Pane > (Pane > Editor) // should match?
// (Pane > Pane) > Editor // should not match?
// Pane > !Workspace <-- should match?
// !Workspace <-- shouldn't match?
Self::Descendant(parent, child) => {
for i in 0..contexts.len() - 1 {
// [Workspace > Pane], [Editor]
if parent.eval_inner(&contexts[..=i], all_contexts) {
if !child.eval_inner(&contexts[i + 1..], &contexts[i + 1..]) {
return false;
}
return true;
}
}
return false;
}
Self::And(left, right) => {
left.eval_inner(contexts, all_contexts) && right.eval_inner(contexts, all_contexts)
}
Self::Or(left, right) => {
left.eval_inner(contexts, all_contexts) || right.eval_inner(contexts, all_contexts)
}
Self::And(left, right) => left.eval(contexts) && right.eval(contexts),
Self::Or(left, right) => left.eval(contexts) || right.eval(contexts),
}
}
@ -285,7 +328,7 @@ impl KeyBindingContextPredicate {
}
match other {
KeyBindingContextPredicate::Child(_, child) => self.is_superset(child),
KeyBindingContextPredicate::Descendant(_, child) => self.is_superset(child),
KeyBindingContextPredicate::And(left, right) => {
self.is_superset(left) || self.is_superset(right)
}
@ -375,7 +418,7 @@ impl KeyBindingContextPredicate {
}
fn new_child(self, other: Self) -> Result<Self> {
Ok(Self::Child(Box::new(self), Box::new(other)))
Ok(Self::Descendant(Box::new(self), Box::new(other)))
}
fn new_eq(self, other: Self) -> Result<Self> {
@ -598,4 +641,122 @@ mod tests {
assert_eq!(a.is_superset(&b), result, "({a:?}).is_superset({b:?})");
}
}
#[test]
fn test_child_operator() {
let predicate = KeyBindingContextPredicate::parse("parent > child").unwrap();
let parent_context = KeyContext::try_from("parent").unwrap();
let child_context = KeyContext::try_from("child").unwrap();
let contexts = vec![parent_context.clone(), child_context.clone()];
assert!(predicate.eval(&contexts));
let grandparent_context = KeyContext::try_from("grandparent").unwrap();
let contexts = vec![
grandparent_context,
parent_context.clone(),
child_context.clone(),
];
assert!(predicate.eval(&contexts));
let other_context = KeyContext::try_from("other").unwrap();
let contexts = vec![other_context.clone(), child_context.clone()];
assert!(!predicate.eval(&contexts));
let contexts = vec![
parent_context.clone(),
other_context.clone(),
child_context.clone(),
];
assert!(predicate.eval(&contexts));
assert!(!predicate.eval(&[]));
assert!(!predicate.eval(&[child_context.clone()]));
assert!(!predicate.eval(&[parent_context]));
let zany_predicate = KeyBindingContextPredicate::parse("child > child").unwrap();
assert!(!zany_predicate.eval(&[child_context.clone()]));
assert!(zany_predicate.eval(&[child_context.clone(), child_context.clone()]));
}
#[test]
fn test_not_operator() {
let not_predicate = KeyBindingContextPredicate::parse("!editor").unwrap();
let editor_context = KeyContext::try_from("editor").unwrap();
let workspace_context = KeyContext::try_from("workspace").unwrap();
let parent_context = KeyContext::try_from("parent").unwrap();
let child_context = KeyContext::try_from("child").unwrap();
assert!(not_predicate.eval(&[workspace_context.clone()]));
assert!(!not_predicate.eval(&[editor_context.clone()]));
assert!(!not_predicate.eval(&[editor_context.clone(), workspace_context.clone()]));
assert!(!not_predicate.eval(&[workspace_context.clone(), editor_context.clone()]));
let complex_not = KeyBindingContextPredicate::parse("!editor && workspace").unwrap();
assert!(complex_not.eval(&[workspace_context.clone()]));
assert!(!complex_not.eval(&[editor_context.clone(), workspace_context.clone()]));
let not_mode_predicate = KeyBindingContextPredicate::parse("!(mode == full)").unwrap();
let mut mode_context = KeyContext::default();
mode_context.set("mode", "full");
assert!(!not_mode_predicate.eval(&[mode_context.clone()]));
let mut other_mode_context = KeyContext::default();
other_mode_context.set("mode", "partial");
assert!(not_mode_predicate.eval(&[other_mode_context]));
let not_descendant = KeyBindingContextPredicate::parse("!(parent > child)").unwrap();
assert!(not_descendant.eval(&[parent_context.clone()]));
assert!(not_descendant.eval(&[child_context.clone()]));
assert!(!not_descendant.eval(&[parent_context.clone(), child_context.clone()]));
let not_descendant = KeyBindingContextPredicate::parse("parent > !child").unwrap();
assert!(!not_descendant.eval(&[parent_context.clone()]));
assert!(!not_descendant.eval(&[child_context.clone()]));
assert!(!not_descendant.eval(&[parent_context.clone(), child_context.clone()]));
let double_not = KeyBindingContextPredicate::parse("!!editor").unwrap();
assert!(double_not.eval(&[editor_context.clone()]));
assert!(!double_not.eval(&[workspace_context.clone()]));
// Test complex descendant cases
let workspace_context = KeyContext::try_from("Workspace").unwrap();
let pane_context = KeyContext::try_from("Pane").unwrap();
let editor_context = KeyContext::try_from("Editor").unwrap();
// Workspace > Pane > Editor
let workspace_pane_editor = vec![
workspace_context.clone(),
pane_context.clone(),
editor_context.clone(),
];
// Pane > (Pane > Editor) - should not match
let pane_pane_editor = KeyBindingContextPredicate::parse("Pane > (Pane > Editor)").unwrap();
assert!(!pane_pane_editor.eval(&workspace_pane_editor));
let workspace_pane_editor_predicate =
KeyBindingContextPredicate::parse("Workspace > Pane > Editor").unwrap();
assert!(workspace_pane_editor_predicate.eval(&workspace_pane_editor));
// (Pane > Pane) > Editor - should not match
let pane_pane_then_editor =
KeyBindingContextPredicate::parse("(Pane > Pane) > Editor").unwrap();
assert!(!pane_pane_then_editor.eval(&workspace_pane_editor));
// Pane > !Workspace - should match
let pane_not_workspace = KeyBindingContextPredicate::parse("Pane > !Workspace").unwrap();
assert!(pane_not_workspace.eval(&[pane_context.clone(), editor_context.clone()]));
assert!(!pane_not_workspace.eval(&[pane_context.clone(), workspace_context.clone()]));
// !Workspace - shouldn't match when Workspace is in the context
let not_workspace = KeyBindingContextPredicate::parse("!Workspace").unwrap();
assert!(!not_workspace.eval(&[workspace_context.clone()]));
assert!(not_workspace.eval(&[pane_context.clone()]));
assert!(not_workspace.eval(&[editor_context.clone()]));
assert!(!not_workspace.eval(&workspace_pane_editor));
}
}

9
crates/language_tools/src/key_context_view.rs
View file

@ -132,14 +132,7 @@ impl KeyContextView {
}
fn matches(&self, predicate: &KeyBindingContextPredicate) -> bool {
let mut stack = self.context_stack.clone();
while !stack.is_empty() {
if predicate.eval(&stack) {
return true;
}
stack.pop();
}
false
predicate.depth_of(&self.context_stack).is_some()
}
fn action_matches(&self, a: &Option<Box<dyn Action>>, b: &dyn Action) -> bool {

2
crates/settings_ui/src/keybindings.rs
View file

@ -3008,7 +3008,7 @@ fn collect_contexts_from_assets() -> Vec<SharedString> {
contexts.insert(ident_a);
contexts.insert(ident_b);
}
gpui::KeyBindingContextPredicate::Child(ctx_a, ctx_b) => {
gpui::KeyBindingContextPredicate::Descendant(ctx_a, ctx_b) => {
queue.push(*ctx_a);
queue.push(*ctx_b);
}