From c10ac318661df0d0122d4ec93141094bb7a2aff4 Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Tue, 18 Feb 2025 12:47:25 -0500 Subject: [PATCH] theme: Don't log errors for missing themes until extensions have loaded (#25098) This PR makes it so we don't log errors for missing themes or icon themes until after the extensions have been loaded. Currently, if you are using a theme that is defined in an extension it is common to see one or more "theme not found" errors in the logs. This is the result of us having to initialize the theme before the extensions have actually finished loading. This means that a theme that _may_ exist once extensions load is considered non-existent before they have loaded. To that end, we now wait until the extensions have loaded before we start logging errors if we can't find the theme or icon theme. Closes https://github.com/zed-industries/zed/issues/24539. Release Notes: - Reduced the number of "theme not found" and "icon theme not found" errors in the logs for themes provided by extensions. --- crates/extension/src/extension_host_proxy.rs | 10 ++++++++ crates/extension_host/src/extension_host.rs | 1 + crates/theme/src/registry.rs | 13 ++++++++++ crates/theme/src/settings.rs | 24 +++++++++++++------ crates/theme_extension/src/theme_extension.rs | 4 ++++ 5 files changed, 45 insertions(+), 7 deletions(-) diff --git a/crates/extension/src/extension_host_proxy.rs b/crates/extension/src/extension_host_proxy.rs index a692795e87..fee590d345 100644 --- a/crates/extension/src/extension_host_proxy.rs +++ b/crates/extension/src/extension_host_proxy.rs @@ -96,6 +96,8 @@ impl ExtensionHostProxy { } pub trait ExtensionThemeProxy: Send + Sync + 'static { + fn set_extensions_loaded(&self); + fn list_theme_names(&self, theme_path: PathBuf, fs: Arc) -> Task>>; fn remove_user_themes(&self, themes: Vec); @@ -123,6 +125,14 @@ pub trait ExtensionThemeProxy: Send + Sync + 'static { } impl ExtensionThemeProxy for ExtensionHostProxy { + fn set_extensions_loaded(&self) { + let Some(proxy) = self.theme_proxy.read().clone() else { + return; + }; + + proxy.set_extensions_loaded() + } + fn list_theme_names(&self, theme_path: PathBuf, fs: Arc) -> Task>> { let Some(proxy) = self.theme_proxy.read().clone() else { return Task::ready(Ok(Vec::new())); diff --git a/crates/extension_host/src/extension_host.rs b/crates/extension_host/src/extension_host.rs index a6f6299136..9c6b2bbcd8 100644 --- a/crates/extension_host/src/extension_host.rs +++ b/crates/extension_host/src/extension_host.rs @@ -1290,6 +1290,7 @@ impl ExtensionStore { } this.wasm_extensions.extend(wasm_extensions); + this.proxy.set_extensions_loaded(); this.proxy.reload_current_theme(cx); this.proxy.reload_current_icon_theme(cx); }) diff --git a/crates/theme/src/registry.rs b/crates/theme/src/registry.rs index ed1d232cf5..43328aaad2 100644 --- a/crates/theme/src/registry.rs +++ b/crates/theme/src/registry.rs @@ -50,6 +50,8 @@ impl Global for GlobalThemeRegistry {} struct ThemeRegistryState { themes: HashMap>, icon_themes: HashMap>, + /// Whether the extensions have been loaded yet. + extensions_loaded: bool, } /// The registry for themes. @@ -82,6 +84,7 @@ impl ThemeRegistry { state: RwLock::new(ThemeRegistryState { themes: HashMap::default(), icon_themes: HashMap::default(), + extensions_loaded: false, }), assets, }; @@ -100,6 +103,16 @@ impl ThemeRegistry { registry } + /// Returns whether the extensions have been loaded. + pub fn extensions_loaded(&self) -> bool { + self.state.read().extensions_loaded + } + + /// Sets the flag indicating that the extensions have loaded. + pub fn set_extensions_loaded(&self) { + self.state.write().extensions_loaded = true; + } + fn insert_theme_families(&self, families: impl IntoIterator) { for family in families.into_iter() { self.insert_themes(family.themes); diff --git a/crates/theme/src/settings.rs b/crates/theme/src/settings.rs index 70e271a108..3735927115 100644 --- a/crates/theme/src/settings.rs +++ b/crates/theme/src/settings.rs @@ -157,7 +157,11 @@ 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); - if theme_registry.get(theme_name).ok().is_none() { + if let Err(err @ ThemeNotFoundError(_)) = theme_registry.get(theme_name) { + if theme_registry.extensions_loaded() { + log::error!("{err}"); + } + theme_name = Self::default_theme(*system_appearance); }; @@ -180,11 +184,13 @@ impl ThemeSettings { // If the selected icon theme doesn't exist, fall back to the default theme. let theme_registry = ThemeRegistry::global(cx); - if theme_registry - .get_icon_theme(icon_theme_name) - .ok() - .is_none() + if let Err(err @ IconThemeNotFoundError(_)) = + theme_registry.get_icon_theme(icon_theme_name) { + if theme_registry.extensions_loaded() { + log::error!("{err}"); + } + icon_theme_name = DEFAULT_ICON_THEME_NAME; }; @@ -848,7 +854,9 @@ impl settings::Settings for ThemeSettings { this.active_theme = theme; } Err(err @ ThemeNotFoundError(_)) => { - log::error!("{err}"); + if themes.extensions_loaded() { + log::error!("{err}"); + } } } } @@ -866,7 +874,9 @@ impl settings::Settings for ThemeSettings { this.active_icon_theme = icon_theme; } Err(err @ IconThemeNotFoundError(_)) => { - log::error!("{err}"); + if themes.extensions_loaded() { + log::error!("{err}"); + } } } } diff --git a/crates/theme_extension/src/theme_extension.rs b/crates/theme_extension/src/theme_extension.rs index 83903da6c6..b9c6ed6d4b 100644 --- a/crates/theme_extension/src/theme_extension.rs +++ b/crates/theme_extension/src/theme_extension.rs @@ -24,6 +24,10 @@ struct ThemeRegistryProxy { } impl ExtensionThemeProxy for ThemeRegistryProxy { + fn set_extensions_loaded(&self) { + self.theme_registry.set_extensions_loaded(); + } + fn list_theme_names(&self, theme_path: PathBuf, fs: Arc) -> Task>> { self.executor.spawn(async move { let themes = theme::read_user_theme(&theme_path, fs).await?;