diff --git a/crates/workspace/src/notifications.rs b/crates/workspace/src/notifications.rs index 0756428a5ceef1..ced055351e31cc 100644 --- a/crates/workspace/src/notifications.rs +++ b/crates/workspace/src/notifications.rs @@ -2,13 +2,21 @@ use crate::{Toast, Workspace}; use anyhow::Context; use anyhow::{anyhow, Result}; use gpui::{ - svg, AppContext, AsyncWindowContext, ClipboardItem, DismissEvent, EventEmitter, PromptLevel, - Render, ScrollHandle, Task, View, ViewContext, VisualContext, WindowContext, + svg, AnyView, AppContext, AsyncWindowContext, ClipboardItem, DismissEvent, EventEmitter, + Global, PromptLevel, Render, ScrollHandle, Task, View, ViewContext, VisualContext, + WindowContext, }; +use std::rc::Rc; use std::{any::TypeId, time::Duration}; use ui::{prelude::*, Tooltip}; use util::ResultExt; +pub fn init(cx: &mut AppContext) { + cx.set_global(GlobalAppNotifications { + app_notifications: Vec::new(), + }) +} + #[derive(Debug, PartialEq, Clone)] pub enum NotificationId { Unique(TypeId), @@ -54,17 +62,33 @@ impl Workspace { cx: &mut ViewContext, build_notification: impl FnOnce(&mut ViewContext) -> View, ) { - self.dismiss_notification_internal(&id, cx); + self.show_notification_without_handling_dismiss_events(&id, cx, |cx| { + let notification = build_notification(cx); + cx.subscribe(¬ification, { + let id = id.clone(); + move |this, _, _: &DismissEvent, cx| { + this.dismiss_notification(&id, cx); + } + }) + .detach(); + notification.into() + }); + } - let notification = build_notification(cx); - cx.subscribe(¬ification, { - let id = id.clone(); - move |this, _, _: &DismissEvent, cx| { - this.dismiss_notification_internal(&id, cx); - } - }) - .detach(); - self.notifications.push((id, notification.into())); + /// Shows a notification in this workspace's window. Caller must handle dismiss. + /// + /// This exists so that the `build_notification` closures stored for app notifications can + /// return `AnyView`. Subscribing to events from an `AnyView` is not supported, so instead that + /// responsibility is pushed to the caller where the `V` type is known. + pub(crate) fn show_notification_without_handling_dismiss_events( + &mut self, + id: &NotificationId, + cx: &mut ViewContext, + build_notification: impl FnOnce(&mut ViewContext) -> AnyView, + ) { + self.dismiss_notification(id, cx); + self.notifications + .push((id.clone(), build_notification(cx))); cx.notify(); } @@ -91,7 +115,14 @@ impl Workspace { } pub fn dismiss_notification(&mut self, id: &NotificationId, cx: &mut ViewContext) { - self.dismiss_notification_internal(id, cx) + self.notifications.retain(|(existing_id, _)| { + if existing_id == id { + cx.notify(); + false + } else { + true + } + }); } pub fn show_toast(&mut self, toast: Toast, cx: &mut ViewContext) { @@ -131,15 +162,18 @@ impl Workspace { cx.notify(); } - fn dismiss_notification_internal(&mut self, id: &NotificationId, cx: &mut ViewContext) { - self.notifications.retain(|(existing_id, _)| { - if existing_id == id { - cx.notify(); - false - } else { - true - } - }); + pub fn show_initial_notifications(&mut self, cx: &mut ViewContext) { + // Allow absence of the global so that tests don't need to initialize it. + let app_notifications = cx + .try_global::() + .iter() + .flat_map(|global| global.app_notifications.iter().cloned()) + .collect::>(); + for (id, build_notification) in app_notifications { + self.show_notification_without_handling_dismiss_events(&id, cx, |cx| { + build_notification(cx) + }); + } } } @@ -467,66 +501,103 @@ pub mod simple_message_notification { } } -/// Shows a notification in the active workspace if there is one, otherwise shows it in all workspaces. -pub fn show_app_notification( +/// Stores app notifications so that they can be shown in new workspaces. +struct GlobalAppNotifications { + app_notifications: Vec<( + NotificationId, + Rc) -> AnyView>, + )>, +} + +impl Global for GlobalAppNotifications {} + +impl GlobalAppNotifications { + pub fn insert( + &mut self, + id: NotificationId, + build_notification: Rc) -> AnyView>, + ) { + self.remove(&id); + self.app_notifications.push((id, build_notification)) + } + + pub fn remove(&mut self, id: &NotificationId) { + self.app_notifications + .retain(|(existing_id, _)| existing_id != id); + } +} + +/// Shows a notification in all workspaces. New workspaces will also receive the notification - this +/// is particularly to handle notifications that occur on initialization before any workspaces +/// exist. If the notification is dismissed within any workspace, it will be removed from all. +pub fn show_app_notification( id: NotificationId, cx: &mut AppContext, - build_notification: impl Fn(&mut ViewContext) -> View, + build_notification: impl Fn(&mut ViewContext) -> View + 'static, ) -> Result<()> { - let workspaces_to_notify = if let Some(active_workspace_window) = cx - .active_window() - .and_then(|window| window.downcast::()) - { - vec![active_workspace_window] - } else { - let mut workspaces_to_notify = Vec::new(); - for window in cx.windows() { - if let Some(workspace_window) = window.downcast::() { - workspaces_to_notify.push(workspace_window); - } + // Handle dismiss events by removing the notification from all workspaces. + let build_notification: Rc) -> AnyView> = Rc::new({ + let id = id.clone(); + move |cx| { + let notification = build_notification(cx); + cx.subscribe(¬ification, { + let id = id.clone(); + move |_, _, _: &DismissEvent, cx| { + dismiss_app_notification(&id, cx); + } + }) + .detach(); + notification.into() } - workspaces_to_notify - }; + }); + + // Store the notification so that new workspaces also receive it. + cx.global_mut::() + .insert(id.clone(), build_notification.clone()); - let mut notified = false; let mut notify_errors = Vec::new(); - for workspace_window in workspaces_to_notify { - let notify_result = workspace_window.update(cx, |workspace, cx| { - workspace.show_notification(id.clone(), cx, &build_notification); - }); - match notify_result { - Ok(()) => notified = true, - Err(notify_err) => notify_errors.push(notify_err), + for window in cx.windows() { + if let Some(workspace_window) = window.downcast::() { + let notify_result = workspace_window.update(cx, |workspace, cx| { + workspace.show_notification_without_handling_dismiss_events(&id, cx, |cx| { + build_notification(cx) + }); + }); + match notify_result { + Ok(()) => {} + Err(notify_err) => notify_errors.push(notify_err), + } } } - if notified { + if notify_errors.is_empty() { Ok(()) } else { - if notify_errors.is_empty() { - Err(anyhow!("Found no workspaces to show notification.")) - } else { - Err(anyhow!( - "No workspaces were able to show notification. Errors:\n\n{}", - notify_errors - .iter() - .map(|e| e.to_string()) - .collect::>() - .join("\n\n") - )) - } + Err(anyhow!( + "No workspaces were able to show notification. Errors:\n\n{}", + notify_errors + .iter() + .map(|e| e.to_string()) + .collect::>() + .join("\n\n") + )) } } pub fn dismiss_app_notification(id: &NotificationId, cx: &mut AppContext) { + cx.global_mut::().remove(id); for window in cx.windows() { if let Some(workspace_window) = window.downcast::() { - workspace_window - .update(cx, |workspace, cx| { - workspace.dismiss_notification(&id, cx); + let id = id.clone(); + // This spawn is necessary in order to dismiss the notification on which the click + // occurred, because in that case we're already in the middle of an update. + cx.spawn(move |mut cx| async move { + workspace_window.update(&mut cx, |workspace, cx| { + workspace.dismiss_notification(&id, cx) }) - .ok(); + }) + .detach_and_log_err(cx); } } } @@ -556,7 +627,7 @@ where match self { Ok(value) => Some(value), Err(err) => { - log::error!("TODO {err:?}"); + log::error!("Showing error notification in workspace: {err:?}"); workspace.show_error(&err, cx); None } @@ -584,10 +655,17 @@ where Ok(value) => Some(value), Err(err) => { let message: SharedString = format!("Error: {err}").into(); - show_app_notification(workspace_error_notification_id(), cx, |cx| { - cx.new_view(|_cx| ErrorMessagePrompt::new(message.clone())) + log::error!("Showing error notification in app: {message}"); + show_app_notification(workspace_error_notification_id(), cx, { + let message = message.clone(); + move |cx| { + cx.new_view({ + let message = message.clone(); + move |_cx| ErrorMessagePrompt::new(message) + }) + } }) - .with_context(|| format!("Showing error notification: {message}")) + .with_context(|| format!("Error while showing error notification: {message}")) .log_err(); None } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 8551d490778484..6b3486fc8c1139 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -368,6 +368,7 @@ fn prompt_and_open_paths( pub fn init(app_state: Arc, cx: &mut AppContext) { init_settings(cx); + notifications::init(cx); theme_preview::init(cx); cx.on_action(Workspace::close_global); @@ -1056,6 +1057,7 @@ impl Workspace { cx.defer(|this, cx| { this.update_window_title(cx); + this.show_initial_notifications(cx); }); Workspace { weak_self: weak_handle.clone(), diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index aa4f08ba699ead..85c22afa7a4a16 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -22,10 +22,10 @@ use feature_flags::FeatureFlagAppExt; use futures::FutureExt; use futures::{channel::mpsc, select_biased, StreamExt}; use gpui::{ - actions, point, px, Action, AnyWindowHandle, AppContext, AsyncAppContext, Context, - DismissEvent, Element, FocusableView, KeyBinding, MenuItem, ParentElement, PathPromptOptions, - PromptLevel, ReadGlobal, SharedString, Styled, Task, TitlebarOptions, View, ViewContext, - VisualContext, WindowKind, WindowOptions, + actions, point, px, Action, AppContext, AsyncAppContext, Context, DismissEvent, Element, + FocusableView, KeyBinding, MenuItem, ParentElement, PathPromptOptions, PromptLevel, ReadGlobal, + SharedString, Styled, Task, TitlebarOptions, View, ViewContext, VisualContext, WindowKind, + WindowOptions, }; pub use open_listener::*; use outline_panel::OutlinePanel; @@ -1017,16 +1017,6 @@ pub fn handle_keymap_file_changes( }) .detach(); - // Need to notify about keymap load errors when new workspaces are created, so that initial - // keymap load errors are shown to the user. - let (new_workspace_window_tx, mut new_workspace_window_rx) = mpsc::unbounded(); - cx.observe_new_views(move |_: &mut Workspace, cx| { - new_workspace_window_tx - .unbounded_send(cx.window_handle()) - .unwrap(); - }) - .detach(); - let mut current_mapping = settings::get_key_equivalents(cx.keyboard_layout()); cx.on_keyboard_layout_change(move |cx| { let next_mapping = settings::get_key_equivalents(cx.keyboard_layout()); @@ -1044,65 +1034,34 @@ pub fn handle_keymap_file_changes( cx.spawn(move |cx| async move { let mut user_keymap_content = String::new(); - enum LastError { - None, - JsonError(anyhow::Error), - LoadError(MarkdownString), - } - let mut last_load_error = LastError::None; loop { - let new_workspace_window = select_biased! { - _ = base_keymap_rx.next() => None, - _ = keyboard_layout_rx.next() => None, - workspace = new_workspace_window_rx.next() => workspace, + 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; } - None } }; cx.update(|cx| { - // No need to reload keymaps when a new workspace is added, just need to send the notification to it. - if new_workspace_window.is_none() { - let load_result = KeymapFile::load(&user_keymap_content, cx); - match load_result { - KeymapFileLoadResult::Success { key_bindings } => { + let load_result = KeymapFile::load(&user_keymap_content, cx); + match load_result { + KeymapFileLoadResult::Success { key_bindings } => { + reload_keymaps(cx, key_bindings); + dismiss_app_notification(¬ification_id, cx); + } + KeymapFileLoadResult::SomeFailedToLoad { + key_bindings, + error_message, + } => { + if !key_bindings.is_empty() { reload_keymaps(cx, key_bindings); - dismiss_app_notification(¬ification_id, cx); - last_load_error = LastError::None; - } - KeymapFileLoadResult::SomeFailedToLoad { - key_bindings, - error_message, - } => { - if !key_bindings.is_empty() { - reload_keymaps(cx, key_bindings); - } - last_load_error = LastError::LoadError(error_message); } - KeymapFileLoadResult::JsonParseFailure { error } => { - last_load_error = LastError::JsonError(error); - } - } - } - match &last_load_error { - LastError::None => {} - LastError::JsonError(err) => { - show_keymap_file_json_error( - new_workspace_window, - notification_id.clone(), - err, - cx, - ); + show_keymap_file_load_error(notification_id.clone(), error_message, cx) } - LastError::LoadError(message) => { - show_keymap_file_load_error( - new_workspace_window, - notification_id.clone(), - message.clone(), - cx, - ); + KeymapFileLoadResult::JsonParseFailure { error } => { + show_keymap_file_json_error(notification_id.clone(), &error, cx) } } }) @@ -1113,32 +1072,26 @@ pub fn handle_keymap_file_changes( } fn show_keymap_file_json_error( - new_workspace_window: Option, notification_id: NotificationId, error: &anyhow::Error, cx: &mut AppContext, ) { let message: SharedString = format!("JSON parse error in keymap file. Bindings not reloaded.\n\n{error}").into(); - show_notification_to_specific_workspace_or_all_workspaces( - new_workspace_window, - notification_id, - cx, - move |cx| { - cx.new_view(|_cx| { - MessageNotification::new(message.clone()) - .with_click_message("Open keymap file") - .on_click(|cx| { - cx.dispatch_action(zed_actions::OpenKeymap.boxed_clone()); - cx.emit(DismissEvent); - }) - }) - }, - ); + show_app_notification(notification_id, cx, move |cx| { + cx.new_view(|_cx| { + MessageNotification::new(message.clone()) + .with_click_message("Open keymap file") + .on_click(|cx| { + cx.dispatch_action(zed_actions::OpenKeymap.boxed_clone()); + cx.emit(DismissEvent); + }) + }) + }) + .log_err(); } fn show_keymap_file_load_error( - new_workspace_window: Option, notification_id: NotificationId, markdown_error_message: MarkdownString, cx: &mut AppContext, @@ -1157,57 +1110,34 @@ fn show_keymap_file_load_error( cx.spawn(move |cx| async move { let parsed_markdown = Rc::new(parsed_markdown.await); cx.update(|cx| { - show_notification_to_specific_workspace_or_all_workspaces( - new_workspace_window, - notification_id, - cx, - move |cx| { - let workspace_handle = cx.view().downgrade(); - let parsed_markdown = parsed_markdown.clone(); - cx.new_view(move |_cx| { - MessageNotification::new_from_builder(move |cx| { - gpui::div() - .text_xs() - .child(markdown_preview::markdown_renderer::render_parsed_markdown( - &parsed_markdown.clone(), - Some(workspace_handle.clone()), - cx, - )) - .into_any() - }) - .with_click_message("Open keymap file") - .on_click(|cx| { - cx.dispatch_action(zed_actions::OpenKeymap.boxed_clone()); - cx.emit(DismissEvent); - }) + show_app_notification(notification_id, cx, move |cx| { + let workspace_handle = cx.view().downgrade(); + let parsed_markdown = parsed_markdown.clone(); + cx.new_view(move |_cx| { + MessageNotification::new_from_builder(move |cx| { + gpui::div() + .text_xs() + .child(markdown_preview::markdown_renderer::render_parsed_markdown( + &parsed_markdown.clone(), + Some(workspace_handle.clone()), + cx, + )) + .into_any() }) - }, - ) + .with_click_message("Open keymap file") + .on_click(|cx| { + cx.dispatch_action(zed_actions::OpenKeymap.boxed_clone()); + cx.emit(DismissEvent); + }) + }) + }) + .log_err(); }) .ok(); }) .detach(); } -fn show_notification_to_specific_workspace_or_all_workspaces( - new_workspace_window: Option, - notification_id: NotificationId, - cx: &mut AppContext, - build_notification: impl Fn(&mut ViewContext) -> View, -) where - V: workspace::notifications::Notification, -{ - if let Some(workspace_window) = new_workspace_window.and_then(|w| w.downcast::()) { - workspace_window - .update(cx, |workspace, cx| { - workspace.show_notification(notification_id, cx, build_notification); - }) - .ok(); - } else { - show_app_notification(notification_id, cx, build_notification).ok(); - } -} - fn reload_keymaps(cx: &mut AppContext, user_key_bindings: Vec) { cx.clear_key_bindings(); load_default_keymap(cx);