Reuse existing remote workspaces when opening files from the CLI (#49307)

John Tur created

Re-lands https://github.com/zed-industries/zed/pull/48891, which was
reverted due to conflicts with multi-workspace.

Before you mark this PR as ready for review, make sure that you have:
- [X] Added a solid test coverage and/or screenshots from doing manual
testing
- [X] Done a self-review taking into account security and performance
aspects

Release Notes:

- N/A

Change summary

crates/project/src/project.rs                    |  21 
crates/project/src/worktree_store.rs             |   4 
crates/recent_projects/src/recent_projects.rs    |   2 
crates/recent_projects/src/remote_connections.rs | 247 +++++++++++++-
crates/remote/src/remote_client.rs               |   2 
crates/util/src/paths.rs                         |  12 
crates/workspace/src/workspace.rs                | 216 +++++++++---
crates/zed/src/zed.rs                            |  20 
crates/zed/src/zed/open_listener.rs              | 288 ++++++++---------
9 files changed, 558 insertions(+), 254 deletions(-)

Detailed changes

crates/project/src/project.rs 🔗

@@ -2343,14 +2343,12 @@ impl Project {
     pub fn visibility_for_paths(
         &self,
         paths: &[PathBuf],
-        metadatas: &[Metadata],
         exclude_sub_dirs: bool,
         cx: &App,
     ) -> Option<bool> {
         paths
             .iter()
-            .zip(metadatas)
-            .map(|(path, metadata)| self.visibility_for_path(path, metadata, exclude_sub_dirs, cx))
+            .map(|path| self.visibility_for_path(path, exclude_sub_dirs, cx))
             .max()
             .flatten()
     }
@@ -2358,17 +2356,26 @@ impl Project {
     pub fn visibility_for_path(
         &self,
         path: &Path,
-        metadata: &Metadata,
         exclude_sub_dirs: bool,
         cx: &App,
     ) -> Option<bool> {
         let path = SanitizedPath::new(path).as_path();
+        let path_style = self.path_style(cx);
         self.worktrees(cx)
             .filter_map(|worktree| {
                 let worktree = worktree.read(cx);
-                let abs_path = worktree.as_local()?.abs_path();
-                let contains = path == abs_path.as_ref()
-                    || (path.starts_with(abs_path) && (!exclude_sub_dirs || !metadata.is_dir));
+                let abs_path = worktree.abs_path();
+                let relative_path = path_style.strip_prefix(path, abs_path.as_ref());
+                let is_dir = relative_path
+                    .as_ref()
+                    .and_then(|p| worktree.entry_for_path(p))
+                    .is_some_and(|e| e.is_dir());
+                // Don't exclude the worktree root itself, only actual subdirectories
+                let is_subdir = relative_path
+                    .as_ref()
+                    .is_some_and(|p| !p.as_ref().as_unix_str().is_empty());
+                let contains =
+                    relative_path.is_some() && (!exclude_sub_dirs || !is_dir || !is_subdir);
                 contains.then(|| worktree.is_visible())
             })
             .max()

crates/project/src/worktree_store.rs 🔗

@@ -223,8 +223,8 @@ impl WorktreeStore {
         let abs_path = SanitizedPath::new(abs_path.as_ref());
         for tree in self.worktrees() {
             let path_style = tree.read(cx).path_style();
-            if let Ok(relative_path) = abs_path.as_ref().strip_prefix(tree.read(cx).abs_path())
-                && let Ok(relative_path) = RelPath::new(relative_path, path_style)
+            if let Some(relative_path) =
+                path_style.strip_prefix(abs_path.as_ref(), tree.read(cx).abs_path().as_ref())
             {
                 return Some((tree.clone(), relative_path.into_arc()));
             }

crates/recent_projects/src/recent_projects.rs 🔗

@@ -16,7 +16,7 @@ mod wsl_picker;
 
 use remote::RemoteConnectionOptions;
 pub use remote_connection::{RemoteConnectionModal, connect};
-pub use remote_connections::open_remote_project;
+pub use remote_connections::{navigate_to_positions, open_remote_project};
 
 use disconnected_overlay::DisconnectedOverlay;
 use fuzzy::{StringMatch, StringMatchCandidate};

crates/recent_projects/src/remote_connections.rs 🔗

@@ -8,7 +8,7 @@ use askpass::EncryptedPassword;
 use editor::Editor;
 use extension_host::ExtensionStore;
 use futures::{FutureExt as _, channel::oneshot, select};
-use gpui::{AppContext, AsyncApp, PromptLevel};
+use gpui::{AppContext, AsyncApp, PromptLevel, WindowHandle};
 
 use language::Point;
 use project::trusted_worktrees;
@@ -19,7 +19,10 @@ use remote::{
 pub use settings::SshConnection;
 use settings::{DevContainerConnection, ExtendingVec, RegisterSetting, Settings, WslConnection};
 use util::paths::PathWithPosition;
-use workspace::{AppState, MultiWorkspace, Workspace};
+use workspace::{
+    AppState, MultiWorkspace, OpenOptions, SerializedWorkspaceLocation, Workspace,
+    find_existing_workspace,
+};
 
 pub use remote_connection::{
     RemoteClientDelegate, RemoteConnectionModal, RemoteConnectionPrompt, SshConnectionHeader,
@@ -131,6 +134,69 @@ pub async fn open_remote_project(
     cx: &mut AsyncApp,
 ) -> Result<()> {
     let created_new_window = open_options.replace_window.is_none();
+
+    let (existing, open_visible) = find_existing_workspace(
+        &paths,
+        &open_options,
+        &SerializedWorkspaceLocation::Remote(connection_options.clone()),
+        cx,
+    )
+    .await;
+
+    if let Some((existing_window, existing_workspace)) = existing {
+        let remote_connection = cx
+            .update(|cx| {
+                existing_workspace
+                    .read(cx)
+                    .project()
+                    .read(cx)
+                    .remote_client()
+                    .and_then(|client| client.read(cx).remote_connection())
+            })
+            .ok_or_else(|| anyhow::anyhow!("no remote connection for existing remote workspace"))?;
+
+        let (resolved_paths, paths_with_positions) =
+            determine_paths_with_positions(&remote_connection, paths).await;
+
+        let open_results = existing_window
+            .update(cx, |multi_workspace, window, cx| {
+                window.activate_window();
+                multi_workspace.activate(existing_workspace.clone(), cx);
+                existing_workspace.update(cx, |workspace, cx| {
+                    workspace.open_paths(
+                        resolved_paths,
+                        OpenOptions {
+                            visible: Some(open_visible),
+                            ..Default::default()
+                        },
+                        None,
+                        window,
+                        cx,
+                    )
+                })
+            })?
+            .await;
+
+        _ = existing_window.update(cx, |multi_workspace, _, cx| {
+            let workspace = multi_workspace.workspace().clone();
+            workspace.update(cx, |workspace, cx| {
+                for item in open_results.iter().flatten() {
+                    if let Err(e) = item {
+                        workspace.show_error(&e, cx);
+                    }
+                }
+            });
+        });
+
+        let items = open_results
+            .into_iter()
+            .map(|r| r.and_then(|r| r.ok()))
+            .collect::<Vec<_>>();
+        navigate_to_positions(&existing_window, items, &paths_with_positions, cx);
+
+        return Ok(());
+    }
+
     let (window, initial_workspace) = if let Some(window) = open_options.replace_window {
         let workspace = window.update(cx, |multi_workspace, _, _| {
             multi_workspace.workspace().clone()
@@ -342,29 +408,7 @@ pub async fn open_remote_project(
             }
 
             Ok(items) => {
-                for (item, path) in items.into_iter().zip(paths_with_positions) {
-                    let Some(item) = item else {
-                        continue;
-                    };
-                    let Some(row) = path.row else {
-                        continue;
-                    };
-                    if let Some(active_editor) = item.downcast::<Editor>() {
-                        window
-                            .update(cx, |_, window, cx| {
-                                active_editor.update(cx, |editor, cx| {
-                                    let row = row.saturating_sub(1);
-                                    let col = path.column.unwrap_or(0).saturating_sub(1);
-                                    editor.go_to_singleton_buffer_point(
-                                        Point::new(row, col),
-                                        window,
-                                        cx,
-                                    );
-                                });
-                            })
-                            .ok();
-                    }
-                }
+                navigate_to_positions(&window, items, &paths_with_positions, cx);
             }
         }
 
@@ -390,6 +434,33 @@ pub async fn open_remote_project(
     Ok(())
 }
 
+pub fn navigate_to_positions(
+    window: &WindowHandle<MultiWorkspace>,
+    items: impl IntoIterator<Item = Option<Box<dyn workspace::item::ItemHandle>>>,
+    positions: &[PathWithPosition],
+    cx: &mut AsyncApp,
+) {
+    for (item, path) in items.into_iter().zip(positions) {
+        let Some(item) = item else {
+            continue;
+        };
+        let Some(row) = path.row else {
+            continue;
+        };
+        if let Some(active_editor) = item.downcast::<Editor>() {
+            window
+                .update(cx, |_, window, cx| {
+                    active_editor.update(cx, |editor, cx| {
+                        let row = row.saturating_sub(1);
+                        let col = path.column.unwrap_or(0).saturating_sub(1);
+                        editor.go_to_singleton_buffer_point(Point::new(row, col), window, cx);
+                    });
+                })
+                .ok();
+        }
+    }
+}
+
 pub(crate) async fn determine_paths_with_positions(
     remote_connection: &Arc<dyn RemoteConnection>,
     mut paths: Vec<PathBuf>,
@@ -444,6 +515,7 @@ mod tests {
     use remote_server::{HeadlessAppState, HeadlessProject};
     use serde_json::json;
     use util::path;
+    use workspace::find_existing_workspace;
 
     #[gpui::test]
     async fn test_open_remote_project_with_mock_connection(
@@ -525,6 +597,131 @@ mod tests {
             .unwrap();
     }
 
+    #[gpui::test]
+    async fn test_reuse_existing_remote_workspace_window(
+        cx: &mut TestAppContext,
+        server_cx: &mut TestAppContext,
+    ) {
+        let app_state = init_test(cx);
+        let executor = cx.executor();
+
+        cx.update(|cx| {
+            release_channel::init(semver::Version::new(0, 0, 0), cx);
+        });
+        server_cx.update(|cx| {
+            release_channel::init(semver::Version::new(0, 0, 0), cx);
+        });
+
+        let (opts, server_session, connect_guard) = RemoteClient::fake_server(cx, server_cx);
+
+        let remote_fs = FakeFs::new(server_cx.executor());
+        remote_fs
+            .insert_tree(
+                path!("/project"),
+                json!({
+                    "src": {
+                        "main.rs": "fn main() {}",
+                        "lib.rs": "pub fn hello() {}",
+                    },
+                    "README.md": "# Test Project",
+                }),
+            )
+            .await;
+
+        server_cx.update(HeadlessProject::init);
+        let http_client = Arc::new(BlockedHttpClient);
+        let node_runtime = NodeRuntime::unavailable();
+        let languages = Arc::new(language::LanguageRegistry::new(server_cx.executor()));
+        let proxy = Arc::new(ExtensionHostProxy::new());
+
+        let _headless = server_cx.new(|cx| {
+            HeadlessProject::new(
+                HeadlessAppState {
+                    session: server_session,
+                    fs: remote_fs.clone(),
+                    http_client,
+                    node_runtime,
+                    languages,
+                    extension_host_proxy: proxy,
+                },
+                false,
+                cx,
+            )
+        });
+
+        drop(connect_guard);
+
+        // First open: create a new window for the remote project.
+        let paths = vec![PathBuf::from(path!("/project"))];
+        let mut async_cx = cx.to_async();
+        open_remote_project(
+            opts.clone(),
+            paths,
+            app_state.clone(),
+            workspace::OpenOptions::default(),
+            &mut async_cx,
+        )
+        .await
+        .expect("first open_remote_project should succeed");
+
+        executor.run_until_parked();
+
+        assert_eq!(
+            cx.update(|cx| cx.windows().len()),
+            1,
+            "First open should create exactly one window"
+        );
+
+        let first_window = cx.update(|cx| cx.windows()[0].downcast::<MultiWorkspace>().unwrap());
+
+        // Verify find_existing_workspace discovers the remote workspace.
+        let search_paths = vec![PathBuf::from(path!("/project/src/lib.rs"))];
+        let (found, _open_visible) = find_existing_workspace(
+            &search_paths,
+            &workspace::OpenOptions::default(),
+            &SerializedWorkspaceLocation::Remote(opts.clone()),
+            &mut async_cx,
+        )
+        .await;
+
+        assert!(
+            found.is_some(),
+            "find_existing_workspace should locate the existing remote workspace"
+        );
+        let (found_window, _found_workspace) = found.unwrap();
+        assert_eq!(
+            found_window, first_window,
+            "find_existing_workspace should return the same window"
+        );
+
+        // Second open with the same connection options should reuse the window.
+        let second_paths = vec![PathBuf::from(path!("/project/src/lib.rs"))];
+        open_remote_project(
+            opts.clone(),
+            second_paths,
+            app_state.clone(),
+            workspace::OpenOptions::default(),
+            &mut async_cx,
+        )
+        .await
+        .expect("second open_remote_project should succeed via reuse");
+
+        executor.run_until_parked();
+
+        assert_eq!(
+            cx.update(|cx| cx.windows().len()),
+            1,
+            "Second open should reuse the existing window, not create a new one"
+        );
+
+        let still_first_window =
+            cx.update(|cx| cx.windows()[0].downcast::<MultiWorkspace>().unwrap());
+        assert_eq!(
+            still_first_window, first_window,
+            "The window handle should be the same after reuse"
+        );
+    }
+
     fn init_test(cx: &mut TestAppContext) -> Arc<AppState> {
         cx.update(|cx| {
             let state = AppState::test(cx);

crates/remote/src/remote_client.rs 🔗

@@ -1132,7 +1132,7 @@ impl RemoteClient {
             .unwrap()
     }
 
-    fn remote_connection(&self) -> Option<Arc<dyn RemoteConnection>> {
+    pub fn remote_connection(&self) -> Option<Arc<dyn RemoteConnection>> {
         self.state
             .as_ref()
             .and_then(|state| state.remote_connection())

crates/util/src/paths.rs 🔗

@@ -428,7 +428,17 @@ impl PathStyle {
             .find_map(|sep| parent.strip_suffix(sep))
             .unwrap_or(parent);
         let child = child.to_str()?;
-        let stripped = child.strip_prefix(parent)?;
+
+        // Match behavior of std::path::Path, which is case-insensitive for drive letters (e.g., "C:" == "c:")
+        let stripped = if self.is_windows()
+            && child.as_bytes().get(1) == Some(&b':')
+            && parent.as_bytes().get(1) == Some(&b':')
+            && child.as_bytes()[0].eq_ignore_ascii_case(&parent.as_bytes()[0])
+        {
+            child[2..].strip_prefix(&parent[2..])?
+        } else {
+            child.strip_prefix(parent)?
+        };
         if let Some(relative) = self
             .separators()
             .iter()

crates/workspace/src/workspace.rs 🔗

@@ -1158,7 +1158,7 @@ pub enum Event {
     ModalOpened,
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub enum OpenVisible {
     All,
     None,
@@ -5992,7 +5992,7 @@ impl Workspace {
             }
         }
 
-        match self.serialize_workspace_location(cx) {
+        match self.workspace_location(cx) {
             WorkspaceLocation::Location(location, paths) => {
                 let breakpoints = self.project.update(cx, |project, cx| {
                     project
@@ -6064,7 +6064,7 @@ impl Workspace {
         self.panes.iter().any(|pane| pane.read(cx).items_len() > 0)
     }
 
-    fn serialize_workspace_location(&self, cx: &App) -> WorkspaceLocation {
+    fn workspace_location(&self, cx: &App) -> WorkspaceLocation {
         let paths = PathList::new(&self.root_paths(cx));
         if let Some(connection) = self.project.read(cx).remote_connection_options(cx) {
             WorkspaceLocation::Location(SerializedWorkspaceLocation::Remote(connection), paths)
@@ -8308,26 +8308,149 @@ fn activate_any_workspace_window(cx: &mut AsyncApp) -> Option<WindowHandle<Multi
 }
 
 pub fn local_workspace_windows(cx: &App) -> Vec<WindowHandle<MultiWorkspace>> {
+    workspace_windows_for_location(&SerializedWorkspaceLocation::Local, cx)
+}
+
+pub fn workspace_windows_for_location(
+    serialized_location: &SerializedWorkspaceLocation,
+    cx: &App,
+) -> Vec<WindowHandle<MultiWorkspace>> {
     cx.windows()
         .into_iter()
         .filter_map(|window| window.downcast::<MultiWorkspace>())
         .filter(|multi_workspace| {
+            let same_host = |left: &RemoteConnectionOptions, right: &RemoteConnectionOptions| match (left, right) {
+                (RemoteConnectionOptions::Ssh(a), RemoteConnectionOptions::Ssh(b)) => {
+                    (&a.host, &a.username, &a.port) == (&b.host, &b.username, &b.port)
+                }
+                (RemoteConnectionOptions::Wsl(a), RemoteConnectionOptions::Wsl(b)) => {
+                    // The WSL username is not consistently populated in the workspace location, so ignore it for now.
+                    a.distro_name == b.distro_name
+                }
+                (RemoteConnectionOptions::Docker(a), RemoteConnectionOptions::Docker(b)) => {
+                    a.container_id == b.container_id
+                }
+                #[cfg(any(test, feature = "test-support"))]
+                (RemoteConnectionOptions::Mock(a), RemoteConnectionOptions::Mock(b)) => {
+                    a.id == b.id
+                }
+                _ => false,
+            };
+
             multi_workspace.read(cx).is_ok_and(|multi_workspace| {
-                multi_workspace
-                    .workspaces()
-                    .iter()
-                    .any(|workspace| workspace.read(cx).project.read(cx).is_local())
+                multi_workspace.workspaces().iter().any(|workspace| {
+                    match workspace.read(cx).workspace_location(cx) {
+                        WorkspaceLocation::Location(location, _) => {
+                            match (&location, serialized_location) {
+                                (
+                                    SerializedWorkspaceLocation::Local,
+                                    SerializedWorkspaceLocation::Local,
+                                ) => true,
+                                (
+                                    SerializedWorkspaceLocation::Remote(a),
+                                    SerializedWorkspaceLocation::Remote(b),
+                                ) => same_host(a, b),
+                                _ => false,
+                            }
+                        }
+                        _ => false,
+                    }
+                })
             })
         })
         .collect()
 }
 
-#[derive(Default)]
+pub async fn find_existing_workspace(
+    abs_paths: &[PathBuf],
+    open_options: &OpenOptions,
+    location: &SerializedWorkspaceLocation,
+    cx: &mut AsyncApp,
+) -> (
+    Option<(WindowHandle<MultiWorkspace>, Entity<Workspace>)>,
+    OpenVisible,
+) {
+    let mut existing: Option<(WindowHandle<MultiWorkspace>, Entity<Workspace>)> = None;
+    let mut open_visible = OpenVisible::All;
+    let mut best_match = None;
+
+    if open_options.open_new_workspace != Some(true) {
+        cx.update(|cx| {
+            for window in workspace_windows_for_location(location, cx) {
+                if let Ok(multi_workspace) = window.read(cx) {
+                    for workspace in multi_workspace.workspaces() {
+                        let project = workspace.read(cx).project.read(cx);
+                        let m = project.visibility_for_paths(
+                            abs_paths,
+                            open_options.open_new_workspace == None,
+                            cx,
+                        );
+                        if m > best_match {
+                            existing = Some((window, workspace.clone()));
+                            best_match = m;
+                        } else if best_match.is_none()
+                            && open_options.open_new_workspace == Some(false)
+                        {
+                            existing = Some((window, workspace.clone()))
+                        }
+                    }
+                }
+            }
+        });
+
+        let all_paths_are_files = existing
+            .as_ref()
+            .and_then(|(_, target_workspace)| {
+                cx.update(|cx| {
+                    let workspace = target_workspace.read(cx);
+                    let project = workspace.project.read(cx);
+                    let path_style = workspace.path_style(cx);
+                    Some(!abs_paths.iter().any(|path| {
+                        let path = util::paths::SanitizedPath::new(path);
+                        project.worktrees(cx).any(|worktree| {
+                            let worktree = worktree.read(cx);
+                            let abs_path = worktree.abs_path();
+                            path_style
+                                .strip_prefix(path.as_ref(), abs_path.as_ref())
+                                .and_then(|rel| worktree.entry_for_path(&rel))
+                                .is_some_and(|e| e.is_dir())
+                        })
+                    }))
+                })
+            })
+            .unwrap_or(false);
+
+        if open_options.open_new_workspace.is_none()
+            && existing.is_some()
+            && open_options.wait
+            && all_paths_are_files
+        {
+            cx.update(|cx| {
+                let windows = workspace_windows_for_location(location, cx);
+                let window = cx
+                    .active_window()
+                    .and_then(|window| window.downcast::<MultiWorkspace>())
+                    .filter(|window| windows.contains(window))
+                    .or_else(|| windows.into_iter().next());
+                if let Some(window) = window {
+                    if let Ok(multi_workspace) = window.read(cx) {
+                        let active_workspace = multi_workspace.workspace().clone();
+                        existing = Some((window, active_workspace));
+                        open_visible = OpenVisible::None;
+                    }
+                }
+            });
+        }
+    }
+    (existing, open_visible)
+}
+
+#[derive(Default, Clone)]
 pub struct OpenOptions {
     pub visible: Option<OpenVisible>,
     pub focus: Option<bool>,
     pub open_new_workspace: Option<bool>,
-    pub prefer_focused_window: bool,
+    pub wait: bool,
     pub replace_window: Option<WindowHandle<MultiWorkspace>>,
     pub env: Option<HashMap<String, String>>,
 }
@@ -8458,16 +8581,23 @@ pub fn open_paths(
     )>,
 > {
     let abs_paths = abs_paths.to_vec();
-    let mut existing: Option<(WindowHandle<MultiWorkspace>, Entity<Workspace>)> = None;
-    let mut best_match = None;
-    let mut open_visible = OpenVisible::All;
     #[cfg(target_os = "windows")]
     let wsl_path = abs_paths
         .iter()
         .find_map(|p| util::paths::WslPath::from_path(p));
 
     cx.spawn(async move |cx| {
-        if open_options.open_new_workspace != Some(true) {
+        let (mut existing, mut open_visible) = find_existing_workspace(
+            &abs_paths,
+            &open_options,
+            &SerializedWorkspaceLocation::Local,
+            cx,
+        )
+        .await;
+
+        // Fallback: if no workspace contains the paths and all paths are files,
+        // prefer an existing local workspace window (active window first).
+        if open_options.open_new_workspace.is_none() && existing.is_none() {
             let all_paths = abs_paths.iter().map(|path| app_state.fs.metadata(path));
             let all_metadatas = futures::future::join_all(all_paths)
                 .await
@@ -8475,60 +8605,22 @@ pub fn open_paths(
                 .filter_map(|result| result.ok().flatten())
                 .collect::<Vec<_>>();
 
-            cx.update(|cx| {
-                for window in local_workspace_windows(cx) {
-                    if let Ok(multi_workspace) = window.read(cx) {
-                        for workspace in multi_workspace.workspaces() {
-                            let m = workspace.read(cx).project.read(cx).visibility_for_paths(
-                                &abs_paths,
-                                &all_metadatas,
-                                open_options.open_new_workspace == None,
-                                cx,
-                            );
-                            if m > best_match {
-                                existing = Some((window, workspace.clone()));
-                                best_match = m;
-                            } else if best_match.is_none()
-                                && open_options.open_new_workspace == Some(false)
-                            {
-                                existing = Some((window, workspace.clone()))
-                            }
-                        }
-                    }
-                }
-            });
-
-            if (open_options.open_new_workspace.is_none()
-                || (open_options.open_new_workspace == Some(false)
-                    && open_options.prefer_focused_window))
-                && (existing.is_none() || open_options.prefer_focused_window)
-                && all_metadatas.iter().all(|file| !file.is_dir)
-            {
+            if all_metadatas.iter().all(|file| !file.is_dir) {
                 cx.update(|cx| {
-                    if let Some(window) = cx
+                    let windows = workspace_windows_for_location(
+                        &SerializedWorkspaceLocation::Local,
+                        cx,
+                    );
+                    let window = cx
                         .active_window()
                         .and_then(|window| window.downcast::<MultiWorkspace>())
-                        && let Ok(multi_workspace) = window.read(cx)
-                    {
-                        let active_workspace = multi_workspace.workspace().clone();
-                        let project = active_workspace.read(cx).project().read(cx);
-                        if project.is_local() && !project.is_via_collab() {
+                        .filter(|window| windows.contains(window))
+                        .or_else(|| windows.into_iter().next());
+                    if let Some(window) = window {
+                        if let Ok(multi_workspace) = window.read(cx) {
+                            let active_workspace = multi_workspace.workspace().clone();
                             existing = Some((window, active_workspace));
                             open_visible = OpenVisible::None;
-                            return;
-                        }
-                    }
-                    'outer: for window in local_workspace_windows(cx) {
-                        if let Ok(multi_workspace) = window.read(cx) {
-                            for workspace in multi_workspace.workspaces() {
-                                let project = workspace.read(cx).project().read(cx);
-                                if project.is_via_collab() {
-                                    continue;
-                                }
-                                existing = Some((window, workspace.clone()));
-                                open_visible = OpenVisible::None;
-                                break 'outer;
-                            }
                         }
                     }
                 });

crates/zed/src/zed.rs 🔗

@@ -2343,7 +2343,7 @@ mod tests {
             .fs
             .as_fake()
             .insert_tree(
-                "/root",
+                path!("/root"),
                 json!({
                     "a": {
                         "aa": null,
@@ -2371,7 +2371,10 @@ mod tests {
 
         cx.update(|cx| {
             open_paths(
-                &[PathBuf::from("/root/a"), PathBuf::from("/root/b")],
+                &[
+                    PathBuf::from(path!("/root/a")),
+                    PathBuf::from(path!("/root/b")),
+                ],
                 app_state.clone(),
                 workspace::OpenOptions::default(),
                 cx,
@@ -2383,7 +2386,7 @@ mod tests {
 
         cx.update(|cx| {
             open_paths(
-                &[PathBuf::from("/root/a")],
+                &[PathBuf::from(path!("/root/a"))],
                 app_state.clone(),
                 workspace::OpenOptions::default(),
                 cx,
@@ -2414,7 +2417,10 @@ mod tests {
 
         cx.update(|cx| {
             open_paths(
-                &[PathBuf::from("/root/c"), PathBuf::from("/root/d")],
+                &[
+                    PathBuf::from(path!("/root/c")),
+                    PathBuf::from(path!("/root/d")),
+                ],
                 app_state.clone(),
                 workspace::OpenOptions::default(),
                 cx,
@@ -2430,7 +2436,7 @@ mod tests {
             .unwrap();
         cx.update(|cx| {
             open_paths(
-                &[PathBuf::from("/root/e")],
+                &[PathBuf::from(path!("/root/e"))],
                 app_state,
                 workspace::OpenOptions {
                     replace_window: Some(window),
@@ -2454,7 +2460,7 @@ mod tests {
                         .worktrees(cx)
                         .map(|w| w.read(cx).abs_path())
                         .collect::<Vec<_>>(),
-                    &[Path::new("/root/e").into()]
+                    &[Path::new(path!("/root/e")).into()]
                 );
                 assert!(workspace.left_dock().read(cx).is_open());
                 assert!(workspace.active_pane().focus_handle(cx).is_focused(window));
@@ -5254,7 +5260,7 @@ mod tests {
 
             cx.update(|cx| {
                 let open_options = OpenOptions {
-                    prefer_focused_window: true,
+                    wait: true,
                     ..Default::default()
                 };
 

crates/zed/src/zed/open_listener.rs 🔗

@@ -4,21 +4,19 @@ use anyhow::{Context as _, Result, anyhow};
 use cli::{CliRequest, CliResponse, ipc::IpcSender};
 use cli::{IpcHandshake, ipc};
 use client::{ZedLink, parse_zed_link};
-use collections::HashMap;
 use db::kvp::KEY_VALUE_STORE;
 use editor::Editor;
 use fs::Fs;
 use futures::channel::mpsc::{UnboundedReceiver, UnboundedSender};
 use futures::channel::{mpsc, oneshot};
 use futures::future;
-use futures::future::join_all;
+
 use futures::{FutureExt, SinkExt, StreamExt};
 use git_ui::{file_diff_view::FileDiffView, multi_diff_view::MultiDiffView};
 use gpui::{App, AsyncApp, Global, WindowHandle};
-use language::Point;
 use onboarding::FIRST_OPEN;
 use onboarding::show_onboarding_view;
-use recent_projects::{RemoteSettings, open_remote_project};
+use recent_projects::{RemoteSettings, navigate_to_positions, open_remote_project};
 use remote::{RemoteConnectionOptions, WslConnectionOptions};
 use settings::Settings;
 use std::path::{Path, PathBuf};
@@ -340,21 +338,9 @@ pub async fn open_paths_with_positions(
     WindowHandle<MultiWorkspace>,
     Vec<Option<Result<Box<dyn ItemHandle>>>>,
 )> {
-    let mut caret_positions = HashMap::default();
-
     let paths = path_positions
         .iter()
-        .map(|path_with_position| {
-            let path = path_with_position.path.clone();
-            if let Some(row) = path_with_position.row
-                && path.is_file()
-            {
-                let row = row.saturating_sub(1);
-                let col = path_with_position.column.unwrap_or(0).saturating_sub(1);
-                caret_positions.insert(path.clone(), Point::new(row, col));
-            }
-            path
-        })
+        .map(|path_with_position| path_with_position.path.clone())
         .collect::<Vec<_>>();
 
     let (multi_workspace, mut items) = cx
@@ -391,25 +377,15 @@ pub async fn open_paths_with_positions(
     for (item, path) in items.iter_mut().zip(&paths) {
         if let Some(Err(error)) = item {
             *error = anyhow!("error opening {path:?}: {error}");
-            continue;
-        }
-        let Some(Ok(item)) = item else {
-            continue;
-        };
-        let Some(point) = caret_positions.remove(path) else {
-            continue;
-        };
-        if let Some(active_editor) = item.downcast::<Editor>() {
-            multi_workspace
-                .update(cx, |_, window, cx| {
-                    active_editor.update(cx, |editor, cx| {
-                        editor.go_to_singleton_buffer_point(point, window, cx);
-                    });
-                })
-                .log_err();
         }
     }
 
+    let items_for_navigation = items
+        .iter()
+        .map(|item| item.as_ref().and_then(|r| r.as_ref().ok()).cloned())
+        .collect::<Vec<_>>();
+    navigate_to_positions(&multi_workspace, items_for_navigation, path_positions, cx);
+
     Ok((multi_workspace, items))
 }
 
@@ -525,64 +501,82 @@ async fn open_workspaces(
                 .detach();
             });
         }
-    } else {
-        // If there are paths to open, open a workspace for each grouping of paths
-        let mut errored = false;
-
-        for (location, workspace_paths) in grouped_locations {
-            match location {
-                SerializedWorkspaceLocation::Local => {
-                    let workspace_paths = workspace_paths
-                        .paths()
-                        .iter()
-                        .map(|path| path.to_string_lossy().into_owned())
-                        .collect();
-
-                    let workspace_failed_to_open = open_local_workspace(
-                        workspace_paths,
-                        diff_paths.clone(),
-                        diff_all,
-                        open_new_workspace,
-                        reuse,
-                        wait,
-                        responses,
-                        env.as_ref(),
-                        &app_state,
-                        cx,
-                    )
-                    .await;
+        return Ok(());
+    }
+    // If there are paths to open, open a workspace for each grouping of paths
+    let mut errored = false;
 
-                    if workspace_failed_to_open {
-                        errored = true
-                    }
+    for (location, workspace_paths) in grouped_locations {
+        // If reuse flag is passed, open a new workspace in an existing window.
+        let (open_new_workspace, replace_window) = if reuse {
+            (
+                Some(true),
+                cx.update(|cx| {
+                    workspace::workspace_windows_for_location(&location, cx)
+                        .into_iter()
+                        .next()
+                }),
+            )
+        } else {
+            (open_new_workspace, None)
+        };
+        let open_options = workspace::OpenOptions {
+            open_new_workspace,
+            replace_window,
+            wait,
+            env: env.clone(),
+            ..Default::default()
+        };
+
+        match location {
+            SerializedWorkspaceLocation::Local => {
+                let workspace_paths = workspace_paths
+                    .paths()
+                    .iter()
+                    .map(|path| path.to_string_lossy().into_owned())
+                    .collect();
+
+                let workspace_failed_to_open = open_local_workspace(
+                    workspace_paths,
+                    diff_paths.clone(),
+                    diff_all,
+                    open_options,
+                    responses,
+                    &app_state,
+                    cx,
+                )
+                .await;
+
+                if workspace_failed_to_open {
+                    errored = true
                 }
-                SerializedWorkspaceLocation::Remote(mut connection) => {
-                    let app_state = app_state.clone();
-                    if let RemoteConnectionOptions::Ssh(options) = &mut connection {
-                        cx.update(|cx| {
-                            RemoteSettings::get_global(cx)
-                                .fill_connection_options_from_settings(options)
-                        });
-                    }
-                    cx.spawn(async move |cx| {
-                        open_remote_project(
-                            connection,
-                            workspace_paths.paths().to_vec(),
-                            app_state,
-                            OpenOptions::default(),
-                            cx,
-                        )
-                        .await
-                        .log_err();
-                    })
-                    .detach();
+            }
+            SerializedWorkspaceLocation::Remote(mut connection) => {
+                let app_state = app_state.clone();
+                if let RemoteConnectionOptions::Ssh(options) = &mut connection {
+                    cx.update(|cx| {
+                        RemoteSettings::get_global(cx)
+                            .fill_connection_options_from_settings(options)
+                    });
                 }
+                cx.spawn(async move |cx| {
+                    open_remote_project(
+                        connection,
+                        workspace_paths.paths().to_vec(),
+                        app_state,
+                        open_options,
+                        cx,
+                    )
+                    .await
+                    .log_err();
+                })
+                .detach();
             }
         }
-
-        anyhow::ensure!(!errored, "failed to open a workspace");
     }
 
+    anyhow::ensure!(!errored, "failed to open a workspace");
+
     Ok(())
 }
 
@@ -590,39 +584,20 @@ async fn open_local_workspace(
     workspace_paths: Vec<String>,
     diff_paths: Vec<[String; 2]>,
     diff_all: bool,
-    open_new_workspace: Option<bool>,
-    reuse: bool,
-    wait: bool,
+    open_options: workspace::OpenOptions,
     responses: &IpcSender<CliResponse>,
-    env: Option<&HashMap<String, String>>,
     app_state: &Arc<AppState>,
     cx: &mut AsyncApp,
 ) -> bool {
     let paths_with_position =
         derive_paths_with_position(app_state.fs.as_ref(), workspace_paths).await;
 
-    // If reuse flag is passed, open a new workspace in an existing window.
-    let (open_new_workspace, replace_window) = if reuse {
-        (
-            Some(true),
-            cx.update(|cx| workspace::local_workspace_windows(cx).into_iter().next()),
-        )
-    } else {
-        (open_new_workspace, None)
-    };
-
     let (workspace, items) = match open_paths_with_positions(
         &paths_with_position,
         &diff_paths,
         diff_all,
         app_state.clone(),
-        workspace::OpenOptions {
-            open_new_workspace,
-            replace_window,
-            prefer_focused_window: wait || open_new_workspace == Some(false),
-            env: env.cloned(),
-            ..Default::default()
-        },
+        open_options.clone(),
         cx,
     )
     .await
@@ -641,10 +616,9 @@ async fn open_local_workspace(
     let mut errored = false;
     let mut item_release_futures = Vec::new();
     let mut subscriptions = Vec::new();
-
     // If --wait flag is used with no paths, or a directory, then wait until
     // the entire workspace is closed.
-    if wait {
+    if open_options.wait {
         let mut wait_for_window_close = paths_with_position.is_empty() && diff_paths.is_empty();
         for path_with_position in &paths_with_position {
             if app_state.fs.is_dir(&path_with_position.path).await {
@@ -667,7 +641,7 @@ async fn open_local_workspace(
     for item in items {
         match item {
             Some(Ok(item)) => {
-                if wait {
+                if open_options.wait {
                     let (release_tx, release_rx) = oneshot::channel();
                     item_release_futures.push(release_rx);
                     subscriptions.push(Ok(cx.update(|cx| {
@@ -692,7 +666,7 @@ async fn open_local_workspace(
         }
     }
 
-    if wait {
+    if open_options.wait {
         let wait = async move {
             let _subscriptions = subscriptions;
             let _ = future::try_join_all(item_release_futures).await;
@@ -723,17 +697,30 @@ pub async fn derive_paths_with_position(
     fs: &dyn Fs,
     path_strings: impl IntoIterator<Item = impl AsRef<str>>,
 ) -> Vec<PathWithPosition> {
-    join_all(path_strings.into_iter().map(|path_str| async move {
-        let canonicalized = fs.canonicalize(Path::new(path_str.as_ref())).await;
-        (path_str, canonicalized)
-    }))
-    .await
-    .into_iter()
-    .map(|(original, canonicalized)| match canonicalized {
-        Ok(canonicalized) => PathWithPosition::from_path(canonicalized),
-        Err(_) => PathWithPosition::parse_str(original.as_ref()),
-    })
-    .collect()
+    let path_strings: Vec<_> = path_strings.into_iter().collect();
+    let mut result = Vec::with_capacity(path_strings.len());
+    for path_str in path_strings {
+        let original_path = Path::new(path_str.as_ref());
+        let mut parsed = PathWithPosition::parse_str(path_str.as_ref());
+
+        // If the unparsed path string actually points to a file, use that file instead of parsing out the line/col number.
+        // Note: The colon syntax is also used to open NTFS alternate data streams (e.g., `file.txt:stream`), which would cause issues.
+        // However, the colon is not valid in NTFS file names, so we can just skip this logic.
+        if !cfg!(windows)
+            && parsed.row.is_some()
+            && parsed.path != original_path
+            && fs.is_file(original_path).await
+        {
+            parsed = PathWithPosition::from_path(original_path.to_path_buf());
+        }
+
+        if let Ok(canonicalized) = fs.canonicalize(&parsed.path).await {
+            parsed.path = canonicalized;
+        }
+
+        result.push(parsed);
+    }
+    result
 }
 
 #[cfg(test)]
@@ -961,11 +948,11 @@ mod tests {
                     workspace_paths,
                     vec![],
                     false,
-                    None,
-                    false,
-                    true,
+                    workspace::OpenOptions {
+                        wait: true,
+                        ..Default::default()
+                    },
                     &response_tx,
-                    None,
                     &app_state,
                     &mut cx,
                 )
@@ -1059,11 +1046,11 @@ mod tests {
                     workspace_paths,
                     vec![],
                     false,
-                    open_new_workspace,
-                    false,
-                    false,
+                    workspace::OpenOptions {
+                        open_new_workspace,
+                        ..Default::default()
+                    },
                     &response_tx,
-                    None,
                     &app_state,
                     &mut cx,
                 )
@@ -1133,11 +1120,8 @@ mod tests {
                         workspace_paths,
                         vec![],
                         false,
-                        None,
-                        false,
-                        false,
+                        workspace::OpenOptions::default(),
                         &response_tx,
-                        None,
                         &app_state,
                         &mut cx,
                     )
@@ -1148,6 +1132,17 @@ mod tests {
 
         // Now test the reuse functionality - should replace the existing workspace
         let workspace_paths_reuse = vec![file1_path.to_string()];
+        let paths: Vec<PathBuf> = workspace_paths_reuse.iter().map(PathBuf::from).collect();
+        let window_to_replace = workspace::find_existing_workspace(
+            &paths,
+            &workspace::OpenOptions::default(),
+            &workspace::SerializedWorkspaceLocation::Local,
+            &mut cx.to_async(),
+        )
+        .await
+        .0
+        .unwrap()
+        .0;
 
         let errored_reuse = cx
             .spawn({
@@ -1158,11 +1153,11 @@ mod tests {
                         workspace_paths_reuse,
                         vec![],
                         false,
-                        None, // open_new_workspace will be overridden by reuse logic
-                        true, // reuse = true
-                        false,
+                        workspace::OpenOptions {
+                            replace_window: Some(window_to_replace),
+                            ..Default::default()
+                        },
                         &response_tx,
-                        None,
                         &app_state,
                         &mut cx,
                     )
@@ -1309,11 +1304,8 @@ mod tests {
                         workspace_paths_1,
                         Vec::new(),
                         false,
-                        None,
-                        false,
-                        false,
+                        workspace::OpenOptions::default(),
                         &response_tx,
-                        None,
                         &app_state,
                         &mut cx,
                     )
@@ -1336,11 +1328,11 @@ mod tests {
                         workspace_paths_2,
                         Vec::new(),
                         false,
-                        Some(true), // Force new window
-                        false,
-                        false,
+                        workspace::OpenOptions {
+                            open_new_workspace: Some(true), // Force new window
+                            ..Default::default()
+                        },
                         &response_tx,
-                        None,
                         &app_state,
                         &mut cx,
                     )
@@ -1382,11 +1374,11 @@ mod tests {
                         workspace_paths_add,
                         Vec::new(),
                         false,
-                        Some(false), // --add flag: open_new_workspace = Some(false)
-                        false,
-                        false,
+                        workspace::OpenOptions {
+                            open_new_workspace: Some(false), // --add flag
+                            ..Default::default()
+                        },
                         &response_tx,
-                        None,
                         &app_state,
                         &mut cx,
                     )