Cargo.lock 🔗
@@ -608,6 +608,7 @@ dependencies = [
"paths",
"schemars",
"serde",
+ "serde_json",
"serde_json_lenient",
"settings",
"workspace-hack",
Fall back to old key when loading agent settings (#30001)
Cole Miller created
This PR updates #29943 to fall back to loading agent panel settings from
the old `assistant` key if the `agent` key is not present. Edits to
these settings will also target `assistant` in this situation instead of
`agent` as before.
Release Notes:
- Agent Beta: Fixed a regression that caused the agent panel not to
load, or buttons in the agent panel not to work.Change summary
Cargo.lock | 1
crates/assistant_settings/Cargo.toml | 1
crates/assistant_settings/src/assistant_settings.rs | 66 ++++++++++++++
crates/settings/src/settings_store.rs | 42 +++++++--
4 files changed, 99 insertions(+), 11 deletions(-)
Detailed changes
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"] }
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<T>(target: &mut T, value: Option<T>) {
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::<SettingsStore, _>(|settings_store, cx| {
+ settings_store.update_user_settings::<AssistantSettings>(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<serde_json_lenient::Value>,
+ }
+
+ let assistant_settings: AssistantSettingsTest = serde_json::from_value(settings).unwrap();
+ assert!(assistant_settings.agent.is_none());
+ }
}
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<T> {
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<DeserializedSetting>;
+ fn deserialize_setting(&self, json: &Value) -> Result<DeserializedSetting> {
+ self.deserialize_setting_with_key(json).1
+ }
+ fn deserialize_setting_with_key(
+ &self,
+ json: &Value,
+ ) -> (Option<&'static str>, Result<DeserializedSetting>);
fn load_setting(
&self,
sources: SettingsSources<DeserializedSetting>,
@@ -537,7 +545,8 @@ impl SettingsStore {
.get(&setting_type_id)
.unwrap_or_else(|| panic!("unregistered setting type {}", type_name::<T>()));
let raw_settings = parse_json_with_comments::<Value>(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::<T::FileContent>().unwrap(),
Err(_) => Box::<<T as Settings>::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<T: Settings> AnySettingValue for SettingValue<T> {
)?))
}
- fn deserialize_setting(&self, mut json: &Value) -> Result<DeserializedSetting> {
- 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<DeserializedSetting>) {
+ 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<SettingsLocation>) -> &dyn Any {
@@ -1211,7 +1230,8 @@ impl<T: Settings> AnySettingValue for SettingValue<T> {
text: &mut String,
edits: &mut Vec<(Range<usize>, 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::<T::FileContent>().unwrap(),
Err(_) => Box::<<T as Settings>::FileContent>::default(),
};
@@ -1222,7 +1242,7 @@ impl<T: Settings> AnySettingValue for SettingValue<T> {
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);
}