Improve LSP diagnostic info #3436
Conversation
This removes the extraneous information from the diagnostic report (filename, line, column), which is already included in the range. It also adds a hard-coded `buf` as the source of the diagnostic.
|
The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).
|
private/buf/buflsp/report.go
Outdated
| Message: err.Unwrap().Error(), | ||
| Source: "buf", |
There was a problem hiding this comment.
I think this change is reasonable, but I believe one of the reasons we print out the position information is because this is also printed into the console of VSCode, for example, so we want more information. But agree that in-line, this is clearer.
There was a problem hiding this comment.
yeah, I figured different clients might have different displays. Definitely open to figuring out what makes sense for most clients.
I will note though that adding the position information is inconsistent with some of our other diagnostics (e.g. our buf lint diagnostics):
There was a problem hiding this comment.
(It's possible that we should consistently use buf-lsp as our source, FWIW?)
There was a problem hiding this comment.
Go lsp doesn't include line diagnostics, so this makes sense to me.
There was a problem hiding this comment.
given @doriable's 👍 to consistently using buf-lsp as a source, did that in d2c6441.
I've seen different LSPs do different things, e.g. I think gopls uses the individual analyzer name as the source, but I think starting consistently with buf-lsp is better for now.
This removes the extraneous information from the diagnostic report (filename, line, column), which is already included in the range.
It also adds a hard-coded
bufas the source of the diagnostic.Before:
After: