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

ASCII color codes are displayed inline for for editors #15443

Open
lindhe opened this issue Aug 11, 2023 · 19 comments
Open

ASCII color codes are displayed inline for for editors #15443

lindhe opened this issue Aug 11, 2023 · 19 comments
Labels
A-diagnostics diagnostics / error reporting C-support Category: support questions

Comments

@lindhe
Copy link

lindhe commented Aug 11, 2023

Sometimes, the error shown in Vim (and apparently VS Code too) is printed with ASCII color codes showing in the string from rust_analyzer. It makes the error very hard to read. See neovim/nvim-lspconfig#2760.

It seems to me like rust-analyzer does not properly distinguish between being executed in a terminal environment or not, and decide from that whether to print ASCII color codes or skip them.

In terminal:

Screenshot from 2023-08-11 16-16-58

In editor:

Screenshot from 2023-08-11 10-51-47

Steps to reproduce

  1. Create the following crate:
cargo new -q foo
cd foo
cargo add -q rocket@0.5.0-rc.3
cat <<EOF > src/main.rs
use rocket::UriDisplayPath;

fn main() {
    println!("Hello, world!");
}

#[derive(UriDisplayPath)]
pub enum Foo {
    Bar,
}
EOF
  1. Optionally run cargo check.
  2. Open src/main.rs with Vim.

Other info

rust-analyzer version: (eg. output of "rust-analyzer: Show RA Version" command, accessible in VSCode via Ctrl/⌘+Shift+P)

Mason reports rust-analyzer 2023-08-07.

rustc version: (eg. output of rustc -V)

$ rustc -V
rustc 1.71.1 (eb26296b5 2023-08-03)

relevant settings: (eg. client settings, or environment variables like CARGO, RUSTC, RUSTUP_HOME or CARGO_HOME)

Not sure, nothing I've set explicitly at least. I run NeoVim with rust-lang/rust.vim and rust_analyzer for lspconfig installed via Mason.

@lindhe lindhe added the C-bug Category: bug label Aug 11, 2023
@Veykril
Copy link
Member

Veykril commented Aug 12, 2023

Your error message for some reason contains a green [note] part, which is what you are seeing inline. That should not be there as that is not the normal diagnostic output of the compiler. This is what I am getting here
image
I don't know what could cause this for you but something seems to fiddle with your compiler output?

@Veykril Veykril added A-diagnostics diagnostics / error reporting C-support Category: support questions and removed C-bug Category: bug labels Aug 12, 2023
@tadeokondrak
Copy link
Contributor

tadeokondrak commented Aug 12, 2023

I think the derive macro is emitting escape codes (when using a stable compiler): https://github.com/SergioBenitez/proc-macro2-diagnostics/blob/master/src/line.rs#L79

@Veykril
Copy link
Member

Veykril commented Aug 12, 2023

Ah good catch, that's indeed gonna be the issue. There isn't really anything we can do here then, r-a just spits out the error messages we get in json verbatim as the expectation is for compiler diagnostics to be just text. This might be something to request as a feature for the proc-macro api, since the compiler itself has a flag that toggles whether the json output has colors in it or not which this should hook into.

@Veykril Veykril closed this as completed Aug 12, 2023
@lindhe
Copy link
Author

lindhe commented Aug 12, 2023

Ah, there it is! Thanks so much for identifying the real culprit for me! I'll continue my hunt over there.

@lindhe
Copy link
Author

lindhe commented Aug 12, 2023

Interestingly, it seems like cargo build and proc-macro properly toggles color output sometimes!

Here's what it looks like in my terminal, and the color codes are not printed!

Screenshot from 2023-08-12 10-01-05

It seems like proc-macro do attempt to use different logic depending on whether or not color is set:

#[cfg(all(feature = "colors", not(nightly_diagnostics)))]

@Veykril, you mentioned "the error messages we get in json". It seems like cargo build --message-format has a few different options that might be of relevance here: json, json-diagnostic-short, json-diagnostic-rendered-ansi and json-render-diagnostics.

What field do you render? Is it message.message?

The full JSON object

{
  "reason": "compiler-message",
  "package_id": "foo 0.1.0 (path+file:///home/andreas/tmp/foo)",
  "manifest_path": "/home/andreas/tmp/foo/Cargo.toml",
  "target": {
    "kind": [
      "bin"
    ],
    "crate_types": [
      "bin"
    ],
    "name": "foo",
    "src_path": "/home/andreas/tmp/foo/src/main.rs",
    "edition": "2021",
    "doc": true,
    "doctest": false,
    "test": true
  },
  "message": {
    "rendered": "error: [note] error occurred while deriving `UriDisplay`\n --> src/main.rs:7:10\n  |\n7 | #[derive(UriDisplayPath)]\n  |          ^^^^^^^^^^^^^^\n  |\n  = note: this error originates in the derive macro `UriDisplayPath` (in Nightly builds, run with -Z macro-backtrace for more info)\n\n",
    "children": [],
    "code": null,
    "level": "error",
    "message": "\u001b[1;34m[\u001b[0m\u001b[1;32mnote\u001b[0m\u001b[1;34m] \u001b[0m\u001b[1;39merror occurred while deriving `UriDisplay`\u001b[0m",
    "spans": [
      {
        "byte_end": 98,
        "byte_start": 84,
        "column_end": 24,
        "column_start": 10,
        "expansion": {
          "def_site_span": {
            "byte_end": 36109,
            "byte_start": 36044,
            "column_end": 66,
            "column_start": 1,
            "expansion": null,
            "file_name": "/home/andreas/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rocket_codegen-0.5.0-rc.3/src/lib.rs",
            "is_primary": false,
            "label": null,
            "line_end": 1060,
            "line_start": 1060,
            "suggested_replacement": null,
            "suggestion_applicability": null,
            "text": [
              {
                "highlight_end": 66,
                "highlight_start": 1,
                "text": "pub fn derive_uri_display_path(input: TokenStream) -> TokenStream {"
              }
            ]
          },
          "macro_decl_name": "#[derive(UriDisplayPath)]",
          "span": {
            "byte_end": 98,
            "byte_start": 84,
            "column_end": 24,
            "column_start": 10,
            "expansion": null,
            "file_name": "src/main.rs",
            "is_primary": false,
            "label": null,
            "line_end": 7,
            "line_start": 7,
            "suggested_replacement": null,
            "suggestion_applicability": null,
            "text": [
              {
                "highlight_end": 24,
                "highlight_start": 10,
                "text": "#[derive(UriDisplayPath)]"
              }
            ]
          }
        },
        "file_name": "src/main.rs",
        "is_primary": true,
        "label": null,
        "line_end": 7,
        "line_start": 7,
        "suggested_replacement": null,
        "suggestion_applicability": null,
        "text": [
          {
            "highlight_end": 24,
            "highlight_start": 10,
            "text": "#[derive(UriDisplayPath)]"
          }
        ]
      }
    ]
  }
}

BTW, I notice that cargo build prints the [note] while rustc does not. What do rust-analyzer use internally to build/check?

EDIT 1: To my surprise, json and json-diagnostic-rendered-ansi has the same behavior:

Screenshot from 2023-08-12 10-48-39

@lindhe
Copy link
Author

lindhe commented Aug 12, 2023

Rats! I think this might be a bug in Cargo. Even with --color=never, the color feature appears to be set when building using --json:

Screenshot from 2023-08-12 11-50-16

@Veykril
Copy link
Member

Veykril commented Aug 12, 2023

No, there is no bug here. The colored [note] is created by the proc-macro itself, no tool has control over that proc-macro emitting color codes or not. Hence why I said

This might be something to request as a feature for the proc-macro api, since the compiler itself has a flag that toggles whether the json output has colors in it or not which this should hook into.

earlier

@lindhe
Copy link
Author

lindhe commented Aug 12, 2023

the compiler itself has a flag that toggles whether the json output has colors in it or not

What flag is that, if not --color?

@Veykril
Copy link
Member

Veykril commented Aug 12, 2023

That flag controls the color of the compilers output colors for diagnostic, the color you are seeing here (that is the [note] part) is created by the proc-macro though, not by the compiler. The compiler merely emits the message that the proc-macro tells it to.

@lindhe
Copy link
Author

lindhe commented Aug 12, 2023

Yes, but proc-macro emits that conditionally on whether or not the color feature is set:

https://github.com/SergioBenitez/proc-macro2-diagnostics/blob/45fa2d68669d10daeaf0f9b664a96576626fae16/src/line.rs#L78C1-L78C1

@Veykril
Copy link
Member

Veykril commented Aug 12, 2023

Yes, that is a feature flag. Passing --color or not to the compiler is rrelevant to that though, as that is not a flag that sets the feature. In fact, crates cannot observe --color being passed or not whatsoever

@lindhe
Copy link
Author

lindhe commented Aug 12, 2023

Oh, that's interesting. Any idea how on earth proc-macro is able to distinguish between colorized and plain output then? Because, as you can see in my screenshots, it does not colorize "[note]" when I pipe it to cat.

@lindhe
Copy link
Author

lindhe commented Aug 12, 2023

the compiler itself has a flag that toggles whether the json output has colors in it or not which this should hook into.

Do you have any pointers to how one checks that flag?

@lindhe
Copy link
Author

lindhe commented Aug 12, 2023

That flag controls the color of the compilers output colors for diagnostic, the color you are seeing here (that is the [note] part) is created by the proc-macro though, not by the compiler. The compiler merely emits the message that the proc-macro tells it to.

Passing --color or not to the compiler is rrelevant to that though, as that is not a flag that sets the feature. In fact, crates cannot observe --color being passed or not whatsoever

I am sure you are right, I just don't understand. What trips me up here is that [note] is printed in color when setting --color=always but printed not in color when setting --color=never (see screenshot below). Unless Cargo or the compiler does a text replacement to strip color codes from the output, which I very much doubt, I fail to see how it can be the case that --color=always is not picked up by the proc-macro.

Screenshot from 2023-08-12 19-03-46

@SergioBenitez
Copy link

All, copying my comments from SergioBenitez/proc-macro2-diagnostics#3 (comment) here for discourse:

The issue [...] is that as far as I can tell, there's no way for a proc-macro to know if it should emit colors or not. The proxy is typically to check whether we're connected to a TTY, but Cargo captures all output, so we're never connected to a TTY, and detecting via this manner would mean never emitting colors, which would be a shame.

Ideally, there would be some way to query 1) whether cargo itself will be emitting colors and/or 2) what message-format is expected. Alternatively, if we could inquire about how the compiler is invoked, we might be able to patch this minimally for rust-analyzer. It doesn't seem like 1) or 2) are viable options now, but the alternative might be, given that rust-analyzer appears to set RUSTC_WRAPPER to rust-analyzer by default. If we can detect this, we can stop emitting colors when rust-analyzer invokes rustc.

[...] the above didn't work. First, the wrapper is only set for build scripts, which is fine-ish as we can forward the env-var along via Cargo. Second, and more importantly, the wrapper is only set the first time the build script is built/run, so the solution only really works once in a given workspace, if it works at all.

Until there's some reliable mechanism to detect whether we should color, I think the "correct" solution here is for rust-analyzer to strip ANSI escapes from any output before it re-emits it. Cargo itself uses https://crates.io/crates/strip-ansi-escapes, which is why using --color=never works as expected.

@Veykril
Copy link
Member

Veykril commented Sep 26, 2023

Interesting, so cargo already handles this specially itself as well? In that case you are right, we should do the same as cargo here when processing compile_error! invocations (and also we should pass --color=never to cargo check, I don't think we are doing that currently)

@DriedYellowPeach
Copy link

Is there any solution I can avoid this right now? I tried add color=never in my .toml, but it only avoid color output in cargo build, not in diagnostics. Can I setup rust-analyzer or something?

@lindhe
Copy link
Author

lindhe commented Nov 11, 2023

I know of no work-around until they fix this.

@SergioBenitez
Copy link

One "work-around" is to use a nightly toolchain, at least for rust-analyzer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics diagnostics / error reporting C-support Category: support questions
Projects
None yet
Development

No branches or pull requests

5 participants