From 25036ba8bce904124e04c4e11e5264a3034c7f52 Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Wed, 22 Apr 2026 06:13:53 -0400 Subject: [PATCH] Handle new profile form in migrate_settings and migrate_language_setting (#53681) I noticed that there are helper migration functions that apply settings transformations to multiple layers (root, platform, release-channel, and profile): `migrate_settings` `migrate_language_setting` Both of these handle the old settings profile forms, so I've updated them to handle either form, for usage in historical and future migrations. Migration `m_2026_04_10` actually introduced a wrapper function to provide to `migrate_settings` to handle both forms, which is what clued me into the fact that I missed these migration helpers when I wrote the `m_2026_04_01` originally (I didn't know they existed). Self-Review Checklist: - [X] I've reviewed my own diff for quality, security, and reliability - [N/A] Unsafe blocks (if any) have justifying comments - [N/A] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [X] Tests cover the new/changed behavior - [X] Performance impact has been considered and is acceptable Closes #ISSUE Release Notes: - N/A --------- Co-authored-by: Smit Barmase --- crates/migrator/src/migrations.rs | 36 ++++++-- .../src/migrations/m_2026_04_10/settings.rs | 16 +--- crates/migrator/src/migrator.rs | 92 +++++++++++++++++++ 3 files changed, 120 insertions(+), 24 deletions(-) diff --git a/crates/migrator/src/migrations.rs b/crates/migrator/src/migrations.rs index ecc9a1030f725055407be1593babf148c9c3a775..8fa8907a16cc4839125975f14ca75327ac6bf6a9 100644 --- a/crates/migrator/src/migrations.rs +++ b/crates/migrator/src/migrations.rs @@ -29,9 +29,16 @@ pub(crate) fn migrate_settings( if let Some(profiles) = root_object.get_mut("profiles") { if let Some(profiles_object) = profiles.as_object_mut() { - for (_profile_name, profile_settings) in profiles_object.iter_mut() { - if let Some(profile_map) = profile_settings.as_object_mut() { - migrate_one(profile_map)?; + for profile_value in profiles_object.values_mut() { + if let Some(profile_map) = profile_value.as_object_mut() { + if let Some(inner) = profile_map + .get_mut("settings") + .and_then(|v| v.as_object_mut()) + { + migrate_one(inner)?; + } else { + migrate_one(profile_map)?; + } } } } @@ -93,12 +100,23 @@ pub(crate) fn migrate_language_setting( if let Some(profiles_object) = profiles.as_object_mut() { let profile_names: Vec = profiles_object.keys().cloned().collect(); for profile_name in &profile_names { - if let Some(profile_settings) = profiles_object.get_mut(profile_name.as_str()) { - apply_to_value_and_languages( - profile_settings, - &["profiles", profile_name], - migrate_fn, - )?; + if let Some(profile_value) = profiles_object.get_mut(profile_name.as_str()) { + if let Some(settings_value) = profile_value + .as_object_mut() + .and_then(|m| m.get_mut("settings")) + { + apply_to_value_and_languages( + settings_value, + &["profiles", profile_name], + migrate_fn, + )?; + } else { + apply_to_value_and_languages( + profile_value, + &["profiles", profile_name], + migrate_fn, + )?; + } } } } diff --git a/crates/migrator/src/migrations/m_2026_04_10/settings.rs b/crates/migrator/src/migrations/m_2026_04_10/settings.rs index 5430523149480772b8070f734f5c03daac8505d2..f72f5ee03d7c4fb62fd84fb3da2ce9e11d22b033 100644 --- a/crates/migrator/src/migrations/m_2026_04_10/settings.rs +++ b/crates/migrator/src/migrations/m_2026_04_10/settings.rs @@ -5,27 +5,13 @@ use crate::migrations::migrate_settings; const AGENT_KEY: &str = "agent"; const PROFILES_KEY: &str = "profiles"; -const SETTINGS_KEY: &str = "settings"; const TOOL_PERMISSIONS_KEY: &str = "tool_permissions"; const TOOLS_KEY: &str = "tools"; const OLD_TOOL_NAME: &str = "web_search"; const NEW_TOOL_NAME: &str = "search_web"; pub fn rename_web_search_to_search_web(value: &mut Value) -> Result<()> { - migrate_settings(value, &mut migrate_one) -} - -fn migrate_one(object: &mut serde_json::Map) -> Result<()> { - migrate_agent_value(object)?; - - // Root-level profiles have a `settings` wrapper after m_2026_04_01, - // but `migrate_settings` calls us with the profile map directly, - // so we need to look inside `settings` too. - if let Some(settings) = object.get_mut(SETTINGS_KEY).and_then(|v| v.as_object_mut()) { - migrate_agent_value(settings)?; - } - - Ok(()) + migrate_settings(value, &mut migrate_agent_value) } fn migrate_agent_value(object: &mut serde_json::Map) -> Result<()> { diff --git a/crates/migrator/src/migrator.rs b/crates/migrator/src/migrator.rs index b62406a2ace64c5552f44e09bf15ff5339d8f379..72cd7723ce69d524923a1ba645c2bf310f7a5b2e 100644 --- a/crates/migrator/src/migrator.rs +++ b/crates/migrator/src/migrator.rs @@ -4933,6 +4933,98 @@ mod tests { ); } + #[test] + fn test_migration_helpers_handle_various_profile_forms() { + let setting = "a_setting"; + let old_value = "old_value"; + let new_value = "new_value"; + + fn language_setting_fn(value: &mut serde_json::Value, _: &[&str]) -> anyhow::Result<()> { + if let Some(obj) = value.as_object_mut() { + if let Some(v) = obj.get_mut("a_setting") { + *v = serde_json::json!("new_value"); + } + } + Ok(()) + } + + let mut settings_fn = |map: &mut serde_json::Map| { + if let Some(v) = map.get_mut(setting) { + *v = serde_json::json!(new_value); + } + Ok(()) + }; + + // Legacy form + let input = serde_json::json!({ + "profiles": { + "work": { + setting: old_value + } + } + }); + let expected = serde_json::json!({ + "profiles": { + "work": { + setting: new_value + } + } + }); + + let mut value = input.clone(); + migrations::migrate_settings(&mut value, &mut settings_fn).unwrap(); + assert_eq!(value, expected); + + let mut value = input; + migrations::migrate_language_setting(&mut value, language_setting_fn).unwrap(); + assert_eq!(value, expected); + + // Form after migration: `m_2026_04_01` + let input = serde_json::json!({ + "profiles": { + "work": { + "settings": { + setting: old_value + } + } + } + }); + let expected = serde_json::json!({ + "profiles": { + "work": { + "settings": { + setting: new_value + } + } + } + }); + + let mut value = input.clone(); + migrations::migrate_settings(&mut value, &mut settings_fn).unwrap(); + assert_eq!(value, expected); + + let mut value = input; + migrations::migrate_language_setting(&mut value, language_setting_fn).unwrap(); + assert_eq!(value, expected); + + // Base-only form after migration: `m_2026_04_01` (no settings to migrate) + let input = serde_json::json!({ + "profiles": { + "work": { + "base": "default" + } + } + }); + + let mut value = input.clone(); + migrations::migrate_settings(&mut value, &mut settings_fn).unwrap(); + assert_eq!(value, input); + + let mut value = input.clone(); + migrations::migrate_language_setting(&mut value, language_setting_fn).unwrap(); + assert_eq!(value, input); + } + #[test] fn test_rename_web_search_to_search_web_root_level_profile() { assert_migrate_with_migrations(