From 76560968149daf88a95d3f579be029dd54656a02 Mon Sep 17 00:00:00 2001
From: Marshall Bowers <elliott.codes@gmail.com>
Date: Mon, 29 Jan 2024 17:51:34 -0500
Subject: [PATCH] Rework access to `ThemeRegistry` global (#7023)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This PR reworks how we access the `ThemeRegistry` global.

Previously we were making calls directly on the context, like
`cx.global::<ThemeRegistry>`. However, one problem with this is that it
spreads out the knowledge of exactly what type is stored in the global.

In order to make it easier to adjust the type we store in the context
(e.g., wrapping the `ThemeRegistry` in an `Arc`), we now call methods on
the `ThemeRegistry` itself for accessing the global.

It would also be interesting to see how we could prevent access to the
`ThemeRegistry` without going through one of these dedicated methods 🤔

Release Notes:

- N/A
---
 crates/storybook/src/storybook.rs           |  2 +-
 crates/theme/src/registry.rs                | 19 ++++++++++++++++++-
 crates/theme/src/settings.rs                |  5 ++---
 crates/theme/src/theme.rs                   |  2 +-
 crates/theme_selector/src/theme_selector.rs |  4 ++--
 5 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/crates/storybook/src/storybook.rs b/crates/storybook/src/storybook.rs
index 47fc3b042a..efbd46665c 100644
--- a/crates/storybook/src/storybook.rs
+++ b/crates/storybook/src/storybook.rs
@@ -71,7 +71,7 @@ fn main() {
 
         let selector = story_selector;
 
-        let theme_registry = cx.global::<ThemeRegistry>();
+        let theme_registry = 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);
diff --git a/crates/theme/src/registry.rs b/crates/theme/src/registry.rs
index c90b1c909e..1fbee2449e 100644
--- a/crates/theme/src/registry.rs
+++ b/crates/theme/src/registry.rs
@@ -2,7 +2,7 @@ use std::collections::HashMap;
 use std::sync::Arc;
 
 use anyhow::{anyhow, Context, Result};
-use gpui::{AssetSource, HighlightStyle, SharedString};
+use gpui::{AppContext, AssetSource, HighlightStyle, SharedString};
 use refineable::Refineable;
 use util::ResultExt;
 
@@ -24,6 +24,23 @@ pub struct ThemeRegistry {
 }
 
 impl ThemeRegistry {
+    /// Returns the global [`ThemeRegistry`].
+    pub fn global(cx: &AppContext) -> &Self {
+        cx.global::<ThemeRegistry>()
+    }
+
+    /// Returns a mutable reference to the global [`ThemeRegistry`].
+    pub fn global_mut(cx: &mut AppContext) -> &mut Self {
+        cx.global_mut::<ThemeRegistry>()
+    }
+
+    /// Returns a mutable reference to the global [`ThemeRegistry`].
+    ///
+    /// Inserts a default [`ThemeRegistry`] if one does not yet exist.
+    pub fn default_global(cx: &mut AppContext) -> &mut Self {
+        cx.default_global::<ThemeRegistry>()
+    }
+
     pub fn new(assets: Box<dyn AssetSource>) -> Self {
         let mut registry = Self {
             assets,
diff --git a/crates/theme/src/settings.rs b/crates/theme/src/settings.rs
index 67e7e7a0d8..828a2f5de5 100644
--- a/crates/theme/src/settings.rs
+++ b/crates/theme/src/settings.rs
@@ -164,7 +164,7 @@ impl settings::Settings for ThemeSettings {
         user_values: &[&Self::FileContent],
         cx: &mut AppContext,
     ) -> Result<Self> {
-        let themes = cx.default_global::<ThemeRegistry>();
+        let themes = ThemeRegistry::default_global(cx);
 
         let mut this = Self {
             ui_font_size: defaults.ui_font_size.unwrap().into(),
@@ -230,8 +230,7 @@ impl settings::Settings for ThemeSettings {
         cx: &AppContext,
     ) -> schemars::schema::RootSchema {
         let mut root_schema = generator.root_schema_for::<ThemeSettingsContent>();
-        let theme_names = cx
-            .global::<ThemeRegistry>()
+        let theme_names = ThemeRegistry::global(cx)
             .list_names(params.staff_mode)
             .map(|theme_name| Value::String(theme_name.to_string()))
             .collect();
diff --git a/crates/theme/src/theme.rs b/crates/theme/src/theme.rs
index 5018a72da0..2686ffe62d 100644
--- a/crates/theme/src/theme.rs
+++ b/crates/theme/src/theme.rs
@@ -63,7 +63,7 @@ pub fn init(themes_to_load: LoadThemes, cx: &mut AppContext) {
     cx.set_global(ThemeRegistry::new(assets));
 
     if load_user_themes {
-        cx.global_mut::<ThemeRegistry>().load_user_themes();
+        ThemeRegistry::global_mut(cx).load_user_themes();
     }
 
     ThemeSettings::register(cx);
diff --git a/crates/theme_selector/src/theme_selector.rs b/crates/theme_selector/src/theme_selector.rs
index 14251c4a53..d67ff9ba71 100644
--- a/crates/theme_selector/src/theme_selector.rs
+++ b/crates/theme_selector/src/theme_selector.rs
@@ -102,7 +102,7 @@ impl ThemeSelectorDelegate {
         let original_theme = cx.theme().clone();
 
         let staff_mode = cx.is_staff();
-        let registry = cx.global::<ThemeRegistry>();
+        let registry = ThemeRegistry::global(cx);
         let mut themes = registry.list(staff_mode).collect::<Vec<_>>();
         themes.sort_unstable_by(|a, b| {
             a.appearance
@@ -135,7 +135,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 = cx.global::<ThemeRegistry>();
+            let registry = ThemeRegistry::global(cx);
             match registry.get(&mat.string) {
                 Ok(theme) => {
                     Self::set_theme(theme, cx);