Error output should observe --color option

[Sindisil]

Neither of CARGO_TERM_COLOR=never or passing the "--color never" option to rustfmt results in non-colored error output. Likewise, the "--color never" option is ineffective when running rustfmt directly. This often makes it very difficult for me to read error messages.

The only viable (if cumbersome to type) workaround I've found is to redirect stderr and pipe it through another command, like 'more' (e.g., "cargo fmt 2>&1 | more").

FWIW, this is on Windows 11 in cmd shell running in Windows Terminal. Same thing happens with the old Windows Console Host, as well as using PowerShell in Windows Terminal.

Example:

[calebcartwright]

To the best of my knowledge, cargo-fmt has never incorporated the CARGO_TERM_COLOR environment variable. Cargo subcommands/plugins do not inherently receive and honor the same environment vars that cargo itself does. In general it's best to refer to the respective tool/plugin/subcommand's docs to see what mechanisms they support.

WRT rustfmt's --color never, the unclosed delim error is coming from rustc itself. rustfmt operates in the earliest stages of the compiler workflow which allows it to be able to work in the presence of various invalid syntax that would never compile. However, unclosed delimiters are a pretty catastrophic error even in the earliest of those parsing phases, and that forces rustfmt to simply return the compiler error message and bail out.

In simpler terms, rustfmt's option does work for rustfmt output (both error and formatting diff), but does not control rustc behavior, and an unclosed delim error was one of the few edge cases where a user will see the raw rustc error text vs. rustfmt output from processing a rustc error.

There's probably some combination of rustc args that could be utilized to skip colors but I think that might be trickier than it seems at first blush (yes rustc has its own --color never flag, but I think that conflicts in an incompatible way with various diagnostic defaults).

This feels significantly edge case-y enough that I'm disinclined to have t-rustfmt invest any significant time into it, though if you are anyone else is sufficiently motivated to submit a PR (either to add CARGO_TERM_COLOR support to cargo-fmt which translates into --color never for rustfmt, and/or to see if there's any incantation rustfmt could pass into rustc's parser process) then we'd probably (eventually) review and take under consideration.

[Sindisil]

The unclosed delimiter error wasn't the one which first showed me the behavior -- I saw it in the course of my normal coding and simply assume I had made an error attempting to disable error colors. When I went to figure out what I was doing wrong, it was simply the first thing I tried WRT to forcing an error, and since it worked, I went with it.

rustc itself seems to honor the --color never flag for this example, which is why I opened this issue.

It sounds like you're open to a fix, and I certainly would like to avoid having to work around the issue, so I'll see if I can carve out some time figure out how to address it within rustfmt (along with adding CARGO_TERM_COLOR support -- rustfmt feels core enough that the lack is jarring).

My assumption would be that, should I find time to address these, they should likely be separate pull requests, yes? Or would it be preferable to make it a package deal? Certainly the env var support seems pointless w/o the --color never fix.

Thanks for taking the time to provide feedback (and for rustfmt in general)!

[Sindisil]

OK, I see the issue. The fn default_handler in parse/session.rs doesn't take the color option into account at all. Passing in the color option value and using that to determine the ColorConfig passed when creating the EmitterWriter fixes the issue.

I can put together a PR for the fix, and perhaps open a separate issue for cargo-fmt supporting CARGO_TERM_COLOR.