diff --git a/crates/command_palette/src/command_palette.rs b/crates/command_palette/src/command_palette.rs index 123c33cba1..4e0e776000 100644 --- a/crates/command_palette/src/command_palette.rs +++ b/crates/command_palette/src/command_palette.rs @@ -2,7 +2,7 @@ use collections::CommandPaletteFilter; use fuzzy::{StringMatch, StringMatchCandidate}; use gpui::{ actions, elements::*, keymap_matcher::Keystroke, Action, AppContext, Element, MouseState, - ViewContext, WindowContext, + ViewContext, }; use picker::{Picker, PickerDelegate, PickerEvent}; use settings::Settings; @@ -41,47 +41,17 @@ struct Command { keystrokes: Vec, } -fn toggle_command_palette(_: &mut Workspace, _: &Toggle, cx: &mut ViewContext) { - let workspace = cx.handle(); - let focused_view_id = cx.focused_view_id().unwrap_or_else(|| workspace.id()); - - cx.window_context().defer(move |cx| { - // Build the delegate before the workspace is put on the stack so we can find it when - // computing the actions. We should really not allow available_actions to be called - // if it's not reliable however. - let delegate = CommandPaletteDelegate::new(focused_view_id, cx); - workspace.update(cx, |workspace, cx| { - workspace.toggle_modal(cx, |_, cx| cx.add_view(|cx| Picker::new(delegate, cx))); - }) +fn toggle_command_palette(workspace: &mut Workspace, _: &Toggle, cx: &mut ViewContext) { + let focused_view_id = cx.focused_view_id().unwrap_or_else(|| cx.view_id()); + workspace.toggle_modal(cx, |_, cx| { + cx.add_view(|cx| Picker::new(CommandPaletteDelegate::new(focused_view_id), cx)) }); } impl CommandPaletteDelegate { - pub fn new(focused_view_id: usize, cx: &mut WindowContext) -> Self { - let actions = cx - .available_actions(focused_view_id) - .filter_map(|(name, action, bindings)| { - if cx.has_global::() { - let filter = cx.global::(); - if filter.filtered_namespaces.contains(action.namespace()) { - return None; - } - } - - Some(Command { - name: humanize_action_name(name), - action, - keystrokes: bindings - .iter() - .map(|binding| binding.keystrokes()) - .last() - .map_or(Vec::new(), |keystrokes| keystrokes.to_vec()), - }) - }) - .collect(); - + pub fn new(focused_view_id: usize) -> Self { Self { - actions, + actions: Default::default(), matches: vec![], selected_ix: 0, focused_view_id, @@ -111,17 +81,46 @@ impl PickerDelegate for CommandPaletteDelegate { query: String, cx: &mut ViewContext>, ) -> gpui::Task<()> { - let candidates = self - .actions - .iter() - .enumerate() - .map(|(ix, command)| StringMatchCandidate { - id: ix, - string: command.name.to_string(), - char_bag: command.name.chars().collect(), - }) - .collect::>(); + let window_id = cx.window_id(); + let view_id = self.focused_view_id; cx.spawn(move |picker, mut cx| async move { + let actions = cx + .available_actions(window_id, view_id) + .into_iter() + .filter_map(|(name, action, bindings)| { + let filtered = cx.read(|cx| { + if cx.has_global::() { + let filter = cx.global::(); + filter.filtered_namespaces.contains(action.namespace()) + } else { + false + } + }); + + if filtered { + None + } else { + Some(Command { + name: humanize_action_name(name), + action, + keystrokes: bindings + .iter() + .map(|binding| binding.keystrokes()) + .last() + .map_or(Vec::new(), |keystrokes| keystrokes.to_vec()), + }) + } + }) + .collect::>(); + let candidates = actions + .iter() + .enumerate() + .map(|(ix, command)| StringMatchCandidate { + id: ix, + string: command.name.to_string(), + char_bag: command.name.chars().collect(), + }) + .collect::>(); let matches = if query.is_empty() { candidates .into_iter() @@ -147,6 +146,7 @@ impl PickerDelegate for CommandPaletteDelegate { picker .update(&mut cx, |picker, _| { let delegate = picker.delegate_mut(); + delegate.actions = actions; delegate.matches = matches; if delegate.matches.is_empty() { delegate.selected_ix = 0; diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 0b618d8e6d..bf3fc0e5b7 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -341,6 +341,15 @@ impl AsyncAppContext { .ok_or_else(|| anyhow!("window not found")) } + pub fn available_actions( + &self, + window_id: usize, + view_id: usize, + ) -> Vec<(&'static str, Box, SmallVec<[Binding; 1]>)> { + self.read_window(window_id, |cx| cx.available_actions(view_id)) + .unwrap_or_default() + } + pub fn has_window(&self, window_id: usize) -> bool { self.read(|cx| cx.windows.contains_key(&window_id)) } @@ -3221,6 +3230,64 @@ impl<'a, 'b, 'c, V: View> LayoutContext<'a, 'b, 'c, V> { pub fn view_context(&mut self) -> &mut ViewContext<'a, 'b, V> { self.view_context } + + /// Return keystrokes that would dispatch the given action on the given view. + pub(crate) fn keystrokes_for_action( + &mut self, + view: &V, + view_id: usize, + action: &dyn Action, + ) -> Option> { + self.notify_if_view_ancestors_change(view_id); + + let window_id = self.window_id; + let mut contexts = Vec::new(); + let mut handler_depth = None; + for (i, view_id) in self.ancestors(view_id).enumerate() { + let view = if view_id == self.view_id { + Some(view as _) + } else { + self.views + .get(&(window_id, view_id)) + .map(|view| view.as_ref()) + }; + + if let Some(view) = view { + if let Some(actions) = self.actions.get(&view.as_any().type_id()) { + if actions.contains_key(&action.as_any().type_id()) { + handler_depth = Some(i); + } + } + contexts.push(view.keymap_context(self)); + } + } + + if self.global_actions.contains_key(&action.as_any().type_id()) { + handler_depth = Some(contexts.len()) + } + + self.keystroke_matcher + .bindings_for_action_type(action.as_any().type_id()) + .find_map(|b| { + handler_depth + .map(|highest_handler| { + if (0..=highest_handler).any(|depth| b.match_context(&contexts[depth..])) { + Some(b.keystrokes().into()) + } else { + None + } + }) + .flatten() + }) + } + + fn notify_if_view_ancestors_change(&mut self, view_id: usize) { + let self_view_id = self.view_id; + self.views_to_notify_if_ancestors_change + .entry(view_id) + .or_default() + .push(self_view_id); + } } impl<'a, 'b, 'c, V: View> Deref for LayoutContext<'a, 'b, 'c, V> { @@ -5740,7 +5807,7 @@ mod tests { } #[crate::test(self)] - fn test_keystrokes_for_action(cx: &mut AppContext) { + fn test_keystrokes_for_action(cx: &mut TestAppContext) { actions!(test, [Action1, Action2, GlobalAction]); struct View1 { @@ -5772,35 +5839,47 @@ mod tests { } } - let (window_id, view_1) = cx.add_window(Default::default(), |cx| { + let (window_id, view_1) = cx.add_window(|cx| { let view_2 = cx.add_view(|cx| { cx.focus_self(); View2 {} }); View1 { child: view_2 } }); - let view_2 = view_1.read(cx).child.clone(); + let view_2 = view_1.read_with(cx, |view, _| view.child.clone()); - cx.add_action(|_: &mut View1, _: &Action1, _cx| {}); - cx.add_action(|_: &mut View2, _: &Action2, _cx| {}); - cx.add_global_action(|_: &GlobalAction, _| {}); + cx.update(|cx| { + cx.add_action(|_: &mut View1, _: &Action1, _cx| {}); + cx.add_action(|_: &mut View2, _: &Action2, _cx| {}); + cx.add_global_action(|_: &GlobalAction, _| {}); + cx.add_bindings(vec![ + Binding::new("a", Action1, Some("View1")), + Binding::new("b", Action2, Some("View1 > View2")), + Binding::new("c", GlobalAction, Some("View3")), // View 3 does not exist + ]); + }); - cx.add_bindings(vec![ - Binding::new("a", Action1, Some("View1")), - Binding::new("b", Action2, Some("View1 > View2")), - Binding::new("c", GlobalAction, Some("View3")), // View 3 does not exist - ]); - - cx.update_window(window_id, |cx| { + let view_1_id = view_1.id(); + view_1.update(cx, |view_1, cx| { // Sanity check + let mut new_parents = Default::default(); + let mut notify_views_if_parents_change = Default::default(); + let mut layout_cx = LayoutContext::new( + cx, + &mut new_parents, + &mut notify_views_if_parents_change, + false, + ); assert_eq!( - cx.keystrokes_for_action(view_1.id(), &Action1) + layout_cx + .keystrokes_for_action(view_1, view_1_id, &Action1) .unwrap() .as_slice(), &[Keystroke::parse("a").unwrap()] ); assert_eq!( - cx.keystrokes_for_action(view_2.id(), &Action2) + layout_cx + .keystrokes_for_action(view_1, view_2.id(), &Action2) .unwrap() .as_slice(), &[Keystroke::parse("b").unwrap()] @@ -5809,44 +5888,53 @@ mod tests { // The 'a' keystroke propagates up the view tree from view_2 // to view_1. The action, Action1, is handled by view_1. assert_eq!( - cx.keystrokes_for_action(view_2.id(), &Action1) + layout_cx + .keystrokes_for_action(view_1, view_2.id(), &Action1) .unwrap() .as_slice(), &[Keystroke::parse("a").unwrap()] ); // Actions that are handled below the current view don't have bindings - assert_eq!(cx.keystrokes_for_action(view_1.id(), &Action2), None); - - // Actions that are handled in other branches of the tree should not have a binding - assert_eq!(cx.keystrokes_for_action(view_2.id(), &GlobalAction), None); - - // Check that global actions do not have a binding, even if a binding does exist in another view assert_eq!( - &available_actions(view_1.id(), cx), - &[ - ("test::Action1", vec![Keystroke::parse("a").unwrap()]), - ("test::GlobalAction", vec![]) - ], + layout_cx.keystrokes_for_action(view_1, view_1_id, &Action2), + None ); - // Check that view 1 actions and bindings are available even when called from view 2 + // Actions that are handled in other branches of the tree should not have a binding assert_eq!( - &available_actions(view_2.id(), cx), - &[ - ("test::Action1", vec![Keystroke::parse("a").unwrap()]), - ("test::Action2", vec![Keystroke::parse("b").unwrap()]), - ("test::GlobalAction", vec![]), - ], + layout_cx.keystrokes_for_action(view_1, view_2.id(), &GlobalAction), + None ); }); + // Check that global actions do not have a binding, even if a binding does exist in another view + assert_eq!( + &available_actions(window_id, view_1.id(), cx), + &[ + ("test::Action1", vec![Keystroke::parse("a").unwrap()]), + ("test::GlobalAction", vec![]) + ], + ); + + // Check that view 1 actions and bindings are available even when called from view 2 + assert_eq!( + &available_actions(window_id, view_2.id(), cx), + &[ + ("test::Action1", vec![Keystroke::parse("a").unwrap()]), + ("test::Action2", vec![Keystroke::parse("b").unwrap()]), + ("test::GlobalAction", vec![]), + ], + ); + // Produces a list of actions and key bindings fn available_actions( + window_id: usize, view_id: usize, - cx: &WindowContext, + cx: &TestAppContext, ) -> Vec<(&'static str, Vec)> { - cx.available_actions(view_id) + cx.available_actions(window_id, view_id) + .into_iter() .map(|(action_name, _, bindings)| { ( action_name, diff --git a/crates/gpui/src/app/test_app_context.rs b/crates/gpui/src/app/test_app_context.rs index af01eec1be..4af436a7b8 100644 --- a/crates/gpui/src/app/test_app_context.rs +++ b/crates/gpui/src/app/test_app_context.rs @@ -1,7 +1,7 @@ use crate::{ executor, geometry::vector::Vector2F, - keymap_matcher::Keystroke, + keymap_matcher::{Binding, Keystroke}, platform, platform::{Event, InputHandler, KeyDownEvent, Platform}, Action, AppContext, BorrowAppContext, BorrowWindowContext, Entity, FontCache, Handle, @@ -12,6 +12,7 @@ use collections::BTreeMap; use futures::Future; use itertools::Itertools; use parking_lot::{Mutex, RwLock}; +use smallvec::SmallVec; use smol::stream::StreamExt; use std::{ any::Any, @@ -71,17 +72,24 @@ impl TestAppContext { cx } - pub fn dispatch_action(&self, window_id: usize, action: A) { - self.cx - .borrow_mut() - .update_window(window_id, |window| { - window.dispatch_action(window.focused_view_id(), &action); - }) - .expect("window not found"); + pub fn dispatch_action(&mut self, window_id: usize, action: A) { + self.update_window(window_id, |window| { + window.dispatch_action(window.focused_view_id(), &action); + }) + .expect("window not found"); } - pub fn dispatch_global_action(&self, action: A) { - self.cx.borrow_mut().dispatch_global_action_any(&action); + pub fn available_actions( + &self, + window_id: usize, + view_id: usize, + ) -> Vec<(&'static str, Box, SmallVec<[Binding; 1]>)> { + self.read_window(window_id, |cx| cx.available_actions(view_id)) + .unwrap_or_default() + } + + pub fn dispatch_global_action(&mut self, action: A) { + self.update(|cx| cx.dispatch_global_action_any(&action)); } pub fn dispatch_keystroke(&mut self, window_id: usize, keystroke: Keystroke, is_held: bool) { diff --git a/crates/gpui/src/app/window.rs b/crates/gpui/src/app/window.rs index a96d937b1b..4ec9a87bba 100644 --- a/crates/gpui/src/app/window.rs +++ b/crates/gpui/src/app/window.rs @@ -356,49 +356,10 @@ impl<'a> WindowContext<'a> { ) } - /// Return keystrokes that would dispatch the given action on the given view. - pub(crate) fn keystrokes_for_action( - &mut self, - view_id: usize, - action: &dyn Action, - ) -> Option> { - let window_id = self.window_id; - let mut contexts = Vec::new(); - let mut handler_depth = None; - for (i, view_id) in self.ancestors(view_id).enumerate() { - if let Some(view) = self.views.get(&(window_id, view_id)) { - if let Some(actions) = self.actions.get(&view.as_any().type_id()) { - if actions.contains_key(&action.as_any().type_id()) { - handler_depth = Some(i); - } - } - contexts.push(view.keymap_context(self)); - } - } - - if self.global_actions.contains_key(&action.as_any().type_id()) { - handler_depth = Some(contexts.len()) - } - - self.keystroke_matcher - .bindings_for_action_type(action.as_any().type_id()) - .find_map(|b| { - handler_depth - .map(|highest_handler| { - if (0..=highest_handler).any(|depth| b.match_context(&contexts[depth..])) { - Some(b.keystrokes().into()) - } else { - None - } - }) - .flatten() - }) - } - - pub fn available_actions( + pub(crate) fn available_actions( &self, view_id: usize, - ) -> impl Iterator, SmallVec<[&Binding; 1]>)> { + ) -> 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(); @@ -441,12 +402,14 @@ impl<'a> WindowContext<'a> { .filter(|b| { (0..=action_depth).any(|depth| b.match_context(&contexts[depth..])) }) + .cloned() .collect(), )) } else { None } }) + .collect() } pub(crate) fn dispatch_keystroke(&mut self, keystroke: &Keystroke) -> bool { diff --git a/crates/gpui/src/elements/keystroke_label.rs b/crates/gpui/src/elements/keystroke_label.rs index c011649b2e..05b6864d56 100644 --- a/crates/gpui/src/elements/keystroke_label.rs +++ b/crates/gpui/src/elements/keystroke_label.rs @@ -42,7 +42,7 @@ impl Element for KeystrokeLabel { cx: &mut LayoutContext, ) -> (Vector2F, AnyElement) { let mut element = if let Some(keystrokes) = - cx.keystrokes_for_action(self.view_id, self.action.as_ref()) + cx.keystrokes_for_action(view, self.view_id, self.action.as_ref()) { Flex::row() .with_children(keystrokes.iter().map(|keystroke| { diff --git a/crates/gpui/src/keymap_matcher/binding.rs b/crates/gpui/src/keymap_matcher/binding.rs index c1cfd14e82..aa40e8c6af 100644 --- a/crates/gpui/src/keymap_matcher/binding.rs +++ b/crates/gpui/src/keymap_matcher/binding.rs @@ -11,6 +11,16 @@ pub struct Binding { context_predicate: Option, } +impl Clone for Binding { + fn clone(&self) -> Self { + Self { + action: self.action.boxed_clone(), + keystrokes: self.keystrokes.clone(), + context_predicate: self.context_predicate.clone(), + } + } +} + impl Binding { pub fn new(keystrokes: &str, action: A, context: Option<&str>) -> Self { Self::load(keystrokes, Box::new(action), context).unwrap() diff --git a/crates/gpui/src/keymap_matcher/keymap_context.rs b/crates/gpui/src/keymap_matcher/keymap_context.rs index bbf6bfc14b..2eca4fbee9 100644 --- a/crates/gpui/src/keymap_matcher/keymap_context.rs +++ b/crates/gpui/src/keymap_matcher/keymap_context.rs @@ -39,7 +39,7 @@ impl KeymapContext { } } -#[derive(Debug, Eq, PartialEq)] +#[derive(Clone, Debug, Eq, PartialEq)] pub enum KeymapContextPredicate { Identifier(String), Equal(String, String), diff --git a/crates/settings/src/settings_file.rs b/crates/settings/src/settings_file.rs index c261879a58..ae0a34e008 100644 --- a/crates/settings/src/settings_file.rs +++ b/crates/settings/src/settings_file.rs @@ -80,7 +80,7 @@ mod tests { watch_files, watched_json::watch_settings_file, EditorSettings, Settings, SoftWrap, }; use fs::FakeFs; - use gpui::{actions, elements::*, Action, Entity, View, ViewContext, WindowContext}; + use gpui::{actions, elements::*, Action, Entity, TestAppContext, View, ViewContext}; use theme::ThemeRegistry; struct TestView; @@ -167,13 +167,12 @@ mod tests { let (window_id, _view) = cx.add_window(|_| TestView); // Test loading the keymap base at all - cx.read_window(window_id, |cx| { - assert_key_bindings_for( - cx, - vec![("backspace", &A), ("k", &ActivatePreviousPane)], - line!(), - ); - }); + assert_key_bindings_for( + window_id, + cx, + vec![("backspace", &A), ("k", &ActivatePreviousPane)], + line!(), + ); // Test modifying the users keymap, while retaining the base keymap fs.save( @@ -195,13 +194,12 @@ mod tests { cx.foreground().run_until_parked(); - cx.read_window(window_id, |cx| { - assert_key_bindings_for( - cx, - vec![("backspace", &B), ("k", &ActivatePreviousPane)], - line!(), - ); - }); + assert_key_bindings_for( + window_id, + cx, + vec![("backspace", &B), ("k", &ActivatePreviousPane)], + line!(), + ); // Test modifying the base, while retaining the users keymap fs.save( @@ -219,31 +217,33 @@ mod tests { cx.foreground().run_until_parked(); - cx.read_window(window_id, |cx| { - assert_key_bindings_for( - cx, - vec![("backspace", &B), ("[", &ActivatePrevItem)], - line!(), - ); - }); + assert_key_bindings_for( + window_id, + cx, + vec![("backspace", &B), ("[", &ActivatePrevItem)], + line!(), + ); } fn assert_key_bindings_for<'a>( - cx: &WindowContext, + window_id: usize, + cx: &TestAppContext, actions: Vec<(&'static str, &'a dyn Action)>, line: u32, ) { for (key, action) in actions { // assert that... assert!( - cx.available_actions(0).any(|(_, bound_action, b)| { - // action names match... - bound_action.name() == action.name() + cx.available_actions(window_id, 0) + .into_iter() + .any(|(_, bound_action, b)| { + // action names match... + bound_action.name() == action.name() && bound_action.namespace() == action.namespace() // and key strokes contain the given key && b.iter() .any(|binding| binding.keystrokes().iter().any(|k| k.key == key)) - }), + }), "On {} Failed to find {} with key binding {}", line, action.name(), diff --git a/crates/workspace/src/dock.rs b/crates/workspace/src/dock.rs index 2ec3dbaea4..7efcb7f9d3 100644 --- a/crates/workspace/src/dock.rs +++ b/crates/workspace/src/dock.rs @@ -726,7 +726,7 @@ mod tests { self.update_workspace(|workspace, cx| Dock::move_dock(workspace, anchor, true, cx)); } - pub fn hide_dock(&self) { + pub fn hide_dock(&mut self) { self.cx.dispatch_action(self.window_id, HideDock); }