Fix server settings (#38477)

Conrad Irwin created 1 month ago

In the settings refactor I'd assumed server settings were like project
settings. This is not the case, they are in fact the normal user
settings;
but just read from the server.

Release Notes:

- N/A

Change summary

Cargo.lock                                       |  1 
crates/editor/Cargo.toml                         |  1 
crates/editor/src/editor.rs                      |  8 +--
crates/project/src/project_settings.rs           |  5 -
crates/remote_server/src/remote_editing_tests.rs |  7 +-
crates/settings/src/settings_content.rs          |  7 --
crates/settings/src/settings_store.rs            | 44 +----------------
7 files changed, 13 insertions(+), 60 deletions(-)

Detailed changes

Cargo.lock

@@ -5295,6 +5295,7 @@ dependencies = [
  "url",
  "util",
  "uuid",
+ "vim_mode_setting",
  "workspace",
  "workspace-hack",
  "zed_actions",

crates/editor/Cargo.toml

@@ -89,6 +89,7 @@ ui.workspace = true
 url.workspace = true
 util.workspace = true
 uuid.workspace = true
+vim_mode_setting.workspace = true
 workspace.workspace = true
 zed_actions.workspace = true
 workspace-hack.workspace = true

crates/editor/src/editor.rs

@@ -21678,12 +21678,10 @@ impl Editor {
     }
 }
 
-// todo(settings_refactor) this should not be!
 fn vim_enabled(cx: &App) -> bool {
-    cx.global::<SettingsStore>()
-        .raw_user_settings()
-        .and_then(|settings| settings.content.vim_mode)
-        == Some(true)
+    vim_mode_setting::VimModeSetting::try_get(cx)
+        .map(|vim_mode| vim_mode.0)
+        .unwrap_or(false)
 }
 
 fn process_completion_for_edit(

crates/project/src/project_settings.rs

@@ -770,12 +770,9 @@ impl SettingsObserver {
         envelope: TypedEnvelope<proto::UpdateUserSettings>,
         cx: AsyncApp,
     ) -> anyhow::Result<()> {
-        let new_settings = serde_json::from_str(&envelope.payload.contents).with_context(|| {
-            format!("deserializing {} user settings", envelope.payload.contents)
-        })?;
         cx.update_global(|settings_store: &mut SettingsStore, cx| {
             settings_store
-                .set_raw_user_settings(new_settings, cx)
+                .set_user_settings(&envelope.payload.contents, cx)
                 .context("setting new user settings")?;
             anyhow::Ok(())
         })??;

crates/remote_server/src/remote_editing_tests.rs

@@ -1797,8 +1797,8 @@ async fn test_remote_external_agent_server(
     pretty_assertions::assert_eq!(names, ["gemini", "claude"]);
     server_cx.update_global::<SettingsStore, _>(|settings_store, cx| {
         settings_store
-            .set_raw_server_settings(
-                Some(json!({
+            .set_server_settings(
+                &json!({
                     "agent_servers": {
                         "foo": {
                             "command": "foo-cli",
@@ -1808,7 +1808,8 @@ async fn test_remote_external_agent_server(
                             }
                         }
                     }
-                })),
+                })
+                .to_string(),
                 cx,
             )
             .unwrap();

crates/settings/src/settings_content.rs

@@ -166,13 +166,6 @@ impl SettingsContent {
     }
 }
 
-#[skip_serializing_none]
-#[derive(Debug, Default, Serialize, Deserialize, JsonSchema, MergeFrom)]
-pub struct ServerSettingsContent {
-    #[serde(flatten)]
-    pub project: ProjectSettingsContent,
-}
-
 #[skip_serializing_none]
 #[derive(Debug, Default, PartialEq, Clone, Serialize, Deserialize, JsonSchema, MergeFrom)]
 pub struct UserSettingsContent {

crates/settings/src/settings_store.rs

@@ -36,8 +36,7 @@ use crate::{
     merge_from::MergeFrom,
     parse_json_with_comments, replace_value_in_json_text,
     settings_content::{
-        ExtensionsSettingsContent, ProjectSettingsContent, ServerSettingsContent, SettingsContent,
-        UserSettingsContent,
+        ExtensionsSettingsContent, ProjectSettingsContent, SettingsContent, UserSettingsContent,
     },
     update_value_in_json_text,
 };
@@ -327,33 +326,6 @@ impl SettingsStore {
         self.user_settings.as_ref()
     }
 
-    /// Replaces current settings with the values from the given JSON.
-    pub fn set_raw_user_settings(
-        &mut self,
-        new_settings: UserSettingsContent,
-        cx: &mut App,
-    ) -> Result<()> {
-        self.user_settings = Some(new_settings);
-        self.recompute_values(None, cx)?;
-        Ok(())
-    }
-
-    /// Replaces current settings with the values from the given JSON.
-    pub fn set_raw_server_settings(
-        &mut self,
-        new_settings: Option<Value>,
-        cx: &mut App,
-    ) -> Result<()> {
-        // Rewrite the server settings into a content type
-        self.server_settings = new_settings
-            .map(|settings| settings.to_string())
-            .and_then(|str| parse_json_with_comments::<SettingsContent>(&str).ok())
-            .map(Box::new);
-
-        self.recompute_values(None, cx)?;
-        Ok(())
-    }
-
     /// Get the configured settings profile names.
     pub fn configured_settings_profiles(&self) -> impl Iterator<Item = &str> {
         self.user_settings
@@ -361,11 +333,6 @@ impl SettingsStore {
             .flat_map(|settings| settings.profiles.keys().map(|k| k.as_str()))
     }
 
-    /// Access the raw JSON value of the default settings.
-    pub fn raw_default_settings(&self) -> &SettingsContent {
-        &self.default_settings
-    }
-
     #[cfg(any(test, feature = "test-support"))]
     pub fn test(cx: &mut App) -> Self {
         Self::new(cx, &crate::test_settings())
@@ -621,19 +588,14 @@ impl SettingsStore {
         server_settings_content: &str,
         cx: &mut App,
     ) -> Result<()> {
-        let settings: Option<ServerSettingsContent> = if server_settings_content.is_empty() {
+        let settings: Option<SettingsContent> = if server_settings_content.is_empty() {
             None
         } else {
             parse_json_with_comments(server_settings_content)?
         };
 
         // Rewrite the server settings into a content type
-        self.server_settings = settings.map(|settings| {
-            Box::new(SettingsContent {
-                project: settings.project,
-                ..Default::default()
-            })
-        });
+        self.server_settings = settings.map(|settings| Box::new(settings));
 
         self.recompute_values(None, cx)?;
         Ok(())