From d633a0da788047787c0b344cae721bf1ac48b64e Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Mon, 29 Apr 2024 16:37:37 -0400 Subject: [PATCH] gpui: Fix `Global` trait (#11187) This PR restores the `Global` trait's status as a marker trait. This was the original intent from #7095, when it was added, that had been lost in #9777. The purpose of the `Global` trait is to statically convey what types can and can't be accessed as `Global` state, as well as provide a way of restricting access to said globals. For example, in the case of the `ThemeRegistry` we have a private `GlobalThemeRegistry` that is marked as `Global`: https://github.com/zed-industries/zed/blob/91b3c24ed35d58438ae33970f07d1ff01d3acfc7/crates/theme/src/registry.rs#L25-L34 We're then able to permit reading the `ThemeRegistry` from the `GlobalThemeRegistry` via a custom getter, while still restricting which callers are able to mutate the global: https://github.com/zed-industries/zed/blob/91b3c24ed35d58438ae33970f07d1ff01d3acfc7/crates/theme/src/registry.rs#L46-L61 Release Notes: - N/A --- crates/assistant2/src/assistant2.rs | 2 +- crates/assistant2/src/completion_provider.rs | 6 +++++- crates/file_icons/src/file_icons.rs | 4 ++++ crates/gpui/src/gpui.rs | 21 +++++++------------- crates/semantic_index/examples/index.rs | 2 +- crates/semantic_index/src/semantic_index.rs | 2 +- crates/settings/src/settings_store.rs | 7 +++++++ crates/tasks_ui/src/modal.rs | 6 +++--- 8 files changed, 29 insertions(+), 21 deletions(-) diff --git a/crates/assistant2/src/assistant2.rs b/crates/assistant2/src/assistant2.rs index 68784862b8..aa0450ac2f 100644 --- a/crates/assistant2/src/assistant2.rs +++ b/crates/assistant2/src/assistant2.rs @@ -11,7 +11,7 @@ use feature_flags::FeatureFlagAppExt as _; use futures::{future::join_all, StreamExt}; use gpui::{ list, prelude::*, AnyElement, AppContext, AsyncWindowContext, EventEmitter, FocusHandle, - FocusableView, Global, ListAlignment, ListState, Render, Task, View, WeakView, + FocusableView, ListAlignment, ListState, Render, Task, View, WeakView, }; use language::{language_settings::SoftWrap, LanguageRegistry}; use open_ai::{FunctionContent, ToolCall, ToolCallContent}; diff --git a/crates/assistant2/src/completion_provider.rs b/crates/assistant2/src/completion_provider.rs index 01970c053e..7351573119 100644 --- a/crates/assistant2/src/completion_provider.rs +++ b/crates/assistant2/src/completion_provider.rs @@ -2,7 +2,7 @@ use anyhow::Result; use assistant_tooling::ToolFunctionDefinition; use client::{proto, Client}; use futures::{future::BoxFuture, stream::BoxStream, FutureExt, StreamExt}; -use gpui::Global; +use gpui::{AppContext, Global}; use std::sync::Arc; pub use open_ai::RequestMessage as CompletionMessage; @@ -11,6 +11,10 @@ pub use open_ai::RequestMessage as CompletionMessage; pub struct CompletionProvider(Arc); impl CompletionProvider { + pub fn get(cx: &AppContext) -> &Self { + cx.global::() + } + pub fn new(backend: impl CompletionProviderBackend) -> Self { Self(Arc::new(backend)) } diff --git a/crates/file_icons/src/file_icons.rs b/crates/file_icons/src/file_icons.rs index 4c72bad43e..63e8558e06 100644 --- a/crates/file_icons/src/file_icons.rs +++ b/crates/file_icons/src/file_icons.rs @@ -31,6 +31,10 @@ pub fn init(assets: impl AssetSource, cx: &mut AppContext) { } impl FileIcons { + pub fn get(cx: &AppContext) -> &Self { + cx.global::() + } + pub fn new(assets: impl AssetSource) -> Self { assets .load("icons/file_icons/file_types.json") diff --git a/crates/gpui/src/gpui.rs b/crates/gpui/src/gpui.rs index 9564d632d0..315ad9257e 100644 --- a/crates/gpui/src/gpui.rs +++ b/crates/gpui/src/gpui.rs @@ -330,19 +330,12 @@ impl Flatten for Result { /// A marker trait for types that can be stored in GPUI's global state. /// +/// This trait exists to provide type-safe access to globals by restricting +/// the scope from which they can be accessed. For instance, the actual type +/// that implements [`Global`] can be private, with public accessor functions +/// that enforce correct usage. +/// /// Implement this on types you want to store in the context as a global. -pub trait Global: 'static + Sized { - /// Access the global of the implementing type. Panics if a global for that type has not been assigned. - fn get(cx: &AppContext) -> &Self { - cx.global() - } - - /// Updates the global of the implementing type with a closure. Unlike `global_mut`, this method provides - /// your closure with mutable access to the `AppContext` and the global simultaneously. - fn update(cx: &mut C, f: impl FnOnce(&mut Self, &mut C) -> R) -> R - where - C: BorrowAppContext, - { - cx.update_global(f) - } +pub trait Global: 'static { + // This trait is intentionally left empty, by virtue of being a marker trait. } diff --git a/crates/semantic_index/examples/index.rs b/crates/semantic_index/examples/index.rs index 6783e07048..ed5f461377 100644 --- a/crates/semantic_index/examples/index.rs +++ b/crates/semantic_index/examples/index.rs @@ -1,6 +1,6 @@ use client::Client; use futures::channel::oneshot; -use gpui::{App, Global}; +use gpui::App; use language::language_settings::AllLanguageSettings; use project::Project; use semantic_index::{OpenAiEmbeddingModel, OpenAiEmbeddingProvider, SemanticIndex}; diff --git a/crates/semantic_index/src/semantic_index.rs b/crates/semantic_index/src/semantic_index.rs index f14438fed6..e26ca0a0a7 100644 --- a/crates/semantic_index/src/semantic_index.rs +++ b/crates/semantic_index/src/semantic_index.rs @@ -921,7 +921,7 @@ fn db_key_for_path(path: &Arc) -> String { mod tests { use super::*; use futures::{future::BoxFuture, FutureExt}; - use gpui::{Global, TestAppContext}; + use gpui::TestAppContext; use language::language_settings::AllLanguageSettings; use project::Project; use settings::SettingsStore; diff --git a/crates/settings/src/settings_store.rs b/crates/settings/src/settings_store.rs index a6bb5838f9..5824414917 100644 --- a/crates/settings/src/settings_store.rs +++ b/crates/settings/src/settings_store.rs @@ -207,6 +207,13 @@ trait AnySettingValue: 'static + Send + Sync { struct DeserializedSetting(Box); impl SettingsStore { + pub fn update(cx: &mut C, f: impl FnOnce(&mut Self, &mut C) -> R) -> R + where + C: BorrowAppContext, + { + cx.update_global(f) + } + /// Add a new type of setting to the store. pub fn register_setting(&mut self, cx: &mut AppContext) { let setting_type_id = TypeId::of::(); diff --git a/crates/tasks_ui/src/modal.rs b/crates/tasks_ui/src/modal.rs index 9e07e57dad..3d895197a4 100644 --- a/crates/tasks_ui/src/modal.rs +++ b/crates/tasks_ui/src/modal.rs @@ -3,9 +3,9 @@ use std::sync::Arc; use crate::{active_item_selection_properties, schedule_resolved_task}; use fuzzy::{StringMatch, StringMatchCandidate}; use gpui::{ - impl_actions, rems, AppContext, DismissEvent, EventEmitter, FocusableView, Global, - InteractiveElement, Model, ParentElement, Render, SharedString, Styled, Subscription, View, - ViewContext, VisualContext, WeakView, + impl_actions, rems, AppContext, DismissEvent, EventEmitter, FocusableView, InteractiveElement, + Model, ParentElement, Render, SharedString, Styled, Subscription, View, ViewContext, + VisualContext, WeakView, }; use picker::{highlighted_match_with_paths::HighlightedText, Picker, PickerDelegate}; use project::{Inventory, TaskSourceKind};