Encapsulate CommandPaletteFilter and CommandPaletteInterceptor (#9402)

This PR refactors the `CommandPaletteFilter` and
`CommandPaletteInterceptor` to better encapsulate their internals.

Previously these globals and their fields were publicly accessible,
which meant that there was a lot of reaching in and making
modifications.

These changes should make it easier to add additional consumers of these
hooks (right now they're primarily used by Vim mode).

Release Notes:

- N/A

Cargo.lock

@@ -2406,6 +2406,7 @@ name = "command_palette_hooks"
 version = "0.1.0"
 dependencies = [
  "collections",
+ "derive_more",
  "gpui",
 ]
 

crates/command_palette/src/command_palette.rs

@@ -27,7 +27,7 @@ actions!(command_palette, [Toggle]);
 pub fn init(cx: &mut AppContext) {
     client::init_settings(cx);
     cx.set_global(HitCounts::default());
-    cx.set_global(CommandPaletteFilter::default());
+    command_palette_hooks::init(cx);
     cx.observe_new_views(CommandPalette::register).detach();
 }
 
@@ -73,23 +73,18 @@ impl CommandPalette {
         telemetry: Arc<Telemetry>,
         cx: &mut ViewContext<Self>,
     ) -> Self {
-        let filter = cx.try_global::<CommandPaletteFilter>();
+        let filter = CommandPaletteFilter::try_global(cx);
 
         let commands = cx
             .available_actions()
             .into_iter()
             .filter_map(|action| {
-                let name = action.name();
-                let namespace = name.split("::").next().unwrap_or("malformed action name");
-                if filter.is_some_and(|f| {
-                    f.hidden_namespaces.contains(namespace)
-                        || f.hidden_action_types.contains(&action.type_id())
-                }) {
+                if filter.is_some_and(|filter| filter.is_hidden(&*action)) {
                     return None;
                 }
 
                 Some(Command {
-                    name: humanize_action_name(&name),
+                    name: humanize_action_name(action.name()),
                     action,
                 })
             })
@@ -185,12 +180,8 @@ impl CommandPaletteDelegate {
     ) {
         self.updating_matches.take();
 
-        let mut intercept_result =
-            if let Some(interceptor) = cx.try_global::<CommandPaletteInterceptor>() {
-                (interceptor.0)(&query, cx)
-            } else {
-                None
-            };
+        let mut intercept_result = CommandPaletteInterceptor::try_global(cx)
+            .and_then(|interceptor| interceptor.intercept(&query, cx));
 
         if parse_zed_link(&query, cx).is_some() {
             intercept_result = Some(CommandInterceptResult {
@@ -523,10 +514,9 @@ mod tests {
 
         // Add namespace filter, and redeploy the palette
         cx.update(|cx| {
-            cx.set_global(CommandPaletteFilter::default());
-            cx.update_global::<CommandPaletteFilter, _>(|filter, _| {
-                filter.hidden_namespaces.insert("editor");
-            })
+            CommandPaletteFilter::update_global(cx, |filter, _| {
+                filter.hide_namespace("editor");
+            });
         });
 
         cx.simulate_keystrokes("cmd-shift-p");

crates/command_palette_hooks/Cargo.toml

@@ -14,4 +14,5 @@ doctest = false
 
 [dependencies]
 collections.workspace = true
+derive_more.workspace = true
 gpui.workspace = true

crates/command_palette_hooks/src/command_palette_hooks.rs

@@ -1,24 +1,142 @@
+//! Provides hooks for customizing the behavior of the command palette.
+
+#![deny(missing_docs)]
+
 use std::any::TypeId;
 
 use collections::HashSet;
+use derive_more::{Deref, DerefMut};
 use gpui::{Action, AppContext, Global};
 
+/// Initializes the command palette hooks.
+pub fn init(cx: &mut AppContext) {
+    cx.set_global(GlobalCommandPaletteFilter::default());
+    cx.set_global(GlobalCommandPaletteInterceptor::default());
+}
+
+/// A filter for the command palette.
 #[derive(Default)]
 pub struct CommandPaletteFilter {
-    pub hidden_namespaces: HashSet<&'static str>,
-    pub hidden_action_types: HashSet<TypeId>,
+    hidden_namespaces: HashSet<&'static str>,
+    hidden_action_types: HashSet<TypeId>,
 }
 
-impl Global for CommandPaletteFilter {}
+#[derive(Deref, DerefMut, Default)]
+struct GlobalCommandPaletteFilter(CommandPaletteFilter);
 
-pub struct CommandPaletteInterceptor(
-    pub Box<dyn Fn(&str, &AppContext) -> Option<CommandInterceptResult>>,
-);
+impl Global for GlobalCommandPaletteFilter {}
 
-impl Global for CommandPaletteInterceptor {}
+impl CommandPaletteFilter {
+    /// Returns the global [`CommandPaletteFilter`], if one is set.
+    pub fn try_global(cx: &AppContext) -> Option<&CommandPaletteFilter> {
+        cx.try_global::<GlobalCommandPaletteFilter>()
+            .map(|filter| &filter.0)
+    }
 
+    /// Returns a mutable reference to the global [`CommandPaletteFilter`].
+    pub fn global_mut(cx: &mut AppContext) -> &mut Self {
+        cx.global_mut::<GlobalCommandPaletteFilter>()
+    }
+
+    /// Updates the global [`CommandPaletteFilter`] using the given closure.
+    pub fn update_global<F, R>(cx: &mut AppContext, update: F) -> R
+    where
+        F: FnOnce(&mut Self, &mut AppContext) -> R,
+    {
+        cx.update_global(|this: &mut GlobalCommandPaletteFilter, cx| update(&mut this.0, cx))
+    }
+
+    /// Returns whether the given [`Action`] is hidden by the filter.
+    pub fn is_hidden(&self, action: &dyn Action) -> bool {
+        let name = action.name();
+        let namespace = name.split("::").next().unwrap_or("malformed action name");
+
+        self.hidden_namespaces.contains(namespace)
+            || self.hidden_action_types.contains(&action.type_id())
+    }
+
+    /// Hides all actions in the given namespace.
+    pub fn hide_namespace(&mut self, namespace: &'static str) {
+        self.hidden_namespaces.insert(namespace);
+    }
+
+    /// Shows all actions in the given namespace.
+    pub fn show_namespace(&mut self, namespace: &'static str) {
+        self.hidden_namespaces.remove(namespace);
+    }
+
+    /// Hides all actions with the given types.
+    pub fn hide_action_types(&mut self, action_types: &[TypeId]) {
+        self.hidden_action_types.extend(action_types);
+    }
+
+    /// Shows all actions with the given types.
+    pub fn show_action_types<'a>(&mut self, action_types: impl Iterator<Item = &'a TypeId>) {
+        for action_type in action_types {
+            self.hidden_action_types.remove(action_type);
+        }
+    }
+}
+
+/// The result of intercepting a command palette command.
 pub struct CommandInterceptResult {
+    /// The action produced as a result of the interception.
     pub action: Box<dyn Action>,
+    // TODO: Document this field.
+    #[allow(missing_docs)]
     pub string: String,
+    // TODO: Document this field.
+    #[allow(missing_docs)]
     pub positions: Vec<usize>,
 }
+
+/// An interceptor for the command palette.
+#[derive(Default)]
+pub struct CommandPaletteInterceptor(
+    Option<Box<dyn Fn(&str, &AppContext) -> Option<CommandInterceptResult>>>,
+);
+
+#[derive(Default)]
+struct GlobalCommandPaletteInterceptor(CommandPaletteInterceptor);
+
+impl Global for GlobalCommandPaletteInterceptor {}
+
+impl CommandPaletteInterceptor {
+    /// Returns the global [`CommandPaletteInterceptor`], if one is set.
+    pub fn try_global(cx: &AppContext) -> Option<&CommandPaletteInterceptor> {
+        cx.try_global::<GlobalCommandPaletteInterceptor>()
+            .map(|interceptor| &interceptor.0)
+    }
+
+    /// Updates the global [`CommandPaletteInterceptor`] using the given closure.
+    pub fn update_global<F, R>(cx: &mut AppContext, update: F) -> R
+    where
+        F: FnOnce(&mut Self, &mut AppContext) -> R,
+    {
+        cx.update_global(|this: &mut GlobalCommandPaletteInterceptor, cx| update(&mut this.0, cx))
+    }
+
+    /// Intercepts the given query from the command palette.
+    pub fn intercept(&self, query: &str, cx: &AppContext) -> Option<CommandInterceptResult> {
+        let Some(handler) = self.0.as_ref() else {
+            return None;
+        };
+
+        (handler)(query, cx)
+    }
+
+    /// Clears the global interceptor.
+    pub fn clear(&mut self) {
+        self.0 = None;
+    }
+
+    /// Sets the global interceptor.
+    ///
+    /// This will override the previous interceptor, if it exists.
+    pub fn set(
+        &mut self,
+        handler: Box<dyn Fn(&str, &AppContext) -> Option<CommandInterceptResult>>,
+    ) {
+        self.0 = Some(handler);
+    }
+}

crates/copilot/src/copilot.rs

@@ -66,33 +66,26 @@ pub fn init(
         let copilot_auth_action_types = [TypeId::of::<SignOut>()];
         let copilot_no_auth_action_types = [TypeId::of::<SignIn>()];
         let status = handle.read(cx).status();
-        let filter = cx.default_global::<CommandPaletteFilter>();
+        let filter = CommandPaletteFilter::global_mut(cx);
 
         match status {
             Status::Disabled => {
-                filter.hidden_action_types.extend(copilot_action_types);
-                filter.hidden_action_types.extend(copilot_auth_action_types);
-                filter
-                    .hidden_action_types
-                    .extend(copilot_no_auth_action_types);
+                filter.hide_action_types(&copilot_action_types);
+                filter.hide_action_types(&copilot_auth_action_types);
+                filter.hide_action_types(&copilot_no_auth_action_types);
             }
             Status::Authorized => {
-                filter
-                    .hidden_action_types
-                    .extend(copilot_no_auth_action_types);
-                for type_id in copilot_action_types
+                filter.hide_action_types(&copilot_no_auth_action_types);
+                filter.show_action_types(
+                    copilot_action_types
                         .iter()
-                    .chain(&copilot_auth_action_types)
-                {
-                    filter.hidden_action_types.remove(type_id);
-                }
+                        .chain(&copilot_auth_action_types),
+                );
             }
             _ => {
-                filter.hidden_action_types.extend(copilot_action_types);
-                filter.hidden_action_types.extend(copilot_auth_action_types);
-                for type_id in &copilot_no_auth_action_types {
-                    filter.hidden_action_types.remove(type_id);
-                }
+                filter.hide_action_types(&copilot_action_types);
+                filter.hide_action_types(&copilot_auth_action_types);
+                filter.show_action_types(copilot_no_auth_action_types.iter());
             }
         }
     })

crates/vim/src/vim.rs

@@ -87,8 +87,8 @@ pub fn init(cx: &mut AppContext) {
     // Any time settings change, update vim mode to match. The Vim struct
     // will be initialized as disabled by default, so we filter its commands
     // out when starting up.
-    cx.update_global::<CommandPaletteFilter, _>(|filter, _| {
-        filter.hidden_namespaces.insert("vim");
+    CommandPaletteFilter::update_global(cx, |filter, _| {
+        filter.hide_namespace(Vim::NAMESPACE);
     });
     cx.update_global(|vim: &mut Vim, cx: &mut AppContext| {
         vim.set_enabled(VimModeSetting::get_global(cx).0, cx)
@@ -191,6 +191,9 @@ struct Vim {
 impl Global for Vim {}
 
 impl Vim {
+    /// The namespace for Vim actions.
+    const NAMESPACE: &'static str = "vim";
+
     fn read(cx: &mut AppContext) -> &Self {
         cx.global::<Self>()
     }
@@ -628,21 +631,23 @@ impl Vim {
             return;
         }
         if !enabled {
-            let _ = cx.remove_global::<CommandPaletteInterceptor>();
-            cx.update_global::<CommandPaletteFilter, _>(|filter, _| {
-                filter.hidden_namespaces.insert("vim");
+            CommandPaletteInterceptor::update_global(cx, |interceptor, _| {
+                interceptor.clear();
+            });
+            CommandPaletteFilter::update_global(cx, |filter, _| {
+                filter.hide_namespace(Self::NAMESPACE);
             });
             *self = Default::default();
             return;
         }
 
         self.enabled = true;
-        cx.update_global::<CommandPaletteFilter, _>(|filter, _| {
-            filter.hidden_namespaces.remove("vim");
+        CommandPaletteFilter::update_global(cx, |filter, _| {
+            filter.show_namespace(Self::NAMESPACE);
+        });
+        CommandPaletteInterceptor::update_global(cx, |interceptor, _| {
+            interceptor.set(Box::new(command::command_interceptor));
         });
-        cx.set_global::<CommandPaletteInterceptor>(CommandPaletteInterceptor(Box::new(
-            command::command_interceptor,
-        )));
 
         if let Some(active_window) = cx
             .active_window()