keymap: Show error notification when keymap is invalid (#15977)

Bennet Bo Fenner created 1 year ago

This adds an error notification that pops up when the user has an
invalid keymap, similar to what we added for settings in #15905.

Release Notes:

- Added a popup that is displayed when the keymap is invalid

Change summary

crates/settings/src/settings_file.rs |  5 +--
crates/zed/src/main.rs               | 35 +++++++++++++++++++++++++----
crates/zed/src/zed.rs                | 17 +++++++++-----
3 files changed, 43 insertions(+), 14 deletions(-)

Detailed changes

crates/settings/src/settings_file.rs

@@ -1,5 +1,4 @@
 use crate::{settings_store::SettingsStore, Settings};
-use anyhow::Result;
 use fs::Fs;
 use futures::{channel::mpsc, StreamExt};
 use gpui::{AppContext, BackgroundExecutor, ReadGlobal, UpdateGlobal};
@@ -67,7 +66,7 @@ pub fn watch_config_file(
 pub fn handle_settings_file_changes(
     mut user_settings_file_rx: mpsc::UnboundedReceiver<String>,
     cx: &mut AppContext,
-    settings_changed: impl Fn(Result<()>, &mut AppContext) + 'static,
+    settings_changed: impl Fn(Option<anyhow::Error>, &mut AppContext) + 'static,
 ) {
     let user_settings_content = cx
         .background_executor()
@@ -85,7 +84,7 @@ pub fn handle_settings_file_changes(
                 if let Err(err) = &result {
                     log::error!("Failed to load user settings: {err}");
                 }
-                settings_changed(result, cx);
+                settings_changed(result.err(), cx);
                 cx.refresh();
             });
             if result.is_err() {

crates/zed/src/main.rs

@@ -430,7 +430,7 @@ fn main() {
 
         settings::init(cx);
         handle_settings_file_changes(user_settings_file_rx, cx, handle_settings_changed);
-        handle_keymap_file_changes(user_keymap_file_rx, cx);
+        handle_keymap_file_changes(user_keymap_file_rx, cx, handle_keymap_changed);
 
         client::init_settings(cx);
         let client = Client::production(cx);
@@ -543,15 +543,39 @@ fn main() {
     });
 }
 
-fn handle_settings_changed(result: Result<()>, cx: &mut AppContext) {
+fn handle_keymap_changed(error: Option<anyhow::Error>, cx: &mut AppContext) {
+    struct KeymapParseErrorNotification;
+    let id = NotificationId::unique::<KeymapParseErrorNotification>();
+
+    for workspace in workspace::local_workspace_windows(cx) {
+        workspace
+            .update(cx, |workspace, cx| match &error {
+                Some(error) => {
+                    workspace.show_notification(id.clone(), cx, |cx| {
+                        cx.new_view(|_| {
+                            MessageNotification::new(format!("Invalid keymap file\n{error}"))
+                                .with_click_message("Open keymap file")
+                                .on_click(|cx| {
+                                    cx.dispatch_action(zed_actions::OpenKeymap.boxed_clone());
+                                    cx.emit(DismissEvent);
+                                })
+                        })
+                    });
+                }
+                None => workspace.dismiss_notification(&id, cx),
+            })
+            .log_err();
+    }
+}
+
+fn handle_settings_changed(error: Option<anyhow::Error>, cx: &mut AppContext) {
     struct SettingsParseErrorNotification;
     let id = NotificationId::unique::<SettingsParseErrorNotification>();
 
     for workspace in workspace::local_workspace_windows(cx) {
         workspace
-            .update(cx, |workspace, cx| match &result {
-                Ok(()) => workspace.dismiss_notification(&id, cx),
-                Err(error) => {
+            .update(cx, |workspace, cx| match &error {
+                Some(error) => {
                     workspace.show_notification(id.clone(), cx, |cx| {
                         cx.new_view(|_| {
                             MessageNotification::new(format!("Invalid settings file\n{error}"))
@@ -563,6 +587,7 @@ fn handle_settings_changed(result: Result<()>, cx: &mut AppContext) {
                         })
                     });
                 }
+                None => workspace.dismiss_notification(&id, cx),
             })
             .log_err();
     }

crates/zed/src/zed.rs

@@ -733,6 +733,7 @@ fn open_log_file(workspace: &mut Workspace, cx: &mut ViewContext<Workspace>) {
 pub fn handle_keymap_file_changes(
     mut user_keymap_file_rx: mpsc::UnboundedReceiver<String>,
     cx: &mut AppContext,
+    keymap_changed: impl Fn(Option<anyhow::Error>, &mut AppContext) + 'static,
 ) {
     BaseKeymap::register(cx);
     VimModeSetting::register(cx);
@@ -761,10 +762,14 @@ pub fn handle_keymap_file_changes(
                 _ = base_keymap_rx.next() => {}
                 user_keymap_content = user_keymap_file_rx.next() => {
                     if let Some(user_keymap_content) = user_keymap_content {
-                        if let Some(keymap_content) = KeymapFile::parse(&user_keymap_content).log_err() {
-                            user_keymap = keymap_content;
-                        } else {
-                            continue
+                        match KeymapFile::parse(&user_keymap_content) {
+                            Ok(keymap_content) => {
+                                cx.update(|cx| keymap_changed(None, cx)).log_err();
+                                user_keymap = keymap_content;
+                            }
+                            Err(error) => {
+                                cx.update(|cx| keymap_changed(Some(error), cx)).log_err();
+                            }
                         }
                     }
                 }
@@ -3097,7 +3102,7 @@ mod tests {
                 PathBuf::from("/keymap.json"),
             );
             handle_settings_file_changes(settings_rx, cx, |_, _| {});
-            handle_keymap_file_changes(keymap_rx, cx);
+            handle_keymap_file_changes(keymap_rx, cx, |_, _| {});
         });
         workspace
             .update(cx, |workspace, cx| {
@@ -3237,7 +3242,7 @@ mod tests {
             );
 
             handle_settings_file_changes(settings_rx, cx, |_, _| {});
-            handle_keymap_file_changes(keymap_rx, cx);
+            handle_keymap_file_changes(keymap_rx, cx, |_, _| {});
         });
 
         cx.background_executor.run_until_parked();