Eliminate OpenPaths global action for workspace and replace with methods

Antonio Scandurra created

We no longer want to invoke this with dispatch_action.

Change summary

crates/recent_projects/src/recent_projects.rs |  61 +++++++--
crates/workspace/src/workspace.rs             | 132 ++++++++++----------
crates/zed/src/main.rs                        |  14 -
3 files changed, 117 insertions(+), 90 deletions(-)

Detailed changes

crates/recent_projects/src/recent_projects.rs 🔗

@@ -5,30 +5,30 @@ use gpui::{
     actions,
     anyhow::Result,
     elements::{Flex, ParentElement},
-    AnyElement, AppContext, Element, Task, ViewContext,
+    AnyElement, AppContext, Element, Task, ViewContext, WeakViewHandle,
 };
 use highlighted_workspace_location::HighlightedWorkspaceLocation;
 use ordered_float::OrderedFloat;
 use picker::{Picker, PickerDelegate, PickerEvent};
 use settings::Settings;
-use std::sync::Arc;
+use std::sync::{Arc, Weak};
 use workspace::{
-    notifications::simple_message_notification::MessageNotification, OpenPaths, Workspace,
+    notifications::simple_message_notification::MessageNotification, AppState, Workspace,
     WorkspaceLocation, WORKSPACE_DB,
 };
 
 actions!(projects, [OpenRecent]);
 
-pub fn init(cx: &mut AppContext) {
-    cx.add_async_action(toggle);
+pub fn init(cx: &mut AppContext, app_state: Weak<AppState>) {
+    cx.add_async_action(
+        move |_: &mut Workspace, _: &OpenRecent, cx: &mut ViewContext<Workspace>| {
+            toggle(app_state.clone(), cx)
+        },
+    );
     RecentProjects::init(cx);
 }
 
-fn toggle(
-    _: &mut Workspace,
-    _: &OpenRecent,
-    cx: &mut ViewContext<Workspace>,
-) -> Option<Task<Result<()>>> {
+fn toggle(app_state: Weak<AppState>, cx: &mut ViewContext<Workspace>) -> Option<Task<Result<()>>> {
     Some(cx.spawn(|workspace, mut cx| async move {
         let workspace_locations: Vec<_> = cx
             .background()
@@ -46,9 +46,17 @@ fn toggle(
         workspace.update(&mut cx, |workspace, cx| {
             if !workspace_locations.is_empty() {
                 workspace.toggle_modal(cx, |_, cx| {
+                    let workspace = cx.weak_handle();
                     cx.add_view(|cx| {
-                        RecentProjects::new(RecentProjectsDelegate::new(workspace_locations), cx)
-                            .with_max_size(800., 1200.)
+                        RecentProjects::new(
+                            RecentProjectsDelegate::new(
+                                workspace,
+                                workspace_locations,
+                                app_state.clone(),
+                            ),
+                            cx,
+                        )
+                        .with_max_size(800., 1200.)
                     })
                 });
             } else {
@@ -64,15 +72,23 @@ fn toggle(
 type RecentProjects = Picker<RecentProjectsDelegate>;
 
 struct RecentProjectsDelegate {
+    workspace: WeakViewHandle<Workspace>,
     workspace_locations: Vec<WorkspaceLocation>,
+    app_state: Weak<AppState>,
     selected_match_index: usize,
     matches: Vec<StringMatch>,
 }
 
 impl RecentProjectsDelegate {
-    fn new(workspace_locations: Vec<WorkspaceLocation>) -> Self {
+    fn new(
+        workspace: WeakViewHandle<Workspace>,
+        workspace_locations: Vec<WorkspaceLocation>,
+        app_state: Weak<AppState>,
+    ) -> Self {
         Self {
+            workspace,
             workspace_locations,
+            app_state,
             selected_match_index: 0,
             matches: Default::default(),
         }
@@ -139,11 +155,22 @@ impl PickerDelegate for RecentProjectsDelegate {
     }
 
     fn confirm(&mut self, cx: &mut ViewContext<RecentProjects>) {
-        if let Some(selected_match) = &self.matches.get(self.selected_index()) {
+        if let Some(((selected_match, workspace), app_state)) = self
+            .matches
+            .get(self.selected_index())
+            .zip(self.workspace.upgrade(cx))
+            .zip(self.app_state.upgrade())
+        {
             let workspace_location = &self.workspace_locations[selected_match.candidate_id];
-            cx.dispatch_action(OpenPaths {
-                paths: workspace_location.paths().as_ref().clone(),
-            });
+            workspace
+                .update(cx, |workspace, cx| {
+                    workspace.open_workspace_for_paths(
+                        workspace_location.paths().as_ref().clone(),
+                        app_state,
+                        cx,
+                    )
+                })
+                .detach_and_log_err(cx);
             cx.emit(PickerEvent::Dismiss);
         }
     }

crates/workspace/src/workspace.rs 🔗

@@ -219,7 +219,6 @@ pub type WorkspaceId = i64;
 impl_internal_actions!(
     workspace,
     [
-        OpenPaths,
         ToggleFollow,
         JoinProject,
         OpenSharedScreen,
@@ -235,81 +234,53 @@ pub fn init(app_state: Arc<AppState>, cx: &mut AppContext) {
     dock::init(cx);
     notifications::init(cx);
 
-    cx.add_global_action(|_: &Open, cx: &mut AppContext| {
-        let mut paths = cx.prompt_for_paths(PathPromptOptions {
-            files: true,
-            directories: true,
-            multiple: true,
-        });
-
-        cx.spawn(|mut cx| async move {
-            if let Some(paths) = paths.recv().await.flatten() {
-                cx.update(|cx| cx.dispatch_global_action(OpenPaths { paths }));
-            }
-        })
-        .detach();
-    });
-    cx.add_action(|_, _: &Open, cx: &mut ViewContext<Workspace>| {
-        let mut paths = cx.prompt_for_paths(PathPromptOptions {
-            files: true,
-            directories: true,
-            multiple: true,
-        });
-
-        let handle = cx.handle().downgrade();
-        cx.spawn(|_, mut cx| async move {
-            if let Some(paths) = paths.recv().await.flatten() {
-                cx.update(|cx| {
-                    cx.dispatch_action_at(handle.window_id(), handle.id(), OpenPaths { paths })
-                })
-            }
-        })
-        .detach();
-    });
     cx.add_global_action({
         let app_state = Arc::downgrade(&app_state);
-        move |action: &OpenPaths, cx: &mut AppContext| {
+        move |_: &Open, cx: &mut AppContext| {
+            let mut paths = cx.prompt_for_paths(PathPromptOptions {
+                files: true,
+                directories: true,
+                multiple: true,
+            });
+
             if let Some(app_state) = app_state.upgrade() {
-                open_paths(&action.paths, &app_state, None, cx).detach();
+                cx.spawn(move |mut cx| async move {
+                    if let Some(paths) = paths.recv().await.flatten() {
+                        cx.update(|cx| {
+                            open_paths(&paths, &app_state, None, cx).detach_and_log_err(cx)
+                        });
+                    }
+                })
+                .detach();
             }
         }
     });
-    cx.add_async_action({
+    cx.add_action({
         let app_state = Arc::downgrade(&app_state);
-        move |workspace, action: &OpenPaths, cx: &mut ViewContext<Workspace>| {
-            if !workspace.project().read(cx).is_local() {
-                cx.propagate_action();
-                return None;
-            }
-
-            let app_state = app_state.upgrade()?;
-            let window_id = cx.window_id();
-            let action = action.clone();
-            let is_remote = workspace.project.read(cx).is_remote();
-            let has_worktree = workspace.project.read(cx).worktrees(cx).next().is_some();
-            let has_dirty_items = workspace.items(cx).any(|item| item.is_dirty(cx));
-            let close_task = if is_remote || has_worktree || has_dirty_items {
-                None
-            } else {
-                Some(workspace.prepare_to_close(false, cx))
-            };
+        move |_, _: &Open, cx: &mut ViewContext<Workspace>| {
+            let mut paths = cx.prompt_for_paths(PathPromptOptions {
+                files: true,
+                directories: true,
+                multiple: true,
+            });
 
-            Some(cx.spawn(|_, mut cx| async move {
-                let window_id_to_replace = if let Some(close_task) = close_task {
-                    if !close_task.await? {
-                        return Ok(());
+            if let Some(app_state) = app_state.upgrade() {
+                cx.spawn(|this, mut cx| async move {
+                    if let Some(paths) = paths.recv().await.flatten() {
+                        if let Some(task) = this
+                            .update(&mut cx, |this, cx| {
+                                this.open_workspace_for_paths(paths, app_state, cx)
+                            })
+                            .log_err()
+                        {
+                            task.await.log_err();
+                        }
                     }
-                    Some(window_id)
-                } else {
-                    None
-                };
-                cx.update(|cx| open_paths(&action.paths, &app_state, window_id_to_replace, cx))
-                    .await?;
-                Ok(())
-            }))
+                })
+                .detach();
+            }
         }
     });
-
     cx.add_global_action({
         let app_state = Arc::downgrade(&app_state);
         move |_: &NewWindow, cx: &mut AppContext| {
@@ -1179,6 +1150,37 @@ impl Workspace {
         })
     }
 
+    pub fn open_workspace_for_paths(
+        &mut self,
+        paths: Vec<PathBuf>,
+        app_state: Arc<AppState>,
+        cx: &mut ViewContext<Self>,
+    ) -> Task<Result<()>> {
+        let window_id = cx.window_id();
+        let is_remote = self.project.read(cx).is_remote();
+        let has_worktree = self.project.read(cx).worktrees(cx).next().is_some();
+        let has_dirty_items = self.items(cx).any(|item| item.is_dirty(cx));
+        let close_task = if is_remote || has_worktree || has_dirty_items {
+            None
+        } else {
+            Some(self.prepare_to_close(false, cx))
+        };
+
+        cx.spawn(|_, mut cx| async move {
+            let window_id_to_replace = if let Some(close_task) = close_task {
+                if !close_task.await? {
+                    return Ok(());
+                }
+                Some(window_id)
+            } else {
+                None
+            };
+            cx.update(|cx| open_paths(&paths, &app_state, window_id_to_replace, cx))
+                .await?;
+            Ok(())
+        })
+    }
+
     #[allow(clippy::type_complexity)]
     pub fn open_paths(
         &mut self,

crates/zed/src/main.rs 🔗

@@ -44,7 +44,7 @@ use theme::ThemeRegistry;
 use util::{channel::RELEASE_CHANNEL, paths, ResultExt, TryFutureExt};
 use workspace::{
     self, dock::FocusDock, item::ItemHandle, notifications::NotifyResultExt, AppState, NewFile,
-    OpenPaths, Workspace,
+    Workspace,
 };
 use zed::{self, build_window_options, initialize_workspace, languages, menus, OpenSettings};
 
@@ -160,7 +160,6 @@ fn main() {
         vim::init(cx);
         terminal_view::init(cx);
         theme_testbench::init(cx);
-        recent_projects::init(cx);
         copilot::init(http.clone(), node_runtime, cx);
 
         cx.spawn(|cx| watch_themes(fs.clone(), themes.clone(), cx))
@@ -194,6 +193,7 @@ fn main() {
         auto_update::init(http, client::ZED_SERVER_URL.clone(), cx);
 
         workspace::init(app_state.clone(), cx);
+        recent_projects::init(cx, Arc::downgrade(&app_state));
 
         journal::init(app_state.clone(), cx);
         language_selector::init(app_state.clone(), cx);
@@ -212,7 +212,7 @@ fn main() {
                 cx.spawn(|cx| async move { restore_or_create_workspace(&app_state, cx).await })
                     .detach()
             } else {
-                cx.dispatch_global_action(OpenPaths { paths });
+                workspace::open_paths(&paths, &app_state, None, cx).detach_and_log_err(cx);
             }
         } else {
             if let Ok(Some(connection)) = cli_connections_rx.try_next() {
@@ -267,11 +267,9 @@ fn main() {
 
 async fn restore_or_create_workspace(app_state: &Arc<AppState>, mut cx: AsyncAppContext) {
     if let Some(location) = workspace::last_opened_workspace_paths().await {
-        cx.update(|cx| {
-            cx.dispatch_global_action(OpenPaths {
-                paths: location.paths().as_ref().clone(),
-            })
-        });
+        cx.update(|cx| workspace::open_paths(location.paths().as_ref(), app_state, None, cx))
+            .await
+            .log_err();
     } else if matches!(KEY_VALUE_STORE.read_kvp(FIRST_OPEN), Ok(None)) {
         cx.update(|cx| show_welcome_experience(app_state, cx));
     } else {