From eaa8224076e20b5d4df70b0cf4c5b8f049fac94c Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 17 Jul 2023 12:24:56 +0300 Subject: [PATCH 1/4] Use id instead of type_id for actions Currently, both are the same thing, so the logic is not changed. --- crates/gpui/src/app.rs | 12 +++++----- crates/gpui/src/app/window.rs | 20 +++++++--------- crates/gpui/src/keymap_matcher.rs | 4 ++-- crates/gpui/src/keymap_matcher/keymap.rs | 29 +++++++++++------------- crates/gpui/src/platform/mac/platform.rs | 2 +- 5 files changed, 30 insertions(+), 37 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 640614324f65dcdcae29c711a698713b48cfd9d0..b40a67db61cf2077d3b22ccc72d8afc9b96e9819 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -1073,7 +1073,7 @@ impl AppContext { pub fn is_action_available(&self, action: &dyn Action) -> bool { let mut available_in_window = false; - let action_type = action.as_any().type_id(); + let action_id = action.id(); if let Some(window_id) = self.platform.main_window_id() { available_in_window = self .read_window(window_id, |cx| { @@ -1083,7 +1083,7 @@ impl AppContext { cx.views_metadata.get(&(window_id, view_id)) { if let Some(actions) = cx.actions.get(&view_metadata.type_id) { - if actions.contains_key(&action_type) { + if actions.contains_key(&action_id) { return true; } } @@ -1094,7 +1094,7 @@ impl AppContext { }) .unwrap_or(false); } - available_in_window || self.global_actions.contains_key(&action_type) + available_in_window || self.global_actions.contains_key(&action_id) } fn actions_mut( @@ -3399,7 +3399,7 @@ impl<'a, 'b, 'c, V: View> LayoutContext<'a, 'b, 'c, V> { for (i, view_id) in self.ancestors(view_id).enumerate() { if let Some(view_metadata) = self.views_metadata.get(&(window_id, view_id)) { if let Some(actions) = self.actions.get(&view_metadata.type_id) { - if actions.contains_key(&action.as_any().type_id()) { + if actions.contains_key(&action.id()) { handler_depth = Some(i); } } @@ -3407,12 +3407,12 @@ impl<'a, 'b, 'c, V: View> LayoutContext<'a, 'b, 'c, V> { } } - if self.global_actions.contains_key(&action.as_any().type_id()) { + if self.global_actions.contains_key(&action.id()) { handler_depth = Some(contexts.len()) } self.keystroke_matcher - .bindings_for_action_type(action.as_any().type_id()) + .bindings_for_action(action.id()) .find_map(|b| { let highest_handler = handler_depth?; if action.eq(b.action()) diff --git a/crates/gpui/src/app/window.rs b/crates/gpui/src/app/window.rs index 49b12d823e69832eb42ae382c3d6b37c39fd1957..1a5611f6aed0fcc17d8509124b1583a9255dcbc0 100644 --- a/crates/gpui/src/app/window.rs +++ b/crates/gpui/src/app/window.rs @@ -363,17 +363,13 @@ impl<'a> WindowContext<'a> { ) -> Vec<(&'static str, Box, SmallVec<[Binding; 1]>)> { let window_id = self.window_id; let mut contexts = Vec::new(); - let mut handler_depths_by_action_type = HashMap::::default(); + let mut handler_depths_by_action_id = HashMap::::default(); for (depth, view_id) in self.ancestors(view_id).enumerate() { if let Some(view_metadata) = self.views_metadata.get(&(window_id, view_id)) { contexts.push(view_metadata.keymap_context.clone()); if let Some(actions) = self.actions.get(&view_metadata.type_id) { - handler_depths_by_action_type.extend( - actions - .keys() - .copied() - .map(|action_type| (action_type, depth)), - ); + handler_depths_by_action_id + .extend(actions.keys().copied().map(|action_id| (action_id, depth))); } } else { log::error!( @@ -383,21 +379,21 @@ impl<'a> WindowContext<'a> { } } - handler_depths_by_action_type.extend( + handler_depths_by_action_id.extend( self.global_actions .keys() .copied() - .map(|action_type| (action_type, contexts.len())), + .map(|action_id| (action_id, contexts.len())), ); self.action_deserializers .iter() - .filter_map(move |(name, (type_id, deserialize))| { - if let Some(action_depth) = handler_depths_by_action_type.get(type_id).copied() { + .filter_map(move |(name, (action_id, deserialize))| { + if let Some(action_depth) = handler_depths_by_action_id.get(action_id).copied() { let action = deserialize(serde_json::Value::Object(Default::default())).ok()?; let bindings = self .keystroke_matcher - .bindings_for_action_type(*type_id) + .bindings_for_action(*action_id) .filter(|b| { action.eq(b.action()) && (0..=action_depth) diff --git a/crates/gpui/src/keymap_matcher.rs b/crates/gpui/src/keymap_matcher.rs index bc70638b2c077bf5aee53f9fedd22e96938d6b4b..9e5d45987dd69cd2ef825d232c18c0c51095487a 100644 --- a/crates/gpui/src/keymap_matcher.rs +++ b/crates/gpui/src/keymap_matcher.rs @@ -47,8 +47,8 @@ impl KeymapMatcher { self.keymap.clear(); } - pub fn bindings_for_action_type(&self, action_type: TypeId) -> impl Iterator { - self.keymap.bindings_for_action_type(action_type) + pub fn bindings_for_action(&self, action_id: TypeId) -> impl Iterator { + self.keymap.bindings_for_action(action_id) } pub fn clear_pending(&mut self) { diff --git a/crates/gpui/src/keymap_matcher/keymap.rs b/crates/gpui/src/keymap_matcher/keymap.rs index 6f358aad3939d3f5dbb4be3dba52e3d5020874ce..bc02ae10facb5e2d67cf07598e627f19068cb5b1 100644 --- a/crates/gpui/src/keymap_matcher/keymap.rs +++ b/crates/gpui/src/keymap_matcher/keymap.rs @@ -1,39 +1,36 @@ use smallvec::SmallVec; -use std::{ - any::{Any, TypeId}, - collections::HashMap, -}; +use std::{any::TypeId, collections::HashMap}; use super::Binding; #[derive(Default)] pub struct Keymap { bindings: Vec, - binding_indices_by_action_type: HashMap>, + binding_indices_by_action_id: HashMap>, } impl Keymap { pub fn new(bindings: Vec) -> Self { - let mut binding_indices_by_action_type = HashMap::new(); + let mut binding_indices_by_action_id = HashMap::new(); for (ix, binding) in bindings.iter().enumerate() { - binding_indices_by_action_type - .entry(binding.action().type_id()) + binding_indices_by_action_id + .entry(binding.action().id()) .or_insert_with(SmallVec::new) .push(ix); } Self { - binding_indices_by_action_type, + binding_indices_by_action_id, bindings, } } - pub(crate) fn bindings_for_action_type( + pub(crate) fn bindings_for_action( &self, - action_type: TypeId, + action_id: TypeId, ) -> impl Iterator { - self.binding_indices_by_action_type - .get(&action_type) + self.binding_indices_by_action_id + .get(&action_id) .map(SmallVec::as_slice) .unwrap_or(&[]) .iter() @@ -42,8 +39,8 @@ impl Keymap { pub(crate) fn add_bindings>(&mut self, bindings: T) { for binding in bindings { - self.binding_indices_by_action_type - .entry(binding.action().as_any().type_id()) + self.binding_indices_by_action_id + .entry(binding.action().id()) .or_default() .push(self.bindings.len()); self.bindings.push(binding); @@ -52,7 +49,7 @@ impl Keymap { pub(crate) fn clear(&mut self) { self.bindings.clear(); - self.binding_indices_by_action_type.clear(); + self.binding_indices_by_action_id.clear(); } pub fn bindings(&self) -> &Vec { diff --git a/crates/gpui/src/platform/mac/platform.rs b/crates/gpui/src/platform/mac/platform.rs index e1d80fe25ceead41828aa18d90f5938eb518a17f..509c979b8536dfde31a9246b34edd7bd0ca88e0e 100644 --- a/crates/gpui/src/platform/mac/platform.rs +++ b/crates/gpui/src/platform/mac/platform.rs @@ -231,7 +231,7 @@ impl MacForegroundPlatform { } => { // TODO let keystrokes = keystroke_matcher - .bindings_for_action_type(action.as_any().type_id()) + .bindings_for_action(action.id()) .find(|binding| binding.action().eq(action.as_ref())) .map(|binding| binding.keystrokes()); let selector = match os_action { From f5eac82e813a7f5bbfda1b6f4d0d6eda4e97be16 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 17 Jul 2023 12:30:42 +0300 Subject: [PATCH 2/4] Reload menu after keybindings change --- crates/zed/src/zed.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 1621ae276a993bfb347825970d2afa652ccd659d..9dffc644ae7fd63a5a722db94fd4bdb25a979a99 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -517,11 +517,7 @@ pub fn handle_keymap_file_changes( let mut settings_subscription = None; while let Some(user_keymap_content) = user_keymap_file_rx.next().await { if let Ok(keymap_content) = KeymapFile::parse(&user_keymap_content) { - cx.update(|cx| { - cx.clear_bindings(); - load_default_keymap(cx); - keymap_content.clone().add_to_cx(cx).log_err(); - }); + cx.update(|cx| reload_keymaps(cx, &keymap_content)); let mut old_base_keymap = cx.read(|cx| *settings::get::(cx)); drop(settings_subscription); @@ -530,10 +526,7 @@ pub fn handle_keymap_file_changes( let new_base_keymap = *settings::get::(cx); if new_base_keymap != old_base_keymap { old_base_keymap = new_base_keymap.clone(); - - cx.clear_bindings(); - load_default_keymap(cx); - keymap_content.clone().add_to_cx(cx).log_err(); + reload_keymaps(cx, &keymap_content); } }) .detach(); @@ -544,6 +537,13 @@ pub fn handle_keymap_file_changes( .detach(); } +fn reload_keymaps(cx: &mut AppContext, keymap_content: &KeymapFile) { + cx.clear_bindings(); + load_default_keymap(cx); + keymap_content.clone().add_to_cx(cx).log_err(); + cx.set_menus(menus::menus()); +} + fn open_local_settings_file( workspace: &mut Workspace, _: &OpenLocalSettings, From 4cc06748c909dced565de925c1198bff95bff911 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 17 Jul 2023 14:22:50 +0300 Subject: [PATCH 3/4] Ignore keybindings with NoAction in config overrides --- crates/gpui/src/keymap_matcher/binding.rs | 4 +- crates/gpui/src/keymap_matcher/keymap.rs | 372 +++++++++++++++++- .../gpui/src/keymap_matcher/keymap_context.rs | 2 +- crates/gpui/src/keymap_matcher/keystroke.rs | 2 +- 4 files changed, 355 insertions(+), 25 deletions(-) diff --git a/crates/gpui/src/keymap_matcher/binding.rs b/crates/gpui/src/keymap_matcher/binding.rs index 4d8334128ba5b0f419e095b173e2568207d18560..527052c85d3668d0716da84c1445880d81faca47 100644 --- a/crates/gpui/src/keymap_matcher/binding.rs +++ b/crates/gpui/src/keymap_matcher/binding.rs @@ -7,8 +7,8 @@ use super::{KeymapContext, KeymapContextPredicate, Keystroke}; pub struct Binding { action: Box, - keystrokes: SmallVec<[Keystroke; 2]>, - context_predicate: Option, + pub(super) keystrokes: SmallVec<[Keystroke; 2]>, + pub(super) context_predicate: Option, } impl std::fmt::Debug for Binding { diff --git a/crates/gpui/src/keymap_matcher/keymap.rs b/crates/gpui/src/keymap_matcher/keymap.rs index bc02ae10facb5e2d67cf07598e627f19068cb5b1..7cb95cab3a28aac3a41f7515e1551d22b1a776f5 100644 --- a/crates/gpui/src/keymap_matcher/keymap.rs +++ b/crates/gpui/src/keymap_matcher/keymap.rs @@ -1,28 +1,24 @@ +use collections::HashSet; use smallvec::SmallVec; use std::{any::TypeId, collections::HashMap}; -use super::Binding; +use crate::{Action, NoAction}; + +use super::{Binding, KeymapContextPredicate, Keystroke}; #[derive(Default)] pub struct Keymap { bindings: Vec, binding_indices_by_action_id: HashMap>, + disabled_keystrokes: HashMap, HashSet>>, } impl Keymap { - pub fn new(bindings: Vec) -> Self { - let mut binding_indices_by_action_id = HashMap::new(); - for (ix, binding) in bindings.iter().enumerate() { - binding_indices_by_action_id - .entry(binding.action().id()) - .or_insert_with(SmallVec::new) - .push(ix); - } - - Self { - binding_indices_by_action_id, - bindings, - } + #[cfg(test)] + pub(super) fn new(bindings: Vec) -> Self { + let mut this = Self::default(); + this.add_bindings(bindings); + this } pub(crate) fn bindings_for_action( @@ -35,24 +31,358 @@ impl Keymap { .unwrap_or(&[]) .iter() .map(|ix| &self.bindings[*ix]) + .filter(|binding| !self.binding_disabled(binding)) } pub(crate) fn add_bindings>(&mut self, bindings: T) { + let no_action_id = (NoAction {}).id(); + let mut new_bindings = Vec::new(); + let mut has_new_disabled_keystrokes = false; for binding in bindings { - self.binding_indices_by_action_id - .entry(binding.action().id()) - .or_default() - .push(self.bindings.len()); - self.bindings.push(binding); + if binding.action().id() == no_action_id { + has_new_disabled_keystrokes |= self + .disabled_keystrokes + .entry(binding.keystrokes) + .or_default() + .insert(binding.context_predicate); + } else { + new_bindings.push(binding); + } + } + + if has_new_disabled_keystrokes { + self.binding_indices_by_action_id.retain(|_, indices| { + indices.retain(|ix| { + let binding = &self.bindings[*ix]; + match self.disabled_keystrokes.get(&binding.keystrokes) { + Some(disabled_predicates) => { + !disabled_predicates.contains(&binding.context_predicate) + } + None => true, + } + }); + !indices.is_empty() + }); + } + + for new_binding in new_bindings { + if !self.binding_disabled(&new_binding) { + self.binding_indices_by_action_id + .entry(new_binding.action().id()) + .or_default() + .push(self.bindings.len()); + self.bindings.push(new_binding); + } } } pub(crate) fn clear(&mut self) { self.bindings.clear(); self.binding_indices_by_action_id.clear(); + self.disabled_keystrokes.clear(); + } + + pub fn bindings(&self) -> Vec<&Binding> { + self.bindings + .iter() + .filter(|binding| !self.binding_disabled(binding)) + .collect() + } + + fn binding_disabled(&self, binding: &Binding) -> bool { + match self.disabled_keystrokes.get(&binding.keystrokes) { + Some(disabled_predicates) => disabled_predicates.contains(&binding.context_predicate), + None => false, + } + } +} + +#[cfg(test)] +mod tests { + use crate::actions; + + use super::*; + + actions!( + keymap_test, + [Present1, Present2, Present3, Duplicate, Missing] + ); + + #[test] + fn regular_keymap() { + let present_1 = Binding::new("ctrl-q", Present1 {}, None); + let present_2 = Binding::new("ctrl-w", Present2 {}, Some("pane")); + let present_3 = Binding::new("ctrl-e", Present3 {}, Some("editor")); + let keystroke_duplicate_to_1 = Binding::new("ctrl-q", Duplicate {}, None); + let full_duplicate_to_2 = Binding::new("ctrl-w", Present2 {}, Some("pane")); + let missing = Binding::new("ctrl-r", Missing {}, None); + let all_bindings = [ + &present_1, + &present_2, + &present_3, + &keystroke_duplicate_to_1, + &full_duplicate_to_2, + &missing, + ]; + + let mut keymap = Keymap::default(); + assert_absent(&keymap, &all_bindings); + assert!(keymap.bindings().is_empty()); + + keymap.add_bindings([present_1.clone(), present_2.clone(), present_3.clone()]); + assert_absent(&keymap, &[&keystroke_duplicate_to_1, &missing]); + assert_present( + &keymap, + &[(&present_1, "q"), (&present_2, "w"), (&present_3, "e")], + ); + + keymap.add_bindings([ + keystroke_duplicate_to_1.clone(), + full_duplicate_to_2.clone(), + ]); + assert_absent(&keymap, &[&missing]); + assert!( + !keymap.binding_disabled(&keystroke_duplicate_to_1), + "Duplicate binding 1 was added and should not be disabled" + ); + assert!( + !keymap.binding_disabled(&full_duplicate_to_2), + "Duplicate binding 2 was added and should not be disabled" + ); + + assert_eq!( + keymap + .bindings_for_action(keystroke_duplicate_to_1.action().id()) + .map(|binding| &binding.keystrokes) + .flatten() + .collect::>(), + vec![&Keystroke { + ctrl: true, + alt: false, + shift: false, + cmd: false, + function: false, + key: "q".to_string() + }], + "{keystroke_duplicate_to_1:?} should have the expected keystroke in the keymap" + ); + assert_eq!( + keymap + .bindings_for_action(full_duplicate_to_2.action().id()) + .map(|binding| &binding.keystrokes) + .flatten() + .collect::>(), + vec![ + &Keystroke { + ctrl: true, + alt: false, + shift: false, + cmd: false, + function: false, + key: "w".to_string() + }, + &Keystroke { + ctrl: true, + alt: false, + shift: false, + cmd: false, + function: false, + key: "w".to_string() + } + ], + "{full_duplicate_to_2:?} should have a duplicated keystroke in the keymap" + ); + + let updated_bindings = keymap.bindings(); + let expected_updated_bindings = vec![ + &present_1, + &present_2, + &present_3, + &keystroke_duplicate_to_1, + &full_duplicate_to_2, + ]; + assert_eq!( + updated_bindings.len(), + expected_updated_bindings.len(), + "Unexpected updated keymap bindings {updated_bindings:?}" + ); + for (i, expected) in expected_updated_bindings.iter().enumerate() { + let keymap_binding = &updated_bindings[i]; + assert_eq!( + keymap_binding.context_predicate, expected.context_predicate, + "Unexpected context predicate for keymap {i} element: {keymap_binding:?}" + ); + assert_eq!( + keymap_binding.keystrokes, expected.keystrokes, + "Unexpected keystrokes for keymap {i} element: {keymap_binding:?}" + ); + } + + keymap.clear(); + assert_absent(&keymap, &all_bindings); + assert!(keymap.bindings().is_empty()); } - pub fn bindings(&self) -> &Vec { - &self.bindings + #[test] + fn keymap_with_ignored() { + let present_1 = Binding::new("ctrl-q", Present1 {}, None); + let present_2 = Binding::new("ctrl-w", Present2 {}, Some("pane")); + let present_3 = Binding::new("ctrl-e", Present3 {}, Some("editor")); + let keystroke_duplicate_to_1 = Binding::new("ctrl-q", Duplicate {}, None); + let full_duplicate_to_2 = Binding::new("ctrl-w", Present2 {}, Some("pane")); + let ignored_1 = Binding::new("ctrl-q", NoAction {}, None); + let ignored_2 = Binding::new("ctrl-w", NoAction {}, Some("pane")); + let ignored_3_with_other_context = + Binding::new("ctrl-e", NoAction {}, Some("other_context")); + + let mut keymap = Keymap::default(); + + keymap.add_bindings([ + ignored_1.clone(), + ignored_2.clone(), + ignored_3_with_other_context.clone(), + ]); + assert_absent(&keymap, &[&present_3]); + assert_disabled( + &keymap, + &[ + &present_1, + &present_2, + &ignored_1, + &ignored_2, + &ignored_3_with_other_context, + ], + ); + assert!(keymap.bindings().is_empty()); + keymap.clear(); + + keymap.add_bindings([ + present_1.clone(), + present_2.clone(), + present_3.clone(), + ignored_1.clone(), + ignored_2.clone(), + ignored_3_with_other_context.clone(), + ]); + assert_present(&keymap, &[(&present_3, "e")]); + assert_disabled( + &keymap, + &[ + &present_1, + &present_2, + &ignored_1, + &ignored_2, + &ignored_3_with_other_context, + ], + ); + keymap.clear(); + + keymap.add_bindings([ + present_1.clone(), + present_2.clone(), + present_3.clone(), + ignored_1.clone(), + ]); + assert_present(&keymap, &[(&present_2, "w"), (&present_3, "e")]); + assert_disabled(&keymap, &[&present_1, &ignored_1]); + assert_absent(&keymap, &[&ignored_2, &ignored_3_with_other_context]); + keymap.clear(); + + keymap.add_bindings([ + present_1.clone(), + present_2.clone(), + present_3.clone(), + keystroke_duplicate_to_1.clone(), + full_duplicate_to_2.clone(), + ignored_1.clone(), + ignored_2.clone(), + ignored_3_with_other_context.clone(), + ]); + assert_present(&keymap, &[(&present_3, "e")]); + assert_disabled( + &keymap, + &[ + &present_1, + &present_2, + &keystroke_duplicate_to_1, + &full_duplicate_to_2, + &ignored_1, + &ignored_2, + &ignored_3_with_other_context, + ], + ); + keymap.clear(); + } + + #[track_caller] + fn assert_present(keymap: &Keymap, expected_bindings: &[(&Binding, &str)]) { + let keymap_bindings = keymap.bindings(); + assert_eq!( + expected_bindings.len(), + keymap_bindings.len(), + "Unexpected keymap bindings {keymap_bindings:?}" + ); + for (i, (expected, expected_key)) in expected_bindings.iter().enumerate() { + assert!( + !keymap.binding_disabled(expected), + "{expected:?} should not be disabled as it was added into keymap for element {i}" + ); + assert_eq!( + keymap + .bindings_for_action(expected.action().id()) + .map(|binding| &binding.keystrokes) + .flatten() + .collect::>(), + vec![&Keystroke { + ctrl: true, + alt: false, + shift: false, + cmd: false, + function: false, + key: expected_key.to_string() + }], + "{expected:?} should have the expected keystroke with key '{expected_key}' in the keymap for element {i}" + ); + + let keymap_binding = &keymap_bindings[i]; + assert_eq!( + keymap_binding.context_predicate, expected.context_predicate, + "Unexpected context predicate for keymap {i} element: {keymap_binding:?}" + ); + assert_eq!( + keymap_binding.keystrokes, expected.keystrokes, + "Unexpected keystrokes for keymap {i} element: {keymap_binding:?}" + ); + } + } + + #[track_caller] + fn assert_absent(keymap: &Keymap, bindings: &[&Binding]) { + for binding in bindings.iter() { + assert!( + !keymap.binding_disabled(binding), + "{binding:?} should not be disabled in the keymap where was not added" + ); + assert_eq!( + keymap.bindings_for_action(binding.action().id()).count(), + 0, + "{binding:?} should have no actions in the keymap where was not added" + ); + } + } + + #[track_caller] + fn assert_disabled(keymap: &Keymap, bindings: &[&Binding]) { + for binding in bindings.iter() { + assert!( + keymap.binding_disabled(binding), + "{binding:?} should be disabled in the keymap" + ); + assert_eq!( + keymap.bindings_for_action(binding.action().id()).count(), + 0, + "{binding:?} should have no actions in the keymap where it was disabled" + ); + } } } diff --git a/crates/gpui/src/keymap_matcher/keymap_context.rs b/crates/gpui/src/keymap_matcher/keymap_context.rs index be61fea531656fb5a3163ba2804dfce1b4fb0e63..fd60a8f4b5d385eb94b7edf0bfeb407a9dce8c20 100644 --- a/crates/gpui/src/keymap_matcher/keymap_context.rs +++ b/crates/gpui/src/keymap_matcher/keymap_context.rs @@ -44,7 +44,7 @@ impl KeymapContext { } } -#[derive(Clone, Debug, Eq, PartialEq)] +#[derive(Clone, Debug, Eq, PartialEq, Hash)] pub enum KeymapContextPredicate { Identifier(String), Equal(String, String), diff --git a/crates/gpui/src/keymap_matcher/keystroke.rs b/crates/gpui/src/keymap_matcher/keystroke.rs index ed3c3f69140f2da02268c5d44ac9ecd77d813761..164dea8aba145aec263dd1c95afbbb79824bb83c 100644 --- a/crates/gpui/src/keymap_matcher/keystroke.rs +++ b/crates/gpui/src/keymap_matcher/keystroke.rs @@ -3,7 +3,7 @@ use std::fmt::Write; use anyhow::anyhow; use serde::Deserialize; -#[derive(Clone, Debug, Eq, PartialEq, Default, Deserialize)] +#[derive(Clone, Debug, Eq, PartialEq, Default, Deserialize, Hash)] pub struct Keystroke { pub ctrl: bool, pub alt: bool, From a4bf19c5bd1fba45785c31d93068cd4f5ecb0da1 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 17 Jul 2023 20:37:58 +0300 Subject: [PATCH 4/4] Simplify NoAction filtering logic co-authored-by: Max Brunsfeld --- crates/gpui/src/app/window.rs | 8 ++------ crates/gpui/src/keymap_matcher.rs | 7 +++++-- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/crates/gpui/src/app/window.rs b/crates/gpui/src/app/window.rs index 1a5611f6aed0fcc17d8509124b1583a9255dcbc0..1dc88d2e717a985be4aed9984597f4abf7b92a1a 100644 --- a/crates/gpui/src/app/window.rs +++ b/crates/gpui/src/app/window.rs @@ -14,8 +14,8 @@ use crate::{ text_layout::TextLayoutCache, util::post_inc, Action, AnyView, AnyViewHandle, AppContext, BorrowAppContext, BorrowWindowContext, Effect, - Element, Entity, Handle, LayoutContext, MouseRegion, MouseRegionId, NoAction, SceneBuilder, - Subscription, View, ViewContext, ViewHandle, WindowInvalidation, + Element, Entity, Handle, LayoutContext, MouseRegion, MouseRegionId, SceneBuilder, Subscription, + View, ViewContext, ViewHandle, WindowInvalidation, }; use anyhow::{anyhow, bail, Result}; use collections::{HashMap, HashSet}; @@ -430,11 +430,7 @@ impl<'a> WindowContext<'a> { MatchResult::None => false, MatchResult::Pending => true, MatchResult::Matches(matches) => { - let no_action_id = (NoAction {}).id(); for (view_id, action) in matches { - if action.id() == no_action_id { - return false; - } if self.dispatch_action(Some(*view_id), action.as_ref()) { self.keystroke_matcher.clear_pending(); handled_by = Some(action.boxed_clone()); diff --git a/crates/gpui/src/keymap_matcher.rs b/crates/gpui/src/keymap_matcher.rs index 9e5d45987dd69cd2ef825d232c18c0c51095487a..8cb7d30dfe4a78012023ee724becc09ced6d3212 100644 --- a/crates/gpui/src/keymap_matcher.rs +++ b/crates/gpui/src/keymap_matcher.rs @@ -8,7 +8,7 @@ use std::{any::TypeId, fmt::Debug}; use collections::HashMap; use smallvec::SmallVec; -use crate::Action; +use crate::{Action, NoAction}; pub use binding::{Binding, BindingMatchResult}; pub use keymap::Keymap; @@ -81,6 +81,7 @@ impl KeymapMatcher { // The key is the reverse position of the binding in the bindings list so that later bindings // match before earlier ones in the user's config let mut matched_bindings: Vec<(usize, Box)> = Default::default(); + let no_action_id = (NoAction {}).id(); let first_keystroke = self.pending_keystrokes.is_empty(); self.pending_keystrokes.push(keystroke.clone()); @@ -108,7 +109,9 @@ impl KeymapMatcher { match binding.match_keys_and_context(&self.pending_keystrokes, &self.contexts[i..]) { BindingMatchResult::Complete(action) => { - matched_bindings.push((*view_id, action)); + if action.id() != no_action_id { + matched_bindings.push((*view_id, action)); + } } BindingMatchResult::Partial => { self.pending_views