From 7ad90fd0a7d951b68ef5994ab4bd6fc1c2ef005a Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 3 May 2023 10:27:05 +0200 Subject: [PATCH] Merge pull request #2436 from zed-industries/close-window-end-call Move methods querying window state into `AsyncAppContext` --- crates/collab_ui/src/collab_ui.rs | 13 ++-- .../src/project_shared_notification.rs | 4 +- .../collab_ui/src/sharing_status_indicator.rs | 4 +- crates/copilot/src/sign_in.rs | 15 ++--- crates/gpui/src/app.rs | 67 +++++++++---------- crates/gpui/src/app/test_app_context.rs | 6 +- crates/gpui/src/test.rs | 2 +- crates/gpui_macros/src/gpui_macros.rs | 4 +- crates/workspace/src/workspace.rs | 48 ++++++------- crates/zed/src/zed.rs | 66 +++++++++--------- 10 files changed, 116 insertions(+), 113 deletions(-) diff --git a/crates/collab_ui/src/collab_ui.rs b/crates/collab_ui/src/collab_ui.rs index 557f9b58d9bc888e6b09b50f595d4055674f1404..5a23fc5d32f7de8f5dee08e918da1ee93f33deda 100644 --- a/crates/collab_ui/src/collab_ui.rs +++ b/crates/collab_ui/src/collab_ui.rs @@ -52,13 +52,16 @@ fn join_project(action: &JoinProject, app_state: Arc, cx: &mut AppCont let project_id = action.project_id; let follow_user_id = action.follow_user_id; cx.spawn(|mut cx| async move { - let existing_workspace = cx.update(|cx| { - cx.window_ids() - .filter_map(|window_id| cx.root_view(window_id)?.clone().downcast::()) - .find(|workspace| { + let existing_workspace = cx + .window_ids() + .into_iter() + .filter_map(|window_id| cx.root_view(window_id)?.clone().downcast::()) + .find(|workspace| { + cx.read_window(workspace.window_id(), |cx| { workspace.read(cx).project().read(cx).remote_id() == Some(project_id) }) - }); + .unwrap_or(false) + }); let workspace = if let Some(existing_workspace) = existing_workspace { existing_workspace.downgrade() diff --git a/crates/collab_ui/src/project_shared_notification.rs b/crates/collab_ui/src/project_shared_notification.rs index 4cdb18d97c9166430852bf77102ebd6b5857ab73..fd02a75e4543930bc2f4e6decedcd6e8d3c1cd50 100644 --- a/crates/collab_ui/src/project_shared_notification.rs +++ b/crates/collab_ui/src/project_shared_notification.rs @@ -62,14 +62,14 @@ pub fn init(cx: &mut AppContext) { room::Event::RemoteProjectUnshared { project_id } => { if let Some(window_ids) = notification_windows.remove(&project_id) { for window_id in window_ids { - cx.remove_window(window_id); + cx.update_window(window_id, |cx| cx.remove_window()); } } } room::Event::Left => { for (_, window_ids) in notification_windows.drain() { for window_id in window_ids { - cx.remove_window(window_id); + cx.update_window(window_id, |cx| cx.remove_window()); } } } diff --git a/crates/collab_ui/src/sharing_status_indicator.rs b/crates/collab_ui/src/sharing_status_indicator.rs index 42c3c886ad6cdf1d034fb9ff045a5c302b942992..efb6934cba140bdfc5cf2c25a0497c1a470818ef 100644 --- a/crates/collab_ui/src/sharing_status_indicator.rs +++ b/crates/collab_ui/src/sharing_status_indicator.rs @@ -20,10 +20,10 @@ pub fn init(cx: &mut AppContext) { status_indicator = Some(cx.add_status_bar_item(|_| SharingStatusIndicator)); } } else if let Some((window_id, _)) = status_indicator.take() { - cx.remove_status_bar_item(window_id); + cx.update_window(window_id, |cx| cx.remove_window()); } } else if let Some((window_id, _)) = status_indicator.take() { - cx.remove_status_bar_item(window_id); + cx.update_window(window_id, |cx| cx.remove_window()); } }) .detach(); diff --git a/crates/copilot/src/sign_in.rs b/crates/copilot/src/sign_in.rs index d7aadb62623c7ca52a83bf5cf557a25fdb5e84e6..62f078cdc99886b5e56ff2e50eac8e72938e41bc 100644 --- a/crates/copilot/src/sign_in.rs +++ b/crates/copilot/src/sign_in.rs @@ -33,14 +33,13 @@ pub fn init(cx: &mut AppContext) { crate::Status::SigningIn { prompt } => { if let Some(code_verification_handle) = code_verification.as_mut() { let window_id = code_verification_handle.window_id(); - if cx.has_window(window_id) { - cx.update_window(window_id, |cx| { - code_verification_handle.update(cx, |code_verification, cx| { - code_verification.set_status(status, cx) - }); - cx.activate_window(); + let updated = cx.update_window(window_id, |cx| { + code_verification_handle.update(cx, |code_verification, cx| { + code_verification.set_status(status.clone(), cx) }); - } else { + cx.activate_window(); + }); + if updated.is_none() { code_verification = Some(create_copilot_auth_window(cx, &status)); } } else if let Some(_prompt) = prompt { @@ -62,7 +61,7 @@ pub fn init(cx: &mut AppContext) { } _ => { if let Some(code_verification) = code_verification.take() { - cx.remove_window(code_verification.window_id()); + cx.update_window(code_verification.window_id(), |cx| cx.remove_window()); } } } diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 173b71668982309c83be514c01a43116b3873ad7..9629b2234da5d9d583731eb31f7a23deb28cfdf7 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -302,6 +302,14 @@ impl AsyncAppContext { self.0.borrow_mut().update(callback) } + pub fn read_window T>( + &self, + window_id: usize, + callback: F, + ) -> Option { + self.0.borrow_mut().read_window(window_id, callback) + } + pub fn update_window T>( &mut self, window_id: usize, @@ -318,6 +326,22 @@ impl AsyncAppContext { })? } + pub fn has_window(&self, window_id: usize) -> bool { + self.read(|cx| cx.windows.contains_key(&window_id)) + } + + pub fn window_is_active(&self, window_id: usize) -> bool { + self.read(|cx| cx.windows.get(&window_id).map_or(false, |w| w.is_active)) + } + + pub fn root_view(&self, window_id: usize) -> Option { + self.read(|cx| cx.windows.get(&window_id).map(|w| w.root_view().clone())) + } + + pub fn window_ids(&self) -> Vec { + self.read(|cx| cx.windows.keys().copied().collect()) + } + pub fn add_model(&mut self, build_model: F) -> ModelHandle where T: Entity, @@ -339,7 +363,7 @@ impl AsyncAppContext { } pub fn remove_window(&mut self, window_id: usize) { - self.update(|cx| cx.remove_window(window_id)) + self.update_window(window_id, |cx| cx.remove_window()); } pub fn activate_window(&mut self, window_id: usize) { @@ -538,7 +562,7 @@ impl AppContext { App(self.weak_self.as_ref().unwrap().upgrade().unwrap()) } - pub fn quit(&mut self) { + fn quit(&mut self) { let mut futures = Vec::new(); self.update(|cx| { @@ -555,7 +579,8 @@ impl AppContext { } }); - self.remove_all_windows(); + self.windows.clear(); + self.flush_effects(); let futures = futures::future::join_all(futures); if self @@ -567,11 +592,6 @@ impl AppContext { } } - pub fn remove_all_windows(&mut self) { - self.windows.clear(); - self.flush_effects(); - } - pub fn foreground(&self) -> &Rc { &self.foreground } @@ -688,24 +708,6 @@ impl AppContext { } } - pub fn has_window(&self, window_id: usize) -> bool { - self.window_ids() - .find(|window| window == &window_id) - .is_some() - } - - pub fn window_is_active(&self, window_id: usize) -> bool { - self.windows.get(&window_id).map_or(false, |w| w.is_active) - } - - pub fn root_view(&self, window_id: usize) -> Option<&AnyViewHandle> { - self.windows.get(&window_id).map(|w| w.root_view()) - } - - pub fn window_ids(&self) -> impl Iterator + '_ { - self.windows.keys().copied() - } - pub fn view_ui_name(&self, window_id: usize, view_id: usize) -> Option<&'static str> { Some(self.views.get(&(window_id, view_id))?.ui_name()) } @@ -1275,15 +1277,6 @@ impl AppContext { }) } - pub fn remove_status_bar_item(&mut self, id: usize) { - self.remove_window(id); - } - - pub fn remove_window(&mut self, window_id: usize) { - self.windows.remove(&window_id); - self.flush_effects(); - } - pub fn build_window( &mut self, window_id: usize, @@ -1342,7 +1335,7 @@ impl AppContext { { let mut app = self.upgrade(); platform_window.on_close(Box::new(move || { - app.update(|cx| cx.remove_window(window_id)); + app.update(|cx| cx.update_window(window_id, |cx| cx.remove_window())); })); } @@ -4717,7 +4710,7 @@ mod tests { assert!(model_release_observed.get()); drop(view); - cx.remove_window(window_id); + cx.update_window(window_id, |cx| cx.remove_window()); assert!(view_released.get()); assert!(view_release_observed.get()); } diff --git a/crates/gpui/src/app/test_app_context.rs b/crates/gpui/src/app/test_app_context.rs index 3a03a81c4707e65982795868c9eb060f5173fced..8462f80d9ad8d53fbee0a72270e1f4a21f85173d 100644 --- a/crates/gpui/src/app/test_app_context.rs +++ b/crates/gpui/src/app/test_app_context.rs @@ -180,7 +180,11 @@ impl TestAppContext { } pub fn window_ids(&self) -> Vec { - self.cx.borrow().window_ids().collect() + self.cx.borrow().windows.keys().copied().collect() + } + + pub fn remove_all_windows(&mut self) { + self.update(|cx| cx.windows.clear()); } pub fn read T>(&self, callback: F) -> T { diff --git a/crates/gpui/src/test.rs b/crates/gpui/src/test.rs index 3b2a5e996037d12abb0210c777659a96f6b280c8..def8ba2ce5ba665b84ff5f25d4c7fd6f0c0e8781 100644 --- a/crates/gpui/src/test.rs +++ b/crates/gpui/src/test.rs @@ -100,7 +100,7 @@ pub fn run_test( test_fn(cx, foreground_platform.clone(), deterministic.clone(), seed); }); - cx.update(|cx| cx.remove_all_windows()); + cx.remove_all_windows(); deterministic.run_until_parked(); cx.update(|cx| cx.clear_globals()); diff --git a/crates/gpui_macros/src/gpui_macros.rs b/crates/gpui_macros/src/gpui_macros.rs index ca15fa14a2c77c57aa34c243fc0076e440785713..e976245e06b3bc29e904d6e02db35f476498e1e0 100644 --- a/crates/gpui_macros/src/gpui_macros.rs +++ b/crates/gpui_macros/src/gpui_macros.rs @@ -137,7 +137,7 @@ pub fn test(args: TokenStream, function: TokenStream) -> TokenStream { ); )); cx_teardowns.extend(quote!( - #cx_varname.update(|cx| cx.remove_all_windows()); + #cx_varname.remove_all_windows(); deterministic.run_until_parked(); #cx_varname.update(|cx| cx.clear_globals()); )); @@ -212,7 +212,7 @@ pub fn test(args: TokenStream, function: TokenStream) -> TokenStream { ); )); cx_teardowns.extend(quote!( - #cx_varname.update(|cx| cx.remove_all_windows()); + #cx_varname.remove_all_windows(); deterministic.run_until_parked(); #cx_varname.update(|cx| cx.clear_globals()); )); diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 26afa544773e1f1d08023c73e0e315223288e096..d2c5576f21cb9c460468c3e1a685b1964cb175a7 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -1110,12 +1110,18 @@ impl Workspace { } pub fn close_global(_: &CloseWindow, cx: &mut AppContext) { - let id = cx.window_ids().find(|&id| cx.window_is_active(id)); - if let Some(id) = id { - //This can only get called when the window's project connection has been lost - //so we don't need to prompt the user for anything and instead just close the window - cx.remove_window(id); - } + cx.spawn(|mut cx| async move { + let id = cx + .window_ids() + .into_iter() + .find(|&id| cx.window_is_active(id)); + if let Some(id) = id { + //This can only get called when the window's project connection has been lost + //so we don't need to prompt the user for anything and instead just close the window + cx.remove_window(id); + } + }) + .detach(); } pub fn close( @@ -1140,19 +1146,14 @@ impl Workspace { ) -> Task> { let active_call = self.active_call().cloned(); let window_id = cx.window_id(); - let workspace_count = cx - .window_ids() - .collect::>() - .into_iter() - .filter_map(|window_id| { - cx.app_context() - .root_view(window_id)? - .clone() - .downcast::() - }) - .count(); cx.spawn(|this, mut cx| async move { + let workspace_count = cx + .window_ids() + .into_iter() + .filter_map(|window_id| cx.root_view(window_id)?.clone().downcast::()) + .count(); + if let Some(active_call) = active_call { if !quitting && workspace_count == 1 @@ -2979,10 +2980,10 @@ impl std::fmt::Debug for OpenPaths { pub struct WorkspaceCreated(WeakViewHandle); pub fn activate_workspace_for_project( - cx: &mut AppContext, + cx: &mut AsyncAppContext, predicate: impl Fn(&mut Project, &mut ModelContext) -> bool, ) -> Option> { - for window_id in cx.window_ids().collect::>() { + for window_id in cx.window_ids() { let handle = cx .update_window(window_id, |cx| { if let Some(workspace_handle) = cx.root_view().clone().downcast::() { @@ -3021,13 +3022,14 @@ pub fn open_paths( > { log::info!("open paths {:?}", abs_paths); - // Open paths in existing workspace if possible - let existing = - activate_workspace_for_project(cx, |project, cx| project.contains_paths(abs_paths, cx)); - let app_state = app_state.clone(); let abs_paths = abs_paths.to_vec(); cx.spawn(|mut cx| async move { + // Open paths in existing workspace if possible + let existing = activate_workspace_for_project(&mut cx, |project, cx| { + project.contains_paths(&abs_paths, cx) + }); + if let Some(existing) = existing { Ok(( existing.clone(), diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index d0ce4118b4f0688b9786b8e70c014ac7e4eb344b..ec44ac05d20cb848e7d07429eecaa4b1162da8ef 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -378,24 +378,25 @@ pub fn build_window_options( } fn restart(_: &Restart, cx: &mut gpui::AppContext) { - let mut workspaces = cx - .window_ids() - .filter_map(|window_id| { - Some( - cx.root_view(window_id)? - .clone() - .downcast::()? - .downgrade(), - ) - }) - .collect::>(); - - // If multiple windows have unsaved changes, and need a save prompt, - // prompt in the active window before switching to a different window. - workspaces.sort_by_key(|workspace| !cx.window_is_active(workspace.window_id())); - let should_confirm = cx.global::().confirm_quit; cx.spawn(|mut cx| async move { + let mut workspaces = cx + .window_ids() + .into_iter() + .filter_map(|window_id| { + Some( + cx.root_view(window_id)? + .clone() + .downcast::()? + .downgrade(), + ) + }) + .collect::>(); + + // If multiple windows have unsaved changes, and need a save prompt, + // prompt in the active window before switching to a different window. + workspaces.sort_by_key(|workspace| !cx.window_is_active(workspace.window_id())); + if let (true, Some(workspace)) = (should_confirm, workspaces.first()) { let answer = cx.prompt( workspace.window_id(), @@ -430,24 +431,25 @@ fn restart(_: &Restart, cx: &mut gpui::AppContext) { } fn quit(_: &Quit, cx: &mut gpui::AppContext) { - let mut workspaces = cx - .window_ids() - .filter_map(|window_id| { - Some( - cx.root_view(window_id)? - .clone() - .downcast::()? - .downgrade(), - ) - }) - .collect::>(); - - // If multiple windows have unsaved changes, and need a save prompt, - // prompt in the active window before switching to a different window. - workspaces.sort_by_key(|workspace| !cx.window_is_active(workspace.window_id())); - let should_confirm = cx.global::().confirm_quit; cx.spawn(|mut cx| async move { + let mut workspaces = cx + .window_ids() + .into_iter() + .filter_map(|window_id| { + Some( + cx.root_view(window_id)? + .clone() + .downcast::()? + .downgrade(), + ) + }) + .collect::>(); + + // If multiple windows have unsaved changes, and need a save prompt, + // prompt in the active window before switching to a different window. + workspaces.sort_by_key(|workspace| !cx.window_is_active(workspace.window_id())); + if let (true, Some(workspace)) = (should_confirm, workspaces.first()) { let answer = cx.prompt( workspace.window_id(),