chore: Remove `settings` dependency on `migrator` (#24642)

Piotr Osiewicz created

Closes #ISSUE

Release Notes:

- N/A

Change summary

Cargo.lock                            |  2 
crates/settings/Cargo.toml            |  1 
crates/settings/src/keymap_file.rs    | 40 -------------
crates/settings/src/settings_store.rs | 49 -----------------
crates/zed/Cargo.toml                 |  1 
crates/zed/src/zed.rs                 |  9 +-
crates/zed/src/zed/migrate.rs         | 79 +++++++++++++++++++++++++++++
7 files changed, 90 insertions(+), 91 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -12035,7 +12035,6 @@ dependencies = [
  "indoc",
  "inventory",
  "log",
- "migrator",
  "paths",
  "pretty_assertions",
  "release_channel",
@@ -16647,6 +16646,7 @@ dependencies = [
  "markdown",
  "markdown_preview",
  "menu",
+ "migrator",
  "mimalloc",
  "nix",
  "node_runtime",

crates/settings/Cargo.toml 🔗

@@ -37,7 +37,6 @@ streaming-iterator.workspace = true
 tree-sitter-json.workspace = true
 tree-sitter.workspace = true
 util.workspace = true
-migrator.workspace = true
 
 [dev-dependencies]
 fs = { workspace = true, features = ["test-support"] }

crates/settings/src/keymap_file.rs 🔗

@@ -1,11 +1,10 @@
-use anyhow::{anyhow, Context as _, Result};
+use anyhow::{anyhow, Result};
 use collections::{BTreeMap, HashMap, IndexMap};
 use fs::Fs;
 use gpui::{
     Action, ActionBuildError, App, InvalidKeystrokeError, KeyBinding, KeyBindingContextPredicate,
     NoAction, SharedString, KEYSTROKE_PARSE_EXPECTED_MESSAGE,
 };
-use migrator::migrate_keymap;
 use schemars::{
     gen::{SchemaGenerator, SchemaSettings},
     schema::{ArrayValidation, InstanceType, Schema, SchemaObject, SubschemaValidation},
@@ -598,7 +597,7 @@ impl KeymapFile {
         self.0.iter()
     }
 
-    async fn load_keymap_file(fs: &Arc<dyn Fs>) -> Result<String> {
+    pub async fn load_keymap_file(fs: &Arc<dyn Fs>) -> Result<String> {
         match fs.load(paths::keymap_file()).await {
             result @ Ok(_) => result,
             Err(err) => {
@@ -611,41 +610,6 @@ impl KeymapFile {
             }
         }
     }
-
-    pub fn should_migrate_keymap(keymap_file: Self) -> bool {
-        let Ok(old_text) = serde_json::to_string(&keymap_file) else {
-            return false;
-        };
-        migrate_keymap(&old_text).is_some()
-    }
-
-    pub async fn migrate_keymap(fs: Arc<dyn Fs>) -> Result<()> {
-        let old_text = Self::load_keymap_file(&fs).await?;
-        let Some(new_text) = migrate_keymap(&old_text) else {
-            return Ok(());
-        };
-        let keymap_path = paths::keymap_file().as_path();
-        if fs.is_file(keymap_path).await {
-            fs.atomic_write(paths::keymap_backup_file().to_path_buf(), old_text)
-                .await
-                .with_context(|| {
-                    "Failed to create settings backup in home directory".to_string()
-                })?;
-            let resolved_path = fs
-                .canonicalize(keymap_path)
-                .await
-                .with_context(|| format!("Failed to canonicalize keymap path {:?}", keymap_path))?;
-            fs.atomic_write(resolved_path.clone(), new_text)
-                .await
-                .with_context(|| format!("Failed to write keymap to file {:?}", resolved_path))?;
-        } else {
-            fs.atomic_write(keymap_path.to_path_buf(), new_text)
-                .await
-                .with_context(|| format!("Failed to write keymap to file {:?}", keymap_path))?;
-        }
-
-        Ok(())
-    }
 }
 
 // Double quotes a string and wraps it in backticks for markdown inline code..

crates/settings/src/settings_store.rs 🔗

@@ -4,7 +4,7 @@ use ec4rs::{ConfigParser, PropertiesSource, Section};
 use fs::Fs;
 use futures::{channel::mpsc, future::LocalBoxFuture, FutureExt, StreamExt};
 use gpui::{App, AsyncApp, BorrowAppContext, Global, Task, UpdateGlobal};
-use migrator::migrate_settings;
+
 use paths::{local_settings_file_relative_path, EDITORCONFIG_NAME};
 use schemars::{gen::SchemaGenerator, schema::RootSchema, JsonSchema};
 use serde::{de::DeserializeOwned, Deserialize, Serialize};
@@ -390,7 +390,7 @@ impl SettingsStore {
         self.set_user_settings(&new_text, cx).unwrap();
     }
 
-    async fn load_settings(fs: &Arc<dyn Fs>) -> Result<String> {
+    pub async fn load_settings(fs: &Arc<dyn Fs>) -> Result<String> {
         match fs.load(paths::settings_file()).await {
             result @ Ok(_) => result,
             Err(err) => {
@@ -996,51 +996,6 @@ impl SettingsStore {
         properties.use_fallbacks();
         Some(properties)
     }
-
-    pub fn should_migrate_settings(settings: &serde_json::Value) -> bool {
-        let Ok(old_text) = serde_json::to_string(settings) else {
-            return false;
-        };
-        migrate_settings(&old_text).is_some()
-    }
-
-    pub fn migrate_settings(&self, fs: Arc<dyn Fs>) {
-        self.setting_file_updates_tx
-            .unbounded_send(Box::new(move |_: AsyncApp| {
-                async move {
-                    let old_text = Self::load_settings(&fs).await?;
-                    let Some(new_text) = migrate_settings(&old_text) else {
-                        return anyhow::Ok(());
-                    };
-                    let settings_path = paths::settings_file().as_path();
-                    if fs.is_file(settings_path).await {
-                        fs.atomic_write(paths::settings_backup_file().to_path_buf(), old_text)
-                            .await
-                            .with_context(|| {
-                                "Failed to create settings backup in home directory".to_string()
-                            })?;
-                        let resolved_path =
-                            fs.canonicalize(settings_path).await.with_context(|| {
-                                format!("Failed to canonicalize settings path {:?}", settings_path)
-                            })?;
-                        fs.atomic_write(resolved_path.clone(), new_text)
-                            .await
-                            .with_context(|| {
-                                format!("Failed to write settings to file {:?}", resolved_path)
-                            })?;
-                    } else {
-                        fs.atomic_write(settings_path.to_path_buf(), new_text)
-                            .await
-                            .with_context(|| {
-                                format!("Failed to write settings to file {:?}", settings_path)
-                            })?;
-                    }
-                    anyhow::Ok(())
-                }
-                .boxed_local()
-            }))
-            .ok();
-    }
 }
 
 #[derive(Debug, Clone, PartialEq)]

crates/zed/Cargo.toml 🔗

@@ -77,6 +77,7 @@ log.workspace = true
 markdown.workspace = true
 markdown_preview.workspace = true
 menu.workspace = true
+migrator.workspace = true
 mimalloc = { version = "0.1", optional = true }
 nix = { workspace = true, features = ["pthread", "signal"] }
 node_runtime.workspace = true

crates/zed/src/zed.rs 🔗

@@ -4,6 +4,7 @@ pub mod inline_completion_registry;
 pub(crate) mod linux_prompts;
 #[cfg(target_os = "macos")]
 pub(crate) mod mac_only_instance;
+mod migrate;
 mod open_listener;
 mod quick_action_bar;
 #[cfg(target_os = "windows")]
@@ -1213,7 +1214,7 @@ fn show_keymap_migration_notification_if_needed(
     notification_id: NotificationId,
     cx: &mut App,
 ) -> bool {
-    if !KeymapFile::should_migrate_keymap(keymap_file) {
+    if !migrate::should_migrate_keymap(keymap_file) {
         return false;
     }
     let message = MarkdownString(format!(
@@ -1228,7 +1229,7 @@ fn show_keymap_migration_notification_if_needed(
         move |_, cx| {
             let fs = <dyn Fs>::global(cx);
             cx.spawn(move |weak_notification, mut cx| async move {
-                KeymapFile::migrate_keymap(fs).await.ok();
+                migrate::migrate_keymap(fs).await.ok();
                 weak_notification
                     .update(&mut cx, |_, cx| {
                         cx.emit(DismissEvent);
@@ -1247,7 +1248,7 @@ fn show_settings_migration_notification_if_needed(
     settings: serde_json::Value,
     cx: &mut App,
 ) {
-    if !SettingsStore::should_migrate_settings(&settings) {
+    if !migrate::should_migrate_settings(&settings) {
         return;
     }
     let message = MarkdownString(format!(
@@ -1261,7 +1262,7 @@ fn show_settings_migration_notification_if_needed(
         "Backup and Migrate Settings".into(),
         move |_, cx| {
             let fs = <dyn Fs>::global(cx);
-            cx.update_global(|store: &mut SettingsStore, _| store.migrate_settings(fs));
+            migrate::migrate_settings(fs, cx);
             cx.emit(DismissEvent);
         },
         cx,

crates/zed/src/zed/migrate.rs 🔗

@@ -0,0 +1,79 @@
+use std::sync::Arc;
+
+use anyhow::Context;
+use fs::Fs;
+use settings::{KeymapFile, SettingsStore};
+
+pub fn should_migrate_settings(settings: &serde_json::Value) -> bool {
+    let Ok(old_text) = serde_json::to_string(settings) else {
+        return false;
+    };
+    migrator::migrate_settings(&old_text).is_some()
+}
+
+pub fn migrate_settings(fs: Arc<dyn Fs>, cx: &mut gpui::App) {
+    cx.background_executor()
+        .spawn(async move {
+            let old_text = SettingsStore::load_settings(&fs).await?;
+            let Some(new_text) = migrator::migrate_settings(&old_text) else {
+                return anyhow::Ok(());
+            };
+            let settings_path = paths::settings_file().as_path();
+            if fs.is_file(settings_path).await {
+                fs.atomic_write(paths::settings_backup_file().to_path_buf(), old_text)
+                    .await
+                    .with_context(|| {
+                        "Failed to create settings backup in home directory".to_string()
+                    })?;
+                let resolved_path = fs.canonicalize(settings_path).await.with_context(|| {
+                    format!("Failed to canonicalize settings path {:?}", settings_path)
+                })?;
+                fs.atomic_write(resolved_path.clone(), new_text)
+                    .await
+                    .with_context(|| {
+                        format!("Failed to write settings to file {:?}", resolved_path)
+                    })?;
+            } else {
+                fs.atomic_write(settings_path.to_path_buf(), new_text)
+                    .await
+                    .with_context(|| {
+                        format!("Failed to write settings to file {:?}", settings_path)
+                    })?;
+            }
+            Ok(())
+        })
+        .detach_and_log_err(cx);
+}
+
+pub fn should_migrate_keymap(keymap_file: KeymapFile) -> bool {
+    let Ok(old_text) = serde_json::to_string(&keymap_file) else {
+        return false;
+    };
+    migrator::migrate_keymap(&old_text).is_some()
+}
+
+pub async fn migrate_keymap(fs: Arc<dyn Fs>) -> anyhow::Result<()> {
+    let old_text = KeymapFile::load_keymap_file(&fs).await?;
+    let Some(new_text) = migrator::migrate_keymap(&old_text) else {
+        return Ok(());
+    };
+    let keymap_path = paths::keymap_file().as_path();
+    if fs.is_file(keymap_path).await {
+        fs.atomic_write(paths::keymap_backup_file().to_path_buf(), old_text)
+            .await
+            .with_context(|| "Failed to create settings backup in home directory".to_string())?;
+        let resolved_path = fs
+            .canonicalize(keymap_path)
+            .await
+            .with_context(|| format!("Failed to canonicalize keymap path {:?}", keymap_path))?;
+        fs.atomic_write(resolved_path.clone(), new_text)
+            .await
+            .with_context(|| format!("Failed to write keymap to file {:?}", resolved_path))?;
+    } else {
+        fs.atomic_write(keymap_path.to_path_buf(), new_text)
+            .await
+            .with_context(|| format!("Failed to write keymap to file {:?}", keymap_path))?;
+    }
+
+    Ok(())
+}