Restore lsp-rust-analyzer-cargo-all-targets #3974
Conversation
This partially reverts emacs-lsp#3893. The rust-analyzer.checkOnSave.allTargets configuration still exists, although it is internally renamed by rust-analyzer to rust-analyzer.check.allTargets which is the new version. Maybe the intent was to remove lsp-rust-all-targets which is indeed unused by lsp-rust.
clients/lsp-rust.el
Outdated
| "Cargo watch all targets or not." | ||
| :type 'boolean | ||
| :group 'lsp-rust-analyzer | ||
| :package-version '(lsp-mode . "6.2.2")) |
There was a problem hiding this comment.
Even if it's a revert? The flag only disappeared for a month or so.
If it counts as a new flag then I'd like to change the name to lsp-rust-analyzer-check-all-targets to match the new naming of rust-analyzer.
There was a problem hiding this comment.
If it counts as a new flag then I'd like to change the name to lsp-rust-analyzer-check-all-targets to match the new naming of rust-analyzer.
Probably better.
I think yes, but it's possible that they changed the path and I didn't realize it https://github.com/rust-lang/rust-analyzer/blame/87d57f51bcf07eb364fcf835f9be987006158961/editors/code/package.json#L705 And then they changed it again rust-lang/rust-analyzer@d2bb62b#diff-00784dde03b1be151004f435a4ca74754cb674a9e789840e12760d8f3ac1728fR564 I currently don't use rust so this is really annoying. Hopefully they stop changing the paths all the time. |
| @@ -747,6 +753,7 @@ or JSON objects in `rust-project.json` format." | |||
| :checkOnSave (:enable ,(lsp-json-bool lsp-rust-analyzer-cargo-watch-enable) | |||
There was a problem hiding this comment.
although it is internally renamed by rust-analyzer to rust-analyzer.check.allTargets which is the new version
So we also have to replace checkonsave with check right ?
There was a problem hiding this comment.
But keep this line for rust-analyzer.checkOnSave
There was a problem hiding this comment.
We don't need to rename checkOnSave (just yet), they do the renaming internally currently (for backward compatibility). However, eventually we probably want to go over all flags and try to follow the newest convention.
There was a problem hiding this comment.
I see, so it's probably the best solution to wait until they are happy and stop changing it =)
Yes, I didn't follow all the reasoning behind those renamings, but I know it generated quite some discussions on their side. I might get some time starting from April to try to go over all flags in lsp-rust if that would be something useful to do. |
ia0
force-pushed
the
restore-check-all-targets
branch
from
fb54638 to
f8fc437
Compare
Sure, thanks! rls was also deprecated a few months ago so it probably makes sense to remove the rls related code completely ? |
I think we can move rls in lsp-rls.el and not load it by default. |
This partially reverts #3893.
The rust-analyzer.checkOnSave.allTargets configuration still exists, although it is internally renamed by rust-analyzer to rust-analyzer.check.allTargets which is the new version. Maybe the intent was to remove lsp-rust-all-targets which is indeed unused by lsp-rust.