Symlink scan option - #53646
Conversation
Introduces a new `scan_symlinks` setting that controls when Zed scans the contents of symlinked directories. Two options are supported: - `expanded` (default): only scan symlinked directories after the user explicitly expands them in the project panel. - `always`: follow all symbolic links and scan their contents on initial project load. Release Notes: - N/A
cla-bot
Bot
added
the
cla-signed
zed-community-bot
Bot
added
the
first contribution
|
@probably-neb FYI, pinging you since you were involved in the related issues/PRs |
|
I was reading #48890 and I think I may agree that symlinks within the project should be ignored. I explicitly tested for symlinks within the project for this PR (in addition to external symlinks). But it could perhaps be updated to completely disable the inclusion of symlinks within the project. |
|
@eholk or anyone else from Zed team: Is there anything I can do to help move this forward? Being able to open files from symlinked directories is mission critical for me, having to expand trees manually just to be able to "cmd+P" to a file kind of defeats the purpose of "quick open". I really don't want to go back to VSCode, but the lack of ability to configure multiple roots in Zed has really been hurting the entire time I've used Zed. I was hoping symlinks to be a reasonable workaround, but that turned out to be unusable too. I would really appreciate a way to be able to add multiple directories from my fs to a single project that I can jump through/search through easily. I don't much care if it's symlinks or configuration of some sort, just as long as it can be done in some way. |
|
@eholk @smitbarmase +1 here, critical for our team to be able to continue using Zed - Possible to have some eyes on here? |
|
No updates on this? I feel this is such a critical feature that's missing rn. I too don't wanna go back to VS Code tbf. I can't just keep opening the project panel to literally find the file amongst hundreds of folders. |
|
See #46469 for rationale behind a "never" option. |
|
Please merge this, waiting for it forever already |
You are able to add multiple folders to a project already, by right clicking in a blank area in the project panel and choosing "Add Folders to Project..." Does this work for you? I'm looking over this PR now... |
eholk
left a comment
eholk
left a comment
There was a problem hiding this comment.
Thanks for the PR! Overall I think it looks like a nice improvement and directionally good. I was hoping to get this reviewed and merged before I head out for the weekend but unfortunately that didn't happen. Instead, here are some thoughts and comments.
There are a couple inline suggestions.
I think it's fine not to include a Never mode in this PR. It adds a lot of the plumbing needed to add one easily as a follow-up, so if there's enough interest we can do that.
It seems worth doing the work to skip within-project symlinks (or land it as a separate PR since that issue already exists without this PR).
Anyway, I'll finish the review next week. I'm happy to help address some of the suggestions I made too.
Yeah, that seems like the correct order to fix it to me.
I'll hold off on making any changes until then. If you want to just push suggestions feel free |
@eholk That works even better! But where's the project configuration saved? I never use UI to open projects, I just do Symlinks was always a hack because last I tried, Zed didn't have a multi-root project config. I faintly remember that one variant worked, but it expanded all directories on open and I had to manually toggle them every time, and I jump around a lot, so doing that every time is also cumbersome. |
|
@pyronaur - You can also use It might be worth considering a |
|
I've been a little swamped this week but I should have time to get back to this starting tomorrow. |
🙏 Please this! |
# Conflicts: # crates/worktree/src/worktree.rs
Moves should_scan_directory from BackgroundScannerState to BackgroundScanner so it can directly read self.settings.scan_symlinks instead of receiving it as a parameter. This matches how every other settings access in this file flows (e.g. is_path_excluded, is_path_hidden, is_path_private). Also collapses the predicate. The previous version introduced a scan_external helper and a carve-out on the scanned_dirs branch so that scan_symlinks was checked twice. The carve-out was not load-bearing: should_scan_directory is a read-only predicate, so multiple true-producing branches do not compound. Folding the external-Always case into the first clause as one extra conjunct produces a single symmetric `scannable` predicate and shrinks the diff from main.
|
I'm doing a Linux smoke test and assuming that goes well I'll merge this. |
|
Hmm, unfortunately my Linux build doesn't seem to be following symbolic links at all. I think this will need some more investigation. Linux isn't my daily driver, so if someone else wants to dive in here and confirm whether it works for them, that would be super helpful! |
|
I tried as well and running this zed version does not follow symlinks in the project search even when folders are expanded. (There shold not be a never option right?) |
Adds RealFs counterparts to test_scan_symlinks_always and test_scan_symlinks_expanded. The existing tests use FakeFs, which does not model fs::canonicalize against a real filesystem, so any platform-specific differences in canonicalization or readdir behavior slip past those tests. A user reproduced behavior on Linux where the worktree does not appear to follow symlinks at all, even with the symlinked directory manually expanded — i.e. a regression of the fix in zed-industries#51382 that is only observable on real filesystems. These tests are intended to surface that class of regression in CI, gated on cfg(unix) so they run on the Linux runner.
|
@pleibers - Thanks for the confirmation. Out of curiosity, if you run a build off main (or just run the latest Stable/Preview/Nightly release), does it follow symlinks? |
|
After some more investigation, the Linux issue I'm seeing seems independent of this PR, so I think we're ready to merge this one and we can fix the other issues separately. |
Adds option to always search/scan symlinks for more thorough coverage of zed-industries#41887 issue This is a significant rewrite and more thoroughly reviewed and tested version of the old PR here: zed-industries#46344 The "never" option was removed from the old PR, since "expanded" is now the default. Perhaps "never" can be added as an option later if there's demand for it. Adding that option may resolve zed-industries#48890 for instance. 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 is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable (significant impact with "always" option, but "expanded" is default) Release Notes: - Add option choose between including expanded symlinks or include all symlinks in project search (zed-industries#41887) --------- Co-authored-by: Eric Holk <eric@zed.dev>
|
NB: I wrongly expected this |
Adds option to always search/scan symlinks for more thorough coverage of zed-industries#41887 issue This is a significant rewrite and more thoroughly reviewed and tested version of the old PR here: zed-industries#46344 The "never" option was removed from the old PR, since "expanded" is now the default. Perhaps "never" can be added as an option later if there's demand for it. Adding that option may resolve zed-industries#48890 for instance. 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 is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable (significant impact with "always" option, but "expanded" is default) Release Notes: - Add option choose between including expanded symlinks or include all symlinks in project search (zed-industries#41887) --------- Co-authored-by: Eric Holk <eric@zed.dev>
Adds option to always search/scan symlinks for more thorough coverage of zed-industries#41887 issue This is a significant rewrite and more thoroughly reviewed and tested version of the old PR here: zed-industries#46344 The "never" option was removed from the old PR, since "expanded" is now the default. Perhaps "never" can be added as an option later if there's demand for it. Adding that option may resolve zed-industries#48890 for instance. 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 is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable (significant impact with "always" option, but "expanded" is default) Release Notes: - Add option choose between including expanded symlinks or include all symlinks in project search (zed-industries#41887) --------- Co-authored-by: Eric Holk <eric@zed.dev>
8 participants
Adds option to always search/scan symlinks for more thorough coverage of #41887 issue
This is a significant rewrite and more thoroughly reviewed and tested version of the old PR here:
#46344
The "never" option was removed from the old PR, since "expanded" is now the default. Perhaps "never" can be added as an option later if there's demand for it. Adding that option may resolve #48890 for instance.
Self-Review Checklist:
Release Notes: