rust-lang / rust-analyzer

A Rust compiler front-end for IDEs
https://rust-analyzer.github.io/
Apache License 2.0
14.06k stars 1.56k forks source link

Custom logging implementation could use VS Code's LSP APIs #13682

Open MariaSolOs opened 1 year ago

MariaSolOs commented 1 year ago

Not really an issue, but just a refactoring suggestion.

I noticed that this repo has a custom logging implementation, but VS Code has a basically identical functionality already implemented here. There's no need to create extra output channels, since VS Code automatically creates one using the client's name.

An example where these LSP client methods are used can be found in the vscode-eslint extension. The output looks as follows:

image

If this sounds like a reasonable suggestion, I would gladly work on the refactoring :)

Veykril commented 1 year ago

We aren't using that logging infrastructure for the client logging as we want to have two separate logging channels, one for client and one for server output. Using that logging infra will log to the server channel as that is what is used by the language client settings.

MariaSolOs commented 1 year ago

@Veykril The extension currently has 3 output channels though. As a user, I find it confusing for a single extension to have all of this logging.

image

IMO it would make sense to just name the LSP client "Rust Analyzer Client" (or use this setting to name the channel as needed) and use the built-in logging methods. There could be a small wrapper function to use user settings before logging.

However this is just a suggestion. If you're satisfied with the current code then that's okay :)

Veykril commented 1 year ago

The trace one we should ideally only spawn if the relevant config is set. The client and server logs do have a reason to be split though, the server logs are the actual logs sent by the server, the client one has the non-lsp client logs. I prefer having the split personally, as I have the server logs set up rather spammy myself (though the client logs don't really log much in general atm, since well the client doesn't do too much). Likewise, we could probably make the client logs lazy as well though, only creating the output channel for it on demand.

Veykril commented 1 year ago

Closing as this is intentional

lnicola commented 1 year ago

Having a single channel might not be so bad, the client usually logs stuff only when it loads. Having two of them does seem to be confusing for some users who only look at one of the channels and not the other. And since most extensions are written in JavaScript, the difference between the client and the server might not be obvious.

joshka commented 1 month ago

VSCode style logs for the client is addressed in https://github.com/rust-lang/rust-analyzer/pull/17722

On the single logs part, I'd suggest considering whether prefixing log lines with the source would reasonably solve this. Perhaps add a developer config option that triggers the multiple panes as you find this to be useful for plugin development, but make the general user level combine them.

When you change from an output channel to a log output channel, VSCode enables configuration of the log level directly on the output pane (and command / cli options). This could be useful instead of a separate server trace pane.

image