settings_ui: Show migration banner (#41112)

Closes #ISSUE

Release Notes:

- N/A *or* Added/Fixed/Improved ...

---------

Co-authored-by: Danilo <danilo@zed.dev>

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]]

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" }

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

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<Option<Str
 
     let mut current_text = text.to_string();
     let mut result: Option<String> = None;
-    let json_indent_size = settings::infer_json_indent_size(&current_text);
+    let json_indent_size = infer_json_indent_size(&current_text);
     for migration in migrations.iter() {
         let migrated_text = match migration {
             MigrationType::TreeSitter(patterns, query) => migrate(&current_text, patterns, query)?,
@@ -83,14 +84,14 @@ fn run_migrations(text: &str, migrations: &[MigrationType]) -> Result<Option<Str
                     return Ok(None);
                 }
                 let old_content: serde_json_lenient::Value =
-                    settings::parse_json_with_comments(&current_text)?;
+                    parse_json_with_comments(&current_text)?;
                 let old_value = serde_json::to_value(&old_content).unwrap();
                 let mut new_value = old_value.clone();
                 callback(&mut new_value)?;
                 if new_value != old_value {
                     let mut current = current_text.clone();
                     let mut edits = vec![];
-                    settings::update_value_in_json_text(
+                    update_value_in_json_text(
                         &mut current,
                         &mut vec![],
                         json_indent_size,

crates/project/src/project_settings.rs

@@ -728,6 +728,7 @@ impl SettingsObserver {
         cx.update_global(|settings_store: &mut SettingsStore, cx| {
             settings_store
                 .set_user_settings(&envelope.payload.contents, cx)
+                .result()
                 .context("setting new user settings")?;
             anyhow::Ok(())
         })??;

crates/settings/Cargo.toml

@@ -32,16 +32,15 @@ schemars.workspace = true
 serde.workspace = true
 serde_json.workspace = true
 serde_json_lenient.workspace = true
-serde_path_to_error.workspace = true
 serde_repr.workspace = true
 serde_with.workspace = true
+settings_json.workspace = true
 settings_macros.workspace = true
 smallvec.workspace = true
 strum.workspace = true
-tree-sitter-json.workspace = true
-tree-sitter.workspace = true
 util.workspace = true
 zlog.workspace = true
+migrator.workspace = true
 
 [dev-dependencies]
 fs = { workspace = true, features = ["test-support"] }

crates/settings/src/keymap_file.rs

@@ -17,8 +17,9 @@ use util::{
     markdown::{MarkdownEscaped, MarkdownInlineCode, MarkdownString},
 };
 
-use crate::{
-    SettingsAssets, append_top_level_array_value_in_json_text, parse_json_with_comments,
+use crate::SettingsAssets;
+use settings_json::{
+    append_top_level_array_value_in_json_text, parse_json_with_comments,
     replace_top_level_array_value_in_json_text,
 };
 

crates/settings/src/settings.rs

@@ -5,7 +5,6 @@ pub mod merge_from;
 mod serde_helper;
 mod settings_content;
 mod settings_file;
-mod settings_json;
 mod settings_store;
 mod vscode_import;
 
@@ -26,8 +25,8 @@ pub use serde_helper::*;
 pub use settings_file::*;
 pub use settings_json::*;
 pub use settings_store::{
-    InvalidSettingsError, LocalSettingsKind, Settings, SettingsFile, SettingsKey, SettingsLocation,
-    SettingsStore,
+    InvalidSettingsError, LocalSettingsKind, MigrationStatus, ParseStatus, Settings, SettingsFile,
+    SettingsJsonSchemaParams, SettingsKey, SettingsLocation, SettingsStore,
 };
 
 pub use vscode_import::{VsCodeSettings, VsCodeSettingsSource};

crates/settings/src/settings_store.rs

@@ -7,7 +7,7 @@ use futures::{
     channel::{mpsc, oneshot},
     future::LocalBoxFuture,
 };
-use gpui::{App, AsyncApp, BorrowAppContext, Global, Task, UpdateGlobal};
+use gpui::{App, AsyncApp, BorrowAppContext, Global, SharedString, Task, UpdateGlobal};
 
 use paths::{EDITORCONFIG_NAME, local_settings_file_relative_path, task_file_name};
 use schemars::{JsonSchema, json_schema};
@@ -32,16 +32,15 @@ pub type EditorconfigProperties = ec4rs::Properties;
 
 use crate::{
     ActiveSettingsProfileName, FontFamilyName, IconThemeName, LanguageSettingsContent,
-    LanguageToSettingsMap, SettingsJsonSchemaParams, ThemeName, VsCodeSettings, WorktreeId,
-    infer_json_indent_size,
+    LanguageToSettingsMap, ThemeName, VsCodeSettings, WorktreeId,
     merge_from::MergeFrom,
-    parse_json_with_comments,
     settings_content::{
         ExtensionsSettingsContent, ProjectSettingsContent, SettingsContent, UserSettingsContent,
     },
-    update_value_in_json_text,
 };
 
+use settings_json::{infer_json_indent_size, parse_json_with_comments, update_value_in_json_text};
+
 pub trait SettingsKey: 'static + Send + Sync {
     /// The name of a key within the JSON file from which this setting should
     /// be deserialized. If this is `None`, then the setting will be deserialized
@@ -148,14 +147,15 @@ pub struct SettingsStore {
     _setting_file_updates: Task<()>,
     setting_file_updates_tx:
         mpsc::UnboundedSender<Box<dyn FnOnce(AsyncApp) -> LocalBoxFuture<'static, Result<()>>>>,
-    file_errors: BTreeMap<SettingsFile, String>,
+    file_errors: BTreeMap<SettingsFile, SettingsParseResult>,
 }
 
 #[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<RelPath>)),
 }
@@ -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<RelPath>, value: Box<dyn Any>);
 }
 
+/// 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::<ActiveSettingsProfileName>(|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<dyn Fs>) -> Result<String> {
@@ -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<R>(
+    #[inline(always)]
+    fn parse_and_migrate_zed_settings<SettingsContentType: serde::de::DeserializeOwned>(
         &mut self,
+        user_settings_content: &str,
         file: SettingsFile,
-        result: Result<R>,
-    ) -> Result<R> {
-        if let Err(err) = result.as_ref() {
-            let message = err.to_string();
-            self.file_errors.insert(file, message);
+    ) -> (Option<SettingsContentType>, 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<String> {
-        self.file_errors.get(&file).cloned()
+    pub fn error_for_file(&self, file: SettingsFile) -> Option<SettingsParseResult> {
+        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::<UserSettingsContent>(
+            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::<SettingsContent>(
+            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<SettingsContent> = 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::<ProjectSettingsContent>(
+                        settings_contents,
                         SettingsFile::Project((root_id, directory_path.clone())),
-                        parse_json_with_comments::<ProjectSettingsContent>(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::<SettingsContent>::new();
         let mut paths_stack = Vec::<Option<(WorktreeId, &RelPath)>>::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<bool> {
+        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<bool> {
+        self.result().ok()
+    }
+
+    pub fn parse_error(&self) -> Option<String> {
+        match &self.parse_status {
+            ParseStatus::Failed { error } => Some(error.clone()),
+            ParseStatus::Success => None,
+        }
+    }
+}
+
 #[derive(Debug, Clone, PartialEq)]
 pub enum InvalidSettingsError {
     LocalSettings { path: Arc<RelPath>, message: String },

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

crates/settings_json/LICENSE-GPL

@@ -0,0 +1 @@
+../../LICENSE-GPL

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>,

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::<SettingsStore>();
         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);
-            }
-
-            warning_banner = v_flex()
-                .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);
-                                    })),
-                            ),
+            fn banner(
+                label: &'static str,
+                error: String,
+                shown_errors: &mut HashSet<String>,
+                cx: &mut Context<SettingsWindow>,
+            ) -> 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()
+                .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()

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
             }
-            Err(err) => {
+            settings::MigrationStatus::NotNeeded => {
+                dismiss_app_notification(&id, cx);
+                false
+            }
+            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
     };