Skip to content

Commit

Permalink
project: Fix issue where Cmd+Click on an import opens the wrong file (#…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
smitbarmase authored Mar 5, 2025
1 parent 89d89b8 commit 387ee46
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 36 deletions.
81 changes: 55 additions & 26 deletions crates/project/src/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3462,41 +3462,70 @@ impl Project {
}
}

let worktrees = self.worktrees(cx).collect::<Vec<_>>();
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);
}
}
}
None
})
}

fn resolve_path_in_worktree(
worktree: &Entity<Worktree>,
path: &PathBuf,
cx: &mut AsyncApp,
) -> Option<ResolvedPath> {
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,
Expand Down
36 changes: 26 additions & 10 deletions crates/remote_server/src/remote_editing_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!({
Expand All @@ -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());
}
Expand Down

0 comments on commit 387ee46

Please sign in to comment.