-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Make the Yarn SDK path relative to the worktree #40062
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
We require contributors to sign our Contributor License Agreement, and we don't have @arcanis on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'. |
We require contributors to sign our Contributor License Agreement, and we don't have @arcanis on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'. |
@cla-bot check |
The cla-bot has been summoned, and re-checked this pull request! |
Huh, |
My understanding is that it only does so for files that aren't part of an ignore list (gitignore?), otherwise it fails with "no worktree entry". In our case we want to check unconditionally, since it's conceivable the user would have gitignored the SDK files (in my case it was by mistake as I had incorrect |
Hmm, zed/crates/project/src/lsp_store.rs Line 12791 in 92e765b
include_ignored: true to traverse a worktree.. zed/crates/worktree/src/worktree.rs Line 2374 in 92e765b
I don't see why it would ignore these entries. Don't get me wrong, I appreciate you filing a PR, but I need to do some further digging myself before I'm able to review this (even though the code looks good at glance). |
I might be wrong, but I suspect it's because |
Let's say you run this:
The
zed
process will execute withcurrent_dir() = ~/proj-a
, but aworktree_root_path() = ~/proj-b
. The old detection was then checking if the Yarn SDK was installed inproj-a
to decide whether to set the tsdk value or not. This was incorrect, as we should instead check for the SDK presence insideproj-b
.Release Notes: