settings: Show error notification when settings are invalid (#15905)

Bennet Bo Fenner and Thorsten created 1 year ago

https://github.com/user-attachments/assets/07627142-e730-4446-a50b-7ef46f8e661c

We want to improve the design in the future, but it fixes a long
standing paper cut for now.

Release Notes:

- Added a popup that is displayed when the settings are invalid

---------

Co-authored-by: Thorsten <[e-mail]>

Change summary

crates/settings/src/settings_file.rs | 10 ++++++--
crates/workspace/src/workspace.rs    |  2 
crates/zed/src/main.rs               | 35 +++++++++++++++++++++++++++--
crates/zed/src/zed.rs                |  4 +-
4 files changed, 42 insertions(+), 9 deletions(-)

Detailed changes

crates/settings/src/settings_file.rs

@@ -1,4 +1,5 @@
 use crate::{settings_store::SettingsStore, Settings};
+use anyhow::Result;
 use fs::Fs;
 use futures::{channel::mpsc, StreamExt};
 use gpui::{AppContext, BackgroundExecutor, ReadGlobal, UpdateGlobal};
@@ -66,6 +67,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,
 ) {
     let user_settings_content = cx
         .background_executor()
@@ -79,9 +81,11 @@ pub fn handle_settings_file_changes(
     cx.spawn(move |mut cx| async move {
         while let Some(user_settings_content) = user_settings_file_rx.next().await {
             let result = cx.update_global(|store: &mut SettingsStore, cx| {
-                store
-                    .set_user_settings(&user_settings_content, cx)
-                    .log_err();
+                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, cx);
                 cx.refresh();
             });
             if result.is_err() {

crates/workspace/src/workspace.rs

@@ -5193,7 +5193,7 @@ fn activate_any_workspace_window(cx: &mut AsyncAppContext) -> Option<WindowHandl
     .flatten()
 }
 
-fn local_workspace_windows(cx: &AppContext) -> Vec<WindowHandle<Workspace>> {
+pub fn local_workspace_windows(cx: &AppContext) -> Vec<WindowHandle<Workspace>> {
     cx.windows()
         .into_iter()
         .filter_map(|window| window.downcast::<Workspace>())

crates/zed/src/main.rs

@@ -19,7 +19,8 @@ use fs::{Fs, RealFs};
 use futures::{future, StreamExt};
 use git::GitHostingProviderRegistry;
 use gpui::{
-    App, AppContext, AsyncAppContext, Context, Global, Task, UpdateGlobal as _, VisualContext,
+    Action, App, AppContext, AsyncAppContext, Context, DismissEvent, Global, Task,
+    UpdateGlobal as _, VisualContext,
 };
 use image_viewer;
 use language::LanguageRegistry;
@@ -46,7 +47,10 @@ use theme::{ActiveTheme, SystemAppearance, ThemeRegistry, ThemeSettings};
 use util::{maybe, parse_env_output, ResultExt, TryFutureExt};
 use uuid::Uuid;
 use welcome::{show_welcome_view, BaseKeymap, FIRST_OPEN};
-use workspace::{AppState, WorkspaceSettings, WorkspaceStore};
+use workspace::{
+    notifications::{simple_message_notification::MessageNotification, NotificationId},
+    AppState, WorkspaceSettings, WorkspaceStore,
+};
 use zed::{
     app_menus, build_window_options, handle_cli_connection, handle_keymap_file_changes,
     initialize_workspace, open_paths_with_positions, OpenListener, OpenRequest,
@@ -425,7 +429,7 @@ fn main() {
         OpenListener::set_global(cx, open_listener.clone());
 
         settings::init(cx);
-        handle_settings_file_changes(user_settings_file_rx, cx);
+        handle_settings_file_changes(user_settings_file_rx, cx, handle_settings_changed);
         handle_keymap_file_changes(user_keymap_file_rx, cx);
 
         client::init_settings(cx);
@@ -539,6 +543,31 @@ fn main() {
     });
 }
 
+fn handle_settings_changed(result: Result<()>, 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) => {
+                    workspace.show_notification(id.clone(), cx, |cx| {
+                        cx.new_view(|_| {
+                            MessageNotification::new(format!("Invalid settings file\n{error}"))
+                                .with_click_message("Open settings file")
+                                .on_click(|cx| {
+                                    cx.dispatch_action(zed_actions::OpenSettings.boxed_clone());
+                                    cx.emit(DismissEvent);
+                                })
+                        })
+                    });
+                }
+            })
+            .log_err();
+    }
+}
+
 fn handle_open_request(
     request: OpenRequest,
     app_state: Arc<AppState>,

crates/zed/src/zed.rs

@@ -3096,7 +3096,7 @@ mod tests {
                 app_state.fs.clone(),
                 PathBuf::from("/keymap.json"),
             );
-            handle_settings_file_changes(settings_rx, cx);
+            handle_settings_file_changes(settings_rx, cx, |_, _| {});
             handle_keymap_file_changes(keymap_rx, cx);
         });
         workspace
@@ -3236,7 +3236,7 @@ mod tests {
                 PathBuf::from("/keymap.json"),
             );
 
-            handle_settings_file_changes(settings_rx, cx);
+            handle_settings_file_changes(settings_rx, cx, |_, _| {});
             handle_keymap_file_changes(keymap_rx, cx);
         });