From 3564e95f271aa872941b5dc1a47641c54467de42 Mon Sep 17 00:00:00 2001 From: Kay Simmons Date: Mon, 20 Feb 2023 01:22:08 -0800 Subject: [PATCH 1/4] Add labeled tasks to gpui and observe them in the activity status to give feedback when we are still waiting for the language server --- .../src/activity_indicator.rs | 147 +++++++++++------- crates/editor/src/editor.rs | 4 +- crates/gpui/src/app.rs | 140 ++++++++++++++++- crates/gpui/src/keymap_matcher.rs | 3 +- 4 files changed, 229 insertions(+), 65 deletions(-) diff --git a/crates/activity_indicator/src/activity_indicator.rs b/crates/activity_indicator/src/activity_indicator.rs index f3a6f7328a..2041bbc793 100644 --- a/crates/activity_indicator/src/activity_indicator.rs +++ b/crates/activity_indicator/src/activity_indicator.rs @@ -33,6 +33,19 @@ struct LspStatus { status: LanguageServerBinaryStatus, } +struct PendingWork<'a> { + language_server_name: &'a str, + progress_token: &'a str, + progress: &'a LanguageServerProgress, +} + +#[derive(Default)] +struct Content { + icon: Option<&'static str>, + message: String, + action: Option>, +} + pub fn init(cx: &mut MutableAppContext) { cx.add_action(ActivityIndicator::show_error_message); cx.add_action(ActivityIndicator::dismiss_error_message); @@ -69,6 +82,8 @@ impl ActivityIndicator { if let Some(auto_updater) = auto_updater.as_ref() { cx.observe(auto_updater, |_, _, cx| cx.notify()).detach(); } + cx.observe_active_labeled_tasks(|_, cx| cx.notify()) + .detach(); Self { statuses: Default::default(), @@ -130,7 +145,7 @@ impl ActivityIndicator { fn pending_language_server_work<'a>( &self, cx: &'a AppContext, - ) -> impl Iterator { + ) -> impl Iterator> { self.project .read(cx) .language_server_statuses() @@ -142,23 +157,29 @@ impl ActivityIndicator { let mut pending_work = status .pending_work .iter() - .map(|(token, progress)| (status.name.as_str(), token.as_str(), progress)) + .map(|(token, progress)| PendingWork { + language_server_name: status.name.as_str(), + progress_token: token.as_str(), + progress, + }) .collect::>(); - pending_work.sort_by_key(|(_, _, progress)| Reverse(progress.last_update_at)); + pending_work.sort_by_key(|work| Reverse(work.progress.last_update_at)); Some(pending_work) } }) .flatten() } - fn content_to_render( - &mut self, - cx: &mut RenderContext, - ) -> (Option<&'static str>, String, Option>) { + fn content_to_render(&mut self, cx: &mut RenderContext) -> Content { // Show any language server has pending activity. let mut pending_work = self.pending_language_server_work(cx); - if let Some((lang_server_name, progress_token, progress)) = pending_work.next() { - let mut message = lang_server_name.to_string(); + if let Some(PendingWork { + language_server_name, + progress_token, + progress, + }) = pending_work.next() + { + let mut message = language_server_name.to_string(); message.push_str(": "); if let Some(progress_message) = progress.message.as_ref() { @@ -176,7 +197,11 @@ impl ActivityIndicator { write!(&mut message, " + {} more", additional_work_count).unwrap(); } - return (None, message, None); + return Content { + icon: None, + message, + action: None, + }; } // Show any language server installation info. @@ -199,19 +224,19 @@ impl ActivityIndicator { } if !downloading.is_empty() { - return ( - Some(DOWNLOAD_ICON), - format!( + return Content { + icon: Some(DOWNLOAD_ICON), + message: format!( "Downloading {} language server{}...", downloading.join(", "), if downloading.len() > 1 { "s" } else { "" } ), - None, - ); + action: None, + }; } else if !checking_for_update.is_empty() { - return ( - Some(DOWNLOAD_ICON), - format!( + return Content { + icon: Some(DOWNLOAD_ICON), + message: format!( "Checking for updates to {} language server{}...", checking_for_update.join(", "), if checking_for_update.len() > 1 { @@ -220,53 +245,61 @@ impl ActivityIndicator { "" } ), - None, - ); + action: None, + }; } else if !failed.is_empty() { - return ( - Some(WARNING_ICON), - format!( + return Content { + icon: Some(WARNING_ICON), + message: format!( "Failed to download {} language server{}. Click to show error.", failed.join(", "), if failed.len() > 1 { "s" } else { "" } ), - Some(Box::new(ShowErrorMessage)), - ); + action: Some(Box::new(ShowErrorMessage)), + }; } // Show any application auto-update info. if let Some(updater) = &self.auto_updater { - match &updater.read(cx).status() { - AutoUpdateStatus::Checking => ( - Some(DOWNLOAD_ICON), - "Checking for Zed updates…".to_string(), - None, - ), - AutoUpdateStatus::Downloading => ( - Some(DOWNLOAD_ICON), - "Downloading Zed update…".to_string(), - None, - ), - AutoUpdateStatus::Installing => ( - Some(DOWNLOAD_ICON), - "Installing Zed update…".to_string(), - None, - ), - AutoUpdateStatus::Updated => ( - None, - "Click to restart and update Zed".to_string(), - Some(Box::new(workspace::Restart)), - ), - AutoUpdateStatus::Errored => ( - Some(WARNING_ICON), - "Auto update failed".to_string(), - Some(Box::new(DismissErrorMessage)), - ), + return match &updater.read(cx).status() { + AutoUpdateStatus::Checking => Content { + icon: Some(DOWNLOAD_ICON), + message: "Checking for Zed updates…".to_string(), + action: None, + }, + AutoUpdateStatus::Downloading => Content { + icon: Some(DOWNLOAD_ICON), + message: "Downloading Zed update…".to_string(), + action: None, + }, + AutoUpdateStatus::Installing => Content { + icon: Some(DOWNLOAD_ICON), + message: "Installing Zed update…".to_string(), + action: None, + }, + AutoUpdateStatus::Updated => Content { + icon: None, + message: "Click to restart and update Zed".to_string(), + action: Some(Box::new(workspace::Restart)), + }, + AutoUpdateStatus::Errored => Content { + icon: Some(WARNING_ICON), + message: "Auto update failed".to_string(), + action: Some(Box::new(DismissErrorMessage)), + }, AutoUpdateStatus::Idle => Default::default(), - } - } else { - Default::default() + }; } + + if let Some(most_recent_active_task) = cx.active_labeled_tasks().last() { + return Content { + icon: None, + message: most_recent_active_task.to_string(), + action: None, + }; + } + + Default::default() } } @@ -280,7 +313,11 @@ impl View for ActivityIndicator { } fn render(&mut self, cx: &mut RenderContext) -> ElementBox { - let (icon, message, action) = self.content_to_render(cx); + let Content { + icon, + message, + action, + } = self.content_to_render(cx); let mut element = MouseEventHandler::::new(0, cx, |state, cx| { let theme = &cx diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index f9d5001985..a77c4fe4bb 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -5066,7 +5066,7 @@ impl Editor { GotoDefinitionKind::Type => project.type_definition(&buffer, head, cx), }); - cx.spawn(|workspace, mut cx| async move { + cx.spawn_labeled("Fetching Definition...", |workspace, mut cx| async move { let definitions = definitions.await?; workspace.update(&mut cx, |workspace, cx| { Editor::navigate_to_definitions(workspace, editor_handle, definitions, cx); @@ -5146,7 +5146,7 @@ impl Editor { let project = workspace.project().clone(); let references = project.update(cx, |project, cx| project.references(&buffer, head, cx)); - Some(cx.spawn(|workspace, mut cx| async move { + Some(cx.spawn_labeled("Finding All References...", |workspace, mut cx| async move { let locations = references.await?; if locations.is_empty() { return Ok(()); diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 60adadb96c..6c1eeef93c 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -31,7 +31,7 @@ use uuid::Uuid; pub use action::*; use callback_collection::CallbackCollection; -use collections::{hash_map::Entry, HashMap, HashSet, VecDeque}; +use collections::{hash_map::Entry, BTreeMap, HashMap, HashSet, VecDeque}; pub use menu::*; use platform::Event; #[cfg(any(test, feature = "test-support"))] @@ -474,6 +474,7 @@ type WindowBoundsCallback = Box>, &mut MutableAppContext) -> bool, >; +type ActiveLabeledTasksCallback = Box bool>; type DeserializeActionCallback = fn(json: &str) -> anyhow::Result>; type WindowShouldCloseSubscriptionCallback = Box bool>; @@ -503,6 +504,7 @@ pub struct MutableAppContext { window_fullscreen_observations: CallbackCollection, window_bounds_observations: CallbackCollection, keystroke_observations: CallbackCollection, + active_labeled_task_observations: CallbackCollection<(), ActiveLabeledTasksCallback>, #[allow(clippy::type_complexity)] presenters_and_platform_windows: @@ -514,6 +516,8 @@ pub struct MutableAppContext { pending_flushes: usize, flushing_effects: bool, halt_action_dispatch: bool, + next_labeled_task_id: usize, + active_labeled_tasks: BTreeMap, } impl MutableAppContext { @@ -562,6 +566,7 @@ impl MutableAppContext { window_bounds_observations: Default::default(), keystroke_observations: Default::default(), action_dispatch_observations: Default::default(), + active_labeled_task_observations: Default::default(), presenters_and_platform_windows: Default::default(), foreground, pending_effects: VecDeque::new(), @@ -570,6 +575,8 @@ impl MutableAppContext { pending_flushes: 0, flushing_effects: false, halt_action_dispatch: false, + next_labeled_task_id: 0, + active_labeled_tasks: Default::default(), } } @@ -794,6 +801,12 @@ impl MutableAppContext { window.screen().display_uuid() } + pub fn active_labeled_tasks<'a>( + &'a self, + ) -> impl DoubleEndedIterator + 'a { + self.active_labeled_tasks.values().cloned() + } + pub fn render_view(&mut self, params: RenderParams) -> Result { let window_id = params.window_id; let view_id = params.view_id; @@ -1156,6 +1169,19 @@ impl MutableAppContext { ) } + pub fn observe_active_labeled_tasks(&mut self, callback: F) -> Subscription + where + F: 'static + FnMut(&mut MutableAppContext) -> bool, + { + let subscription_id = post_inc(&mut self.next_subscription_id); + self.active_labeled_task_observations + .add_callback((), subscription_id, Box::new(callback)); + Subscription::ActiveLabeledTasksObservation( + self.active_labeled_task_observations + .subscribe((), subscription_id), + ) + } + pub fn defer(&mut self, callback: impl 'static + FnOnce(&mut MutableAppContext)) { self.pending_effects.push_back(Effect::Deferred { callback: Box::new(callback), @@ -2038,6 +2064,17 @@ impl MutableAppContext { handled_by, result, } => self.handle_keystroke_effect(window_id, keystroke, handled_by, result), + Effect::ActiveLabeledTasksChanged => { + self.handle_active_labeled_tasks_changed_effect() + } + Effect::ActiveLabeledTasksObservation { + subscription_id, + callback, + } => self.active_labeled_task_observations.add_callback( + (), + subscription_id, + callback, + ), } self.pending_notifications.clear(); self.remove_dropped_entities(); @@ -2445,26 +2482,68 @@ impl MutableAppContext { } } + fn handle_active_labeled_tasks_changed_effect(&mut self) { + self.active_labeled_task_observations + .clone() + .emit((), self, move |callback, this| { + callback(this); + true + }); + } + pub fn focus(&mut self, window_id: usize, view_id: Option) { self.pending_effects .push_back(Effect::Focus { window_id, view_id }); } - pub fn spawn(&self, f: F) -> Task + fn spawn_internal(&mut self, task_name: Option<&'static str>, f: F) -> Task where F: FnOnce(AsyncAppContext) -> Fut, Fut: 'static + Future, T: 'static, { + let label_id = task_name.map(|task_name| { + let id = post_inc(&mut self.next_labeled_task_id); + self.active_labeled_tasks.insert(id, task_name); + self.pending_effects + .push_back(Effect::ActiveLabeledTasksChanged); + id + }); + let future = f(self.to_async()); let cx = self.to_async(); self.foreground.spawn(async move { let result = future.await; - cx.0.borrow_mut().flush_effects(); + let mut cx = cx.0.borrow_mut(); + + if let Some(completed_label_id) = label_id { + cx.active_labeled_tasks.remove(&completed_label_id); + cx.pending_effects + .push_back(Effect::ActiveLabeledTasksChanged); + } + cx.flush_effects(); result }) } + pub fn spawn_labeled(&mut self, task_name: &'static str, f: F) -> Task + where + F: FnOnce(AsyncAppContext) -> Fut, + Fut: 'static + Future, + T: 'static, + { + self.spawn_internal(Some(task_name), f) + } + + pub fn spawn(&mut self, f: F) -> Task + where + F: FnOnce(AsyncAppContext) -> Fut, + Fut: 'static + Future, + T: 'static, + { + self.spawn_internal(None, f) + } + pub fn to_async(&self) -> AsyncAppContext { AsyncAppContext(self.weak_self.as_ref().unwrap().upgrade().unwrap()) } @@ -2903,6 +2982,11 @@ pub enum Effect { window_id: usize, callback: WindowShouldCloseSubscriptionCallback, }, + ActiveLabeledTasksChanged, + ActiveLabeledTasksObservation { + subscription_id: usize, + callback: ActiveLabeledTasksCallback, + }, } impl Debug for Effect { @@ -3062,6 +3146,16 @@ impl Debug for Effect { ) .field("result", result) .finish(), + Effect::ActiveLabeledTasksChanged => { + f.debug_struct("Effect::ActiveLabeledTasksChanged").finish() + } + Effect::ActiveLabeledTasksObservation { + subscription_id, + callback: _, + } => f + .debug_struct("Effect::ActiveLabeledTasksObservation") + .field("subscription_id", subscription_id) + .finish(), } } } @@ -3476,7 +3570,7 @@ impl<'a, T: Entity> ModelContext<'a, T> { WeakModelHandle::new(self.model_id) } - pub fn spawn(&self, f: F) -> Task + pub fn spawn(&mut self, f: F) -> Task where F: FnOnce(ModelHandle, AsyncAppContext) -> Fut, Fut: 'static + Future, @@ -3486,7 +3580,7 @@ impl<'a, T: Entity> ModelContext<'a, T> { self.app.spawn(|cx| f(handle, cx)) } - pub fn spawn_weak(&self, f: F) -> Task + pub fn spawn_weak(&mut self, f: F) -> Task where F: FnOnce(WeakModelHandle, AsyncAppContext) -> Fut, Fut: 'static + Future, @@ -3939,6 +4033,23 @@ impl<'a, T: View> ViewContext<'a, T> { }) } + pub fn observe_active_labeled_tasks(&mut self, mut callback: F) -> Subscription + where + F: 'static + FnMut(&mut T, &mut ViewContext), + { + let observer = self.weak_handle(); + self.app.observe_active_labeled_tasks(move |cx| { + if let Some(observer) = observer.upgrade(cx) { + observer.update(cx, |observer, cx| { + callback(observer, cx); + }); + true + } else { + false + } + }) + } + pub fn emit(&mut self, payload: T::Event) { self.app.pending_effects.push_back(Effect::Event { entity_id: self.view_id, @@ -3985,7 +4096,17 @@ impl<'a, T: View> ViewContext<'a, T> { self.app.halt_action_dispatch = false; } - pub fn spawn(&self, f: F) -> Task + pub fn spawn_labeled(&mut self, task_label: &'static str, f: F) -> Task + where + F: FnOnce(ViewHandle, AsyncAppContext) -> Fut, + Fut: 'static + Future, + S: 'static, + { + let handle = self.handle(); + self.app.spawn_labeled(task_label, |cx| f(handle, cx)) + } + + pub fn spawn(&mut self, f: F) -> Task where F: FnOnce(ViewHandle, AsyncAppContext) -> Fut, Fut: 'static + Future, @@ -3995,7 +4116,7 @@ impl<'a, T: View> ViewContext<'a, T> { self.app.spawn(|cx| f(handle, cx)) } - pub fn spawn_weak(&self, f: F) -> Task + pub fn spawn_weak(&mut self, f: F) -> Task where F: FnOnce(WeakViewHandle, AsyncAppContext) -> Fut, Fut: 'static + Future, @@ -5113,6 +5234,9 @@ pub enum Subscription { KeystrokeObservation(callback_collection::Subscription), ReleaseObservation(callback_collection::Subscription), ActionObservation(callback_collection::Subscription<(), ActionObservationCallback>), + ActiveLabeledTasksObservation( + callback_collection::Subscription<(), ActiveLabeledTasksCallback>, + ), } impl Subscription { @@ -5129,6 +5253,7 @@ impl Subscription { Subscription::KeystrokeObservation(subscription) => subscription.id(), Subscription::ReleaseObservation(subscription) => subscription.id(), Subscription::ActionObservation(subscription) => subscription.id(), + Subscription::ActiveLabeledTasksObservation(subscription) => subscription.id(), } } @@ -5145,6 +5270,7 @@ impl Subscription { Subscription::WindowBoundsObservation(subscription) => subscription.detach(), Subscription::ReleaseObservation(subscription) => subscription.detach(), Subscription::ActionObservation(subscription) => subscription.detach(), + Subscription::ActiveLabeledTasksObservation(subscription) => subscription.detach(), } } } diff --git a/crates/gpui/src/keymap_matcher.rs b/crates/gpui/src/keymap_matcher.rs index cfc26d6869..fb75d1063b 100644 --- a/crates/gpui/src/keymap_matcher.rs +++ b/crates/gpui/src/keymap_matcher.rs @@ -227,7 +227,7 @@ mod tests { #[test] fn test_push_keystroke() -> Result<()> { - actions!(test, [B, AB, C, D, DA]); + actions!(test, [B, AB, C, D, DA, E, EF]); let mut context1 = KeymapContext::default(); context1.set.insert("1".into()); @@ -286,6 +286,7 @@ mod tests { matcher.push_keystroke(Keystroke::parse("d")?, dispatch_path.clone()), MatchResult::Matches(vec![(2, Box::new(D)), (1, Box::new(D))]), ); + // If none of the d action handlers consume the binding, a pending // binding may then be used assert_eq!( From 6b6e4e3bfe777a816703a355a5b4832920f3a8db Mon Sep 17 00:00:00 2001 From: Kay Simmons Date: Tue, 21 Feb 2023 16:14:22 -0800 Subject: [PATCH 2/4] Add basic test for labeled tasks --- crates/gpui/src/app.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 6c1eeef93c..6e9090db59 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -5279,6 +5279,7 @@ impl Subscription { mod tests { use super::*; use crate::{actions, elements::*, impl_actions, MouseButton, MouseButtonEvent}; + use postage::{sink::Sink, stream::Stream}; use serde::Deserialize; use smol::future::poll_once; use std::{ @@ -6894,6 +6895,23 @@ mod tests { assert_eq!(presenter.borrow().rendered_views.len(), 1); } + #[crate::test(self)] + async fn test_labeled_tasks(cx: &mut TestAppContext) { + assert_eq!(None, cx.update(|cx| cx.active_labeled_tasks().next())); + let (mut sender, mut reciever) = postage::oneshot::channel::<()>(); + let task = cx + .update(|cx| cx.spawn_labeled("Test Label", |_| async move { reciever.recv().await })); + + assert_eq!( + Some("Test Label"), + cx.update(|cx| cx.active_labeled_tasks().next()) + ); + sender.send(()).await; + task.await; + + assert_eq!(None, cx.update(|cx| cx.active_labeled_tasks().next())); + } + #[crate::test(self)] async fn test_window_activation(cx: &mut TestAppContext) { struct View(&'static str); From 1c69e289b70d728c79c07db2077a4c71f12804a7 Mon Sep 17 00:00:00 2001 From: Kay Simmons Date: Tue, 21 Feb 2023 21:07:07 -0800 Subject: [PATCH 3/4] Fix formatting --- crates/editor/src/editor.rs | 55 ++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index a77c4fe4bb..05dda07231 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -84,7 +84,7 @@ use std::{ }; pub use sum_tree::Bias; use theme::{DiagnosticStyle, Theme}; -use util::{post_inc, ResultExt, TryFutureExt, RangeExt}; +use util::{post_inc, RangeExt, ResultExt, TryFutureExt}; use workspace::{ItemNavHistory, ViewId, Workspace, WorkspaceId}; use crate::git::diff_hunk_to_display; @@ -4806,7 +4806,7 @@ impl Editor { if !in_bracket_range && best_in_bracket_range { continue; } - + // Prefer smaller lengths unless best is inside and current isn't if length > best_length && (best_inside || !inside) { continue; @@ -5146,31 +5146,36 @@ impl Editor { let project = workspace.project().clone(); let references = project.update(cx, |project, cx| project.references(&buffer, head, cx)); - Some(cx.spawn_labeled("Finding All References...", |workspace, mut cx| async move { - let locations = references.await?; - if locations.is_empty() { - return Ok(()); - } + Some(cx.spawn_labeled( + "Finding All References...", + |workspace, mut cx| async move { + let locations = references.await?; + if locations.is_empty() { + return Ok(()); + } - workspace.update(&mut cx, |workspace, cx| { - let title = locations - .first() - .as_ref() - .map(|location| { - let buffer = location.buffer.read(cx); - format!( - "References to `{}`", - buffer - .text_for_range(location.range.clone()) - .collect::() - ) - }) - .unwrap(); - Self::open_locations_in_multibuffer(workspace, locations, replica_id, title, cx); - }); + workspace.update(&mut cx, |workspace, cx| { + let title = locations + .first() + .as_ref() + .map(|location| { + let buffer = location.buffer.read(cx); + format!( + "References to `{}`", + buffer + .text_for_range(location.range.clone()) + .collect::() + ) + }) + .unwrap(); + Self::open_locations_in_multibuffer( + workspace, locations, replica_id, title, cx, + ); + }); - Ok(()) - })) + Ok(()) + }, + )) } /// Opens a multibuffer with the given project locations in it From 46af9a90ce3030930697ebb8268a52a35d836e28 Mon Sep 17 00:00:00 2001 From: Kay Simmons Date: Tue, 21 Feb 2023 21:13:03 -0800 Subject: [PATCH 4/4] fix test warning --- crates/gpui/src/app.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 6e9090db59..a313cf65c7 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -6906,7 +6906,10 @@ mod tests { Some("Test Label"), cx.update(|cx| cx.active_labeled_tasks().next()) ); - sender.send(()).await; + sender + .send(()) + .await + .expect("Could not send message to complete task"); task.await; assert_eq!(None, cx.update(|cx| cx.active_labeled_tasks().next()));