From 793cd88792be70c5210c49da48c8b5f3db1a1b2a Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner Date: Thu, 8 Aug 2024 14:11:46 +0200 Subject: [PATCH] keymap: Show error notification when keymap is invalid (#15977) 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 --- 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(-) diff --git a/crates/settings/src/settings_file.rs b/crates/settings/src/settings_file.rs index f40e13c40e0a60cab5d972e6aa28dd04a0b08ded..823b75ef42ae17b421411e1dd06d955d5fea311a 100644 --- a/crates/settings/src/settings_file.rs +++ b/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, cx: &mut AppContext, - settings_changed: impl Fn(Result<()>, &mut AppContext) + 'static, + settings_changed: impl Fn(Option, &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() { diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 3d9297cba8e3477ca0cdb628633f72a94c720be8..2eb5b3fc05842e8f307f3280dd4df910e4155433 100644 --- a/crates/zed/src/main.rs +++ b/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, cx: &mut AppContext) { + struct KeymapParseErrorNotification; + let id = NotificationId::unique::(); + + 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, cx: &mut AppContext) { struct SettingsParseErrorNotification; let id = NotificationId::unique::(); 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(); } diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 1799852cc7dedbb96b60ea9045dd53f2d5afd006..1cce7af447a0faf9754074654a1616090b51213a 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -733,6 +733,7 @@ fn open_log_file(workspace: &mut Workspace, cx: &mut ViewContext) { pub fn handle_keymap_file_changes( mut user_keymap_file_rx: mpsc::UnboundedReceiver, cx: &mut AppContext, + keymap_changed: impl Fn(Option, &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();