SergioBenitez / proc-macro2-diagnostics

Diagnostics for stable and nightly proc-macros!
Apache License 2.0
10 stars 5 forks source link

ASCII color codes are displayed inline for editors #3

Open lindhe opened 1 year ago

lindhe commented 1 year ago

I'm using Rocket (which is awesome, BTW) and noticed that sometimes error messages in my editor would be mangled with ASCII color codes, making the error messages very hard to read.

In terminal:

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

In editor:

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

I thought this was an issue with rust-analyzer, since I do not see the ASCII color codes printed if I pipe cargo build into cat, so I posted an issue there. But the people over there say that it is caused by your implementation of Display here:

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

You should really read https://github.com/rust-lang/rust-analyzer/issues/15443 first to get all of the context.

I have not yet wrapped my head around all the parts involved here, since I am pretty new to Rust. But I thought I'd post this here to see what you think.

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.
SergioBenitez commented 1 year ago

Right, this is problematic.

The issue, as you identified in https://github.com/rust-lang/rust-analyzer/issues/15443, 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.

I'll give this a try now.

SergioBenitez commented 1 year ago

Following-up: 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.

lindhe commented 1 year ago

Right. I'm glad you see the issue here. Too bad it does not sound like there is an easy fix around the corner.

Would you mind posting an issue to rust-analyzer or commenting on the issue I posted? I feel like I was unsuccessful convincing them before, so it's probably better to give them your perspective on this.