rust-lang / rust-analyzer

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

New "full compiler diagnostic" view could use some color #13648

Closed jplatte closed 1 year ago

jplatte commented 1 year ago

Thanks @Veykril for adding this nice feature in #13633! How hard would it be to use the same colors in the compiler diagnostic view that you also get when seeing the same error in the terminal?

Veykril commented 1 year ago

Pretty hard given the rendered output in the json output doesn't contain the color, and reconstructing that ourselves isn't really an option. cc @estebank

HKalbasi commented 1 year ago

Reconstructing the error isn't that hard, using libraries like ariadne (I did it one time for evcxr, jupyter kernel for rust) but given that r-a doesn't need to change line numbers and such, it is way more reasonable to ask rustc for colored output inside json.

bjorn3 commented 1 year ago

Rustc supports outputting ansi color codes in the json messages and in fact that is what cargo itself uses. Not sure if cargo supports passing this through, but if it does you could parse the ansi escape codes to get the colors.

Veykril commented 1 year ago

Ye i think cargo might be eating the colors here then from what I've seen.

estebank commented 1 year ago

Adding an HTML formatted output option to enum HumanReadableErrorType1 and using that instead in json_rendered2 (if a flag is set) then you wouldn't have to do any of these hacks (other than maybe supplying CSS).

ian-h-chamberlain commented 1 year ago

As a not-optimal workaround, I found a way to get back the colors that Cargo displays by default (in VSCode at least):

  "rust-analyzer.checkOnSave.overrideCommand": [
    "cargo",
    "check",
    "--message-format=json-diagnostic-rendered-ansi"
  ],

Then use an extension like https://marketplace.visualstudio.com/items?itemName=iliazeus.vscode-ansi to show the rendered colors. Unfortunately it seems like there is no native support in the VSCode buffer view (https://github.com/microsoft/vscode/issues/38834) so it would probably require effectively re-implementing what they've done in that extension to render the ANSI codes.

Not sure how other editors might handle this kind of scenario, but for ones that do support rendering ANSI codes natively, a simple flag to switch between message-formats might be nice to have.

Veykril commented 1 year ago

Other editors don't have this feature, and yes, I would expect us to implement ANSI code parsing and effectively transforming things into html or similar that we can emit in a colored fashion.

estebank commented 1 year ago

We should bite the bullet and provide a --message-format=json-diagnostic-rendered-html on the rustc side. If we gave you annotated spans that would be enough for you to provide your own CSS, right?

Veykril commented 1 year ago

Ye we should be able to provide the css there

ian-h-chamberlain commented 1 year ago

In the example of the above extension, they actually use a text decoration provider rather than any CSS/HTML output. I suppose if the intention is to eventually switch to a webview or something like that, the HTML/CSS would be fine, but for the existing editor implementation it would have to then translate from HTML to the VSCode text decoration API, right? Is there a significant different parsing ANSI codes (already available with cargo) vs HTML (not yet implemented)?

Veykril commented 1 year ago

Well there are two ways to solve this. Either via a normal text editor colored with decorators/custom syntax highlighting or via a webview that renders the html.

ian-h-chamberlain commented 1 year ago

I am interested in working on this and actually made some decent progress on a proof of concept, but I had some questions around licensing and whether this is actually the desired approach... I posted over on Zulip but haven't heard there so thought I'd repost on this issue to try and get some input here:

  1. Is it worth pursuing the ANSI escape code / text decoration approach, or should I hold off in favor of a future webview / HTML approach?
  2. For proof-of-concept, I've copied a bunch of code from https://github.com/iliazeus/vscode-ansi (MIT licensed) — is it viable to include this in the final product with a copy of their license, or would I need to basically reimplement it from scratch (or find an alternative npm dependency that has similar functionality)? I couldn't find any other "vendored" code in the repo (and rather few npm deps) so wondering what the expectation /policy on that might be
Veykril commented 1 year ago

Sorry for not receiving any replies regarding this for so long.

The ANSI approach seems fine to me, both approaches should be feature equivalent for our purposes. If there is no simple npm package that offers the relevant things we'd want here then copying from that extension seems fine imo, ideally keeping the copied parts in a separate ts file that we can put the license together (so a subdir in src) to make the licensing stuff clear there. That would also simplify removing that again should we move to the html output at some point.

ian-h-chamberlain commented 1 year ago

Cool! I ended up finding a small package that works well enough and basically rewrote everything I had been using from vscode-ansi so I don't believe there should be any issues there. Just opened a PR with the initial implementation I came up with.

Edit: Oops I guess I should have done this sooner:

@rustbot claim