zed: Fix migration banner not hiding after migration has been carried out (#31723)

Smit Barmase created 6 months ago

- https://github.com/zed-industries/zed/pull/30444

This PR broke migration notification to only emit event when content is
migrated. This resulted in the migration banner not going away after
clicking "Backup and Migrate". It should also emit event when it's not
migrated which removes the banner.

Future: I think we should have better tests in place for banner
visibility.

Release Notes:

- Fixed an issue where migration banner wouldn't go away after clicking
"Backup and Migrate".

Change summary

crates/zed/src/zed.rs         | 26 +++++++++++---------------
crates/zed/src/zed/migrate.rs |  6 +++---
2 files changed, 14 insertions(+), 18 deletions(-)

Detailed changes

crates/zed/src/zed.rs

@@ -1183,19 +1183,15 @@ pub fn handle_settings_file_changes(
             };
 
             let result = cx.update_global(|store: &mut SettingsStore, cx| {
-                let content_migrated = process_settings(content, is_user, store, cx);
-
-                if content_migrated {
-                    if let Some(notifier) = MigrationNotification::try_global(cx) {
-                        notifier.update(cx, |_, cx| {
-                            cx.emit(MigrationEvent::ContentChanged {
-                                migration_type: MigrationType::Settings,
-                                migrated: true,
-                            });
+                let migrating_in_memory = process_settings(content, is_user, store, cx);
+                if let Some(notifier) = MigrationNotification::try_global(cx) {
+                    notifier.update(cx, |_, cx| {
+                        cx.emit(MigrationEvent::ContentChanged {
+                            migration_type: MigrationType::Settings,
+                            migrating_in_memory,
                         });
-                    }
+                    });
                 }
-
                 cx.refresh_windows();
             });
 
@@ -1247,7 +1243,7 @@ pub fn handle_keymap_file_changes(
 
     cx.spawn(async move |cx| {
         let mut user_keymap_content = String::new();
-        let mut content_migrated = false;
+        let mut migrating_in_memory = false;
         loop {
             select_biased! {
                 _ = base_keymap_rx.next() => {},
@@ -1256,10 +1252,10 @@ pub fn handle_keymap_file_changes(
                     if let Some(content) = content {
                         if let Ok(Some(migrated_content)) = migrate_keymap(&content) {
                             user_keymap_content = migrated_content;
-                            content_migrated = true;
+                            migrating_in_memory = true;
                         } else {
                             user_keymap_content = content;
-                            content_migrated = false;
+                            migrating_in_memory = false;
                         }
                     }
                 }
@@ -1269,7 +1265,7 @@ pub fn handle_keymap_file_changes(
                     notifier.update(cx, |_, cx| {
                         cx.emit(MigrationEvent::ContentChanged {
                             migration_type: MigrationType::Keymap,
-                            migrated: content_migrated,
+                            migrating_in_memory,
                         });
                     });
                 }

crates/zed/src/zed/migrate.rs

@@ -29,7 +29,7 @@ pub struct MigrationBanner {
 pub enum MigrationEvent {
     ContentChanged {
         migration_type: MigrationType,
-        migrated: bool,
+        migrating_in_memory: bool,
     },
 }
 
@@ -74,9 +74,9 @@ impl MigrationBanner {
         match event {
             MigrationEvent::ContentChanged {
                 migration_type,
-                migrated,
+                migrating_in_memory,
             } => {
-                if *migrated {
+                if *migrating_in_memory {
                     self.migration_type = Some(*migration_type);
                     self.show(cx);
                 } else {