Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discussion: as-you-type squiggles vs build-time squiggles #510

Open
ljw1004 opened this issue Jun 28, 2018 · 7 comments
Open

Discussion: as-you-type squiggles vs build-time squiggles #510

ljw1004 opened this issue Jun 28, 2018 · 7 comments
Labels
diagnostics feature-request Request for new features or functionality
Milestone

Comments

@ljw1004
Copy link
Contributor

ljw1004 commented Jun 28, 2018

In Visual Studio, it has two kinds of error squiggles: blue squiggles come from when you build a project (using the Project>Build menu item) and are obtained by scraping the output of the build tool; red squiggles come "as-you-type" from the language service and are only ever generated for the file you're currently editing, and even then only up to a limit (a few hundred?). It will be common that a red squiggle and a blue squiggle coincide (because the same error is reported both by the project-build and by the language service) - in this case the red one replaces the blue one.

The idea is that (1) you can't reasonably expect the full set of errors for a project to be recalculated every keystroke; (2) the user deserves some indication that the build-time squiggles are "stale" and require a manual step to generate.

How do folks feel about this in VSCode and LSP? (I'm wanting to do this for my language service).

OPTION 1 This is a question that should be unrelated to LSP. LSP should solely be for the "red" squiggles. The editor itself should figure out when to invoke a command-line build, and scrape blue squiggles from it, and display them, and the editor should figure out how to de-dupe red and blue squiggles.

OPTION 2 This functionality should be rolled into LSP. The "publishDiagnostics" message from the LSP server should indicate whether a given squiggle is blue or red. There should be an additional LSP method to "kick off a build".

@jvican
Copy link

jvican commented Jun 29, 2018

How do folks feel about this in VSCode and LSP? (I'm wanting to do this for my language service).

What you describe may be done with the Build Server Protocol (currently developed at the Scala Center and JetBrains). The idea is that the language server connects to the build tool to fetch compiler diagnostics directly from it and then it handles them. Then, LSP can combine them with the diagnostics coming from LSP in whatever way it prefers depending on things like correctness, last compiler results, etc.

Currently, BSP diagnostics have the same structure as LSP diagnostics. In the future, we'd like to enrich these diagnostics so that the build tool can give us structured build messages that the clients/editors know how to display (a message could be composed of different fields; one short summary, one long summary, a code snippet with a suggestion/actionable feedback, etc).

@ShaneDelmore
Copy link

^ It would be great if the structured messages could include references to code actions in the cases that an automated fix is available.

@jvican
Copy link

jvican commented Jun 29, 2018

I have opened a ticket with my thoughts on how we could support this in the build tool side in build-server-protocol/build-server-protocol#29, but LSP support would be required as well so that language servers can forward our more structured data to editors.

@dbaeumer I think that there's value in allowing diagnostics to provide the following fields in LSP:

  1. Summary.
  2. Long description of the diagnostic.
  3. Link to doc/reference.

For a description of how I think this information is visually important for the user, see the linked ticket. The major advantage of this approach is that language servers can get linter output directly from the build tool -- no need for custom linter integrations in language servers, the integrations in the build tools are reused.

(Note that there's no need to support code actions/fixes since those are already supported by workspace/codeAction and a language server can interpret them from the BSP diagnostics.)

EDIT: Judging from #500 (comment), it looks like any change to diagnostics is cumbersome because they flow in both ways. I think it would make sense to add this in LSP 4.0 -- we're not rushed in our side to implement rich diagnostics. Another option would be to add an independent endpoint for rich diagnostics and enable it via capabilities.

@dbaeumer dbaeumer added feature-request Request for new features or functionality and removed discussion labels Oct 31, 2019
@dbaeumer dbaeumer added this to the On Deck milestone Oct 31, 2019
@matklad
Copy link
Contributor

matklad commented May 21, 2020

Kind of an annoying +1, but we could really use this for Rust :)

Our language server is pretty nascent at the moment, and can show only few errors. In contrast, errors from the compiler are outright stunning in quality.

We currently union the two sets of diagnostics on the server side, but this is awkward to implement, and awkward for the user. It would be cool if:

  • build time and on-the-fly diagnostics had different presentations, so that users are not confused about reporting errors to rust-analyzer or rustc
  • build-time error ranges are automatically shifted when the user types (and marked as potentially obsolete)
  • there's a way to save all text files before invoking build
  • there's a way to configure the build to be executed automatically on every save or after a timeout.

@ljw1004
Copy link
Contributor Author

ljw1004 commented May 21, 2020

@matklad I wonder... wouldn't you also want on-the-fly ranges to be automatically shifted when the user types?

@matklad
Copy link
Contributor

matklad commented May 21, 2020

@ljw1004 yes, but that's not nearly as important, as they are updated more or less instantly by the server anyway.

@dbaeumer
Copy link
Member

We had this discussion in VS Code as well and one solution is to differentiate between builder and reconcile problems. See microsoft/vscode#61140 (comment)

But we so far carefully state away in the LSP to talk about position adjustment. So I would like to leave it that way.

There is also the build protocol: https://build-server-protocol.github.io/docs/specification

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
diagnostics feature-request Request for new features or functionality
Projects
None yet
Development

No branches or pull requests

5 participants