From d2c922b81511daafbec8cbedc0f4abab942f0128 Mon Sep 17 00:00:00 2001 From: John Tur Date: Tue, 10 Feb 2026 17:48:54 -0500 Subject: [PATCH] Reuse existing windows when opening files in remote workspaces (#48891) Closes https://github.com/zed-industries/zed/issues/42366 - [ ] Tests or screenshots needed? - [X] Code Reviewed - [X] Manual QA Release Notes: - Opening files with the Zed CLI will reuse existing windows for remote workspaces. --- crates/project/src/project.rs | 21 +- crates/project/src/worktree_store.rs | 4 +- crates/recent_projects/src/recent_projects.rs | 2 +- .../recent_projects/src/remote_connections.rs | 113 ++++++-- crates/remote/src/remote_client.rs | 2 +- crates/workspace/src/workspace.rs | 199 +++++++++---- crates/zed/Cargo.toml | 2 +- crates/zed/src/main.rs | 4 +- crates/zed/src/zed.rs | 20 +- crates/zed/src/zed/open_listener.rs | 268 +++++++++--------- 10 files changed, 396 insertions(+), 239 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 265c8147da40ee1b1832c3e6d72def0b56188794..4e0b0b382fd3da67fb8c29b09f7526bb48ae8ee1 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -2325,14 +2325,12 @@ impl Project { pub fn visibility_for_paths( &self, paths: &[PathBuf], - metadatas: &[Metadata], exclude_sub_dirs: bool, cx: &App, ) -> Option { 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() } @@ -2340,17 +2338,26 @@ impl Project { pub fn visibility_for_path( &self, path: &Path, - metadata: &Metadata, exclude_sub_dirs: bool, cx: &App, ) -> Option { 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() diff --git a/crates/project/src/worktree_store.rs b/crates/project/src/worktree_store.rs index 0f004b8653cbecd33e6aecc66ae20d6187d67ee4..31a6cc041eda875f3c7ee5b33b77519d7ee2b142 100644 --- a/crates/project/src/worktree_store.rs +++ b/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())); } diff --git a/crates/recent_projects/src/recent_projects.rs b/crates/recent_projects/src/recent_projects.rs index 7f67627ad4d6841419292e58b5b41a5fd5ef7d5b..8d003ead0e578bc0638291e14148c657a702c942 100644 --- a/crates/recent_projects/src/recent_projects.rs +++ b/crates/recent_projects/src/recent_projects.rs @@ -11,7 +11,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}; diff --git a/crates/recent_projects/src/remote_connections.rs b/crates/recent_projects/src/remote_connections.rs index 9d6199786cb6f251a792fa32e8caccd9351d00d3..9aabc635508ea812f89a843cefe840442760c84f 100644 --- a/crates/recent_projects/src/remote_connections.rs +++ b/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,9 @@ use remote::{ pub use settings::SshConnection; use settings::{DevContainerConnection, ExtendingVec, RegisterSetting, Settings, WslConnection}; use util::paths::PathWithPosition; -use workspace::{AppState, Workspace}; +use workspace::{ + AppState, OpenOptions, SerializedWorkspaceLocation, Workspace, find_existing_workspace, +}; pub use remote_connection::{ RemoteClientDelegate, RemoteConnectionModal, RemoteConnectionPrompt, SshConnectionHeader, @@ -131,6 +133,62 @@ 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) = existing { + let remote_connection = existing + .update(cx, |workspace, _, cx| { + workspace + .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 + .update(cx, |workspace, window, cx| { + window.activate_window(); + workspace.open_paths( + resolved_paths, + OpenOptions { + visible: Some(open_visible), + ..Default::default() + }, + None, + window, + cx, + ) + })? + .await; + + _ = existing.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::>(); + navigate_to_positions(&existing, items, &paths_with_positions, cx); + + return Ok(()); + } + let window = if let Some(window) = open_options.replace_window { window } else { @@ -337,29 +395,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::() { - 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); } } @@ -379,6 +415,33 @@ pub async fn open_remote_project( Ok(()) } +pub fn navigate_to_positions( + window: &WindowHandle, + items: impl IntoIterator>>, + 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::() { + 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, mut paths: Vec, diff --git a/crates/remote/src/remote_client.rs b/crates/remote/src/remote_client.rs index 0b4eed025e6c1e47e979af108514485c32aaccae..479cfeae6523b6a45ab0a5157d2573a1326f5fab 100644 --- a/crates/remote/src/remote_client.rs +++ b/crates/remote/src/remote_client.rs @@ -1132,7 +1132,7 @@ impl RemoteClient { .unwrap() } - fn remote_connection(&self) -> Option> { + pub fn remote_connection(&self) -> Option> { self.state .as_ref() .and_then(|state| state.remote_connection()) diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 8dff340e264abd583c471b95d96c90e14486a2c5..1c42d4250d94ea5e728055a6035010df039b00e2 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -1128,7 +1128,7 @@ pub enum Event { ModalOpened, } -#[derive(Debug)] +#[derive(Debug, Clone)] pub enum OpenVisible { All, None, @@ -5872,7 +5872,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 @@ -5944,7 +5944,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) @@ -5959,6 +5959,16 @@ impl Workspace { } } + pub fn serialized_workspace_location(&self, cx: &App) -> Option { + if let Some(connection) = self.project.read(cx).remote_connection_options(cx) { + Some(SerializedWorkspaceLocation::Remote(connection)) + } else if self.project.read(cx).is_local() && self.has_any_items_open(cx) { + Some(SerializedWorkspaceLocation::Local) + } else { + None + } + } + fn update_history(&self, cx: &mut App) { let Some(id) = self.database_id() else { return; @@ -8169,24 +8179,60 @@ fn activate_any_workspace_window(cx: &mut AsyncApp) -> Option Vec> { +pub fn workspace_windows_for_location( + serialized_location: &SerializedWorkspaceLocation, + cx: &App, +) -> Vec> { cx.windows() .into_iter() .filter_map(|window| window.downcast::()) .filter(|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, + }; + workspace .read(cx) - .is_ok_and(|workspace| workspace.project.read(cx).is_local()) + .is_ok_and(|workspace| match workspace.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)] +#[derive(Default, Clone)] pub struct OpenOptions { pub visible: Option, pub focus: Option, pub open_new_workspace: Option, - pub prefer_focused_window: bool, + pub wait: bool, pub replace_window: Option>, pub env: Option>, } @@ -8268,6 +8314,83 @@ pub fn open_workspace_by_id( }) } +pub async fn find_existing_workspace( + abs_paths: &[PathBuf], + open_options: &OpenOptions, + location: &SerializedWorkspaceLocation, + cx: &mut AsyncApp, +) -> (Option>, OpenVisible) { + let mut existing = 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(workspace) = window.read(cx) { + let project = workspace.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); + best_match = m; + } else if best_match.is_none() && open_options.open_new_workspace == Some(false) + { + existing = Some(window) + } + } + } + }); + + let all_paths_are_files = existing + .and_then(|workspace| { + cx.update(|cx| { + workspace + .read(cx) + .map(|workspace| { + let project = workspace.project.read(cx); + let path_style = workspace.path_style(cx); + !abs_paths.iter().any(|path| { + project.worktrees(cx).any(|worktree| { + let worktree = worktree.read(cx); + let abs_path = worktree.abs_path(); + path_style + .strip_prefix(path, abs_path.as_ref()) + .and_then(|rel| worktree.entry_for_path(&rel)) + .is_some_and(|e| e.is_dir()) + }) + }) + }) + .ok() + }) + }) + .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::()) + .filter(|window| windows.contains(window)) + .or_else(|| windows.into_iter().next()); + if let Some(window) = window { + existing = Some(window); + open_visible = OpenVisible::None; + } + }); + } + } + return (existing, open_visible); +} + #[allow(clippy::type_complexity)] pub fn open_paths( abs_paths: &[PathBuf], @@ -8281,16 +8404,17 @@ pub fn open_paths( )>, > { let abs_paths = abs_paths.to_vec(); - let mut existing = 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 @@ -8298,54 +8422,17 @@ pub fn open_paths( .filter_map(|result| result.ok().flatten()) .collect::>(); - cx.update(|cx| { - for window in local_workspace_windows(cx) { - if let Ok(workspace) = window.read(cx) { - let m = workspace.project.read(cx).visibility_for_paths( - &abs_paths, - &all_metadatas, - open_options.open_new_workspace == None, - cx, - ); - if m > best_match { - existing = Some(window); - best_match = m; - } else if best_match.is_none() - && open_options.open_new_workspace == Some(false) - { - existing = Some(window) - } - } - } - }); - - if open_options.open_new_workspace.is_none() - && (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::()) - && let Ok(workspace) = window.read(cx) - { - let project = workspace.project().read(cx); - if project.is_local() && !project.is_via_collab() { - existing = Some(window); - open_visible = OpenVisible::None; - return; - } - } - for window in local_workspace_windows(cx) { - if let Ok(workspace) = window.read(cx) { - let project = workspace.project().read(cx); - if project.is_via_collab() { - continue; - } - existing = Some(window); - open_visible = OpenVisible::None; - break; - } + .filter(|window| windows.contains(window)) + .or_else(|| windows.into_iter().next()); + if let Some(window) = window { + existing = Some(window); + open_visible = OpenVisible::None; } }); } diff --git a/crates/zed/Cargo.toml b/crates/zed/Cargo.toml index 052aba46b67a32c7607c60a049c3eb3eba3f06dd..f7035be8442b0fefb8e74e3c25f3d94d9f184ab4 100644 --- a/crates/zed/Cargo.toml +++ b/crates/zed/Cargo.toml @@ -249,9 +249,9 @@ pretty_assertions.workspace = true project = { workspace = true, features = ["test-support"] } semver.workspace = true terminal_view = { workspace = true, features = ["test-support"] } +title_bar = { workspace = true, features = ["test-support"] } tree-sitter-md.workspace = true tree-sitter-rust.workspace = true -title_bar = { workspace = true, features = ["test-support"] } workspace = { workspace = true, features = ["test-support"] } image.workspace = true agent_ui = { workspace = true, features = ["test-support"] } diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 9a2cea33882b1a9d8434bf0f3e2cb1dba8471007..322b0d2a245894362b501fff1746ea69c26a137c 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -37,7 +37,7 @@ use parking_lot::Mutex; use project::{project_settings::ProjectSettings, trusted_worktrees}; use proto; use recent_projects::{RemoteSettings, open_remote_project}; -use release_channel::{AppCommitSha, AppVersion, ReleaseChannel}; +use release_channel::{AppCommitSha, AppVersion}; use session::{AppSession, Session}; use settings::{BaseKeymap, Settings, SettingsStore, watch_config_file}; use std::{ @@ -322,7 +322,7 @@ fn main() { let (open_listener, mut open_rx) = OpenListener::new(); let failed_single_instance_check = if *zed_env_vars::ZED_STATELESS - || *release_channel::RELEASE_CHANNEL == ReleaseChannel::Dev + // || *release_channel::RELEASE_CHANNEL == ReleaseChannel::Dev { false } else { diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 6e80f13cec5bebe67062aed2a2f722af4269b2e1..242d3eb5943a5032dffc3b021250ab1da3773cff 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -2413,7 +2413,7 @@ mod tests { .fs .as_fake() .insert_tree( - "/root", + path!("/root"), json!({ "a": { "aa": null, @@ -2441,7 +2441,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, @@ -2453,7 +2456,7 @@ mod tests { cx.update(|cx| { open_paths( - &[PathBuf::from("/root/a")], + &[PathBuf::from(path!("/root/a"))], app_state.clone(), workspace::OpenOptions::default(), cx, @@ -2482,7 +2485,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, @@ -2498,7 +2504,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), @@ -2521,7 +2527,7 @@ mod tests { .worktrees(cx) .map(|w| w.read(cx).abs_path()) .collect::>(), - &[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)); @@ -5285,7 +5291,7 @@ mod tests { cx.update(|cx| { let open_options = OpenOptions { - prefer_focused_window: true, + wait: true, ..Default::default() }; diff --git a/crates/zed/src/zed/open_listener.rs b/crates/zed/src/zed/open_listener.rs index 86d35d558dc024931a901c479f26e502de381ca7..985f26b0217b6a4ba7f4436a48c2f4a81f61e0c4 100644 --- a/crates/zed/src/zed/open_listener.rs +++ b/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, Vec>>>, )> { - 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::>(); let (workspace, mut items) = cx @@ -386,25 +372,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::() { - 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::>(); + navigate_to_positions(&workspace, items_for_navigation, path_positions, cx); + Ok((workspace, items)) } @@ -527,64 +503,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(()) } @@ -592,39 +586,20 @@ async fn open_local_workspace( workspace_paths: Vec, diff_paths: Vec<[String; 2]>, diff_all: bool, - open_new_workspace: Option, - reuse: bool, - wait: bool, + open_options: workspace::OpenOptions, responses: &IpcSender, - env: Option<&HashMap>, app_state: &Arc, 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, - env: env.cloned(), - ..Default::default() - }, + open_options.clone(), cx, ) .await @@ -643,10 +618,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 { @@ -669,7 +643,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| { @@ -694,7 +668,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; @@ -725,17 +699,30 @@ pub async fn derive_paths_with_position( fs: &dyn Fs, path_strings: impl IntoIterator>, ) -> Vec { - 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 a 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)] @@ -755,7 +742,7 @@ mod tests { use serde_json::json; use std::{sync::Arc, task::Poll}; use util::path; - use workspace::{AppState, Workspace}; + use workspace::{AppState, Workspace, find_existing_workspace}; #[gpui::test] fn test_parse_ssh_url(cx: &mut TestAppContext) { @@ -957,11 +944,11 @@ mod tests { workspace_paths, vec![], false, - None, - false, - true, + workspace::OpenOptions { + wait: true, + ..Default::default() + }, &response_tx, - None, &app_state, &mut cx, ) @@ -1049,11 +1036,11 @@ mod tests { workspace_paths, vec![], false, - open_new_workspace, - false, - false, + OpenOptions { + open_new_workspace, + ..Default::default() + }, &response_tx, - None, &app_state, &mut cx, ) @@ -1123,11 +1110,8 @@ mod tests { workspace_paths, vec![], false, - None, - false, - false, + workspace::OpenOptions::default(), &response_tx, - None, &app_state, &mut cx, ) @@ -1138,6 +1122,16 @@ mod tests { // Now test the reuse functionality - should replace the existing workspace let workspace_paths_reuse = vec![file1_path.to_string()]; + let paths: Vec = workspace_paths_reuse.iter().map(PathBuf::from).collect(); + let window_to_replace = find_existing_workspace( + &paths, + &workspace::OpenOptions::default(), + &workspace::SerializedWorkspaceLocation::Local, + &mut cx.to_async(), + ) + .await + .0 + .unwrap(); let errored_reuse = cx .spawn({ @@ -1148,11 +1142,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, )