Closes #28164 This PR adresses inproper keybinds being shown in MacOS application menus. The issue arises because the keybinds shown in MacOS application menus are unaware of keybind contexts (they are only ever updated [on a keymap-change](https://github.com/zed-industries/zed/blob/6d1dd109f554579bdf676c14b69deb9e16acc043/crates/zed/src/zed.rs#L1421)). Thus, using the keybind that was added last in the keymap can result in incorrect keybindings being shown quite frequently, as they might belong to a different context not generally available (applies the same for the default keymap as well as for user-keymaps). For example, the linked issue arises because the keybind found last in the iterator is https://github.com/zed-industries/zed/blob/6d1dd109f554579bdf676c14b69deb9e16acc043/assets/keymaps/vim.json#L759, which is not even available in most contexts (and, additionally, the `e` of `escape` is rendered here as a keybind which seems to be a seperate issue). Additionally, this would result in inconsistent behavior with some Vim-keybinds. A vim-keybind would be used only when available but otherwise the default binding would be shown (see `Undo` and `Redo` as an example below), which seems inconsistent. This PR fixes this by instead using the first keybind found in keymaps, which is expected to be the keybind available in most contexts. Additionally, this allows rendering some more keybinds for actions which vim-keybind cannot be displayed (Find In Project for example) .This seems to be more reasonable until [this related comment](https://github.com/zed-industries/zed/blob/6d1dd109f554579bdf676c14b69deb9e16acc043/crates/gpui/src/keymap.rs#L199-L204) is resolved. This includes a revert of #25878 as well. With this change, the change made in #25878 becomes obsolete and would also regress the behavior back to the state prior to that PR. | | `main` | This PR | | --- | --- | --- | | Edit-menu | <img width="220" alt="main_edit" src="https://github.com/user-attachments/assets/9f793b64-80b6-4a5b-b7e5-628f0d552166" /> | <img width="220" alt="PR_edit" src="https://github.com/user-attachments/assets/bccb444c-7a49-41d5-9377-d90b1639a3ed" /> | | View-menu | <img width="214" alt="main_view" src="https://github.com/user-attachments/assets/0e6a6632-df02-4883-9f5a-facb4d0263b5" /> | <img width="214" alt="PR_view" src="https://github.com/user-attachments/assets/14600ece-fcaa-447a-94ef-4fa350eca49c" /> | Release Notes: - Improved keybinds displayed for actions in MacOS application menus.
340 lines
12 KiB
Rust
340 lines
12 KiB
Rust
mod binding;
|
|
mod context;
|
|
|
|
pub use binding::*;
|
|
pub use context::*;
|
|
|
|
use crate::{Action, Keystroke, is_no_action};
|
|
use collections::HashMap;
|
|
use smallvec::SmallVec;
|
|
use std::any::TypeId;
|
|
|
|
/// An opaque identifier of which version of the keymap is currently active.
|
|
/// The keymap's version is changed whenever bindings are added or removed.
|
|
#[derive(Copy, Clone, Eq, PartialEq, Default)]
|
|
pub struct KeymapVersion(usize);
|
|
|
|
/// A collection of key bindings for the user's application.
|
|
#[derive(Default)]
|
|
pub struct Keymap {
|
|
bindings: Vec<KeyBinding>,
|
|
binding_indices_by_action_id: HashMap<TypeId, SmallVec<[usize; 3]>>,
|
|
no_action_binding_indices: Vec<usize>,
|
|
version: KeymapVersion,
|
|
}
|
|
|
|
impl Keymap {
|
|
/// Create a new keymap with the given bindings.
|
|
pub fn new(bindings: Vec<KeyBinding>) -> Self {
|
|
let mut this = Self::default();
|
|
this.add_bindings(bindings);
|
|
this
|
|
}
|
|
|
|
/// Get the current version of the keymap.
|
|
pub fn version(&self) -> KeymapVersion {
|
|
self.version
|
|
}
|
|
|
|
/// Add more bindings to the keymap.
|
|
pub fn add_bindings<T: IntoIterator<Item = KeyBinding>>(&mut self, bindings: T) {
|
|
for binding in bindings {
|
|
let action_id = binding.action().as_any().type_id();
|
|
if is_no_action(&*binding.action) {
|
|
self.no_action_binding_indices.push(self.bindings.len());
|
|
} else {
|
|
self.binding_indices_by_action_id
|
|
.entry(action_id)
|
|
.or_default()
|
|
.push(self.bindings.len());
|
|
}
|
|
self.bindings.push(binding);
|
|
}
|
|
|
|
self.version.0 += 1;
|
|
}
|
|
|
|
/// Reset this keymap to its initial state.
|
|
pub fn clear(&mut self) {
|
|
self.bindings.clear();
|
|
self.binding_indices_by_action_id.clear();
|
|
self.no_action_binding_indices.clear();
|
|
self.version.0 += 1;
|
|
}
|
|
|
|
/// Iterate over all bindings, in the order they were added.
|
|
pub fn bindings(&self) -> impl DoubleEndedIterator<Item = &KeyBinding> {
|
|
self.bindings.iter()
|
|
}
|
|
|
|
/// Iterate over all bindings for the given action, in the order they were added. For display,
|
|
/// the last binding should take precedence.
|
|
pub fn bindings_for_action<'a>(
|
|
&'a self,
|
|
action: &'a dyn Action,
|
|
) -> impl 'a + DoubleEndedIterator<Item = &'a KeyBinding> {
|
|
let action_id = action.type_id();
|
|
let binding_indices = self
|
|
.binding_indices_by_action_id
|
|
.get(&action_id)
|
|
.map_or(&[] as _, SmallVec::as_slice)
|
|
.iter();
|
|
|
|
binding_indices.filter_map(|ix| {
|
|
let binding = &self.bindings[*ix];
|
|
if !binding.action().partial_eq(action) {
|
|
return None;
|
|
}
|
|
|
|
for null_ix in &self.no_action_binding_indices {
|
|
if null_ix > ix {
|
|
let null_binding = &self.bindings[*null_ix];
|
|
if null_binding.keystrokes == binding.keystrokes {
|
|
let null_binding_matches =
|
|
match (&null_binding.context_predicate, &binding.context_predicate) {
|
|
(None, _) => true,
|
|
(Some(_), None) => false,
|
|
(Some(null_predicate), Some(predicate)) => {
|
|
null_predicate.is_superset(predicate)
|
|
}
|
|
};
|
|
if null_binding_matches {
|
|
return None;
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
Some(binding)
|
|
})
|
|
}
|
|
|
|
/// 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()
|
|
.filter_map(|binding| {
|
|
binding.match_keystrokes(input).filter(|pending| !pending)?;
|
|
Some(binding.clone())
|
|
})
|
|
.collect()
|
|
}
|
|
|
|
/// 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.
|
|
///
|
|
/// 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.
|
|
pub fn bindings_for_input(
|
|
&self,
|
|
input: &[Keystroke],
|
|
context_stack: &[KeyContext],
|
|
) -> (SmallVec<[KeyBinding; 1]>, bool) {
|
|
let possibilities = self.bindings().rev().filter_map(|binding| {
|
|
binding
|
|
.match_keystrokes(input)
|
|
.map(|pending| (binding, pending))
|
|
});
|
|
|
|
let mut bindings: SmallVec<[(KeyBinding, usize); 1]> = SmallVec::new();
|
|
let mut is_pending = None;
|
|
|
|
'outer: for (binding, pending) in possibilities {
|
|
for depth in (0..=context_stack.len()).rev() {
|
|
if self.binding_enabled(binding, &context_stack[0..depth]) {
|
|
if is_pending.is_none() {
|
|
is_pending = Some(pending);
|
|
}
|
|
if !pending {
|
|
bindings.push((binding.clone(), depth));
|
|
continue 'outer;
|
|
}
|
|
}
|
|
}
|
|
}
|
|
bindings.sort_by(|a, b| a.1.cmp(&b.1).reverse());
|
|
let bindings = bindings
|
|
.into_iter()
|
|
.map_while(|(binding, _)| {
|
|
if is_no_action(&*binding.action) {
|
|
None
|
|
} else {
|
|
Some(binding)
|
|
}
|
|
})
|
|
.collect();
|
|
|
|
(bindings, is_pending.unwrap_or_default())
|
|
}
|
|
|
|
/// 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,
|
|
if let Some(predicate) = &binding.context_predicate {
|
|
if !predicate.eval(context) {
|
|
return false;
|
|
}
|
|
}
|
|
|
|
true
|
|
}
|
|
|
|
/// WARN: Assumes the bindings are in the order they were added to the keymap
|
|
/// returns the last binding for the given bindings, which
|
|
/// should be the user's binding in their keymap.json if they've set one,
|
|
/// otherwise, the last declared binding for this action in the base keymaps
|
|
/// (with Vim mode bindings being considered as declared later if Vim mode
|
|
/// is enabled)
|
|
///
|
|
/// If you are considering changing the behavior of this function
|
|
/// (especially to fix a user reported issue) see issues #23621, #24931,
|
|
/// and possibly others as evidence that it has swapped back and forth a
|
|
/// couple times. The decision as of now is to pick a side and leave it
|
|
/// as is, until we have a better way to decide which binding to display
|
|
/// that is consistent and not confusing.
|
|
pub fn binding_to_display_from_bindings(mut bindings: Vec<KeyBinding>) -> Option<KeyBinding> {
|
|
bindings.pop()
|
|
}
|
|
|
|
/// Returns the first binding present in the iterator, which tends to be the
|
|
/// default binding without any key context. This is useful for cases where no
|
|
/// key context is available on binding display. Otherwise, bindings with a
|
|
/// more specific key context would take precedence and result in a
|
|
/// potentially invalid keybind being returned.
|
|
pub fn default_binding_from_bindings_iterator<'a>(
|
|
mut bindings: impl Iterator<Item = &'a KeyBinding>,
|
|
) -> Option<&'a KeyBinding> {
|
|
bindings.next()
|
|
}
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use super::*;
|
|
use crate as gpui;
|
|
use gpui::{NoAction, actions};
|
|
|
|
actions!(
|
|
keymap_test,
|
|
[ActionAlpha, ActionBeta, ActionGamma, ActionDelta,]
|
|
);
|
|
|
|
#[test]
|
|
fn test_keymap() {
|
|
let bindings = [
|
|
KeyBinding::new("ctrl-a", ActionAlpha {}, None),
|
|
KeyBinding::new("ctrl-a", ActionBeta {}, Some("pane")),
|
|
KeyBinding::new("ctrl-a", ActionGamma {}, Some("editor && mode==full")),
|
|
];
|
|
|
|
let mut keymap = Keymap::default();
|
|
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()]));
|
|
|
|
// 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!(!keymap.binding_enabled(&bindings[2], &[KeyContext::parse("editor").unwrap()]));
|
|
assert!(keymap.binding_enabled(
|
|
&bindings[2],
|
|
&[KeyContext::parse("editor mode=full").unwrap()]
|
|
));
|
|
}
|
|
|
|
#[test]
|
|
fn test_keymap_disabled() {
|
|
let bindings = [
|
|
KeyBinding::new("ctrl-a", ActionAlpha {}, Some("editor")),
|
|
KeyBinding::new("ctrl-b", ActionAlpha {}, Some("editor")),
|
|
KeyBinding::new("ctrl-a", NoAction {}, Some("editor && mode==full")),
|
|
KeyBinding::new("ctrl-b", NoAction {}, None),
|
|
];
|
|
|
|
let mut keymap = Keymap::default();
|
|
keymap.add_bindings(bindings.clone());
|
|
|
|
// binding is only enabled in a specific context
|
|
assert!(
|
|
keymap
|
|
.bindings_for_input(
|
|
&[Keystroke::parse("ctrl-a").unwrap()],
|
|
&[KeyContext::parse("barf").unwrap()],
|
|
)
|
|
.0
|
|
.is_empty()
|
|
);
|
|
assert!(
|
|
!keymap
|
|
.bindings_for_input(
|
|
&[Keystroke::parse("ctrl-a").unwrap()],
|
|
&[KeyContext::parse("editor").unwrap()],
|
|
)
|
|
.0
|
|
.is_empty()
|
|
);
|
|
|
|
// binding is disabled in a more specific context
|
|
assert!(
|
|
keymap
|
|
.bindings_for_input(
|
|
&[Keystroke::parse("ctrl-a").unwrap()],
|
|
&[KeyContext::parse("editor mode=full").unwrap()],
|
|
)
|
|
.0
|
|
.is_empty()
|
|
);
|
|
|
|
// binding is globally disabled
|
|
assert!(
|
|
keymap
|
|
.bindings_for_input(
|
|
&[Keystroke::parse("ctrl-b").unwrap()],
|
|
&[KeyContext::parse("barf").unwrap()],
|
|
)
|
|
.0
|
|
.is_empty()
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn test_bindings_for_action() {
|
|
let bindings = [
|
|
KeyBinding::new("ctrl-a", ActionAlpha {}, Some("pane")),
|
|
KeyBinding::new("ctrl-b", ActionBeta {}, Some("editor && mode == full")),
|
|
KeyBinding::new("ctrl-c", ActionGamma {}, Some("workspace")),
|
|
KeyBinding::new("ctrl-a", NoAction {}, Some("pane && active")),
|
|
KeyBinding::new("ctrl-b", NoAction {}, Some("editor")),
|
|
];
|
|
|
|
let mut keymap = Keymap::default();
|
|
keymap.add_bindings(bindings.clone());
|
|
|
|
assert_bindings(&keymap, &ActionAlpha {}, &["ctrl-a"]);
|
|
assert_bindings(&keymap, &ActionBeta {}, &[]);
|
|
assert_bindings(&keymap, &ActionGamma {}, &["ctrl-c"]);
|
|
|
|
#[track_caller]
|
|
fn assert_bindings(keymap: &Keymap, action: &dyn Action, expected: &[&str]) {
|
|
let actual = keymap
|
|
.bindings_for_action(action)
|
|
.map(|binding| binding.keystrokes[0].unparse())
|
|
.collect::<Vec<_>>();
|
|
assert_eq!(actual, expected, "{:?}", action);
|
|
}
|
|
}
|
|
}
|