diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 0c70ff983a55b713cdda618f19a2a966205ac42c..9e37802213dfb8df5cf63af5648044ae8ec65ecb 100644 --- a/crates/project/src/project.rs +++ b/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 { 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 { 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 09ba8200e06198f7bb2ce8984e423449dccaf711..137b4d1da3a7785b5c28a87a84facefef521cba2 100644 --- a/crates/recent_projects/src/recent_projects.rs +++ b/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}; diff --git a/crates/recent_projects/src/remote_connections.rs b/crates/recent_projects/src/remote_connections.rs index 90bce513500eadf77e198cc4745fb0c40dda88d6..fa193a22d99a71981d085dec8e7334744d7e7d65 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,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::>(); + 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::() { - 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, + 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, @@ -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::().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::().unwrap()); + assert_eq!( + still_first_window, first_window, + "The window handle should be the same after reuse" + ); + } + fn init_test(cx: &mut TestAppContext) -> Arc { cx.update(|cx| { let state = AppState::test(cx); 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/util/src/paths.rs b/crates/util/src/paths.rs index 2e014413eff563eda7d80e1e8762a8da97eb4e7a..4f018e6d19237441125b6e35a1f4a76497437b3d 100644 --- a/crates/util/src/paths.rs +++ b/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() diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index d0cd04b0b08f814352b6d0e0dbed4975e7dfcfee..09242c49b24ea048b7fca2f1e48e28651af4b345 100644 --- a/crates/workspace/src/workspace.rs +++ b/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 Vec> { + workspace_windows_for_location(&SerializedWorkspaceLocation::Local, cx) +} + +pub fn workspace_windows_for_location( + serialized_location: &SerializedWorkspaceLocation, + cx: &App, +) -> Vec> { cx.windows() .into_iter() .filter_map(|window| window.downcast::()) .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, Entity)>, + OpenVisible, +) { + let mut existing: Option<(WindowHandle, Entity)> = 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::()) + .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, pub focus: Option, pub open_new_workspace: Option, - pub prefer_focused_window: bool, + pub wait: bool, pub replace_window: Option>, pub env: Option>, } @@ -8458,16 +8581,23 @@ pub fn open_paths( )>, > { let abs_paths = abs_paths.to_vec(); - let mut existing: Option<(WindowHandle, Entity)> = 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::>(); - 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::()) - && 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; - } } } }); diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 124904fec059005a961fe962ab81cf01f8e4d482..24f91e1d953d99c7c1e53c3ea76a9551ad0def7e 100644 --- a/crates/zed/src/zed.rs +++ b/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::>(), - &[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() }; diff --git a/crates/zed/src/zed/open_listener.rs b/crates/zed/src/zed/open_listener.rs index 293ba9059be565995332c55596050f1c0c9f447d..94e125ab98c44282c037704158c48e69d7b3a785 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 (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::() { - 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::>(); + 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, 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 || 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>, ) -> 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 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 = 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, )