theme: Turn ThemeRegistry into a trait (#20076)

This PR converts the `ThemeRegistry` type into a trait instead of a
concrete implementation.

This allows for the extension store to depend on an abstraction rather
than the concrete theme registry implementation.

We currently have two `ThemeRegistry` implementations:

- `RealThemeRegistry` — this was previously the `ThemeRegistry` and
contains the real implementation of the registry.
- `VoidThemeRegistry` — a null object that doesn't have any behavior.

Release Notes:

- N/A

Cargo.lock

@@ -12078,6 +12078,7 @@ name = "theme"
 version = "0.1.0"
 dependencies = [
  "anyhow",
+ "async-trait",
  "collections",
  "derive_more",
  "fs",

crates/extension/src/extension_store.rs

@@ -114,7 +114,7 @@ pub struct ExtensionStore {
     outstanding_operations: BTreeMap<Arc<str>, ExtensionOperation>,
     index_path: PathBuf,
     language_registry: Arc<LanguageRegistry>,
-    theme_registry: Arc<ThemeRegistry>,
+    theme_registry: Arc<dyn ThemeRegistry>,
     slash_command_registry: Arc<SlashCommandRegistry>,
     indexed_docs_registry: Arc<IndexedDocsRegistry>,
     snippet_registry: Arc<SnippetRegistry>,
@@ -179,7 +179,7 @@ pub fn init(
     client: Arc<Client>,
     node_runtime: NodeRuntime,
     language_registry: Arc<LanguageRegistry>,
-    theme_registry: Arc<ThemeRegistry>,
+    theme_registry: Arc<dyn ThemeRegistry>,
     cx: &mut AppContext,
 ) {
     ExtensionSettings::register(cx);
@@ -230,7 +230,7 @@ impl ExtensionStore {
         telemetry: Option<Arc<Telemetry>>,
         node_runtime: NodeRuntime,
         language_registry: Arc<LanguageRegistry>,
-        theme_registry: Arc<ThemeRegistry>,
+        theme_registry: Arc<dyn ThemeRegistry>,
         slash_command_registry: Arc<SlashCommandRegistry>,
         indexed_docs_registry: Arc<IndexedDocsRegistry>,
         snippet_registry: Arc<SnippetRegistry>,
@@ -1358,7 +1358,7 @@ impl ExtensionStore {
                     continue;
                 };
 
-                let Some(theme_family) = ThemeRegistry::read_user_theme(&theme_path, fs.clone())
+                let Some(theme_family) = theme::read_user_theme(&theme_path, fs.clone())
                     .await
                     .log_err()
                 else {

crates/extension/src/extension_store_test.rs

@@ -27,7 +27,7 @@ use std::{
     path::{Path, PathBuf},
     sync::Arc,
 };
-use theme::ThemeRegistry;
+use theme::{RealThemeRegistry, ThemeRegistry};
 use util::test::temp_tree;
 
 #[cfg(test)]
@@ -260,7 +260,7 @@ async fn test_extension_store(cx: &mut TestAppContext) {
     };
 
     let language_registry = Arc::new(LanguageRegistry::test(cx.executor()));
-    let theme_registry = Arc::new(ThemeRegistry::new(Box::new(())));
+    let theme_registry = Arc::new(RealThemeRegistry::new(Box::new(())));
     let slash_command_registry = SlashCommandRegistry::new();
     let indexed_docs_registry = Arc::new(IndexedDocsRegistry::new(cx.executor()));
     let snippet_registry = Arc::new(SnippetRegistry::new());
@@ -486,7 +486,7 @@ async fn test_extension_store_with_test_extension(cx: &mut TestAppContext) {
     let project = Project::test(fs.clone(), [project_dir.as_path()], cx).await;
 
     let language_registry = project.read_with(cx, |project, _cx| project.languages().clone());
-    let theme_registry = Arc::new(ThemeRegistry::new(Box::new(())));
+    let theme_registry = Arc::new(RealThemeRegistry::new(Box::new(())));
     let slash_command_registry = SlashCommandRegistry::new();
     let indexed_docs_registry = Arc::new(IndexedDocsRegistry::new(cx.executor()));
     let snippet_registry = Arc::new(SnippetRegistry::new());

crates/extension_cli/src/main.rs

@@ -15,7 +15,6 @@ use extension::{
 };
 use language::LanguageConfig;
 use reqwest_client::ReqwestClient;
-use theme::ThemeRegistry;
 use tree_sitter::{Language, Query, WasmStore};
 
 #[derive(Parser, Debug)]
@@ -267,7 +266,7 @@ async fn test_themes(
 ) -> Result<()> {
     for relative_theme_path in &manifest.themes {
         let theme_path = extension_path.join(relative_theme_path);
-        let theme_family = ThemeRegistry::read_user_theme(&theme_path, fs.clone()).await?;
+        let theme_family = theme::read_user_theme(&theme_path, fs.clone()).await?;
         log::info!("loaded theme family {}", theme_family.name);
     }
 

crates/settings_ui/src/appearance_settings_controls.rs

@@ -83,7 +83,7 @@ impl RenderOnce for ThemeControl {
             "theme",
             value.clone(),
             ContextMenu::build(cx, |mut menu, cx| {
-                let theme_registry = ThemeRegistry::global(cx);
+                let theme_registry = <dyn ThemeRegistry>::global(cx);
 
                 for theme in theme_registry.list_names(false) {
                     menu = menu.custom_entry(

crates/storybook/src/storybook.rs

@@ -76,7 +76,7 @@ fn main() {
 
         let selector = story_selector;
 
-        let theme_registry = ThemeRegistry::global(cx);
+        let theme_registry = <dyn ThemeRegistry>::global(cx);
         let mut theme_settings = ThemeSettings::get_global(cx).clone();
         theme_settings.active_theme = theme_registry.get(&theme_name).unwrap();
         ThemeSettings::override_global(theme_settings, cx);

crates/theme/Cargo.toml

@@ -18,6 +18,7 @@ doctest = false
 
 [dependencies]
 anyhow.workspace = true
+async-trait.workspace = true
 collections.workspace = true
 derive_more.workspace = true
 fs.workspace = true

crates/theme/src/registry.rs

@@ -1,7 +1,8 @@
 use std::sync::Arc;
 use std::{fmt::Debug, path::Path};
 
-use anyhow::{anyhow, Context, Result};
+use anyhow::{anyhow, bail, Context, Result};
+use async_trait::async_trait;
 use collections::HashMap;
 use derive_more::{Deref, DerefMut};
 use fs::Fs;
@@ -10,7 +11,9 @@ use gpui::{AppContext, AssetSource, Global, SharedString};
 use parking_lot::RwLock;
 use util::ResultExt;
 
-use crate::{refine_theme_family, Appearance, Theme, ThemeFamily, ThemeFamilyContent};
+use crate::{
+    read_user_theme, refine_theme_family, Appearance, Theme, ThemeFamily, ThemeFamilyContent,
+};
 
 /// The metadata for a theme.
 #[derive(Debug, Clone)]
@@ -27,22 +30,34 @@ pub struct ThemeMeta {
 /// inserting the [`ThemeRegistry`] into the context as a global.
 ///
 /// This should not be exposed outside of this module.
-#[derive(Default, Deref, DerefMut)]
+#[derive(Deref, DerefMut)]
-struct GlobalThemeRegistry(Arc<ThemeRegistry>);
+struct GlobalThemeRegistry(Arc<dyn ThemeRegistry>);
 
 impl Global for GlobalThemeRegistry {}
 
-struct ThemeRegistryState {
+/// A registry for themes.
-    themes: HashMap<SharedString, Arc<Theme>>,
+#[async_trait]
+pub trait ThemeRegistry: Send + Sync + 'static {
+    /// Returns the names of all themes in the registry.
+    fn list_names(&self, _staff: bool) -> Vec<SharedString>;
+
+    /// Returns the metadata of all themes in the registry.
+    fn list(&self, _staff: bool) -> Vec<ThemeMeta>;
+
+    /// Returns the theme with the given name.
+    fn get(&self, name: &str) -> Result<Arc<Theme>>;
+
+    /// Loads the user theme from the specified path and adds it to the registry.
+    async fn load_user_theme(&self, theme_path: &Path, fs: Arc<dyn Fs>) -> Result<()>;
+
+    /// Loads the user themes from the specified directory and adds them to the registry.
+    async fn load_user_themes(&self, themes_path: &Path, fs: Arc<dyn Fs>) -> Result<()>;
+
+    /// Removes the themes with the given names from the registry.
+    fn remove_user_themes(&self, themes_to_remove: &[SharedString]);
 }
 
-/// The registry for themes.
+impl dyn ThemeRegistry {
-pub struct ThemeRegistry {
-    state: RwLock<ThemeRegistryState>,
-    assets: Box<dyn AssetSource>,
-}
-
-impl ThemeRegistry {
     /// Returns the global [`ThemeRegistry`].
     pub fn global(cx: &AppContext) -> Arc<Self> {
         cx.global::<GlobalThemeRegistry>().0.clone()
@@ -52,18 +67,37 @@ impl ThemeRegistry {
     ///
     /// Inserts a default [`ThemeRegistry`] if one does not yet exist.
     pub fn default_global(cx: &mut AppContext) -> Arc<Self> {
-        cx.default_global::<GlobalThemeRegistry>().0.clone()
+        if let Some(registry) = cx.try_global::<GlobalThemeRegistry>() {
+            return registry.0.clone();
         }
 
+        let registry = Arc::new(RealThemeRegistry::default());
+        cx.set_global(GlobalThemeRegistry(registry.clone()));
+
+        registry
+    }
+}
+
+struct RealThemeRegistryState {
+    themes: HashMap<SharedString, Arc<Theme>>,
+}
+
+/// The registry for themes.
+pub struct RealThemeRegistry {
+    state: RwLock<RealThemeRegistryState>,
+    assets: Box<dyn AssetSource>,
+}
+
+impl RealThemeRegistry {
     /// Sets the global [`ThemeRegistry`].
-    pub(crate) fn set_global(assets: Box<dyn AssetSource>, cx: &mut AppContext) {
+    pub(crate) fn set_global(self: Arc<Self>, cx: &mut AppContext) {
-        cx.set_global(GlobalThemeRegistry(Arc::new(ThemeRegistry::new(assets))));
+        cx.set_global(GlobalThemeRegistry(self));
     }
 
     /// Creates a new [`ThemeRegistry`] with the given [`AssetSource`].
     pub fn new(assets: Box<dyn AssetSource>) -> Self {
         let registry = Self {
-            state: RwLock::new(ThemeRegistryState {
+            state: RwLock::new(RealThemeRegistryState {
                 themes: HashMap::default(),
             }),
             assets,
@@ -98,49 +132,11 @@ impl ThemeRegistry {
         }
     }
 
-    /// Removes the themes with the given names from the registry.
-    pub fn remove_user_themes(&self, themes_to_remove: &[SharedString]) {
-        self.state
-            .write()
-            .themes
-            .retain(|name, _| !themes_to_remove.contains(name))
-    }
-
     /// Removes all themes from the registry.
     pub fn clear(&self) {
         self.state.write().themes.clear();
     }
 
-    /// Returns the names of all themes in the registry.
-    pub fn list_names(&self, _staff: bool) -> Vec<SharedString> {
-        let mut names = self.state.read().themes.keys().cloned().collect::<Vec<_>>();
-        names.sort();
-        names
-    }
-
-    /// Returns the metadata of all themes in the registry.
-    pub fn list(&self, _staff: bool) -> Vec<ThemeMeta> {
-        self.state
-            .read()
-            .themes
-            .values()
-            .map(|theme| ThemeMeta {
-                name: theme.name.clone(),
-                appearance: theme.appearance(),
-            })
-            .collect()
-    }
-
-    /// Returns the theme with the given name.
-    pub fn get(&self, name: &str) -> Result<Arc<Theme>> {
-        self.state
-            .read()
-            .themes
-            .get(name)
-            .ok_or_else(|| anyhow!("theme not found: {}", name))
-            .cloned()
-    }
-
     /// Loads the themes bundled with the Zed binary and adds them to the registry.
     pub fn load_bundled_themes(&self) {
         let theme_paths = self
@@ -165,9 +161,52 @@ impl ThemeRegistry {
             self.insert_user_theme_families([theme_family]);
         }
     }
+}
 
-    /// Loads the user themes from the specified directory and adds them to the registry.
+impl Default for RealThemeRegistry {
-    pub async fn load_user_themes(&self, themes_path: &Path, fs: Arc<dyn Fs>) -> Result<()> {
+    fn default() -> Self {
+        Self::new(Box::new(()))
+    }
+}
+
+#[async_trait]
+impl ThemeRegistry for RealThemeRegistry {
+    fn list_names(&self, _staff: bool) -> Vec<SharedString> {
+        let mut names = self.state.read().themes.keys().cloned().collect::<Vec<_>>();
+        names.sort();
+        names
+    }
+
+    fn list(&self, _staff: bool) -> Vec<ThemeMeta> {
+        self.state
+            .read()
+            .themes
+            .values()
+            .map(|theme| ThemeMeta {
+                name: theme.name.clone(),
+                appearance: theme.appearance(),
+            })
+            .collect()
+    }
+
+    fn get(&self, name: &str) -> Result<Arc<Theme>> {
+        self.state
+            .read()
+            .themes
+            .get(name)
+            .ok_or_else(|| anyhow!("theme not found: {}", name))
+            .cloned()
+    }
+
+    async fn load_user_theme(&self, theme_path: &Path, fs: Arc<dyn Fs>) -> Result<()> {
+        let theme = read_user_theme(theme_path, fs).await?;
+
+        self.insert_user_theme_families([theme]);
+
+        Ok(())
+    }
+
+    async fn load_user_themes(&self, themes_path: &Path, fs: Arc<dyn Fs>) -> Result<()> {
         let mut theme_paths = fs
             .read_dir(themes_path)
             .await
@@ -186,40 +225,38 @@ impl ThemeRegistry {
         Ok(())
     }
 
-    /// Asynchronously reads the user theme from the specified path.
+    fn remove_user_themes(&self, themes_to_remove: &[SharedString]) {
-    pub async fn read_user_theme(theme_path: &Path, fs: Arc<dyn Fs>) -> Result<ThemeFamilyContent> {
+        self.state
-        let reader = fs.open_sync(theme_path).await?;
+            .write()
-        let theme_family: ThemeFamilyContent = serde_json_lenient::from_reader(reader)?;
+            .themes
-
+            .retain(|name, _| !themes_to_remove.contains(name))
-        for theme in &theme_family.themes {
-            if theme
-                .style
-                .colors
-                .deprecated_scrollbar_thumb_background
-                .is_some()
-            {
-                log::warn!(
-                    r#"Theme "{theme_name}" is using a deprecated style property: scrollbar_thumb.background. Use `scrollbar.thumb.background` instead."#,
-                    theme_name = theme.name
-                )
     }
 }
 
-        Ok(theme_family)
+/// A theme registry that doesn't have any behavior.
+pub struct VoidThemeRegistry;
+
+#[async_trait]
+impl ThemeRegistry for VoidThemeRegistry {
+    fn list_names(&self, _staff: bool) -> Vec<SharedString> {
+        Vec::new()
     }
 
-    /// Loads the user theme from the specified path and adds it to the registry.
+    fn list(&self, _staff: bool) -> Vec<ThemeMeta> {
-    pub async fn load_user_theme(&self, theme_path: &Path, fs: Arc<dyn Fs>) -> Result<()> {
+        Vec::new()
-        let theme = Self::read_user_theme(theme_path, fs).await?;
+    }
 
-        self.insert_user_theme_families([theme]);
+    fn get(&self, name: &str) -> Result<Arc<Theme>> {
+        bail!("cannot retrieve theme {name:?} from a void theme registry")
+    }
 
+    async fn load_user_theme(&self, _theme_path: &Path, _fs: Arc<dyn Fs>) -> Result<()> {
         Ok(())
     }
+
+    async fn load_user_themes(&self, _themes_path: &Path, _fs: Arc<dyn Fs>) -> Result<()> {
+        Ok(())
     }
 
-impl Default for ThemeRegistry {
+    fn remove_user_themes(&self, _themes_to_remove: &[SharedString]) {}
-    fn default() -> Self {
-        Self::new(Box::new(()))
-    }
 }

crates/theme/src/settings.rs

@@ -150,7 +150,7 @@ impl ThemeSettings {
 
             // If the selected theme doesn't exist, fall back to a default theme
             // based on the system appearance.
-            let theme_registry = ThemeRegistry::global(cx);
+            let theme_registry = <dyn ThemeRegistry>::global(cx);
             if theme_registry.get(theme_name).ok().is_none() {
                 theme_name = Self::default_theme(*system_appearance);
             };
@@ -446,7 +446,7 @@ impl ThemeSettings {
     /// Returns a `Some` containing the new theme if it was successful.
     /// Returns `None` otherwise.
     pub fn switch_theme(&mut self, theme: &str, cx: &mut AppContext) -> Option<Arc<Theme>> {
-        let themes = ThemeRegistry::default_global(cx);
+        let themes = <dyn ThemeRegistry>::default_global(cx);
 
         let mut new_theme = None;
 
@@ -598,7 +598,7 @@ impl settings::Settings for ThemeSettings {
     type FileContent = ThemeSettingsContent;
 
     fn load(sources: SettingsSources<Self::FileContent>, cx: &mut AppContext) -> Result<Self> {
-        let themes = ThemeRegistry::default_global(cx);
+        let themes = <dyn ThemeRegistry>::default_global(cx);
         let system_appearance = SystemAppearance::default_global(cx);
 
         let defaults = sources.default;
@@ -710,7 +710,7 @@ impl settings::Settings for ThemeSettings {
         cx: &AppContext,
     ) -> schemars::schema::RootSchema {
         let mut root_schema = generator.root_schema_for::<ThemeSettingsContent>();
-        let theme_names = ThemeRegistry::global(cx)
+        let theme_names = <dyn ThemeRegistry>::global(cx)
             .list_names(params.staff_mode)
             .into_iter()
             .map(|theme_name| Value::String(theme_name.to_string()))

crates/theme/src/theme.rs

@@ -17,17 +17,12 @@ mod schema;
 mod settings;
 mod styles;
 
+use std::path::Path;
 use std::sync::Arc;
 
 use ::settings::{Settings, SettingsStore};
-pub use default_colors::*;
+use anyhow::Result;
-pub use font_family_cache::*;
+use fs::Fs;
-pub use registry::*;
-pub use scale::*;
-pub use schema::*;
-pub use settings::*;
-pub use styles::*;
-
 use gpui::{
     px, AppContext, AssetSource, HighlightStyle, Hsla, Pixels, Refineable, SharedString,
     WindowAppearance, WindowBackgroundAppearance,
@@ -35,6 +30,14 @@ use gpui::{
 use serde::Deserialize;
 use uuid::Uuid;
 
+pub use crate::default_colors::*;
+pub use crate::font_family_cache::*;
+pub use crate::registry::*;
+pub use crate::scale::*;
+pub use crate::schema::*;
+pub use crate::settings::*;
+pub use crate::styles::*;
+
 /// Defines window border radius for platforms that use client side decorations.
 pub const CLIENT_SIDE_DECORATION_ROUNDING: Pixels = px(10.0);
 /// Defines window shadow size for platforms that use client side decorations.
@@ -85,10 +88,11 @@ pub fn init(themes_to_load: LoadThemes, cx: &mut AppContext) {
         LoadThemes::JustBase => (Box::new(()) as Box<dyn AssetSource>, false),
         LoadThemes::All(assets) => (assets, true),
     };
-    ThemeRegistry::set_global(assets, cx);
+    let registry = Arc::new(RealThemeRegistry::new(assets));
+    registry.clone().set_global(cx);
 
     if load_user_themes {
-        ThemeRegistry::global(cx).load_bundled_themes();
+        registry.load_bundled_themes();
     }
 
     ThemeSettings::register(cx);
@@ -321,3 +325,25 @@ pub fn color_alpha(color: Hsla, alpha: f32) -> Hsla {
     color.a = alpha;
     color
 }
+
+/// Asynchronously reads the user theme from the specified path.
+pub async fn read_user_theme(theme_path: &Path, fs: Arc<dyn Fs>) -> Result<ThemeFamilyContent> {
+    let reader = fs.open_sync(theme_path).await?;
+    let theme_family: ThemeFamilyContent = serde_json_lenient::from_reader(reader)?;
+
+    for theme in &theme_family.themes {
+        if theme
+            .style
+            .colors
+            .deprecated_scrollbar_thumb_background
+            .is_some()
+        {
+            log::warn!(
+                r#"Theme "{theme_name}" is using a deprecated style property: scrollbar_thumb.background. Use `scrollbar.thumb.background` instead."#,
+                theme_name = theme.name
+            )
+        }
+    }
+
+    Ok(theme_family)
+}

crates/theme_selector/src/theme_selector.rs

@@ -97,7 +97,7 @@ impl ThemeSelectorDelegate {
         let original_theme = cx.theme().clone();
 
         let staff_mode = cx.is_staff();
-        let registry = ThemeRegistry::global(cx);
+        let registry = <dyn ThemeRegistry>::global(cx);
         let mut themes = registry
             .list(staff_mode)
             .into_iter()
@@ -142,7 +142,7 @@ impl ThemeSelectorDelegate {
 
     fn show_selected_theme(&mut self, cx: &mut ViewContext<Picker<ThemeSelectorDelegate>>) {
         if let Some(mat) = self.matches.get(self.selected_index) {
-            let registry = ThemeRegistry::global(cx);
+            let registry = <dyn ThemeRegistry>::global(cx);
             match registry.get(&mat.string) {
                 Ok(theme) => {
                     Self::set_theme(theme, cx);

crates/zed/src/main.rs

@@ -407,7 +407,7 @@ fn main() {
             app_state.client.clone(),
             app_state.node_runtime.clone(),
             app_state.languages.clone(),
-            ThemeRegistry::global(cx),
+            <dyn ThemeRegistry>::global(cx),
             cx,
         );
         recent_projects::init(cx);
@@ -1160,8 +1160,9 @@ fn load_user_themes_in_background(fs: Arc<dyn fs::Fs>, cx: &mut AppContext) {
     cx.spawn({
         let fs = fs.clone();
         |cx| async move {
-            if let Some(theme_registry) =
+            if let Some(theme_registry) = cx
-                cx.update(|cx| ThemeRegistry::global(cx).clone()).log_err()
+                .update(|cx| <dyn ThemeRegistry>::global(cx).clone())
+                .log_err()
             {
                 let themes_dir = paths::themes_dir().as_ref();
                 match fs
@@ -1200,8 +1201,9 @@ fn watch_themes(fs: Arc<dyn fs::Fs>, cx: &mut AppContext) {
         while let Some(paths) = events.next().await {
             for event in paths {
                 if fs.metadata(&event.path).await.ok().flatten().is_some() {
-                    if let Some(theme_registry) =
+                    if let Some(theme_registry) = cx
-                        cx.update(|cx| ThemeRegistry::global(cx).clone()).log_err()
+                        .update(|cx| <dyn ThemeRegistry>::global(cx).clone())
+                        .log_err()
                     {
                         if let Some(()) = theme_registry
                             .load_user_theme(&event.path, fs.clone())

crates/zed/src/zed.rs

@@ -1139,7 +1139,7 @@ mod tests {
         path::{Path, PathBuf},
         time::Duration,
     };
-    use theme::{ThemeRegistry, ThemeSettings};
+    use theme::{RealThemeRegistry, ThemeRegistry, ThemeSettings};
     use workspace::{
         item::{Item, ItemHandle},
         open_new, open_paths, pane, NewFile, OpenVisible, SaveIntent, SplitDirection,
@@ -3419,7 +3419,7 @@ mod tests {
                     .unwrap(),
             ])
             .unwrap();
-        let themes = ThemeRegistry::default();
+        let themes = RealThemeRegistry::default();
         settings::init(cx);
         theme::init(theme::LoadThemes::JustBase, cx);