From 74565ed0b86afa85fb8e26cb35219182ecf667b7 Mon Sep 17 00:00:00 2001 From: Mikayla Date: Fri, 25 Aug 2023 17:00:53 -0700 Subject: [PATCH] Add feature flags handling to the client, rewrite staff mode to a trait extension style --- Cargo.lock | 28 +++---- Cargo.toml | 2 +- crates/channel/Cargo.toml | 2 +- crates/client/Cargo.toml | 2 +- crates/client/src/telemetry.rs | 2 - crates/client/src/user.rs | 35 ++++---- crates/collab_ui/Cargo.toml | 2 +- crates/collab_ui/src/collab_panel.rs | 35 ++++---- .../{staff_mode => feature_flags}/Cargo.toml | 4 +- crates/feature_flags/src/feature_flags.rs | 79 +++++++++++++++++++ crates/rpc/src/rpc.rs | 2 +- crates/settings/Cargo.toml | 2 +- crates/staff_mode/src/staff_mode.rs | 36 --------- crates/theme_selector/Cargo.toml | 2 +- crates/theme_selector/src/theme_selector.rs | 4 +- crates/zed/Cargo.toml | 2 +- crates/zed/src/languages/json.rs | 4 +- crates/zed/src/main.rs | 7 +- 18 files changed, 143 insertions(+), 107 deletions(-) rename crates/{staff_mode => feature_flags}/Cargo.toml (71%) create mode 100644 crates/feature_flags/src/feature_flags.rs delete mode 100644 crates/staff_mode/src/staff_mode.rs diff --git a/Cargo.lock b/Cargo.lock index 8197f883c0..bfdb1b6092 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1200,6 +1200,7 @@ dependencies = [ "client", "collections", "db", + "feature_flags", "futures 0.3.28", "gpui", "image", @@ -1215,7 +1216,6 @@ dependencies = [ "serde_derive", "settings", "smol", - "staff_mode", "sum_tree", "tempfile", "text", @@ -1374,6 +1374,7 @@ dependencies = [ "async-tungstenite", "collections", "db", + "feature_flags", "futures 0.3.28", "gpui", "image", @@ -1388,7 +1389,6 @@ dependencies = [ "serde_derive", "settings", "smol", - "staff_mode", "sum_tree", "tempfile", "text", @@ -1528,6 +1528,7 @@ dependencies = [ "context_menu", "db", "editor", + "feature_flags", "feedback", "futures 0.3.28", "fuzzy", @@ -1543,7 +1544,6 @@ dependencies = [ "serde", "serde_derive", "settings", - "staff_mode", "theme", "theme_selector", "util", @@ -2529,6 +2529,14 @@ version = "2.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6999dc1837253364c2ebb0704ba97994bd874e8f195d665c50b7548f6ea92764" +[[package]] +name = "feature_flags" +version = "0.1.0" +dependencies = [ + "anyhow", + "gpui", +] + [[package]] name = "feedback" version = "0.1.0" @@ -6834,6 +6842,7 @@ version = "0.1.0" dependencies = [ "anyhow", "collections", + "feature_flags", "fs", "futures 0.3.28", "gpui", @@ -6849,7 +6858,6 @@ dependencies = [ "serde_json_lenient", "smallvec", "sqlez", - "staff_mode", "toml 0.5.11", "tree-sitter", "tree-sitter-json 0.19.0", @@ -7284,14 +7292,6 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3" -[[package]] -name = "staff_mode" -version = "0.1.0" -dependencies = [ - "anyhow", - "gpui", -] - [[package]] name = "static_assertions" version = "1.1.0" @@ -7672,6 +7672,7 @@ name = "theme_selector" version = "0.1.0" dependencies = [ "editor", + "feature_flags", "fs", "fuzzy", "gpui", @@ -7681,7 +7682,6 @@ dependencies = [ "postage", "settings", "smol", - "staff_mode", "theme", "util", "workspace", @@ -9726,6 +9726,7 @@ dependencies = [ "diagnostics", "editor", "env_logger 0.9.3", + "feature_flags", "feedback", "file_finder", "fs", @@ -9772,7 +9773,6 @@ dependencies = [ "simplelog", "smallvec", "smol", - "staff_mode", "sum_tree", "tempdir", "terminal_view", diff --git a/Cargo.toml b/Cargo.toml index 0fb8f0b6b7..5938ecb402 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -62,7 +62,7 @@ members = [ "crates/snippet", "crates/sqlez", "crates/sqlez_macros", - "crates/staff_mode", + "crates/feature_flags", "crates/sum_tree", "crates/terminal", "crates/text", diff --git a/crates/channel/Cargo.toml b/crates/channel/Cargo.toml index 0978462a1a..c2191fdfa3 100644 --- a/crates/channel/Cargo.toml +++ b/crates/channel/Cargo.toml @@ -21,7 +21,7 @@ rpc = { path = "../rpc" } text = { path = "../text" } language = { path = "../language" } settings = { path = "../settings" } -staff_mode = { path = "../staff_mode" } +feature_flags = { path = "../feature_flags" } sum_tree = { path = "../sum_tree" } anyhow.workspace = true diff --git a/crates/client/Cargo.toml b/crates/client/Cargo.toml index 64d8f02c8a..e3038e5bcc 100644 --- a/crates/client/Cargo.toml +++ b/crates/client/Cargo.toml @@ -19,7 +19,7 @@ util = { path = "../util" } rpc = { path = "../rpc" } text = { path = "../text" } settings = { path = "../settings" } -staff_mode = { path = "../staff_mode" } +feature_flags = { path = "../feature_flags" } sum_tree = { path = "../sum_tree" } anyhow.workspace = true diff --git a/crates/client/src/telemetry.rs b/crates/client/src/telemetry.rs index 48886377ba..9cc5d13af0 100644 --- a/crates/client/src/telemetry.rs +++ b/crates/client/src/telemetry.rs @@ -135,8 +135,6 @@ impl Telemetry { } } - /// This method takes the entire TelemetrySettings struct in order to force client code - /// to pull the struct out of the settings global. Do not remove! pub fn set_authenticated_user_info( self: &Arc, metrics_id: Option, diff --git a/crates/client/src/user.rs b/crates/client/src/user.rs index 1dc384da17..5f13aa40ac 100644 --- a/crates/client/src/user.rs +++ b/crates/client/src/user.rs @@ -1,11 +1,11 @@ use super::{proto, Client, Status, TypedEnvelope}; use anyhow::{anyhow, Context, Result}; use collections::{hash_map::Entry, HashMap, HashSet}; +use feature_flags::FeatureFlagAppExt; use futures::{channel::mpsc, future, AsyncReadExt, Future, StreamExt}; use gpui::{AsyncAppContext, Entity, ImageData, ModelContext, ModelHandle, Task}; use postage::{sink::Sink, watch}; use rpc::proto::{RequestMessage, UsersResponse}; -use staff_mode::StaffMode; use std::sync::{Arc, Weak}; use util::http::HttpClient; use util::TryFutureExt as _; @@ -145,26 +145,23 @@ impl UserStore { let fetch_metrics_id = client.request(proto::GetPrivateUserInfo {}).log_err(); let (user, info) = futures::join!(fetch_user, fetch_metrics_id); - cx.read(|cx| { - client.telemetry.set_authenticated_user_info( - info.as_ref().map(|info| info.metrics_id.clone()), - info.as_ref().map(|info| info.staff).unwrap_or(false), - cx, - ) - }); - cx.update(|cx| { - cx.update_default_global(|staff_mode: &mut StaffMode, _| { - if !staff_mode.0 { - *staff_mode = StaffMode( - info.as_ref() - .map(|info| info.staff) - .unwrap_or_default(), - ) - } - () + if let Some(info) = info { + cx.update(|cx| { + cx.update_flags(info.staff, info.flags); + client.telemetry.set_authenticated_user_info( + Some(info.metrics_id.clone()), + info.staff, + cx, + ) }); - }); + } else { + cx.read(|cx| { + client + .telemetry + .set_authenticated_user_info(None, false, cx) + }); + } current_user_tx.send(user).await.ok(); diff --git a/crates/collab_ui/Cargo.toml b/crates/collab_ui/Cargo.toml index 1ecb4b8422..da32308558 100644 --- a/crates/collab_ui/Cargo.toml +++ b/crates/collab_ui/Cargo.toml @@ -40,7 +40,7 @@ picker = { path = "../picker" } project = { path = "../project" } recent_projects = {path = "../recent_projects"} settings = { path = "../settings" } -staff_mode = {path = "../staff_mode"} +feature_flags = {path = "../feature_flags"} theme = { path = "../theme" } theme_selector = { path = "../theme_selector" } vcs_menu = { path = "../vcs_menu" } diff --git a/crates/collab_ui/src/collab_panel.rs b/crates/collab_ui/src/collab_panel.rs index 411a3a2598..0593bfcb1f 100644 --- a/crates/collab_ui/src/collab_panel.rs +++ b/crates/collab_ui/src/collab_panel.rs @@ -9,6 +9,8 @@ use client::{proto::PeerId, Client, Contact, User, UserStore}; use context_menu::{ContextMenu, ContextMenuItem}; use db::kvp::KEY_VALUE_STORE; use editor::{Cancel, Editor}; + +use feature_flags::{ChannelsAlpha, FeatureFlagAppExt, FeatureFlagViewExt}; use futures::StreamExt; use fuzzy::{match_strings, StringMatchCandidate}; use gpui::{ @@ -33,7 +35,6 @@ use panel_settings::{CollaborationPanelDockPosition, CollaborationPanelSettings} use project::{Fs, Project}; use serde_derive::{Deserialize, Serialize}; use settings::SettingsStore; -use staff_mode::StaffMode; use std::{borrow::Cow, mem, sync::Arc}; use theme::{components::ComponentExt, IconButton}; use util::{iife, ResultExt, TryFutureExt}; @@ -182,9 +183,9 @@ pub struct CollabPanel { } #[derive(Serialize, Deserialize)] -struct SerializedChannelsPanel { +struct SerializedCollabPanel { width: Option, - collapsed_channels: Vec, + collapsed_channels: Option>, } #[derive(Debug)] @@ -472,9 +473,10 @@ impl CollabPanel { })); this.subscriptions .push(cx.observe(&active_call, |this, _, cx| this.update_entries(true, cx))); - this.subscriptions.push( - cx.observe_global::(move |this, cx| this.update_entries(true, cx)), - ); + this.subscriptions + .push(cx.observe_flag::(move |_, this, cx| { + this.update_entries(true, cx) + })); this.subscriptions.push(cx.subscribe( &this.channel_store, |this, _channel_store, e, cx| match e { @@ -510,7 +512,7 @@ impl CollabPanel { .log_err() .flatten() { - Some(serde_json::from_str::(&panel)?) + Some(serde_json::from_str::(&panel)?) } else { None }; @@ -520,7 +522,9 @@ impl CollabPanel { if let Some(serialized_panel) = serialized_panel { panel.update(cx, |panel, cx| { panel.width = serialized_panel.width; - panel.collapsed_channels = serialized_panel.collapsed_channels; + panel.collapsed_channels = serialized_panel + .collapsed_channels + .unwrap_or_else(|| Vec::new()); cx.notify(); }); } @@ -537,9 +541,9 @@ impl CollabPanel { KEY_VALUE_STORE .write_kvp( COLLABORATION_PANEL_KEY.into(), - serde_json::to_string(&SerializedChannelsPanel { + serde_json::to_string(&SerializedCollabPanel { width, - collapsed_channels, + collapsed_channels: Some(collapsed_channels), })?, ) .await?; @@ -672,7 +676,8 @@ impl CollabPanel { } let mut request_entries = Vec::new(); - if self.include_channels_section(cx) { + + if cx.has_flag::() { self.entries.push(ListEntry::Header(Section::Channels, 0)); if channel_store.channel_count() > 0 || self.channel_editing_state.is_some() { @@ -1909,14 +1914,6 @@ impl CollabPanel { .into_any() } - fn include_channels_section(&self, cx: &AppContext) -> bool { - if cx.has_global::() { - cx.global::().0 - } else { - false - } - } - fn deploy_channel_context_menu( &mut self, position: Option, diff --git a/crates/staff_mode/Cargo.toml b/crates/feature_flags/Cargo.toml similarity index 71% rename from crates/staff_mode/Cargo.toml rename to crates/feature_flags/Cargo.toml index 2193bd11b1..af273fe403 100644 --- a/crates/staff_mode/Cargo.toml +++ b/crates/feature_flags/Cargo.toml @@ -1,11 +1,11 @@ [package] -name = "staff_mode" +name = "feature_flags" version = "0.1.0" edition = "2021" publish = false [lib] -path = "src/staff_mode.rs" +path = "src/feature_flags.rs" [dependencies] gpui = { path = "../gpui" } diff --git a/crates/feature_flags/src/feature_flags.rs b/crates/feature_flags/src/feature_flags.rs new file mode 100644 index 0000000000..d14152b04c --- /dev/null +++ b/crates/feature_flags/src/feature_flags.rs @@ -0,0 +1,79 @@ +use gpui::{AppContext, Subscription, ViewContext}; + +#[derive(Default)] +struct FeatureFlags { + flags: Vec, + staff: bool, +} + +impl FeatureFlags { + fn has_flag(&self, flag: &str) -> bool { + self.staff || self.flags.iter().find(|f| f.as_str() == flag).is_some() + } +} + +pub trait FeatureFlag { + const NAME: &'static str; +} + +pub enum ChannelsAlpha {} + +impl FeatureFlag for ChannelsAlpha { + const NAME: &'static str = "channels_alpha"; +} + +pub trait FeatureFlagViewExt { + fn observe_flag(&mut self, callback: F) -> Subscription + where + F: Fn(bool, &mut V, &mut ViewContext) + 'static; +} + +impl FeatureFlagViewExt for ViewContext<'_, '_, V> { + fn observe_flag(&mut self, callback: F) -> Subscription + where + F: Fn(bool, &mut V, &mut ViewContext) + 'static, + { + self.observe_global::(move |v, cx| { + let feature_flags = cx.global::(); + callback(feature_flags.has_flag(::NAME), v, cx); + }) + } +} + +pub trait FeatureFlagAppExt { + fn update_flags(&mut self, staff: bool, flags: Vec); + fn set_staff(&mut self, staff: bool); + fn has_flag(&self) -> bool; + fn is_staff(&self) -> bool; +} + +impl FeatureFlagAppExt for AppContext { + fn update_flags(&mut self, staff: bool, flags: Vec) { + self.update_default_global::(|feature_flags, _| { + feature_flags.staff = staff; + feature_flags.flags = flags; + }) + } + + fn set_staff(&mut self, staff: bool) { + self.update_default_global::(|feature_flags, _| { + feature_flags.staff = staff; + }) + } + + fn has_flag(&self) -> bool { + if self.has_global::() { + self.global::().has_flag(T::NAME) + } else { + false + } + } + + fn is_staff(&self) -> bool { + if self.has_global::() { + return self.global::().staff; + } else { + false + } + } +} diff --git a/crates/rpc/src/rpc.rs b/crates/rpc/src/rpc.rs index d64cbae929..bc9dd6f80b 100644 --- a/crates/rpc/src/rpc.rs +++ b/crates/rpc/src/rpc.rs @@ -6,4 +6,4 @@ pub use conn::Connection; pub use peer::*; mod macros; -pub const PROTOCOL_VERSION: u32 = 62; +pub const PROTOCOL_VERSION: u32 = 61; diff --git a/crates/settings/Cargo.toml b/crates/settings/Cargo.toml index 06b81a0c61..f89b80902d 100644 --- a/crates/settings/Cargo.toml +++ b/crates/settings/Cargo.toml @@ -16,7 +16,7 @@ collections = { path = "../collections" } gpui = { path = "../gpui" } sqlez = { path = "../sqlez" } fs = { path = "../fs" } -staff_mode = { path = "../staff_mode" } +feature_flags = { path = "../feature_flags" } util = { path = "../util" } anyhow.workspace = true diff --git a/crates/staff_mode/src/staff_mode.rs b/crates/staff_mode/src/staff_mode.rs deleted file mode 100644 index 49fadc0b2c..0000000000 --- a/crates/staff_mode/src/staff_mode.rs +++ /dev/null @@ -1,36 +0,0 @@ -use gpui::AppContext; - -#[derive(Debug, Default)] -pub struct StaffMode(pub bool); - -impl std::ops::Deref for StaffMode { - type Target = bool; - - fn deref(&self) -> &Self::Target { - &self.0 - } -} - -/// Despite what the type system requires me to tell you, the init function will only be called a once -/// as soon as we know that the staff mode is enabled. -pub fn staff_mode(cx: &mut AppContext, mut init: F) { - if **cx.default_global::() { - init(cx) - } else { - let mut once = Some(()); - cx.observe_global::(move |cx| { - if **cx.global::() && once.take().is_some() { - init(cx); - } - }) - .detach(); - } -} - -/// Immediately checks and runs the init function if the staff mode is not enabled. -/// This is only included for symettry with staff_mode() above -pub fn not_staff_mode(cx: &mut AppContext, init: F) { - if !**cx.default_global::() { - init(cx) - } -} diff --git a/crates/theme_selector/Cargo.toml b/crates/theme_selector/Cargo.toml index 377f64aad6..7e97d39186 100644 --- a/crates/theme_selector/Cargo.toml +++ b/crates/theme_selector/Cargo.toml @@ -16,7 +16,7 @@ gpui = { path = "../gpui" } picker = { path = "../picker" } theme = { path = "../theme" } settings = { path = "../settings" } -staff_mode = { path = "../staff_mode" } +feature_flags = { path = "../feature_flags" } workspace = { path = "../workspace" } util = { path = "../util" } log.workspace = true diff --git a/crates/theme_selector/src/theme_selector.rs b/crates/theme_selector/src/theme_selector.rs index 5510005733..1969b0256a 100644 --- a/crates/theme_selector/src/theme_selector.rs +++ b/crates/theme_selector/src/theme_selector.rs @@ -1,9 +1,9 @@ +use feature_flags::FeatureFlagAppExt; use fs::Fs; use fuzzy::{match_strings, StringMatch, StringMatchCandidate}; use gpui::{actions, elements::*, AnyElement, AppContext, Element, MouseState, ViewContext}; use picker::{Picker, PickerDelegate, PickerEvent}; use settings::{update_settings_file, SettingsStore}; -use staff_mode::StaffMode; use std::sync::Arc; use theme::{Theme, ThemeMeta, ThemeRegistry, ThemeSettings}; use util::ResultExt; @@ -54,7 +54,7 @@ impl ThemeSelectorDelegate { fn new(fs: Arc, cx: &mut ViewContext) -> Self { let original_theme = theme::current(cx).clone(); - let staff_mode = **cx.default_global::(); + let staff_mode = cx.is_staff(); let registry = cx.global::>(); let mut theme_names = registry.list(staff_mode).collect::>(); theme_names.sort_unstable_by(|a, b| a.is_light.cmp(&b.is_light).then(a.name.cmp(&b.name))); diff --git a/crates/zed/Cargo.toml b/crates/zed/Cargo.toml index 92900f84cb..2a97764647 100644 --- a/crates/zed/Cargo.toml +++ b/crates/zed/Cargo.toml @@ -60,7 +60,7 @@ quick_action_bar = { path = "../quick_action_bar" } recent_projects = { path = "../recent_projects" } rpc = { path = "../rpc" } settings = { path = "../settings" } -staff_mode = { path = "../staff_mode" } +feature_flags = { path = "../feature_flags" } sum_tree = { path = "../sum_tree" } text = { path = "../text" } terminal_view = { path = "../terminal_view" } diff --git a/crates/zed/src/languages/json.rs b/crates/zed/src/languages/json.rs index b7e4ab4ba7..61d19ce5b6 100644 --- a/crates/zed/src/languages/json.rs +++ b/crates/zed/src/languages/json.rs @@ -1,6 +1,7 @@ use anyhow::{anyhow, Result}; use async_trait::async_trait; use collections::HashMap; +use feature_flags::FeatureFlagAppExt; use futures::{future::BoxFuture, FutureExt, StreamExt}; use gpui::AppContext; use language::{LanguageRegistry, LanguageServerName, LspAdapter, LspAdapterDelegate}; @@ -9,7 +10,6 @@ use node_runtime::NodeRuntime; use serde_json::json; use settings::{KeymapFile, SettingsJsonSchemaParams, SettingsStore}; use smol::fs; -use staff_mode::StaffMode; use std::{ any::Any, ffi::OsString, @@ -104,7 +104,7 @@ impl LspAdapter for JsonLspAdapter { cx: &mut AppContext, ) -> Option> { let action_names = cx.all_action_names().collect::>(); - let staff_mode = cx.default_global::().0; + let staff_mode = cx.is_staff(); let language_names = &self.languages.language_names(); let settings_schema = cx.global::().json_schema( &SettingsJsonSchemaParams { diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 3b1fccb927..da726eef65 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -53,8 +53,6 @@ use uuid::Uuid; use welcome::{show_welcome_experience, FIRST_OPEN}; use fs::RealFs; -#[cfg(debug_assertions)] -use staff_mode::StaffMode; use util::{channel::RELEASE_CHANNEL, paths, ResultExt, TryFutureExt}; use workspace::AppState; use zed::{ @@ -122,7 +120,10 @@ fn main() { cx.set_global(*RELEASE_CHANNEL); #[cfg(debug_assertions)] - cx.set_global(StaffMode(true)); + { + use feature_flags::FeatureFlagAppExt; + cx.set_staff(true); + } let mut store = SettingsStore::default(); store