diff --git a/crates/agent_ui/src/agent_configuration.rs b/crates/agent_ui/src/agent_configuration.rs index 5f71df75d6287822c77eedfcb2f8fb96487b7950..fda3cb9907b2f02cce29ff0ae8c4762e6efa625a 100644 --- a/crates/agent_ui/src/agent_configuration.rs +++ b/crates/agent_ui/src/agent_configuration.rs @@ -1392,38 +1392,45 @@ async fn open_new_agent_servers_entry_in_settings_editor( let settings = cx.global::(); let mut unique_server_name = None; - let edits = settings.edits_for_update(&text, |settings| { - let server_name: Option = (0..u8::MAX) - .map(|i| { - if i == 0 { - "your_agent".to_string() - } else { - format!("your_agent_{}", i) - } - }) - .find(|name| { - !settings - .agent_servers - .as_ref() - .is_some_and(|agent_servers| agent_servers.contains_key(name.as_str())) - }); - if let Some(server_name) = server_name { - unique_server_name = Some(SharedString::from(server_name.clone())); - settings.agent_servers.get_or_insert_default().insert( - server_name, - settings::CustomAgentServerSettings::Custom { - path: "path_to_executable".into(), - args: vec![], - env: HashMap::default(), - default_mode: None, - default_model: None, - favorite_models: vec![], - default_config_options: Default::default(), - favorite_config_option_values: Default::default(), - }, - ); - } - }); + let Some(edits) = settings + .edits_for_update(&text, |settings| { + let server_name: Option = (0..u8::MAX) + .map(|i| { + if i == 0 { + "your_agent".to_string() + } else { + format!("your_agent_{}", i) + } + }) + .find(|name| { + !settings + .agent_servers + .as_ref() + .is_some_and(|agent_servers| { + agent_servers.contains_key(name.as_str()) + }) + }); + if let Some(server_name) = server_name { + unique_server_name = Some(SharedString::from(server_name.clone())); + settings.agent_servers.get_or_insert_default().insert( + server_name, + settings::CustomAgentServerSettings::Custom { + path: "path_to_executable".into(), + args: vec![], + env: HashMap::default(), + default_mode: None, + default_model: None, + favorite_models: vec![], + default_config_options: Default::default(), + favorite_config_option_values: Default::default(), + }, + ); + } + }) + .log_err() + else { + return; + }; if edits.is_empty() { return; diff --git a/crates/edit_prediction_ui/src/edit_prediction_button.rs b/crates/edit_prediction_ui/src/edit_prediction_button.rs index 377e53da265e4c2b6ada252b68402960f39b18dc..e7aff1271f0505d9c87899cc8b555e377ca3fbd0 100644 --- a/crates/edit_prediction_ui/src/edit_prediction_button.rs +++ b/crates/edit_prediction_ui/src/edit_prediction_button.rs @@ -1357,14 +1357,19 @@ async fn open_disabled_globs_setting_in_editor( let settings = cx.global::(); // Ensure that we always have "edit_predictions { "disabled_globs": [] }" - let edits = settings.edits_for_update(&text, |file| { - file.project - .all_languages - .edit_predictions - .get_or_insert_with(Default::default) - .disabled_globs - .get_or_insert_with(Vec::new); - }); + let Some(edits) = settings + .edits_for_update(&text, |file| { + file.project + .all_languages + .edit_predictions + .get_or_insert_with(Default::default) + .disabled_globs + .get_or_insert_with(Vec::new); + }) + .log_err() + else { + return; + }; if !edits.is_empty() { item.edit( diff --git a/crates/settings/src/settings_store.rs b/crates/settings/src/settings_store.rs index cc5a9105d69d2b35cdff6f10b49e08e0f9a506cb..d157b29abd1958ae47fd340effa698b11e8e04af 100644 --- a/crates/settings/src/settings_store.rs +++ b/crates/settings/src/settings_store.rs @@ -542,9 +542,9 @@ impl SettingsStore { update: impl 'static + Send + FnOnce(&mut SettingsContent, &App), ) { _ = self.update_settings_file_inner(fs, move |old_text: String, cx: AsyncApp| { - Ok(cx.read_global(|store: &SettingsStore, cx| { + cx.read_global(|store: &SettingsStore, cx| { store.new_text_for_update(old_text, |content| update(content, cx)) - })) + }) }); } @@ -554,9 +554,9 @@ impl SettingsStore { vscode_settings: VsCodeSettings, ) -> oneshot::Receiver> { self.update_settings_file_inner(fs, move |old_text: String, cx: AsyncApp| { - Ok(cx.read_global(|store: &SettingsStore, _cx| { + cx.read_global(|store: &SettingsStore, _cx| { store.get_vscode_edits(old_text, &vscode_settings) - })) + }) }) } @@ -747,16 +747,16 @@ impl SettingsStore { &self, old_text: String, update: impl FnOnce(&mut SettingsContent), - ) -> String { - let edits = self.edits_for_update(&old_text, update); + ) -> Result { + let edits = self.edits_for_update(&old_text, update)?; let mut new_text = old_text; for (range, replacement) in edits.into_iter() { new_text.replace_range(range, &replacement); } - new_text + Ok(new_text) } - pub fn get_vscode_edits(&self, old_text: String, vscode: &VsCodeSettings) -> String { + pub fn get_vscode_edits(&self, old_text: String, vscode: &VsCodeSettings) -> Result { self.new_text_for_update(old_text, |content| { content.merge_from(&vscode.settings_content()) }) @@ -768,10 +768,17 @@ impl SettingsStore { &self, text: &str, update: impl FnOnce(&mut SettingsContent), - ) -> Vec<(Range, String)> { - let old_content = UserSettingsContent::parse_json_with_comments(text) - .log_err() - .unwrap_or_default(); + ) -> Result, String)>> { + let old_content = if text.trim().is_empty() { + UserSettingsContent::default() + } else { + let (old_content, parse_status) = UserSettingsContent::parse_json(text); + if let ParseStatus::Failed { error } = &parse_status { + log::error!("Failed to parse settings for update: {error}"); + } + old_content + .context("Settings file could not be parsed. Fix syntax errors before updating.")? + }; let mut new_content = old_content.clone(); update(&mut new_content.content); @@ -790,7 +797,7 @@ impl SettingsStore { &new_value, &mut edits, ); - edits + Ok(edits) } /// Mutates the default settings in place and recomputes all setting values. @@ -1699,7 +1706,7 @@ mod tests { cx: &mut App, ) { store.set_user_settings(&old_json, cx).ok(); - let edits = store.edits_for_update(&old_json, update); + let edits = store.edits_for_update(&old_json, update).unwrap(); let mut new_json = old_json; for (range, replacement) in edits.into_iter() { new_json.replace_range(range, &replacement); @@ -1887,6 +1894,39 @@ mod tests { ); } + #[gpui::test] + fn test_edits_for_update_preserves_unknown_keys(cx: &mut App) { + let mut store = SettingsStore::new(cx, &test_settings()); + store.register_setting::(); + + let old_json = r#"{ + "some_unknown_key": "should_be_preserved", + "auto_update": false + }"# + .unindent(); + + check_settings_update( + &mut store, + old_json, + |settings| settings.auto_update = Some(true), + r#"{ + "some_unknown_key": "should_be_preserved", + "auto_update": true + }"# + .unindent(), + cx, + ); + } + + #[gpui::test] + fn test_edits_for_update_returns_error_on_invalid_json(cx: &mut App) { + let store = SettingsStore::new(cx, &test_settings()); + + let invalid_json = r#"{ this is not valid json at all !!!"#; + let result = store.edits_for_update(invalid_json, |_| {}); + assert!(result.is_err()); + } + #[gpui::test] fn test_vscode_import(cx: &mut App) { let mut store = SettingsStore::new(cx, &test_settings()); @@ -2007,10 +2047,12 @@ mod tests { cx: &mut App, ) { store.set_user_settings(&old, cx).ok(); - let new = store.get_vscode_edits( - old, - &VsCodeSettings::from_str(&vscode, VsCodeSettingsSource::VsCode).unwrap(), - ); + let new = store + .get_vscode_edits( + old, + &VsCodeSettings::from_str(&vscode, VsCodeSettingsSource::VsCode).unwrap(), + ) + .unwrap(); pretty_assertions::assert_eq!(new, expected); } @@ -2018,14 +2060,16 @@ mod tests { fn test_update_git_settings(cx: &mut App) { let store = SettingsStore::new(cx, &test_settings()); - let actual = store.new_text_for_update("{}".to_string(), |current| { - current - .git - .get_or_insert_default() - .inline_blame - .get_or_insert_default() - .enabled = Some(true); - }); + let actual = store + .new_text_for_update("{}".to_string(), |current| { + current + .git + .get_or_insert_default() + .inline_blame + .get_or_insert_default() + .enabled = Some(true); + }) + .unwrap(); pretty_assertions::assert_str_eq!( actual, r#"{ diff --git a/crates/settings_ui/src/settings_ui.rs b/crates/settings_ui/src/settings_ui.rs index 568d00115dcb760fcdacb264a9dd8d36829df9a4..89268b66f4c2f20411358eb63925187c6c3f382d 100644 --- a/crates/settings_ui/src/settings_ui.rs +++ b/crates/settings_ui/src/settings_ui.rs @@ -3932,10 +3932,13 @@ impl ProjectSettingsUpdateQueue { buffer.update(cx, |buffer, cx| { let current_text = buffer.text(); - let new_text = cx + if let Some(new_text) = cx .global::() - .new_text_for_update(current_text, |settings| update(settings, cx)); - buffer.edit([(0..buffer.len(), new_text)], None, cx); + .new_text_for_update(current_text, |settings| update(settings, cx)) + .log_err() + { + buffer.edit([(0..buffer.len(), new_text)], None, cx); + } }); buffer_store