diff --git a/Cargo.lock b/Cargo.lock index ecf93e2c0536bee05e23226c2dd21f28eebf5c26..14d247645e9dcd1beafce69e5f3ddec9cab9abfe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -439,6 +439,7 @@ dependencies = [ "semantic_index", "serde", "serde_json", + "serde_json_lenient", "settings", "similar", "smol", diff --git a/crates/assistant/Cargo.toml b/crates/assistant/Cargo.toml index 6cf00085cfdb84d7d9d1c58f062a674244967ab2..29e460b8a6c22af7f4b69d52bf1175400bcbccde 100644 --- a/crates/assistant/Cargo.toml +++ b/crates/assistant/Cargo.toml @@ -85,5 +85,6 @@ language = { workspace = true, features = ["test-support"] } log.workspace = true project = { workspace = true, features = ["test-support"] } rand.workspace = true +serde_json_lenient.workspace = true text = { workspace = true, features = ["test-support"] } unindent.workspace = true diff --git a/crates/assistant/src/assistant_settings.rs b/crates/assistant/src/assistant_settings.rs index 3242096d814de79ea8abb8fb0086388403acc45d..a438e842d626aa7b9617e55ae420ae26b6afcd8a 100644 --- a/crates/assistant/src/assistant_settings.rs +++ b/crates/assistant/src/assistant_settings.rs @@ -456,6 +456,8 @@ pub struct LegacyAssistantSettingsContent { impl Settings for AssistantSettings { const KEY: Option<&'static str> = Some("assistant"); + const PRESERVED_KEYS: Option<&'static [&'static str]> = Some(&["version"]); + type FileContent = AssistantSettingsContent; fn load( @@ -497,103 +499,70 @@ fn merge(target: &mut T, value: Option) { } } -// #[cfg(test)] -// mod tests { -// use gpui::{AppContext, UpdateGlobal}; -// use settings::SettingsStore; - -// use super::*; - -// #[gpui::test] -// fn test_deserialize_assistant_settings(cx: &mut AppContext) { -// let store = settings::SettingsStore::test(cx); -// cx.set_global(store); - -// // Settings default to gpt-4-turbo. -// AssistantSettings::register(cx); -// assert_eq!( -// AssistantSettings::get_global(cx).provider, -// AssistantProvider::OpenAi { -// model: OpenAiModel::FourOmni, -// api_url: open_ai::OPEN_AI_API_URL.into(), -// low_speed_timeout_in_seconds: None, -// available_models: Default::default(), -// } -// ); - -// // Ensure backward-compatibility. -// SettingsStore::update_global(cx, |store, cx| { -// store -// .set_user_settings( -// r#"{ -// "assistant": { -// "openai_api_url": "test-url", -// } -// }"#, -// cx, -// ) -// .unwrap(); -// }); -// assert_eq!( -// AssistantSettings::get_global(cx).provider, -// AssistantProvider::OpenAi { -// model: OpenAiModel::FourOmni, -// api_url: "test-url".into(), -// low_speed_timeout_in_seconds: None, -// available_models: Default::default(), -// } -// ); -// SettingsStore::update_global(cx, |store, cx| { -// store -// .set_user_settings( -// r#"{ -// "assistant": { -// "default_open_ai_model": "gpt-4-0613" -// } -// }"#, -// cx, -// ) -// .unwrap(); -// }); -// assert_eq!( -// AssistantSettings::get_global(cx).provider, -// AssistantProvider::OpenAi { -// model: OpenAiModel::Four, -// api_url: open_ai::OPEN_AI_API_URL.into(), -// low_speed_timeout_in_seconds: None, -// available_models: Default::default(), -// } -// ); - -// // The new version supports setting a custom model when using zed.dev. -// SettingsStore::update_global(cx, |store, cx| { -// store -// .set_user_settings( -// r#"{ -// "assistant": { -// "version": "1", -// "provider": { -// "name": "zed.dev", -// "default_model": { -// "custom": { -// "name": "custom-provider" -// } -// } -// } -// } -// }"#, -// cx, -// ) -// .unwrap(); -// }); -// assert_eq!( -// AssistantSettings::get_global(cx).provider, -// AssistantProvider::ZedDotDev { -// model: CloudModel::Custom { -// name: "custom-provider".into(), -// max_tokens: None -// } -// } -// ); -// } -// } +#[cfg(test)] +mod tests { + use gpui::{ReadGlobal, TestAppContext}; + + use super::*; + + #[gpui::test] + async fn test_deserialize_assistant_settings_with_version(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 test_settings = settings::SettingsStore::test(cx); + cx.set_global(test_settings); + AssistantSettings::register(cx); + }); + + cx.update(|cx| { + assert!(!AssistantSettings::get_global(cx).using_outdated_settings_version); + assert_eq!( + AssistantSettings::get_global(cx).default_model, + AssistantDefaultModel { + provider: "openai".into(), + model: "gpt-4o".into(), + } + ); + }); + + cx.update(|cx| { + settings::SettingsStore::global(cx).update_settings_file::( + fs.clone(), + |settings, _| { + *settings = AssistantSettingsContent::Versioned( + VersionedAssistantSettingsContent::V2(AssistantSettingsContentV2 { + default_model: Some(AssistantDefaultModel { + provider: "test-provider".into(), + model: "gpt-99".into(), + }), + enabled: None, + button: None, + dock: None, + default_width: None, + default_height: None, + }), + ) + }, + ); + }); + + cx.run_until_parked(); + + let raw_settings_value = fs.load(paths::settings_file()).await.unwrap(); + assert!(raw_settings_value.contains(r#""version": "2""#)); + + #[derive(Debug, Deserialize)] + struct AssistantSettingsTest { + assistant: AssistantSettingsContent, + } + + let assistant_settings: AssistantSettingsTest = + serde_json_lenient::from_str(&raw_settings_value).unwrap(); + + assert!(!assistant_settings.assistant.is_version_outdated()); + } +} diff --git a/crates/settings/src/settings_store.rs b/crates/settings/src/settings_store.rs index 2833d31f677f9ce579f209dded430adce1861b3c..daad11c1ca35af36feba889184a729b29d37f4aa 100644 --- a/crates/settings/src/settings_store.rs +++ b/crates/settings/src/settings_store.rs @@ -28,6 +28,14 @@ pub trait Settings: 'static + Send + Sync { /// from the root object. const KEY: Option<&'static str>; + /// The name of the keys in the [`FileContent`] that should always be written to + /// a settings file, even if their value matches the default value. + /// + /// This is useful for tagged [`FileContent`]s where the tag is a "version" field + /// that should always be persisted, even if the current user settings match the + /// current version of the settings. + const PRESERVED_KEYS: Option<&'static [&'static str]> = None; + /// The type that is stored in an individual JSON file. type FileContent: Clone + Default + Serialize + DeserializeOwned + JsonSchema; @@ -407,6 +415,8 @@ impl SettingsStore { ) -> Vec<(Range, String)> { let setting_type_id = TypeId::of::(); + let preserved_keys = T::PRESERVED_KEYS.unwrap_or_default(); + let setting = self .setting_values .get(&setting_type_id) @@ -436,6 +446,7 @@ impl SettingsStore { tab_size, &old_value, &new_value, + preserved_keys, &mut edits, ); edits @@ -874,6 +885,7 @@ fn update_value_in_json_text<'a>( tab_size: usize, old_value: &'a serde_json::Value, new_value: &'a serde_json::Value, + preserved_keys: &[&str], edits: &mut Vec<(Range, String)>, ) { // If the old and new values are both objects, then compare them key by key, @@ -891,6 +903,7 @@ fn update_value_in_json_text<'a>( tab_size, old_sub_value, new_sub_value, + preserved_keys, edits, ); key_path.pop(); @@ -904,12 +917,17 @@ fn update_value_in_json_text<'a>( tab_size, &serde_json::Value::Null, new_sub_value, + preserved_keys, edits, ); } key_path.pop(); } - } else if old_value != new_value { + } else if key_path + .last() + .map_or(false, |key| preserved_keys.contains(&key)) + || old_value != new_value + { let mut new_value = new_value.clone(); if let Some(new_object) = new_value.as_object_mut() { new_object.retain(|_, v| !v.is_null());