Skip to content
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

Extend VFS to allow dynamic addition of source roots #293

Closed
matklad opened this issue Dec 19, 2018 · 4 comments
Closed

Extend VFS to allow dynamic addition of source roots #293

matklad opened this issue Dec 19, 2018 · 4 comments
Labels
E-hard E-has-instructions Issue has some instructions and pointers to code to get started fun A technically challenging issue with high impact

Comments

@matklad
Copy link
Member

matklad commented Dec 19, 2018

Currently, VFS requires the set of roots in its new. Ideally, we should allow to dynamically add source roots.

This is tricky, however: an added root can be withing some previously existing root, so we'll need to move files from the old root to the new one. (so, both Remove & Add files in the pending_changes).

Another thing to keep in mind is that we can add a new, nested root when the old root is being scanned! So, we need to account for that as well.

The relevant code is here: https://github.com/rust-analyzer/rust-analyzer/blob/1491fafddbe16bb5851c7ad8f82011e224e469ed/crates/ra_vfs/src/lib.rs#L119

@matklad matklad added E-hard fun A technically challenging issue with high impact E-has-instructions Issue has some instructions and pointers to code to get started labels Dec 29, 2018
@pasa
Copy link
Contributor

pasa commented May 7, 2019

I would like to do this. As I understand vfs was moved to the separate repo. Do you have some best practice manage with multiple repo in this project?

@matklad
Copy link
Member Author

matklad commented May 7, 2019

Vfs is indeed in a separate repo: https://github.com/rust-analyzer/ra_vfs

We publish it to crates.io and use a usual cargo dependency.

For hackig, it's a good idea to add something like

ra_vfs = { path = "../ra_vfs" }

to the [patch] section in the root Cargo.toml.

I am not actually sure it is a best issue to work on right now: there's hope that the new notify (file watching crate we use) will implement an API which will make VFS implementation much more straitforward. If that happens, we'll perhaps will need to re-think how code is split between vfs and rust-analyzer.

I'd even say that the best way to move this issue forward is to hack on notify to add 175.

@pasa
Copy link
Contributor

pasa commented May 7, 2019

Ok. It is pity, but seems that all another e-mentor issues are taken. Will try to find something I can cope with.

@matklad
Copy link
Member Author

matklad commented Jan 17, 2020

I think is obsoleted, we should re-design VFS API based on https://docs.rs/watchman_client/0.4.0/watchman_client/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-hard E-has-instructions Issue has some instructions and pointers to code to get started fun A technically challenging issue with high impact
Projects
None yet
Development

No branches or pull requests

2 participants