From 387ee46c466f6e2c736e35fb823fea1e5e4c7ea5 Mon Sep 17 00:00:00 2001 From: smit <0xtimsb@gmail.com> Date: Wed, 5 Mar 2025 16:49:39 +0530 Subject: [PATCH] project: Fix issue where Cmd+Click on an import opens the wrong file (#26120) Closes #21974 `resolve_path_in_worktrees` function looks for provided path in each worktree until valid file is found. In this PR we priortize current buffer worktree before other worktrees, because of edge case where, file with same name might exists in other worktrees. Updated tests to handle this case. Release Notes: - Fixed an issue where the wrong file from a different worktree would open when using `Cmd + Click` on a file import. --- crates/project/src/project.rs | 81 +++++++++++++------ .../remote_server/src/remote_editing_tests.rs | 36 ++++++--- 2 files changed, 81 insertions(+), 36 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 2c61bdee4a415062c34305b5546c3e57c91c0f89..4be3fb5349c118d25ef4c4102f2df0b25d2b0f26 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -3462,34 +3462,39 @@ impl Project { } } - let worktrees = self.worktrees(cx).collect::>(); + let buffer_worktree_id = buffer.read(cx).file().map(|file| file.worktree_id(cx)); + let worktrees_with_ids: Vec<_> = self + .worktrees(cx) + .map(|worktree| { + let id = worktree.read(cx).id(); + (worktree, id) + }) + .collect(); + cx.spawn(|_, mut cx| async move { - for worktree in worktrees { + if let Some(buffer_worktree_id) = buffer_worktree_id { + if let Some((worktree, _)) = worktrees_with_ids + .iter() + .find(|(_, id)| *id == buffer_worktree_id) + { + for candidate in candidates.iter() { + if let Some(path) = + Self::resolve_path_in_worktree(&worktree, candidate, &mut cx) + { + return Some(path); + } + } + } + } + for (worktree, id) in worktrees_with_ids { + if Some(id) == buffer_worktree_id { + continue; + } for candidate in candidates.iter() { - let path = worktree - .update(&mut cx, |worktree, _| { - let root_entry_path = &worktree.root_entry()?.path; - - let resolved = resolve_path(root_entry_path, candidate); - - let stripped = - resolved.strip_prefix(root_entry_path).unwrap_or(&resolved); - - worktree.entry_for_path(stripped).map(|entry| { - let project_path = ProjectPath { - worktree_id: worktree.id(), - path: entry.path.clone(), - }; - ResolvedPath::ProjectPath { - project_path, - is_dir: entry.is_dir(), - } - }) - }) - .ok()?; - - if path.is_some() { - return path; + if let Some(path) = + Self::resolve_path_in_worktree(&worktree, candidate, &mut cx) + { + return Some(path); } } } @@ -3497,6 +3502,30 @@ impl Project { }) } + fn resolve_path_in_worktree( + worktree: &Entity, + path: &PathBuf, + cx: &mut AsyncApp, + ) -> Option { + worktree + .update(cx, |worktree, _| { + let root_entry_path = &worktree.root_entry()?.path; + let resolved = resolve_path(root_entry_path, path); + let stripped = resolved.strip_prefix(root_entry_path).unwrap_or(&resolved); + worktree.entry_for_path(stripped).map(|entry| { + let project_path = ProjectPath { + worktree_id: worktree.id(), + path: entry.path.clone(), + }; + ResolvedPath::ProjectPath { + project_path, + is_dir: entry.is_dir(), + } + }) + }) + .ok()? + } + pub fn list_directory( &self, query: String, diff --git a/crates/remote_server/src/remote_editing_tests.rs b/crates/remote_server/src/remote_editing_tests.rs index 7cc6cea1dffdfcc8cb923997f719a6e9c5f73650..08b56e6126b2f910ea04fdf78d60521706eec69c 100644 --- a/crates/remote_server/src/remote_editing_tests.rs +++ b/crates/remote_server/src/remote_editing_tests.rs @@ -787,6 +787,7 @@ async fn test_remote_resolve_path_in_buffer( server_cx: &mut TestAppContext, ) { let fs = FakeFs::new(server_cx.executor()); + // Even though we are not testing anything from project1, it is necessary to test if project2 is picking up correct worktree fs.insert_tree( path!("/code"), json!({ @@ -797,60 +798,75 @@ async fn test_remote_resolve_path_in_buffer( "lib.rs": "fn one() -> usize { 1 }" } }, + "project2": { + ".git": {}, + "README.md": "# project 2", + "src": { + "lib.rs": "fn two() -> usize { 2 }" + } + } }), ) .await; let (project, _headless) = init_test(&fs, cx, server_cx).await; - let (worktree, _) = project + + let _ = project .update(cx, |project, cx| { project.find_or_create_worktree(path!("/code/project1"), true, cx) }) .await .unwrap(); - let worktree_id = cx.update(|cx| worktree.read(cx).id()); + let (worktree2, _) = project + .update(cx, |project, cx| { + project.find_or_create_worktree(path!("/code/project2"), true, cx) + }) + .await + .unwrap(); - let buffer = project + let worktree2_id = cx.update(|cx| worktree2.read(cx).id()); + + let buffer2 = project .update(cx, |project, cx| { - project.open_buffer((worktree_id, Path::new("src/lib.rs")), cx) + project.open_buffer((worktree2_id, Path::new("src/lib.rs")), cx) }) .await .unwrap(); let path = project .update(cx, |project, cx| { - project.resolve_path_in_buffer(path!("/code/project1/README.md"), &buffer, cx) + project.resolve_path_in_buffer(path!("/code/project2/README.md"), &buffer2, cx) }) .await .unwrap(); assert!(path.is_file()); assert_eq!( path.abs_path().unwrap().to_string_lossy(), - path!("/code/project1/README.md") + path!("/code/project2/README.md") ); let path = project .update(cx, |project, cx| { - project.resolve_path_in_buffer("../README.md", &buffer, cx) + project.resolve_path_in_buffer("../README.md", &buffer2, cx) }) .await .unwrap(); assert!(path.is_file()); assert_eq!( path.project_path().unwrap().clone(), - ProjectPath::from((worktree_id, "README.md")) + ProjectPath::from((worktree2_id, "README.md")) ); let path = project .update(cx, |project, cx| { - project.resolve_path_in_buffer("../src", &buffer, cx) + project.resolve_path_in_buffer("../src", &buffer2, cx) }) .await .unwrap(); assert_eq!( path.project_path().unwrap().clone(), - ProjectPath::from((worktree_id, "src")) + ProjectPath::from((worktree2_id, "src")) ); assert!(path.is_dir()); }