feature: teach rust-analyzer to discover linked_projects
#17246
Conversation
rustbot
added
the
S-waiting-on-review
davidbarsky
force-pushed
the
david/move-rust-project-generation-to-server
branch
4 times, most recently
from
378ef57
to
e62932f
Compare
| if self.config.linked_or_discovered_projects() != old_config.linked_or_discovered_projects() | ||
| { | ||
| self.fetch_workspaces_queue.request_op("discovered projects changed".to_owned(), false) | ||
| self.fetch_workspaces_queue.request_op("discovered projects changed".to_owned(), true) |
There was a problem hiding this comment.
Seeing this we should probably make this an enum (I'm at fault for introducing a bool here and being lazy)
There was a problem hiding this comment.
I can change this to an enum, since I'm doing a bunch of surgery here anyways: would something like (bad namesincoming!) ReloadBehavior { Force, Done } be a decent starting point?
| self.show_message( | ||
| lsp_types::MessageType::ERROR, | ||
| message, | ||
| tracing::enabled!(tracing::Level::ERROR), | ||
| ); | ||
| } | ||
| None => { | ||
| tracing::error!("{}", &message); | ||
| self.send_notification::<lsp_types::notification::ShowMessage>( | ||
| lsp_types::ShowMessageParams { typ: lsp_types::MessageType::ERROR, message }, | ||
| ); | ||
| } | ||
| } | ||
| self.show_message( | ||
| lsp_types::MessageType::ERROR, | ||
| message, | ||
| tracing::enabled!(tracing::Level::ERROR), | ||
| ); |
There was a problem hiding this comment.
This unnecessarily opens the log button notification now when there is no additional info in the logs
There was a problem hiding this comment.
where should error messages from flycheck be printed? flycheck eagerly gets that, but printing with additional information interrupts the error message.
that's probably a sign that the the stuff in flycheck isn't sufficient here, huh
There was a problem hiding this comment.
I don't think I understand, why is flycheck relevant here? This is just for logging errors, if the error has no extra information we show all the info in the popup, if it has additional info we log that and tell the user to check the logs by giving the pop up a button. I'm a bit confused as to why you need to make changes here.
There was a problem hiding this comment.
right, sorry: it's the interaction of several different features.
tracing::enabled!(tracing::Level::ERROR)is almost always true (since it's the default level fortracing-subscriber)CargoActorinflycheckwill be printing out the lines it couldn't parse as JSON (which will often contain the error itself...), but that's interleaved with theadditional_info.
If I'm not make this change, the end result is the following (where the additional_info is "unable to do the things"):
Screenshot of mangled logs
that's probably a sign that the the stuff in flycheck isn't sufficient here, huh
This is what I meant when I said "that's probably a sign that the the stuff in flycheck isn't sufficient here, huh". However, having thought about it, it's not that flycheck is incorrect! It's that the CLI that i'm calling here, rust-project, that's wrong. It should communicate the underlying error in a more structured matter.
I'll hack on something today and propose something in this PR, which would let me undo the change here.
The flycheck crate is a mess now (at least name wise) anyways as it also does stuff for the test explorer, so that doesn't really make things a lot worse now... |
davidbarsky
force-pushed
the
david/move-rust-project-generation-to-server
branch
2 times, most recently
from
ace6d7d
to
1003808
Compare
|
☔ The latest upstream changes (presumably #17287) made this pull request unmergeable. Please resolve the merge conflicts. |
davidbarsky
force-pushed
the
david/move-rust-project-generation-to-server
branch
from
1003808
to
57fa458
Compare
|
☔ The latest upstream changes (presumably #17275) made this pull request unmergeable. Please resolve the merge conflicts. |
Veykril
added
S-waiting-on-author
davidbarsky
force-pushed
the
david/move-rust-project-generation-to-server
branch
from
57fa458
to
1404d02
Compare
davidbarsky
force-pushed
the
david/move-rust-project-generation-to-server
branch
from
1404d02
to
ce2c55c
Compare
|
☔ The latest upstream changes (presumably #17278) made this pull request unmergeable. Please resolve the merge conflicts. |
davidbarsky
force-pushed
the
david/move-rust-project-generation-to-server
branch
from
ce2c55c
to
be92e74
Compare
|
For "reload rust-analyzer reasons when a |
Summary: To support rust-lang/rust-analyzer#17246, I've added an argument for logging using JSON to that rust-analyzer can use these fields for progress reporting. Reviewed By: Wilfred Differential Revision: D57924551 fbshipit-source-id: 992e9e096fdea1eefe2b199d8dd84be7b247a732
|
☔ The latest upstream changes (presumably #17346) made this pull request unmergeable. Please resolve the merge conflicts. |
This PR's been a long-time coming, but like the title says, it introduces server-side project discovery and removes the extension hooks I previously introduced. I don't think this PR is ready to land, but here are the things I'm feeling squishy about:
cargo-metadatacommand-but-for-everything-else in theflycheckmodule, but the progress reporting infrastructure was too convenient to pass up. Happy to move it elsewhere.Here are the things I know I need to change:
serde_json::Valuethat corresponds totracing_subsciber::fmt::Layer's JSON output. I'd like to make this a bit more structured/documented than the current nonsense I wrote.Anyway, here's a video of rust-analyzer discovering a Buck target.
rust-project.mov