From cd7dccd30c93935c523af47d90fd19eccdeb4a55 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 20 Apr 2021 12:28:30 +0200 Subject: [PATCH] Replace remaining usages of `finish_pending_tasks` with `condition` --- gpui/src/app.rs | 138 ++-------------------------- zed/src/test.rs | 16 ---- zed/src/workspace/pane.rs | 9 +- zed/src/workspace/workspace_view.rs | 112 ++++++++-------------- 4 files changed, 50 insertions(+), 225 deletions(-) diff --git a/gpui/src/app.rs b/gpui/src/app.rs index 25fe3cbc15..4b37c2dbfa 100644 --- a/gpui/src/app.rs +++ b/gpui/src/app.rs @@ -325,10 +325,6 @@ impl TestAppContext { result } - pub fn finish_pending_tasks(&self) -> impl Future { - self.0.borrow().finish_pending_tasks() - } - pub fn font_cache(&self) -> Arc { self.0.borrow().font_cache.clone() } @@ -1066,6 +1062,12 @@ impl MutableAppContext { .or_default() .updated .insert(view_id); + + if let Entry::Occupied(mut entry) = self.async_observations.entry(view_id) { + if entry.get_mut().blocking_send(()).is_err() { + entry.remove_entry(); + } + } } fn focus(&mut self, window_id: usize, focused_id: usize) { @@ -1207,40 +1209,6 @@ impl MutableAppContext { result } - pub fn finish_pending_tasks(&self) -> impl Future { - let mut pending_tasks = self - .future_handlers - .borrow() - .keys() - .cloned() - .collect::>(); - pending_tasks.extend(self.stream_handlers.borrow().keys()); - - let task_done = self.task_done.clone(); - let future_handlers = self.future_handlers.clone(); - let stream_handlers = self.stream_handlers.clone(); - - async move { - // A Condvar expects the condition to be protected by a Mutex, but in this case we know - // that this logic will always run on the main thread. - let mutex = async_std::sync::Mutex::new(()); - loop { - { - let future_handlers = future_handlers.borrow(); - let stream_handlers = stream_handlers.borrow(); - pending_tasks.retain(|task_id| { - future_handlers.contains_key(task_id) - || stream_handlers.contains_key(task_id) - }); - if pending_tasks.is_empty() { - break; - } - } - task_done.wait(mutex.lock().await).await; - } - } - } - pub fn write_to_clipboard(&self, item: ClipboardItem) { self.platform.write_to_clipboard(item); } @@ -3388,98 +3356,4 @@ mod tests { // assert!(invalidation.removed.is_empty()); // }); // } - - #[test] - fn test_finish_pending_tasks() { - struct View; - - impl Entity for View { - type Event = (); - } - - impl super::View for View { - fn render<'a>(&self, _: &AppContext) -> ElementBox { - Empty::new().boxed() - } - - fn ui_name() -> &'static str { - "View" - } - } - - struct Model; - - impl Entity for Model { - type Event = (); - } - - App::test_async((), |mut app| async move { - let model = app.add_model(|_| Model); - let (_, view) = app.add_window(|_| View); - - model.update(&mut app, |_, ctx| { - ctx.spawn(async {}, |_, _, _| {}).detach(); - // Cancel this task - drop(ctx.spawn(async {}, |_, _, _| {})); - }); - - view.update(&mut app, |_, ctx| { - ctx.spawn(async {}, |_, _, _| {}).detach(); - // Cancel this task - drop(ctx.spawn(async {}, |_, _, _| {})); - }); - - assert!(!app.0.borrow().future_handlers.borrow().is_empty()); - app.finish_pending_tasks().await; - assert!(app.0.borrow().future_handlers.borrow().is_empty()); - app.finish_pending_tasks().await; // Don't block if there are no tasks - - model.update(&mut app, |_, ctx| { - ctx.spawn_stream(smol::stream::iter(vec![1, 2, 3]), |_, _, _| {}, |_, _| {}) - .detach(); - // Cancel this task - drop(ctx.spawn_stream(smol::stream::iter(vec![1, 2, 3]), |_, _, _| {}, |_, _| {})); - }); - - view.update(&mut app, |_, ctx| { - ctx.spawn_stream(smol::stream::iter(vec![1, 2, 3]), |_, _, _| {}, |_, _| {}) - .detach(); - // Cancel this task - drop(ctx.spawn_stream(smol::stream::iter(vec![1, 2, 3]), |_, _, _| {}, |_, _| {})); - }); - - assert!(!app.0.borrow().stream_handlers.borrow().is_empty()); - app.finish_pending_tasks().await; - assert!(app.0.borrow().stream_handlers.borrow().is_empty()); - app.finish_pending_tasks().await; // Don't block if there are no tasks - - // Tasks are considered finished when we drop handles - let mut tasks = Vec::new(); - model.update(&mut app, |_, ctx| { - tasks.push(Box::new(ctx.spawn(async {}, |_, _, _| {}))); - tasks.push(Box::new(ctx.spawn_stream( - smol::stream::iter(vec![1, 2, 3]), - |_, _, _| {}, - |_, _| {}, - ))); - }); - - view.update(&mut app, |_, ctx| { - tasks.push(Box::new(ctx.spawn(async {}, |_, _, _| {}))); - tasks.push(Box::new(ctx.spawn_stream( - smol::stream::iter(vec![1, 2, 3]), - |_, _, _| {}, - |_, _| {}, - ))); - }); - - assert!(!app.0.borrow().stream_handlers.borrow().is_empty()); - - let finish_pending_tasks = app.finish_pending_tasks(); - drop(tasks); - finish_pending_tasks.await; - assert!(app.0.borrow().stream_handlers.borrow().is_empty()); - app.finish_pending_tasks().await; // Don't block if there are no tasks - }); - } } diff --git a/zed/src/test.rs b/zed/src/test.rs index 3850616671..61579908f7 100644 --- a/zed/src/test.rs +++ b/zed/src/test.rs @@ -6,7 +6,6 @@ use simplelog::SimpleLogger; use std::{ collections::BTreeMap, path::{Path, PathBuf}, - time::{Duration, Instant}, }; use tempdir::TempDir; @@ -144,18 +143,3 @@ fn write_tree(path: &Path, tree: serde_json::Value) { panic!("You must pass a JSON object to this helper") } } - -pub async fn assert_condition(poll_interval: u64, timeout: u64, mut f: impl FnMut() -> bool) { - let poll_interval = Duration::from_millis(poll_interval); - let timeout = Duration::from_millis(timeout); - let start = Instant::now(); - loop { - if f() { - return; - } else if Instant::now().duration_since(start) < timeout { - smol::Timer::after(poll_interval).await; - } else { - panic!("timed out waiting on condition"); - } - } -} diff --git a/zed/src/workspace/pane.rs b/zed/src/workspace/pane.rs index c352a522eb..ea3226715a 100644 --- a/zed/src/workspace/pane.rs +++ b/zed/src/workspace/pane.rs @@ -106,11 +106,10 @@ impl Pane { } pub fn activate_entry(&mut self, entry_id: (usize, u64), ctx: &mut ViewContext) -> bool { - if let Some(index) = self - .items - .iter() - .position(|item| item.entry_id(ctx.as_ref()).map_or(false, |id| id == entry_id)) - { + if let Some(index) = self.items.iter().position(|item| { + item.entry_id(ctx.as_ref()) + .map_or(false, |id| id == entry_id) + }) { self.activate_item(index, ctx); true } else { diff --git a/zed/src/workspace/workspace_view.rs b/zed/src/workspace/workspace_view.rs index bf741edbab..205806490c 100644 --- a/zed/src/workspace/workspace_view.rs +++ b/zed/src/workspace/workspace_view.rs @@ -385,9 +385,9 @@ mod tests { App::test_async((), |mut app| async move { let dir = temp_tree(json!({ "a": { - "aa": "aa contents", - "ab": "ab contents", - "ac": "ab contents", + "file1": "contents 1", + "file2": "contents 2", + "file3": "contents 3", }, })); @@ -396,74 +396,44 @@ mod tests { app.read(|ctx| workspace.read(ctx).worktree_scans_complete(ctx)) .await; let entries = app.read(|ctx| workspace.file_entries(ctx)); + let file1 = entries[0]; + let file2 = entries[1]; + let file3 = entries[2]; let (_, workspace_view) = app.add_window(|ctx| WorkspaceView::new(workspace.clone(), settings, ctx)); + let pane = app.read(|ctx| workspace_view.read(ctx).active_pane().clone()); // Open the first entry - workspace_view.update(&mut app, |w, ctx| w.open_entry(entries[0], ctx)); - - workspace_view - .condition(&app, |workspace_view, ctx| { - workspace_view.active_pane().read(ctx).items().len() == 1 - }) + workspace_view.update(&mut app, |w, ctx| w.open_entry(file1, ctx)); + pane.condition(&app, |pane, _| pane.items().len() == 1) .await; // Open the second entry - workspace_view.update(&mut app, |w, ctx| w.open_entry(entries[1], ctx)); - - workspace_view - .condition(&app, |workspace_view, ctx| { - workspace_view.active_pane().read(ctx).items().len() == 2 - }) + workspace_view.update(&mut app, |w, ctx| w.open_entry(file2, ctx)); + pane.condition(&app, |pane, _| pane.items().len() == 2) .await; - app.read(|ctx| { - assert_eq!( - workspace_view - .read(ctx) - .active_pane() - .read(ctx) - .active_item() - .unwrap() - .entry_id(ctx), - Some(entries[1]) - ); + let pane = pane.read(ctx); + assert_eq!(pane.active_item().unwrap().entry_id(ctx), Some(file2)); }); // Open the first entry again - workspace_view.update(&mut app, |w, ctx| w.open_entry(entries[0], ctx)); - - { - let entries = entries.clone(); - workspace_view - .condition(&app, move |workspace_view, ctx| { - workspace_view - .active_pane() - .read(ctx) - .active_item() - .unwrap() - .entry_id(ctx) - == Some(entries[0]) - }) - .await; - } - + workspace_view.update(&mut app, |w, ctx| w.open_entry(file1, ctx)); + pane.condition(&app, move |pane, ctx| { + pane.active_item().unwrap().entry_id(ctx) == Some(file1) + }) + .await; app.read(|ctx| { - let active_pane = workspace_view.read(ctx).active_pane().read(ctx); - assert_eq!(active_pane.items().len(), 2); + assert_eq!(pane.read(ctx).items().len(), 2); }); // Open the third entry twice concurrently workspace_view.update(&mut app, |w, ctx| { - w.open_entry(entries[2], ctx); - w.open_entry(entries[2], ctx); + w.open_entry(file3, ctx); + w.open_entry(file3, ctx); }); - - workspace_view - .condition(&app, |workspace_view, ctx| { - workspace_view.active_pane().read(ctx).items().len() == 3 - }) + pane.condition(&app, |pane, _| pane.items().len() == 3) .await; }); } @@ -475,44 +445,42 @@ mod tests { let dir = temp_tree(json!({ "a": { - "aa": "aa contents", - "ab": "ab contents", - "ac": "ab contents", + "file1": "contents 1", + "file2": "contents 2", + "file3": "contents 3", }, })); let settings = settings::channel(&app.font_cache()).unwrap().1; let workspace = app.add_model(|ctx| Workspace::new(vec![dir.path().into()], ctx)); - app.finish_pending_tasks().await; // Open and populate worktree. + app.read(|ctx| workspace.read(ctx).worktree_scans_complete(ctx)) + .await; let entries = app.read(|ctx| workspace.file_entries(ctx)); + let file1 = entries[0]; let (window_id, workspace_view) = app.add_window(|ctx| WorkspaceView::new(workspace.clone(), settings, ctx)); - - workspace_view.update(&mut app, |w, ctx| w.open_entry(entries[0], ctx)); - app.finish_pending_tasks().await; - let pane_1 = app.read(|ctx| workspace_view.read(ctx).active_pane().clone()); + workspace_view.update(&mut app, |w, ctx| w.open_entry(file1, ctx)); + pane_1 + .condition(&app, move |pane, ctx| { + pane.active_item().and_then(|i| i.entry_id(ctx)) == Some(file1) + }) + .await; + app.dispatch_action(window_id, vec![pane_1.id()], "pane:split_right", ()); app.update(|ctx| { let pane_2 = workspace_view.read(ctx).active_pane().clone(); assert_ne!(pane_1, pane_2); - assert_eq!( - pane_2 - .read(ctx) - .active_item() - .unwrap() - .entry_id(ctx.as_ref()), - Some(entries[0]) - ); + let pane2_item = pane_2.read(ctx).active_item().unwrap(); + assert_eq!(pane2_item.entry_id(ctx.as_ref()), Some(file1)); ctx.dispatch_action(window_id, vec![pane_2.id()], "pane:close_active_item", ()); - - let w = workspace_view.read(ctx); - assert_eq!(w.panes.len(), 1); - assert_eq!(w.active_pane(), &pane_1); + let workspace_view = workspace_view.read(ctx); + assert_eq!(workspace_view.panes.len(), 1); + assert_eq!(workspace_view.active_pane(), &pane_1); }); }); }