From 2f7045f72427d8cfb4998405c8bc5a0c4459eea6 Mon Sep 17 00:00:00 2001 From: Ben Kunkle Date: Mon, 27 Oct 2025 07:13:26 -0700 Subject: [PATCH] settings_ui: Show migration banner (#41112) Closes #ISSUE Release Notes: - N/A *or* Added/Fixed/Improved ... --------- Co-authored-by: Danilo --- Cargo.lock | 21 +- Cargo.toml | 2 + crates/migrator/Cargo.toml | 2 +- crates/migrator/src/migrator.rs | 7 +- crates/project/src/project_settings.rs | 1 + crates/settings/Cargo.toml | 5 +- crates/settings/src/keymap_file.rs | 5 +- crates/settings/src/settings.rs | 5 +- crates/settings/src/settings_store.rs | 284 +++++++++++++----- crates/settings_json/Cargo.toml | 35 +++ crates/settings_json/LICENSE-GPL | 1 + .../src/settings_json.rs | 9 - crates/settings_ui/src/settings_ui.rs | 94 ++++-- crates/zed/src/zed.rs | 42 +-- 14 files changed, 371 insertions(+), 142 deletions(-) create mode 100644 crates/settings_json/Cargo.toml create mode 120000 crates/settings_json/LICENSE-GPL rename crates/{settings => settings_json}/src/settings_json.rs (99%) diff --git a/Cargo.lock b/Cargo.lock index 4d1ae4f58bcafffb65af23b5985c94fcd593bcb7..a21ab94b659510ad067071993fe5d0ae0f2db97a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9837,7 +9837,7 @@ dependencies = [ "pretty_assertions", "serde_json", "serde_json_lenient", - "settings", + "settings_json", "streaming-iterator", "tree-sitter", "tree-sitter-json", @@ -15260,6 +15260,7 @@ dependencies = [ "indoc", "inventory", "log", + "migrator", "paths", "pretty_assertions", "release_channel", @@ -15268,17 +15269,31 @@ dependencies = [ "serde", "serde_json", "serde_json_lenient", - "serde_path_to_error", "serde_repr", "serde_with", + "settings_json", "settings_macros", "smallvec", "strum 0.27.2", + "unindent", + "util", + "zlog", +] + +[[package]] +name = "settings_json" +version = "0.1.0" +dependencies = [ + "anyhow", + "pretty_assertions", + "serde", + "serde_json", + "serde_json_lenient", + "serde_path_to_error", "tree-sitter", "tree-sitter-json", "unindent", "util", - "zlog", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index d69d768f33eb59e9f2c6e3194a976e612d5944ab..50828f67afca2974fcc4149bec9cfe377b41553b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -148,6 +148,7 @@ members = [ "crates/semantic_version", "crates/session", "crates/settings", + "crates/settings_json", "crates/settings_macros", "crates/settings_profile_selector", "crates/settings_ui", @@ -380,6 +381,7 @@ search = { path = "crates/search" } semantic_version = { path = "crates/semantic_version" } session = { path = "crates/session" } settings = { path = "crates/settings" } +settings_json = { path = "crates/settings_json" } settings_macros = { path = "crates/settings_macros" } settings_ui = { path = "crates/settings_ui" } snippet = { path = "crates/snippet" } diff --git a/crates/migrator/Cargo.toml b/crates/migrator/Cargo.toml index edb48a00e2ca93232d9022b6fb778449d2ecc7e4..e0a75784749c2d3a2a981b44cbbe449a7685c605 100644 --- a/crates/migrator/Cargo.toml +++ b/crates/migrator/Cargo.toml @@ -22,7 +22,7 @@ tree-sitter-json.workspace = true tree-sitter.workspace = true serde_json_lenient.workspace = true serde_json.workspace = true -settings.workspace = true +settings_json.workspace = true [dev-dependencies] pretty_assertions.workspace = true diff --git a/crates/migrator/src/migrator.rs b/crates/migrator/src/migrator.rs index 28021042825988ee70c04993ca71c5e9abe86bb4..ff9635dcef7664b17eb02a03b7584ea18ac9a91b 100644 --- a/crates/migrator/src/migrator.rs +++ b/crates/migrator/src/migrator.rs @@ -15,6 +15,7 @@ //! You only need to write replacement logic for x-1 to x because you can be certain that, internally, every user will be at x-1, regardless of their on disk state. use anyhow::{Context as _, Result}; +use settings_json::{infer_json_indent_size, parse_json_with_comments, update_value_in_json_text}; use std::{cmp::Reverse, ops::Range, sync::LazyLock}; use streaming_iterator::StreamingIterator; use tree_sitter::{Query, QueryMatch}; @@ -74,7 +75,7 @@ fn run_migrations(text: &str, migrations: &[MigrationType]) -> Result = None; - let json_indent_size = settings::infer_json_indent_size(¤t_text); + let json_indent_size = infer_json_indent_size(¤t_text); for migration in migrations.iter() { let migrated_text = match migration { MigrationType::TreeSitter(patterns, query) => migrate(¤t_text, patterns, query)?, @@ -83,14 +84,14 @@ fn run_migrations(text: &str, migrations: &[MigrationType]) -> Result, setting_file_updates_tx: mpsc::UnboundedSender LocalBoxFuture<'static, Result<()>>>>, - file_errors: BTreeMap, + file_errors: BTreeMap, } #[derive(Clone, PartialEq, Eq, Debug)] pub enum SettingsFile { + Default, + Global, User, Server, - Default, /// Represents project settings in ssh projects as well as local projects Project((WorktreeId, Arc)), } @@ -184,6 +184,8 @@ impl Ord for SettingsFile { (_, Server) => Ordering::Greater, (User, _) => Ordering::Less, (_, User) => Ordering::Greater, + (Global, _) => Ordering::Less, + (_, Global) => Ordering::Greater, } } } @@ -235,6 +237,14 @@ trait AnySettingValue: 'static + Send + Sync { fn set_local_value(&mut self, root_id: WorktreeId, path: Arc, value: Box); } +/// Parameters that are used when generating some JSON schemas at runtime. +pub struct SettingsJsonSchemaParams<'a> { + pub language_names: &'a [String], + pub font_names: &'a [String], + pub theme_names: &'a [SharedString], + pub icon_theme_names: &'a [SharedString], +} + impl SettingsStore { pub fn new(cx: &App, default_settings: &str) -> Self { let (setting_file_updates_tx, mut setting_file_updates_rx) = mpsc::unbounded(); @@ -264,7 +274,7 @@ impl SettingsStore { pub fn observe_active_settings_profile_name(cx: &mut App) -> gpui::Subscription { cx.observe_global::(|cx| { Self::update_global(cx, |store, cx| { - store.recompute_values(None, cx).log_err(); + store.recompute_values(None, cx); }); }) } @@ -386,7 +396,7 @@ impl SettingsStore { ..Default::default() }) .unwrap(); - self.set_user_settings(&new_text, cx).unwrap(); + _ = self.set_user_settings(&new_text, cx); } pub async fn load_settings(fs: &Arc) -> Result { @@ -515,6 +525,7 @@ impl SettingsStore { SettingsFile::Default => Some(self.default_settings.as_ref()), SettingsFile::Server => self.server_settings.as_deref(), SettingsFile::Project(ref key) => self.local_settings.get(key), + SettingsFile::Global => self.global_settings.as_deref(), } } @@ -617,22 +628,58 @@ impl SettingsStore { (SettingsFile::Default, None) } - fn handle_potential_file_error( + #[inline(always)] + fn parse_and_migrate_zed_settings( &mut self, + user_settings_content: &str, file: SettingsFile, - result: Result, - ) -> Result { - if let Err(err) = result.as_ref() { - let message = err.to_string(); - self.file_errors.insert(file, message); + ) -> (Option, SettingsParseResult) { + let mut migration_status = MigrationStatus::NotNeeded; + let settings: SettingsContentType = if user_settings_content.is_empty() { + parse_json_with_comments("{}").expect("Empty settings should always be valid") } else { - self.file_errors.remove(&file); - } - return result; + let migration_res = migrator::migrate_settings(user_settings_content); + let content = match &migration_res { + Ok(Some(content)) => content, + Ok(None) => user_settings_content, + Err(_) => user_settings_content, + }; + let parse_result = parse_json_with_comments(content); + migration_status = match migration_res { + Ok(Some(_)) => MigrationStatus::Succeeded, + Ok(None) => MigrationStatus::NotNeeded, + Err(err) => MigrationStatus::Failed { + error: err.to_string(), + }, + }; + match parse_result { + Ok(settings) => settings, + Err(err) => { + let result = SettingsParseResult { + parse_status: ParseStatus::Failed { + error: err.to_string(), + }, + migration_status, + }; + self.file_errors.insert(file, result.clone()); + return (None, result); + } + } + }; + + let result = SettingsParseResult { + parse_status: ParseStatus::Success, + migration_status, + }; + self.file_errors.insert(file, result.clone()); + return (Some(settings), result); } - pub fn error_for_file(&self, file: SettingsFile) -> Option { - self.file_errors.get(&file).cloned() + pub fn error_for_file(&self, file: SettingsFile) -> Option { + self.file_errors + .get(&file) + .filter(|parse_result| parse_result.requires_user_action()) + .cloned() } } @@ -697,41 +744,46 @@ impl SettingsStore { cx: &mut App, ) -> Result<()> { self.default_settings = parse_json_with_comments(default_settings_content)?; - self.recompute_values(None, cx)?; + self.recompute_values(None, cx); Ok(()) } /// Sets the user settings via a JSON string. - pub fn set_user_settings(&mut self, user_settings_content: &str, cx: &mut App) -> Result<()> { - let settings: UserSettingsContent = if user_settings_content.is_empty() { - parse_json_with_comments("{}")? - } else { - self.handle_potential_file_error( - SettingsFile::User, - parse_json_with_comments(user_settings_content), - )? - }; + #[must_use] + pub fn set_user_settings( + &mut self, + user_settings_content: &str, + cx: &mut App, + ) -> SettingsParseResult { + let (settings, parse_result) = self.parse_and_migrate_zed_settings::( + user_settings_content, + SettingsFile::User, + ); - self.user_settings = Some(settings); - self.recompute_values(None, cx)?; - Ok(()) + if let Some(settings) = settings { + self.user_settings = Some(settings); + self.recompute_values(None, cx); + } + return parse_result; } /// Sets the global settings via a JSON string. + #[must_use] pub fn set_global_settings( &mut self, global_settings_content: &str, cx: &mut App, - ) -> Result<()> { - let settings: SettingsContent = if global_settings_content.is_empty() { - parse_json_with_comments("{}")? - } else { - parse_json_with_comments(global_settings_content)? - }; + ) -> SettingsParseResult { + let (settings, parse_result) = self.parse_and_migrate_zed_settings::( + global_settings_content, + SettingsFile::Global, + ); - self.global_settings = Some(Box::new(settings)); - self.recompute_values(None, cx)?; - Ok(()) + if let Some(settings) = settings { + self.global_settings = Some(Box::new(settings)); + self.recompute_values(None, cx); + } + return parse_result; } pub fn set_server_settings( @@ -742,16 +794,13 @@ impl SettingsStore { let settings: Option = if server_settings_content.is_empty() { None } else { - self.handle_potential_file_error( - SettingsFile::Server, - parse_json_with_comments(server_settings_content), - )? + parse_json_with_comments(server_settings_content)? }; // Rewrite the server settings into a content type self.server_settings = settings.map(|settings| Box::new(settings)); - self.recompute_values(None, cx)?; + self.recompute_values(None, cx); Ok(()) } @@ -803,31 +852,36 @@ impl SettingsStore { .remove(&(root_id, directory_path.clone())); } (LocalSettingsKind::Settings, Some(settings_contents)) => { - let new_settings = self - .handle_potential_file_error( + let (new_settings, parse_result) = self + .parse_and_migrate_zed_settings::( + settings_contents, SettingsFile::Project((root_id, directory_path.clone())), - parse_json_with_comments::(settings_contents), - ) - .map_err(|e| InvalidSettingsError::LocalSettings { + ); + match parse_result.parse_status { + ParseStatus::Success => Ok(()), + ParseStatus::Failed { error } => Err(InvalidSettingsError::LocalSettings { path: directory_path.join(local_settings_file_relative_path()), - message: e.to_string(), - })?; - match self.local_settings.entry((root_id, directory_path.clone())) { - btree_map::Entry::Vacant(v) => { - v.insert(SettingsContent { - project: new_settings, - ..Default::default() - }); - zed_settings_changed = true; - } - btree_map::Entry::Occupied(mut o) => { - if &o.get().project != &new_settings { - o.insert(SettingsContent { + message: error, + }), + }?; + if let Some(new_settings) = new_settings { + match self.local_settings.entry((root_id, directory_path.clone())) { + btree_map::Entry::Vacant(v) => { + v.insert(SettingsContent { project: new_settings, ..Default::default() }); zed_settings_changed = true; } + btree_map::Entry::Occupied(mut o) => { + if &o.get().project != &new_settings { + o.insert(SettingsContent { + project: new_settings, + ..Default::default() + }); + zed_settings_changed = true; + } + } } } } @@ -874,7 +928,7 @@ impl SettingsStore { }; if zed_settings_changed { - self.recompute_values(Some((root_id, &directory_path)), cx)?; + self.recompute_values(Some((root_id, &directory_path)), cx); } Ok(()) } @@ -891,7 +945,7 @@ impl SettingsStore { }, ..Default::default() })); - self.recompute_values(None, cx)?; + self.recompute_values(None, cx); Ok(()) } @@ -899,7 +953,7 @@ impl SettingsStore { pub fn clear_local_settings(&mut self, root_id: WorktreeId, cx: &mut App) -> Result<()> { self.local_settings .retain(|(worktree_id, _), _| worktree_id != &root_id); - self.recompute_values(Some((root_id, RelPath::empty())), cx)?; + self.recompute_values(Some((root_id, RelPath::empty())), cx); Ok(()) } @@ -989,12 +1043,11 @@ impl SettingsStore { .to_value() } - // todo -> this function never fails, and should not return a result fn recompute_values( &mut self, changed_local_path: Option<(WorktreeId, &RelPath)>, cx: &mut App, - ) -> std::result::Result<(), InvalidSettingsError> { + ) { // Reload the global and local values for every setting. let mut project_settings_stack = Vec::::new(); let mut paths_stack = Vec::>::new(); @@ -1054,7 +1107,6 @@ impl SettingsStore { setting_value.set_local_value(*root_id, directory_path.clone(), value); } } - Ok(()) } pub fn editorconfig_properties( @@ -1087,6 +1139,96 @@ impl SettingsStore { } } +/// The result of parsing settings, including any migration attempts +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SettingsParseResult { + /// The result of parsing the settings file (possibly after migration) + pub parse_status: ParseStatus, + /// The result of attempting to migrate the settings file + pub migration_status: MigrationStatus, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ParseStatus { + /// Settings were parsed successfully + Success, + /// Settings failed to parse + Failed { error: String }, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum MigrationStatus { + /// No migration was needed - settings are up to date + NotNeeded, + /// Settings were automatically migrated in memory, but the file needs to be updated + Succeeded, + /// Migration was attempted but failed. Original settings were parsed instead. + Failed { error: String }, +} + +impl Default for SettingsParseResult { + fn default() -> Self { + Self { + parse_status: ParseStatus::Success, + migration_status: MigrationStatus::NotNeeded, + } + } +} + +impl SettingsParseResult { + pub fn unwrap(self) -> bool { + self.result().unwrap() + } + + pub fn expect(self, message: &str) -> bool { + self.result().expect(message) + } + + /// Formats the ParseResult as a Result type. This is a lossy conversion + pub fn result(self) -> Result { + let migration_result = match self.migration_status { + MigrationStatus::NotNeeded => Ok(false), + MigrationStatus::Succeeded => Ok(true), + MigrationStatus::Failed { error } => { + Err(anyhow::format_err!(error)).context("Failed to migrate settings") + } + }; + + let parse_result = match self.parse_status { + ParseStatus::Success => Ok(()), + ParseStatus::Failed { error } => { + Err(anyhow::format_err!(error)).context("Failed to parse settings") + } + }; + + match (migration_result, parse_result) { + (migration_result @ Ok(_), Ok(())) => migration_result, + (Err(migration_err), Ok(())) => Err(migration_err), + (_, Err(parse_err)) => Err(parse_err), + } + } + + /// Returns true if there were any errors migrating and parsing the settings content or if migration was required but there were no errors + pub fn requires_user_action(&self) -> bool { + matches!(self.parse_status, ParseStatus::Failed { .. }) + || matches!( + self.migration_status, + MigrationStatus::Succeeded | MigrationStatus::Failed { .. } + ) + } + + pub fn ok(self) -> Option { + self.result().ok() + } + + pub fn parse_error(&self) -> Option { + match &self.parse_status { + ParseStatus::Failed { error } => Some(error.clone()), + ParseStatus::Success => None, + } + } +} + #[derive(Debug, Clone, PartialEq)] pub enum InvalidSettingsError { LocalSettings { path: Arc, message: String }, diff --git a/crates/settings_json/Cargo.toml b/crates/settings_json/Cargo.toml new file mode 100644 index 0000000000000000000000000000000000000000..2ba9887ca016b645bafa2974bbd9029373348838 --- /dev/null +++ b/crates/settings_json/Cargo.toml @@ -0,0 +1,35 @@ +[package] +name = "settings_json" +version = "0.1.0" +edition.workspace = true +publish.workspace = true +license = "GPL-3.0-or-later" + +[lints] +workspace = true + +[lib] +path = "src/settings_json.rs" + +[features] +default = [] + +[dependencies] +anyhow.workspace = true +tree-sitter.workspace = true +tree-sitter-json.workspace = true +util.workspace = true +serde.workspace = true +serde_json.workspace = true +serde_json_lenient.workspace = true +serde_path_to_error.workspace = true + +[dev-dependencies] +unindent.workspace = true +pretty_assertions.workspace = true + +# Uncomment other workspace dependencies as needed +# assistant.workspace = true +# client.workspace = true +# project.workspace = true +# settings.workspace = true diff --git a/crates/settings_json/LICENSE-GPL b/crates/settings_json/LICENSE-GPL new file mode 120000 index 0000000000000000000000000000000000000000..89e542f750cd3860a0598eff0dc34b56d7336dc4 --- /dev/null +++ b/crates/settings_json/LICENSE-GPL @@ -0,0 +1 @@ +../../LICENSE-GPL \ No newline at end of file diff --git a/crates/settings/src/settings_json.rs b/crates/settings_json/src/settings_json.rs similarity index 99% rename from crates/settings/src/settings_json.rs rename to crates/settings_json/src/settings_json.rs index 29ca2a1c9b8bbd64baf88d94bf2f684d8ef988b4..5198e475af82c7a69f7ff568cd58c6945a2453bb 100644 --- a/crates/settings/src/settings_json.rs +++ b/crates/settings_json/src/settings_json.rs @@ -1,19 +1,10 @@ use anyhow::Result; -use gpui::SharedString; use serde::{Serialize, de::DeserializeOwned}; use serde_json::Value; use std::{ops::Range, sync::LazyLock}; use tree_sitter::{Query, StreamingIterator as _}; use util::RangeExt; -/// Parameters that are used when generating some JSON schemas at runtime. -pub struct SettingsJsonSchemaParams<'a> { - pub language_names: &'a [String], - pub font_names: &'a [String], - pub theme_names: &'a [SharedString], - pub icon_theme_names: &'a [SharedString], -} - pub fn update_value_in_json_text<'a>( text: &mut String, key_path: &mut Vec<&'a str>, diff --git a/crates/settings_ui/src/settings_ui.rs b/crates/settings_ui/src/settings_ui.rs index 3b47cb51081a3fe956d832342eae7c54cbb41215..f7f5a975b78d6de3fd3a696d9e223baf7807071c 100644 --- a/crates/settings_ui/src/settings_ui.rs +++ b/crates/settings_ui/src/settings_ui.rs @@ -1139,6 +1139,7 @@ impl SettingsUiFile { settings::SettingsFile::Project(location) => SettingsUiFile::Project(location), settings::SettingsFile::Server => SettingsUiFile::Server("todo: server name"), settings::SettingsFile::Default => return None, + settings::SettingsFile::Global => return None, }) } @@ -1730,7 +1731,10 @@ impl SettingsWindow { let prev_files = self.files.clone(); let settings_store = cx.global::(); let mut ui_files = vec![]; - let all_files = settings_store.get_all_files(); + let mut all_files = settings_store.get_all_files(); + if !all_files.contains(&settings::SettingsFile::User) { + all_files.push(settings::SettingsFile::User); + } for file in all_files { let Some(settings_ui_file) = SettingsUiFile::from_settings(file) else { continue; @@ -2685,40 +2689,72 @@ impl SettingsWindow { if let Some(error) = SettingsStore::global(cx).error_for_file(self.current_file.to_settings()) { - if self.shown_errors.insert(error.clone()) { - telemetry::event!("Settings Error Shown", error = &error); + fn banner( + label: &'static str, + error: String, + shown_errors: &mut HashSet, + cx: &mut Context, + ) -> impl IntoElement { + if shown_errors.insert(error.clone()) { + telemetry::event!("Settings Error Shown", label = label, error = &error); + } + Banner::new() + .severity(Severity::Warning) + .child( + v_flex() + .my_0p5() + .gap_0p5() + .child(Label::new(label)) + .child(Label::new(error).size(LabelSize::Small).color(Color::Muted)), + ) + .action_slot( + div().pr_1().child( + Button::new("fix-in-json", "Fix in settings.json") + .tab_index(0_isize) + .style(ButtonStyle::Tinted(ui::TintColor::Warning)) + .on_click(cx.listener(|this, _, _, cx| { + this.open_current_settings_file(cx); + })), + ), + ) } - + let parse_error = error.parse_error(); + let parse_failed = parse_error.is_some(); warning_banner = v_flex() + .gap_2() .pb_4() - .child( - Banner::new() - .severity(Severity::Warning) - .child( - v_flex() - .my_0p5() - .gap_0p5() - .child(Label::new("Your settings file is in an invalid state.")) - .child( - Label::new(error).size(LabelSize::Small).color(Color::Muted), - ), - ) - .action_slot( - div().pr_1().child( - Button::new("fix-in-json", "Fix in settings.json") - .tab_index(0_isize) - .style(ButtonStyle::Tinted(ui::TintColor::Warning)) - .on_click(cx.listener(|this, _, _, cx| { - this.open_current_settings_file(cx); - })), - ), - ), - ) + .when_some(parse_error, |this, err| { + this.child(banner( + "Failed to load your settings. Some values may be incorrect and changes may be lost.", + err, + &mut self.shown_errors, + cx, + )) + }) + .map(|this| match &error.migration_status { + settings::MigrationStatus::Succeeded => this.child(banner( + "Your settings are out of date, and need to be updated.", + match &self.current_file { + SettingsUiFile::User => "They can be automatically migrated to the latest version.", + SettingsUiFile::Server(_) | SettingsUiFile::Project(_) => "They must be manually migrated to the latest version." + }.to_string(), + &mut self.shown_errors, + cx, + )), + settings::MigrationStatus::Failed { error: err } if !parse_failed => this + .child(banner( + "Your settings file is out of date, automatic migration failed", + err.clone(), + &mut self.shown_errors, + cx, + )), + _ => this, + }) .into_any_element() } return v_flex() - .id("Settings-ui-page") + .id("settings-ui-page") .on_action(cx.listener(|this, _: &menu::SelectNext, window, cx| { if !sub_page_stack().is_empty() { window.focus_next(); @@ -2789,8 +2825,8 @@ impl SettingsWindow { .pt_6() .px_8() .bg(cx.theme().colors().editor_background) - .child(warning_banner) .child(page_header) + .child(warning_banner) .child( div() .size_full() diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index cc2906adfc0f1d8e3a78423e69b93e5ee5909da0..ee29e93da38e4de6d32a00f11b171401d8d4f2c4 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -39,7 +39,7 @@ use language_onboarding::BasedPyrightBanner; use language_tools::lsp_button::{self, LspButton}; use language_tools::lsp_log_view::LspLogToolbarItemView; use migrate::{MigrationBanner, MigrationEvent, MigrationNotification, MigrationType}; -use migrator::{migrate_keymap, migrate_settings}; +use migrator::migrate_keymap; use onboarding::DOCS_URL; use onboarding::multibuffer_hint::MultibufferHint; pub use open_listener::*; @@ -1298,18 +1298,24 @@ pub fn handle_settings_file_changes( store: &mut SettingsStore, cx: &mut App| -> bool { + let result = if is_user { + store.set_user_settings(&content, cx) + } else { + store.set_global_settings(&content, cx) + }; + let id = NotificationId::Named("failed-to-migrate-settings".into()); // Apply migrations to both user and global settings - let (processed_content, content_migrated) = match migrate_settings(&content) { - Ok(result) => { + let content_migrated = match result.migration_status { + settings::MigrationStatus::Succeeded => { dismiss_app_notification(&id, cx); - if let Some(migrated_content) = result { - (migrated_content, true) - } else { - (content, false) - } + true + } + settings::MigrationStatus::NotNeeded => { + dismiss_app_notification(&id, cx); + false } - Err(err) => { + settings::MigrationStatus::Failed { error: err } => { show_app_notification(id, cx, move |cx| { cx.new(|cx| { MessageNotification::new( @@ -1328,22 +1334,22 @@ pub fn handle_settings_file_changes( }) }); // notify user here - (content, false) + false } }; - let result = if is_user { - store.set_user_settings(&processed_content, cx) - } else { - store.set_global_settings(&processed_content, cx) - }; - - if let Err(err) = &result { + if let settings::ParseStatus::Failed { error: err } = &result.parse_status { let settings_type = if is_user { "user" } else { "global" }; log::error!("Failed to load {} settings: {err}", settings_type); } - settings_changed(result.err(), cx); + settings_changed( + match result.parse_status { + settings::ParseStatus::Failed { error } => Some(anyhow::format_err!(error)), + settings::ParseStatus::Success => None, + }, + cx, + ); content_migrated };