diff --git a/Cargo.lock b/Cargo.lock index e4cbfa125c..a333386afe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -608,6 +608,7 @@ dependencies = [ "paths", "schemars", "serde", + "serde_json", "serde_json_lenient", "settings", "workspace-hack", diff --git a/crates/assistant_settings/Cargo.toml b/crates/assistant_settings/Cargo.toml index 763be50bc0..6c0cf3709b 100644 --- a/crates/assistant_settings/Cargo.toml +++ b/crates/assistant_settings/Cargo.toml @@ -33,4 +33,5 @@ fs.workspace = true gpui = { workspace = true, features = ["test-support"] } paths.workspace = true serde_json_lenient.workspace = true +serde_json.workspace = true settings = { workspace = true, features = ["test-support"] } diff --git a/crates/assistant_settings/src/assistant_settings.rs b/crates/assistant_settings/src/assistant_settings.rs index 4d6ee1eea3..07b63121e1 100644 --- a/crates/assistant_settings/src/assistant_settings.rs +++ b/crates/assistant_settings/src/assistant_settings.rs @@ -695,6 +695,8 @@ pub struct LegacyAssistantSettingsContent { impl Settings for AssistantSettings { const KEY: Option<&'static str> = Some("agent"); + const FALLBACK_KEY: Option<&'static str> = Some("assistant"); + const PRESERVED_KEYS: Option<&'static [&'static str]> = Some(&["version"]); type FileContent = AssistantSettingsContent; @@ -826,6 +828,7 @@ fn merge(target: &mut T, value: Option) { mod tests { use fs::Fs; use gpui::{ReadGlobal, TestAppContext}; + use settings::SettingsStore; use super::*; @@ -902,4 +905,67 @@ mod tests { assert!(!assistant_settings.agent.is_version_outdated()); } + + #[gpui::test] + async fn test_load_settings_from_old_key(cx: &mut TestAppContext) { + let fs = fs::FakeFs::new(cx.executor().clone()); + fs.create_dir(paths::settings_file().parent().unwrap()) + .await + .unwrap(); + + cx.update(|cx| { + let mut test_settings = settings::SettingsStore::test(cx); + let user_settings_content = r#"{ + "assistant": { + "enabled": true, + "version": "2", + "default_model": { + "provider": "zed.dev", + "model": "gpt-99" + }, + }}"#; + test_settings + .set_user_settings(user_settings_content, cx) + .unwrap(); + cx.set_global(test_settings); + AssistantSettings::register(cx); + }); + + cx.run_until_parked(); + + let assistant_settings = cx.update(|cx| AssistantSettings::get_global(cx).clone()); + assert!(assistant_settings.enabled); + assert!(!assistant_settings.using_outdated_settings_version); + assert_eq!(assistant_settings.default_model.model, "gpt-99"); + + cx.update_global::(|settings_store, cx| { + settings_store.update_user_settings::(cx, |settings| { + *settings = AssistantSettingsContent { + inner: Some(AssistantSettingsContentInner::for_v2( + AssistantSettingsContentV2 { + enabled: Some(false), + default_model: Some(LanguageModelSelection { + provider: "xai".to_owned(), + model: "grok".to_owned(), + }), + ..Default::default() + }, + )), + }; + }); + }); + + cx.run_until_parked(); + + let settings = cx.update(|cx| SettingsStore::global(cx).raw_user_settings().clone()); + + #[derive(Debug, Deserialize)] + struct AssistantSettingsTest { + assistant: AssistantSettingsContent, + agent: Option, + } + + let assistant_settings: AssistantSettingsTest = serde_json::from_value(settings).unwrap(); + assert!(assistant_settings.agent.is_none()); + } } diff --git a/crates/settings/src/settings_store.rs b/crates/settings/src/settings_store.rs index 3b845574a9..ff1d8089ba 100644 --- a/crates/settings/src/settings_store.rs +++ b/crates/settings/src/settings_store.rs @@ -37,6 +37,8 @@ pub trait Settings: 'static + Send + Sync { /// from the root object. const KEY: Option<&'static str>; + const FALLBACK_KEY: Option<&'static str> = None; + /// The name of the keys in the [`FileContent`](Self::FileContent) that should /// always be written to a settings file, even if their value matches the default /// value. @@ -231,7 +233,13 @@ struct SettingValue { trait AnySettingValue: 'static + Send + Sync { fn key(&self) -> Option<&'static str>; fn setting_type_name(&self) -> &'static str; - fn deserialize_setting(&self, json: &Value) -> Result; + fn deserialize_setting(&self, json: &Value) -> Result { + self.deserialize_setting_with_key(json).1 + } + fn deserialize_setting_with_key( + &self, + json: &Value, + ) -> (Option<&'static str>, Result); fn load_setting( &self, sources: SettingsSources, @@ -537,7 +545,8 @@ impl SettingsStore { .get(&setting_type_id) .unwrap_or_else(|| panic!("unregistered setting type {}", type_name::())); let raw_settings = parse_json_with_comments::(text).unwrap_or_default(); - let old_content = match setting.deserialize_setting(&raw_settings) { + let (key, deserialized_setting) = setting.deserialize_setting_with_key(&raw_settings); + let old_content = match deserialized_setting { Ok(content) => content.0.downcast::().unwrap(), Err(_) => Box::<::FileContent>::default(), }; @@ -548,7 +557,7 @@ impl SettingsStore { let new_value = serde_json::to_value(new_content).unwrap(); let mut key_path = Vec::new(); - if let Some(key) = T::KEY { + if let Some(key) = key { key_path.push(key); } @@ -1153,17 +1162,27 @@ impl AnySettingValue for SettingValue { )?)) } - fn deserialize_setting(&self, mut json: &Value) -> Result { - if let Some(key) = T::KEY { - if let Some(value) = json.get(key) { + fn deserialize_setting_with_key( + &self, + mut json: &Value, + ) -> (Option<&'static str>, Result) { + let mut key = None; + if let Some(k) = T::KEY { + if let Some(value) = json.get(k) { json = value; + key = Some(k); + } else if let Some((k, value)) = T::FALLBACK_KEY.and_then(|k| Some((k, json.get(k)?))) { + json = value; + key = Some(k); } else { let value = T::FileContent::default(); - return Ok(DeserializedSetting(Box::new(value))); + return (T::KEY, Ok(DeserializedSetting(Box::new(value)))); } } - let value = T::FileContent::deserialize(json)?; - Ok(DeserializedSetting(Box::new(value))) + let value = T::FileContent::deserialize(json) + .map(|value| DeserializedSetting(Box::new(value))) + .map_err(anyhow::Error::from); + (key, value) } fn value_for_path(&self, path: Option) -> &dyn Any { @@ -1211,7 +1230,8 @@ impl AnySettingValue for SettingValue { text: &mut String, edits: &mut Vec<(Range, String)>, ) { - let old_content = match self.deserialize_setting(raw_settings) { + let (key, deserialized_setting) = self.deserialize_setting_with_key(raw_settings); + let old_content = match deserialized_setting { Ok(content) => content.0.downcast::().unwrap(), Err(_) => Box::<::FileContent>::default(), }; @@ -1222,7 +1242,7 @@ impl AnySettingValue for SettingValue { let new_value = serde_json::to_value(new_content).unwrap(); let mut key_path = Vec::new(); - if let Some(key) = T::KEY { + if let Some(key) = key { key_path.push(key); }