migrator: In-memory migration and improved UX (#24621)

This PR adds: - Support for deprecated keymap and settings (In-memory migration) - Migration prompt only shown in `settings.json` / `keymap.json`. Release Notes: - The migration banner will only appear in `settings.json` and `keymap.json` if you have deprecated settings or keybindings, allowing you to migrate them to work with the new version on Zed.
2025-02-12 06:47:08 +05:30 · 2025-02-12 06:47:08 +05:30 · 65934ae181
commit 65934ae181
parent 498bb518ff
6 changed files with 375 additions and 196 deletions
--- a/crates/zed/src/main.rs
+++ b/crates/zed/src/main.rs
@ -34,7 +34,7 @@ use project::project_settings::ProjectSettings;
 use recent_projects::{open_ssh_project, SshSettings};
 use release_channel::{AppCommitSha, AppVersion, ReleaseChannel};
 use session::{AppSession, Session};
-use settings::{handle_settings_file_changes, watch_config_file, Settings, SettingsStore};
+use settings::{watch_config_file, Settings, SettingsStore};
 use simplelog::ConfigBuilder;
 use std::{
    env,
@ -52,8 +52,9 @@ use welcome::{show_welcome_view, BaseKeymap, FIRST_OPEN};
 use workspace::{AppState, SerializedWorkspaceLocation, WorkspaceSettings, WorkspaceStore};
 use zed::{
    app_menus, build_window_options, derive_paths_with_position, handle_cli_connection,
-    handle_keymap_file_changes, handle_settings_changed, initialize_workspace,
-    inline_completion_registry, open_paths_with_positions, OpenListener, OpenRequest,
+    handle_keymap_file_changes, handle_settings_changed, handle_settings_file_changes,
+    initialize_workspace, inline_completion_registry, open_paths_with_positions, OpenListener,
+    OpenRequest,
 };

 #[cfg(unix)]
--- a/crates/zed/src/zed.rs
+++ b/crates/zed/src/zed.rs
@ -21,14 +21,16 @@ use command_palette_hooks::CommandPaletteFilter;
 use editor::ProposedChangesEditorToolbar;
 use editor::{scroll::Autoscroll, Editor, MultiBuffer};
 use feature_flags::{FeatureFlagAppExt, FeatureFlagViewExt, GitUiFeatureFlag};
-use fs::Fs;
 use futures::{channel::mpsc, select_biased, StreamExt};
 use gpui::{
    actions, point, px, Action, App, AppContext as _, AsyncApp, Context, DismissEvent, Element,
    Entity, Focusable, KeyBinding, MenuItem, ParentElement, PathPromptOptions, PromptLevel,
-    ReadGlobal, SharedString, Styled, Task, TitlebarOptions, Window, WindowKind, WindowOptions,
+    ReadGlobal, SharedString, Styled, Task, TitlebarOptions, UpdateGlobal, Window, WindowKind,
+    WindowOptions,
 };
 use image_viewer::ImageInfo;
+use migrate::{MigrationType, MigratorBanner, MigratorEvent, MigratorNotification};
+use migrator::{migrate_keymap, migrate_settings};
 pub use open_listener::*;
 use outline_panel::OutlinePanel;
 use paths::{local_settings_file_relative_path, local_tasks_file_relative_path};
@ -150,6 +152,7 @@ pub fn initialize_workspace(
        let workspace_handle = cx.entity().clone();
        let center_pane = workspace.active_pane().clone();
        initialize_pane(workspace, &center_pane, window, cx);
+
        cx.subscribe_in(&workspace_handle, window, {
            move |workspace, _, event, window, cx| match event {
                workspace::Event::PaneAdded(pane) => {
@ -855,7 +858,6 @@ fn initialize_pane(
            toolbar.add_item(breadcrumbs, window, cx);
            let buffer_search_bar = cx.new(|cx| search::BufferSearchBar::new(window, cx));
            toolbar.add_item(buffer_search_bar.clone(), window, cx);
-
            let proposed_change_bar = cx.new(|_| ProposedChangesEditorToolbar::new());
            toolbar.add_item(proposed_change_bar, window, cx);
            let quick_action_bar =
@ -869,6 +871,8 @@ fn initialize_pane(
            toolbar.add_item(lsp_log_item, window, cx);
            let syntax_tree_item = cx.new(|_| language_tools::SyntaxTreeToolbarItemView::new());
            toolbar.add_item(syntax_tree_item, window, cx);
+            let migrator_banner = cx.new(|cx| MigratorBanner::new(workspace, cx));
+            toolbar.add_item(migrator_banner, window, cx);
        })
    });
 }
@ -1097,6 +1101,68 @@ fn open_log_file(workspace: &mut Workspace, window: &mut Window, cx: &mut Contex
        .detach();
 }

+pub fn handle_settings_file_changes(
+    mut user_settings_file_rx: mpsc::UnboundedReceiver<String>,
+    cx: &mut App,
+    settings_changed: impl Fn(Option<anyhow::Error>, &mut App) + 'static,
+) {
+    MigratorNotification::set_global(cx.new(|_| MigratorNotification), cx);
+    let content = cx
+        .background_executor()
+        .block(user_settings_file_rx.next())
+        .unwrap();
+    let user_settings_content = if let Ok(Some(migrated_content)) = migrate_settings(&content) {
+        migrated_content
+    } else {
+        content
+    };
+    SettingsStore::update_global(cx, |store, cx| {
+        let result = store.set_user_settings(&user_settings_content, cx);
+        if let Err(err) = &result {
+            log::error!("Failed to load user settings: {err}");
+        }
+        settings_changed(result.err(), cx);
+    });
+    cx.spawn(move |cx| async move {
+        while let Some(content) = user_settings_file_rx.next().await {
+            let user_settings_content;
+            let content_migrated;
+
+            if let Ok(Some(migrated_content)) = migrate_settings(&content) {
+                user_settings_content = migrated_content;
+                content_migrated = true;
+            } else {
+                user_settings_content = content;
+                content_migrated = false;
+            }
+
+            cx.update(|cx| {
+                if let Some(notifier) = MigratorNotification::try_global(cx) {
+                    notifier.update(cx, |_, cx| {
+                        cx.emit(MigratorEvent::ContentChanged {
+                            migration_type: MigrationType::Settings,
+                            migrated: content_migrated,
+                        });
+                    });
+                }
+            })
+            .ok();
+            let result = cx.update_global(|store: &mut SettingsStore, cx| {
+                let result = store.set_user_settings(&user_settings_content, cx);
+                if let Err(err) = &result {
+                    log::error!("Failed to load user settings: {err}");
+                }
+                settings_changed(result.err(), cx);
+                cx.refresh_windows();
+            });
+            if result.is_err() {
+                break; // App dropped
+            }
+        }
+    })
+    .detach();
+}
+
 pub fn handle_keymap_file_changes(
    mut user_keymap_file_rx: mpsc::UnboundedReceiver<String>,
    cx: &mut App,
@ -1137,47 +1203,46 @@ pub fn handle_keymap_file_changes(

    cx.spawn(move |cx| async move {
        let mut user_keymap_content = String::new();
+        let mut content_migrated = false;
        loop {
            select_biased! {
                _ = base_keymap_rx.next() => {},
                _ = keyboard_layout_rx.next() => {},
                content = user_keymap_file_rx.next() => {
                    if let Some(content) = content {
-                        user_keymap_content = content;
+                        if let Ok(Some(migrated_content)) = migrate_keymap(&content) {
+                            user_keymap_content = migrated_content;
+                            content_migrated = true;
+                        } else {
+                            user_keymap_content = content;
+                            content_migrated = false;
+                        }
                    }
                }
            };
            cx.update(|cx| {
+                if let Some(notifier) = MigratorNotification::try_global(cx) {
+                    notifier.update(cx, |_, cx| {
+                        cx.emit(MigratorEvent::ContentChanged {
+                            migration_type: MigrationType::Keymap,
+                            migrated: content_migrated,
+                        });
+                    });
+                }
                let load_result = KeymapFile::load(&user_keymap_content, cx);
                match load_result {
-                    KeymapFileLoadResult::Success {
-                        key_bindings,
-                        keymap_file,
-                    } => {
+                    KeymapFileLoadResult::Success { key_bindings } => {
                        reload_keymaps(cx, key_bindings);
-                        dismiss_app_notification(&notification_id, cx);
-                        show_keymap_migration_notification_if_needed(
-                            keymap_file,
-                            notification_id.clone(),
-                            cx,
-                        );
+                        dismiss_app_notification(&notification_id.clone(), cx);
                    }
                    KeymapFileLoadResult::SomeFailedToLoad {
                        key_bindings,
-                        keymap_file,
                        error_message,
                    } => {
                        if !key_bindings.is_empty() {
                            reload_keymaps(cx, key_bindings);
                        }
-                        dismiss_app_notification(&notification_id, cx);
-                        if !show_keymap_migration_notification_if_needed(
-                            keymap_file,
-                            notification_id.clone(),
-                            cx,
-                        ) {
-                            show_keymap_file_load_error(notification_id.clone(), error_message, cx);
-                        }
+                        show_keymap_file_load_error(notification_id.clone(), error_message, cx);
                    }
                    KeymapFileLoadResult::JsonParseFailure { error } => {
                        show_keymap_file_json_error(notification_id.clone(), &error, cx)
@ -1209,66 +1274,6 @@ fn show_keymap_file_json_error(
    });
 }

-fn show_keymap_migration_notification_if_needed(
-    keymap_file: KeymapFile,
-    notification_id: NotificationId,
-    cx: &mut App,
-) -> bool {
-    if !migrate::should_migrate_keymap(keymap_file) {
-        return false;
-    }
-    let message = MarkdownString(format!(
-        "Keymap migration needed, as the format for some actions has changed. \
-        You can migrate your keymap by clicking below. A backup will be created at {}.",
-        MarkdownString::inline_code(&paths::keymap_backup_file().to_string_lossy())
-    ));
-    show_markdown_app_notification(
-        notification_id,
-        message,
-        "Backup and Migrate Keymap".into(),
-        move |_, cx| {
-            let fs = <dyn Fs>::global(cx);
-            cx.spawn(move |weak_notification, mut cx| async move {
-                migrate::migrate_keymap(fs).await.ok();
-                weak_notification
-                    .update(&mut cx, |_, cx| {
-                        cx.emit(DismissEvent);
-                    })
-                    .ok();
-            })
-            .detach();
-        },
-        cx,
-    );
-    return true;
-}
-
-fn show_settings_migration_notification_if_needed(
-    notification_id: NotificationId,
-    settings: serde_json::Value,
-    cx: &mut App,
-) {
-    if !migrate::should_migrate_settings(&settings) {
-        return;
-    }
-    let message = MarkdownString(format!(
-        "Settings migration needed, as the format for some settings has changed. \
-            You can migrate your settings by clicking below. A backup will be created at {}.",
-        MarkdownString::inline_code(&paths::settings_backup_file().to_string_lossy())
-    ));
-    show_markdown_app_notification(
-        notification_id,
-        message,
-        "Backup and Migrate Settings".into(),
-        move |_, cx| {
-            let fs = <dyn Fs>::global(cx);
-            migrate::migrate_settings(fs, cx);
-            cx.emit(DismissEvent);
-        },
-        cx,
-    );
-}
-
 fn show_keymap_file_load_error(
    notification_id: NotificationId,
    error_message: MarkdownString,
@ -1363,12 +1368,12 @@ pub fn load_default_keymap(cx: &mut App) {
    }
 }

-pub fn handle_settings_changed(result: Result<serde_json::Value, anyhow::Error>, cx: &mut App) {
+pub fn handle_settings_changed(error: Option<anyhow::Error>, cx: &mut App) {
    struct SettingsParseErrorNotification;
    let id = NotificationId::unique::<SettingsParseErrorNotification>();

-    match result {
-        Err(error) => {
+    match error {
+        Some(error) => {
            if let Some(InvalidSettingsError::LocalSettings { .. }) =
                error.downcast_ref::<InvalidSettingsError>()
            {
@ -1387,9 +1392,8 @@ pub fn handle_settings_changed(result: Result<serde_json::Value, anyhow::Error>,
                })
            });
        }
-        Ok(settings) => {
+        None => {
            dismiss_app_notification(&id, cx);
-            show_settings_migration_notification_if_needed(id, settings, cx);
        }
    }
 }
@ -1672,7 +1676,7 @@ mod tests {
    use language::{LanguageMatcher, LanguageRegistry};
    use project::{project_settings::ProjectSettings, Project, ProjectPath, WorktreeSettings};
    use serde_json::json;
-    use settings::{handle_settings_file_changes, watch_config_file, SettingsStore};
+    use settings::{watch_config_file, SettingsStore};
    use std::{
        path::{Path, PathBuf},
        time::Duration,
--- a/crates/zed/src/zed/migrate.rs
+++ b/crates/zed/src/zed/migrate.rs
@ -1,63 +1,248 @@
+use anyhow::{Context as _, Result};
+use editor::Editor;
+use fs::Fs;
+use markdown_preview::markdown_elements::ParsedMarkdown;
+use markdown_preview::markdown_renderer::render_parsed_markdown;
+use migrator::{migrate_keymap, migrate_settings};
+use settings::{KeymapFile, SettingsStore};
+use util::markdown::MarkdownString;
+use util::ResultExt;
+
 use std::sync::Arc;

-use anyhow::Context;
-use fs::Fs;
-use settings::{KeymapFile, SettingsStore};
+use gpui::{Entity, EventEmitter, Global, WeakEntity};
+use ui::prelude::*;
+use workspace::item::ItemHandle;
+use workspace::{ToolbarItemEvent, ToolbarItemLocation, ToolbarItemView, Workspace};

-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)
-        .ok()
-        .flatten()
-        .is_some()
+#[derive(Debug, Copy, Clone, PartialEq)]
+pub enum MigrationType {
+    Keymap,
+    Settings,
 }

-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)
-                    })?;
+pub struct MigratorBanner {
+    migration_type: Option<MigrationType>,
+    message: ParsedMarkdown,
+    workspace: WeakEntity<Workspace>,
+}
+
+pub enum MigratorEvent {
+    ContentChanged {
+        migration_type: MigrationType,
+        migrated: bool,
+    },
+}
+
+pub struct MigratorNotification;
+
+impl EventEmitter<MigratorEvent> for MigratorNotification {}
+
+impl MigratorNotification {
+    pub fn try_global(cx: &App) -> Option<Entity<Self>> {
+        cx.try_global::<GlobalMigratorNotification>()
+            .map(|notifier| notifier.0.clone())
+    }
+
+    pub fn set_global(notifier: Entity<Self>, cx: &mut App) {
+        cx.set_global(GlobalMigratorNotification(notifier));
+    }
+}
+
+struct GlobalMigratorNotification(Entity<MigratorNotification>);
+
+impl Global for GlobalMigratorNotification {}
+
+impl MigratorBanner {
+    pub fn new(workspace: &Workspace, cx: &mut Context<'_, Self>) -> Self {
+        if let Some(notifier) = MigratorNotification::try_global(cx) {
+            cx.subscribe(
+                &notifier,
+                move |migrator_banner, _, event: &MigratorEvent, cx| {
+                    migrator_banner.handle_notification(event, cx);
+                },
+            )
+            .detach();
+        }
+        Self {
+            migration_type: None,
+            message: ParsedMarkdown { children: vec![] },
+            workspace: workspace.weak_handle(),
+        }
+    }
+    fn handle_notification(&mut self, event: &MigratorEvent, cx: &mut Context<'_, Self>) {
+        match event {
+            MigratorEvent::ContentChanged {
+                migration_type,
+                migrated,
+            } => {
+                if self.migration_type == Some(*migration_type) {
+                    let location = if *migrated {
+                        ToolbarItemLocation::Secondary
+                    } else {
+                        ToolbarItemLocation::Hidden
+                    };
+                    cx.emit(ToolbarItemEvent::ChangeLocation(location));
+                    cx.notify();
+                }
            }
-            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).ok().flatten().is_some()
+impl EventEmitter<ToolbarItemEvent> for MigratorBanner {}
+
+impl ToolbarItemView for MigratorBanner {
+    fn set_active_pane_item(
+        &mut self,
+        active_pane_item: Option<&dyn ItemHandle>,
+        window: &mut Window,
+        cx: &mut Context<Self>,
+    ) -> ToolbarItemLocation {
+        cx.notify();
+        let Some(target) = active_pane_item
+            .and_then(|item| item.act_as::<Editor>(cx))
+            .and_then(|editor| editor.update(cx, |editor, cx| editor.target_file_abs_path(cx)))
+        else {
+            return ToolbarItemLocation::Hidden;
+        };
+
+        if &target == paths::keymap_file() {
+            self.migration_type = Some(MigrationType::Keymap);
+            let fs = <dyn Fs>::global(cx);
+            let should_migrate = should_migrate_keymap(fs);
+            cx.spawn_in(window, |this, mut cx| async move {
+                if let Ok(true) = should_migrate.await {
+                    this.update(&mut cx, |_, cx| {
+                        cx.emit(ToolbarItemEvent::ChangeLocation(
+                            ToolbarItemLocation::Secondary,
+                        ));
+                        cx.notify();
+                    })
+                    .log_err();
+                }
+            })
+            .detach();
+        } else if &target == paths::settings_file() {
+            self.migration_type = Some(MigrationType::Settings);
+            let fs = <dyn Fs>::global(cx);
+            let should_migrate = should_migrate_settings(fs);
+            cx.spawn_in(window, |this, mut cx| async move {
+                if let Ok(true) = should_migrate.await {
+                    this.update(&mut cx, |_, cx| {
+                        cx.emit(ToolbarItemEvent::ChangeLocation(
+                            ToolbarItemLocation::Secondary,
+                        ));
+                        cx.notify();
+                    })
+                    .log_err();
+                }
+            })
+            .detach();
+        }
+
+        if let Some(migration_type) = self.migration_type {
+            cx.spawn_in(window, |this, mut cx| async move {
+                let message = MarkdownString(format!(
+                    "Your {} require migration to support this version of Zed. A backup will be saved to {}.",
+                    match migration_type {
+                        MigrationType::Keymap => "keymap",
+                        MigrationType::Settings => "settings",
+                    },
+                    match migration_type {
+                        MigrationType::Keymap => paths::keymap_backup_file().to_string_lossy(),
+                        MigrationType::Settings => paths::settings_backup_file().to_string_lossy(),
+                    },
+                ));
+                let parsed_markdown = cx
+                    .background_executor()
+                    .spawn(async move {
+                        let file_location_directory = None;
+                        let language_registry = None;
+                        markdown_preview::markdown_parser::parse_markdown(
+                            &message.0,
+                            file_location_directory,
+                            language_registry,
+                        )
+                        .await
+                    })
+                    .await;
+                this
+                    .update(&mut cx, |this, _| {
+                        this.message = parsed_markdown;
+                    })
+                    .log_err();
+            })
+            .detach();
+        }
+
+        return ToolbarItemLocation::Hidden;
+    }
 }

-pub async fn migrate_keymap(fs: Arc<dyn Fs>) -> anyhow::Result<()> {
+impl Render for MigratorBanner {
+    fn render(&mut self, window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
+        let migration_type = self.migration_type;
+        h_flex()
+            .py_1()
+            .px_2()
+            .justify_between()
+            .bg(cx.theme().status().info_background)
+            .rounded_md()
+            .gap_2()
+            .overflow_hidden()
+            .child(
+                render_parsed_markdown(&self.message, Some(self.workspace.clone()), window, cx)
+                    .text_ellipsis(),
+            )
+            .child(
+                Button::new(
+                    SharedString::from("backup-and-migrate"),
+                    "Backup and Migrate",
+                )
+                .style(ButtonStyle::Filled)
+                .on_click(move |_, _, cx| {
+                    let fs = <dyn Fs>::global(cx);
+                    match migration_type {
+                        Some(MigrationType::Keymap) => {
+                            cx.spawn(
+                                move |_| async move { write_keymap_migration(&fs).await.ok() },
+                            )
+                            .detach();
+                        }
+                        Some(MigrationType::Settings) => {
+                            cx.spawn(
+                                move |_| async move { write_settings_migration(&fs).await.ok() },
+                            )
+                            .detach();
+                        }
+                        None => unreachable!(),
+                    }
+                }),
+            )
+            .into_any_element()
+    }
+}
+
+async fn should_migrate_keymap(fs: Arc<dyn Fs>) -> Result<bool> {
    let old_text = KeymapFile::load_keymap_file(&fs).await?;
-    let Some(new_text) = migrator::migrate_keymap(&old_text)? else {
+    if let Ok(Some(_)) = migrate_keymap(&old_text) {
+        return Ok(true);
+    };
+    Ok(false)
+}
+
+async fn should_migrate_settings(fs: Arc<dyn Fs>) -> Result<bool> {
+    let old_text = SettingsStore::load_settings(&fs).await?;
+    if let Ok(Some(_)) = migrate_settings(&old_text) {
+        return Ok(true);
+    };
+    Ok(false)
+}
+
+async fn write_keymap_migration(fs: &Arc<dyn Fs>) -> Result<()> {
+    let old_text = KeymapFile::load_keymap_file(fs).await?;
+    let Ok(Some(new_text)) = migrate_keymap(&old_text) else {
        return Ok(());
    };
    let keymap_path = paths::keymap_file().as_path();
@ -77,6 +262,30 @@ pub async fn migrate_keymap(fs: Arc<dyn Fs>) -> anyhow::Result<()> {
            .await
            .with_context(|| format!("Failed to write keymap to file {:?}", keymap_path))?;
    }
-
+    Ok(())
+}
+
+async fn write_settings_migration(fs: &Arc<dyn Fs>) -> Result<()> {
+    let old_text = SettingsStore::load_settings(fs).await?;
+    let Ok(Some(new_text)) = migrate_settings(&old_text) else {
+        return 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(())
 }