-
Notifications
You must be signed in to change notification settings - Fork 257
Adding a dependency to Cargo.toml require RLS + IDE restart #1240
Comments
Rls should do a full cargo rebuild on manifest changes, is it possible your client isn't telling Rls? |
Why should my client tell RLS? RLS alreay knows I'm changing EDIT: I've just checked and in fact RLS doesn't seem to be aware of my The role of the RLS is so that that the client doesn't have to worry about file changes. |
We can try to associate RLS with the Cargo.toml files so it gets notified of updates on that file (and could even provide some edition assistance for it). |
It's well worth a shot. No idea how to do it though. |
If the RLS already does this then this issue should be moved to Corrosion. @mickaelistria WDYT? |
That isn't true, LSP relies on clients watching for file changes and notifying the language server. Sending workspace/didChangeWatchedFiles for all Cargo.toml & Cargo.lock files (plus ./target directory deletion, but not ./target contents) will allow Rls to trigger a full cargo build on changes to these. |
Does RLS somehow declare automatically which files it's supposed to watch for best results, or do we have to hardcode it in our integration? IMHO best behavior should be default and shouldn't require extra-effort. |
Am I misunderstanding how this works? Is this another feature that has to be implemented in each and every client?
Out of curiosity, what's the rationale behind RLS not actually doing this (in fact it does look like it's not recommended although the reasons for the recommendation on the link above are very unconvincing) @mickaelistria putting things together, it does look like the file monitoring should be done by LSP4E. I have opened an issue in the Eclipse bug reporter. https://bugs.eclipse.org/bugs/show_bug.cgi?id=543627 On a side note, is there any chance that we can have LSP4E issue reporting on GitHub? The Eclipse one is very clunky and cross-referencing all the other GitHub project is very inconvenient. |
It may work if we assume all file changes are done within the Corrosion "registered" editors which is not true but it's what most developers would do anyway. It's possibly a useful stepping stone towards "correct" implementation. Again, I wonder why this is left to each and every client while this clearly is an option that should be implemented in the server as a default - with the option of disabling on in the server if the client can provide a more efficient implementation. |
I have a local version of Eclipse which now detects changes in
|
Accidentally closed |
@alexheretic Here's what I see after adding the RLS appears to be ignoring the change.
|
Thanks for the experiment @norru . |
Yep I think it makes sense to support textDocument/didChange in that way, seems doable to me. |
There is an issue that didChange notifications are usually pre-save. A cargo rebuild can only work post-save afaik, as cargo/rustc reads from disk. So we'd have to handle that somehow. Maybe the lsp messaging is already clear enough to figure that out server side, I'm not sure. |
Using |
Yes, |
Support textDocument/didSave manifest cargo rebuilds * Support cargo rebuilding when client does not send _workspace/didChangeWatchedFiles_ notifications. * Update clippy `1838bfe5a9ff951ffd716e4632156113d95d14a4` * Update cargo `907c0febe7045fa02dff2a35c5e36d3bd59ea50d` Should fix #1240
I'll hook up the
|
Signed-off-by: Nico Orru <nigu.orru@gmail.com>
Signed-off-by: Nico Orru <nigu.orru@gmail.com>
Not fixed as for |
Not fixed as
|
So we're correctly issuing the |
Adding a dependency to
Cargo.toml
doesn't trigger a rebuild. As a result, any reference to the added dependency results in a compilation error.For
main.rs:
RLS log:
Workaround: restarting the RLS (and the IDE) triggers a rebuild and the missing crate is linked correctly.
The text was updated successfully, but these errors were encountered: