Deduplicate worktree entries from LSP paths with mismatched casing (#61392)
# Objective On case-insensitive volumes (the macOS default), language servers may return `Location` URIs whose path casing differs from the worktree's stored casing - e.g. `Utils/helpers.py` when the on-disk (and worktree) path is `utils/helpers.py`. `LspStore::open_local_buffer_via_lsp` used the LSP path verbatim, and worktree selection (`WorktreeStore::find_worktree`) does a case-sensitive prefix match, so the existing worktree was either missed (creating a duplicate invisible worktree) or matched but with a relative path that retained the LSP's intermediate-component casing. That relative path was then used to `load_file`, which inserted a brand-new `Entry` keyed by the differently-cased path alongside the existing one - producing duplicate file entries in the project panel. This was most visible when navigating Python imports (Go to Definition) where the LSP returned differently-cased paths. https://github.com/user-attachments/assets/e36930b5-8dfd-4b43-8fe4-5317d401e198 ## Solution Canonicalize the LSP-provided absolute path via `fs.canonicalize` before the worktree lookup in `LspStore::open_local_buffer_via_lsp`, so the path casing matches the filesystem and the existing worktree/entry is reused. Canonicalization failures (e.g. a path that doesn't exist on disk yet) fall back to the original path to preserve prior behavior. To enable testing this on a fake filesystem, `FakeFs` now supports `set_case_sensitive(false)` and its `canonicalize` resolves names case-insensitively, returning the stored (canonical) casing. ## Testing Added `test_open_buffer_via_lsp_case_variant_no_duplicate` in `crates/project/tests/integration/lsp_store.rs`. It opens a buffer via an LSP URI with differently-cased intermediate component (`/root/SRC/main.rs` vs `/root/src/main.rs`) on a case-insensitive FakeFs and asserts that no differently-cased entry is created and the canonical entry is preserved. Verified the test fails without the fix (`SRC/main.rs` duplicate appears) and passes with it. Existing worktree, fs, and project_panel test suites remain green. ## Self-Review Checklist: - [X] I've reviewed my own diff for quality, security, and reliability - [X] Unsafe blocks (if any) have justifying comments - [X] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [X] Tests cover the new/changed behavior - [X] Performance impact has been considered and is acceptable ## Showcase https://github.com/user-attachments/assets/f052cf10-8b45-4547-8425-00ce4050ee7c --- Release Notes: - Fixed duplicate file entries in the project panel on macOS when navigating to definitions via the language server returned paths with different casing than the worktree root. --------- Co-authored-by: Christopher Biscardi <chris@christopherbiscardi.com>
This commit is contained in:
1 parent
dbdcb310d1
commit
3ea4d186a1
3 files changed
+93
-3
No files matched your search
+23
-3
@@ -1419,6 +1419,7 @@ struct FakeFsState {
|
||||
trash: Mutex<SlotMap<TrashId, (TrashedEntry, FakeFsEntry)>>,
|
||||
file_to_create_before_watch_add: Option<(PathBuf, PathBuf)>,
|
||||
remove_dir_errors: std::collections::HashMap<PathBuf, String>,
|
||||
case_sensitive: bool,
|
||||
}
|
||||
|
||||
#[cfg(feature = "test-support")]
|
||||
@@ -1582,7 +1583,20 @@ impl FakeFsState {
|
||||
Component::Normal(name) => {
|
||||
let current_entry = *entry_stack.last()?;
|
||||
if let FakeFsEntry::Dir { entries, .. } = current_entry {
|
||||
let entry = entries.get(name.to_str().unwrap())?;
|
||||
let name_str = name.to_str().unwrap();
|
||||
let (canonical_name, entry) = match entries.get(name_str) {
|
||||
Some(entry) => (name_str, entry),
|
||||
None => {
|
||||
if !self.case_sensitive {
|
||||
entries
|
||||
.iter()
|
||||
.find(|(key, _)| key.eq_ignore_ascii_case(name_str))
|
||||
.map(|(key, entry)| (key.as_str(), entry))?
|
||||
} else {
|
||||
return None;
|
||||
}
|
||||
}
|
||||
};
|
||||
if (path_components.peek().is_some() || follow_symlink)
|
||||
&& let FakeFsEntry::Symlink { target, .. } = entry
|
||||
{
|
||||
@@ -1592,7 +1606,7 @@ impl FakeFsState {
|
||||
continue 'outer;
|
||||
}
|
||||
entry_stack.push(entry);
|
||||
canonical_path = canonical_path.join(name);
|
||||
canonical_path = canonical_path.join(canonical_name);
|
||||
} else {
|
||||
return None;
|
||||
}
|
||||
@@ -1739,6 +1753,7 @@ impl FakeFs {
|
||||
trash: Mutex::new(SlotMap::with_key()),
|
||||
file_to_create_before_watch_add: None,
|
||||
remove_dir_errors: Default::default(),
|
||||
case_sensitive: true,
|
||||
})),
|
||||
});
|
||||
|
||||
@@ -1758,6 +1773,11 @@ impl FakeFs {
|
||||
this
|
||||
}
|
||||
|
||||
/// Configures whether the fake filesystem reports as case-sensitive.
|
||||
pub fn set_case_sensitive(&self, case_sensitive: bool) {
|
||||
self.state.lock().case_sensitive = case_sensitive;
|
||||
}
|
||||
|
||||
pub fn set_next_mtime(&self, next_mtime: SystemTime) {
|
||||
let mut state = self.state.lock();
|
||||
state.next_mtime = next_mtime;
|
||||
@@ -3332,7 +3352,7 @@ impl Fs for FakeFs {
|
||||
}
|
||||
|
||||
async fn is_case_sensitive(&self) -> bool {
|
||||
true
|
||||
self.state.lock().case_sensitive
|
||||
}
|
||||
|
||||
fn subscribe_to_jobs(&self) -> JobEventReceiver {
|
||||
|
||||
@@ -9409,6 +9409,15 @@ impl LspStore {
|
||||
let abs_path = abs_path
|
||||
.to_file_path_ext(path_style)
|
||||
.map_err(|()| anyhow!("can't convert URI to path"))?;
|
||||
|
||||
let fs = lsp_store.update(cx, |lsp_store, _cx| {
|
||||
lsp_store.as_local().map(|local| local.fs.clone())
|
||||
})?;
|
||||
let abs_path = if let Some(fs) = fs {
|
||||
fs.canonicalize(&abs_path).await.unwrap_or(abs_path)
|
||||
} else {
|
||||
abs_path
|
||||
};
|
||||
let p = abs_path.clone();
|
||||
let yarn_worktree = lsp_store
|
||||
.update(cx, move |lsp_store, cx| match lsp_store.as_local() {
|
||||
|
||||
@@ -102,6 +102,67 @@ async fn test_removing_invisible_worktree_cleans_reused_lsp_bookkeeping(cx: &mut
|
||||
});
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_open_buffer_via_lsp_case_variant_no_duplicate(cx: &mut TestAppContext) {
|
||||
init_test(cx);
|
||||
cx.executor().allow_parking();
|
||||
|
||||
let fs = FakeFs::new(cx.executor());
|
||||
fs.set_case_sensitive(false);
|
||||
fs.insert_tree(
|
||||
path!("/root"),
|
||||
json!({ "src": { "main.rs": "fn main() {}" } }),
|
||||
)
|
||||
.await;
|
||||
|
||||
let project = Project::test(fs.clone(), [path!("/root").as_ref()], cx).await;
|
||||
let language_registry = project.read_with(cx, |project, _| project.languages().clone());
|
||||
language_registry.add(rust_lang());
|
||||
let mut fake_servers = language_registry.register_fake_lsp("Rust", FakeLspAdapter::default());
|
||||
|
||||
project
|
||||
.update(cx, |project, cx| {
|
||||
project.open_local_buffer_with_lsp(path!("/root/src/main.rs"), cx)
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
fake_servers.next().await.unwrap();
|
||||
cx.run_until_parked();
|
||||
|
||||
let server_id = project.read_with(cx, |project, cx| {
|
||||
project
|
||||
.lsp_store()
|
||||
.read(cx)
|
||||
.language_server_statuses()
|
||||
.next()
|
||||
.unwrap()
|
||||
.0
|
||||
});
|
||||
|
||||
project
|
||||
.update(cx, |project, cx| {
|
||||
project.open_local_buffer_via_lsp(
|
||||
Uri::from_file_path(path!("/root/SRC/main.rs")).unwrap(),
|
||||
server_id,
|
||||
cx,
|
||||
)
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
cx.run_until_parked();
|
||||
|
||||
project.read_with(cx, |project, cx| {
|
||||
let worktree = project.worktrees(cx).next().unwrap();
|
||||
let entries: Vec<_> = worktree
|
||||
.read(cx)
|
||||
.snapshot()
|
||||
.entries(true, 0)
|
||||
.map(|entry| entry.path.as_unix_str().to_string())
|
||||
.collect();
|
||||
assert_eq!(entries, vec!["", "src", "src/main.rs"]);
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_rpc_log_grouping_separates_timed_messages() {
|
||||
for (received, direction) in [(false, "Send"), (true, "Receive")] {
|
||||
|
||||
Reference in new issue
Block a user