migrator: Add migration for settings changed prior to migrator-introduction (#27375)

Finn Evers created

This PR updates two existing settings to use the settings migrator
instead of a manually implemented visitor. Both of these settings were
changed prior to the introduction of automatic migrations and the
visitor ensured that the settings were kept backwards compatible. See
https://github.com/zed-industries/zed/pull/22200 and
https://github.com/zed-industries/zed/pull/22364 respectively.

WIth this change, existing user configurations are updated accordingly
and the corresponding settings can derive `Deserialize` again.

I also added tests for the replacement of settings values, as there was
no test for this behaviour. Additionally, I added a seperate test for
the existing migration of `always_show_close_button`, since that
migration updated both the key and value.

Release Notes:

- N/A

Change summary

crates/collab_ui/src/panel_settings.rs                  | 46 -------
crates/editor/src/editor_settings.rs                    | 51 --------
crates/migrator/src/migrations.rs                       |  6 +
crates/migrator/src/migrations/m_2025_01_02/settings.rs | 62 +++++++++++
crates/migrator/src/migrator.rs                         | 58 ++++++++++
5 files changed, 128 insertions(+), 95 deletions(-)

Detailed changes

crates/collab_ui/src/panel_settings.rs 🔗

@@ -11,7 +11,7 @@ pub struct CollaborationPanelSettings {
     pub default_width: Pixels,
 }
 
-#[derive(Clone, Copy, Default, Serialize, JsonSchema, Debug)]
+#[derive(Clone, Copy, Default, Serialize, Deserialize, JsonSchema, Debug)]
 #[serde(rename_all = "snake_case")]
 pub enum ChatPanelButton {
     Never,
@@ -20,50 +20,6 @@ pub enum ChatPanelButton {
     WhenInCall,
 }
 
-impl<'de> Deserialize<'de> for ChatPanelButton {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: serde::Deserializer<'de>,
-    {
-        struct Visitor;
-
-        impl serde::de::Visitor<'_> for Visitor {
-            type Value = ChatPanelButton;
-
-            fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
-                write!(
-                    f,
-                    r#"a boolean or one of "never", "always", "when_in_call""#
-                )
-            }
-
-            fn visit_bool<E>(self, b: bool) -> Result<Self::Value, E>
-            where
-                E: serde::de::Error,
-            {
-                match b {
-                    false => Ok(ChatPanelButton::Never),
-                    true => Ok(ChatPanelButton::Always),
-                }
-            }
-
-            fn visit_str<E>(self, s: &str) -> Result<Self::Value, E>
-            where
-                E: serde::de::Error,
-            {
-                match s {
-                    "never" => Ok(ChatPanelButton::Never),
-                    "always" => Ok(ChatPanelButton::Always),
-                    "when_in_call" => Ok(ChatPanelButton::WhenInCall),
-                    _ => Err(E::unknown_variant(s, &["never", "always", "when_in_call"])),
-                }
-            }
-        }
-
-        deserializer.deserialize_any(Visitor)
-    }
-}
-
 #[derive(Deserialize, Debug)]
 pub struct ChatPanelSettings {
     pub button: ChatPanelButton,

crates/editor/src/editor_settings.rs 🔗

@@ -156,7 +156,7 @@ pub struct ScrollbarAxes {
 /// Which diagnostic indicators to show in the scrollbar.
 ///
 /// Default: all
-#[derive(Copy, Clone, Debug, Serialize, JsonSchema, PartialEq, Eq)]
+#[derive(Copy, Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq, Eq)]
 #[serde(rename_all = "lowercase")]
 pub enum ScrollbarDiagnostics {
     /// Show all diagnostic levels: hint, information, warnings, error.
@@ -171,55 +171,6 @@ pub enum ScrollbarDiagnostics {
     None,
 }
 
-impl<'de> Deserialize<'de> for ScrollbarDiagnostics {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: serde::Deserializer<'de>,
-    {
-        struct Visitor;
-
-        impl serde::de::Visitor<'_> for Visitor {
-            type Value = ScrollbarDiagnostics;
-
-            fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
-                write!(
-                    f,
-                    r#"a boolean or one of "all", "information", "warning", "error", "none""#
-                )
-            }
-
-            fn visit_bool<E>(self, b: bool) -> Result<Self::Value, E>
-            where
-                E: serde::de::Error,
-            {
-                match b {
-                    false => Ok(ScrollbarDiagnostics::None),
-                    true => Ok(ScrollbarDiagnostics::All),
-                }
-            }
-
-            fn visit_str<E>(self, s: &str) -> Result<Self::Value, E>
-            where
-                E: serde::de::Error,
-            {
-                match s {
-                    "all" => Ok(ScrollbarDiagnostics::All),
-                    "information" => Ok(ScrollbarDiagnostics::Information),
-                    "warning" => Ok(ScrollbarDiagnostics::Warning),
-                    "error" => Ok(ScrollbarDiagnostics::Error),
-                    "none" => Ok(ScrollbarDiagnostics::None),
-                    _ => Err(E::unknown_variant(
-                        s,
-                        &["all", "information", "warning", "error", "none"],
-                    )),
-                }
-            }
-        }
-
-        deserializer.deserialize_any(Visitor)
-    }
-}
-
 /// The key to use for adding multiple cursors
 ///
 /// Default: alt

crates/migrator/src/migrations.rs 🔗

@@ -1,3 +1,9 @@
+pub(crate) mod m_2025_01_02 {
+    mod settings;
+
+    pub(crate) use settings::SETTINGS_PATTERNS;
+}
+
 pub(crate) mod m_2025_01_29 {
     mod keymap;
     mod settings;

crates/migrator/src/migrations/m_2025_01_02/settings.rs 🔗

@@ -0,0 +1,62 @@
+use collections::HashMap;
+use std::{ops::Range, sync::LazyLock};
+use tree_sitter::{Query, QueryMatch};
+
+use crate::patterns::SETTINGS_NESTED_KEY_VALUE_PATTERN;
+use crate::MigrationPatterns;
+
+pub const SETTINGS_PATTERNS: MigrationPatterns = &[(
+    SETTINGS_NESTED_KEY_VALUE_PATTERN,
+    replace_deprecated_settings_values,
+)];
+
+fn replace_deprecated_settings_values(
+    contents: &str,
+    mat: &QueryMatch,
+    query: &Query,
+) -> Option<(Range<usize>, String)> {
+    let parent_object_capture_ix = query.capture_index_for_name("parent_key")?;
+    let parent_object_range = mat
+        .nodes_for_capture_index(parent_object_capture_ix)
+        .next()?
+        .byte_range();
+    let parent_object_name = contents.get(parent_object_range.clone())?;
+
+    let setting_name_ix = query.capture_index_for_name("setting_name")?;
+    let setting_name_range = mat
+        .nodes_for_capture_index(setting_name_ix)
+        .next()?
+        .byte_range();
+    let setting_name = contents.get(setting_name_range.clone())?;
+
+    let setting_value_ix = query.capture_index_for_name("setting_value")?;
+    let setting_value_range = mat
+        .nodes_for_capture_index(setting_value_ix)
+        .next()?
+        .byte_range();
+    let setting_value = contents.get(setting_value_range.clone())?;
+
+    UPDATED_SETTINGS
+        .get(&(parent_object_name, setting_name))
+        .and_then(|new_values| {
+            new_values
+                .iter()
+                .find_map(|(old_value, new_value)| {
+                    (*old_value == setting_value).then(|| new_value.to_string())
+                })
+                .map(|new_value| (setting_value_range, new_value))
+        })
+}
+
+static UPDATED_SETTINGS: LazyLock<HashMap<(&str, &str), Vec<(&str, &str)>>> = LazyLock::new(|| {
+    HashMap::from_iter([
+        (
+            ("chat_panel", "button"),
+            vec![("true", "\"always\""), ("false", "\"never\"")],
+        ),
+        (
+            ("scrollbar", "diagnostics"),
+            vec![("true", "\"all\""), ("false", "\"none\"")],
+        ),
+    ])
+});

crates/migrator/src/migrator.rs 🔗

@@ -104,6 +104,10 @@ pub fn migrate_keymap(text: &str) -> Result<Option<String>> {
 
 pub fn migrate_settings(text: &str) -> Result<Option<String>> {
     let migrations: &[(MigrationPatterns, &Query)] = &[
+        (
+            migrations::m_2025_01_02::SETTINGS_PATTERNS,
+            &SETTINGS_QUERY_2025_01_02,
+        ),
         (
             migrations::m_2025_01_29::SETTINGS_PATTERNS,
             &SETTINGS_QUERY_2025_01_29,
@@ -166,6 +170,10 @@ define_query!(
 );
 
 // settings
+define_query!(
+    SETTINGS_QUERY_2025_01_02,
+    migrations::m_2025_01_02::SETTINGS_PATTERNS
+);
 define_query!(
     SETTINGS_QUERY_2025_01_29,
     migrations::m_2025_01_29::SETTINGS_PATTERNS
@@ -461,4 +469,54 @@ mod tests {
             ),
         )
     }
+
+    #[test]
+    fn test_replace_settings_value() {
+        assert_migrate_settings(
+            r#"
+                {
+                    "scrollbar": {
+                        "diagnostics": true
+                    },
+                    "chat_panel": {
+                        "button": true
+                    }
+                }
+            "#,
+            Some(
+                r#"
+                {
+                    "scrollbar": {
+                        "diagnostics": "all"
+                    },
+                    "chat_panel": {
+                        "button": "always"
+                    }
+                }
+            "#,
+            ),
+        )
+    }
+
+    #[test]
+    fn test_replace_settings_name_and_value() {
+        assert_migrate_settings(
+            r#"
+                {
+                    "tabs": {
+                        "always_show_close_button": true
+                    }
+                }
+            "#,
+            Some(
+                r#"
+                {
+                    "tabs": {
+                        "show_close_button": "always"
+                    }
+                }
+            "#,
+            ),
+        )
+    }
 }