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

Compile errors are no longer colorized #49322

Closed
aloucks opened this issue Mar 24, 2018 · 10 comments
Closed

Compile errors are no longer colorized #49322

aloucks opened this issue Mar 24, 2018 · 10 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@aloucks
Copy link
Contributor

aloucks commented Mar 24, 2018

Previously, the line numbers were in cyan and error codes and error underlines were in red.

Possibly related: #48588

image

@retep998
Copy link
Member

cc @alexcrichton who wrote the likely offending PR.

@alexcrichton
Copy link
Member

Thanks for the report @aloucks! This is almost for sure due to #48588

The interesting part here is that it looks like rustc is detecting that it should use colors because somet of the messages look bold. It looks, though, like there may be a bug in how rustc is colorizing things as the wrong colors are coming out!

To confirm, @aloucks are you using cmd.exe as a shell here?

@alexcrichton alexcrichton added regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 24, 2018
@aloucks
Copy link
Contributor Author

aloucks commented Mar 24, 2018

@alexcrichton - Yes, the screen shot is using cmd.exe. I first noticed it from VSCode with the embedded powershell terminal.

I noticed it was showing bold too. It is strange that it detects that it should use color, but doesn't apply it correctly.

@ehuss
Copy link
Contributor

ehuss commented Mar 26, 2018

I did some investigation on this. It looks like when you use both bold and intense that the color doesn't work. It appears to be due to the fact that termcolor sets the color before issuing the bold VT100 command. The Windows console currently does not support true "bold" fonts, and instead just shows a "bright" color. It seems like setting "bold" is overriding the "intense" color. I've noted a few other people having various issues with bold text at Microsoft/console.

@BurntSushi do you have any particular insight on this? I did a quick fix by just moving the code to set bold to before the write_color in this function:

https://github.com/BurntSushi/ripgrep/blob/d7c9323a689239ac8b29baba63b1c3a26e12afda/termcolor/src/lib.rs#L972-L987

I didn't do significant testing, but it seems to avoid the bold setting from overriding the color setting.

@BurntSushi
Copy link
Member

@ehuss I don't have any special insight, no, but I do recall running into those sorts of problems when adding VT100 support for Windows. I can't remember the details. If re-arranging the ordering fixes things, then that seems like a fine thing to me. I'd be happy to take your patch!

ehuss added a commit to ehuss/ripgrep that referenced this issue Mar 26, 2018
There is an issue with the Windows 10 console where if you issue the bold
escape sequence after one of the extended foreground colors, it overrides the
color.  This happens in termcolor if you have bold, intense, and color set.
The workaround is to issue the bold sequence before the color.

This is for rust-lang/rust#49322.
@ehuss
Copy link
Contributor

ehuss commented Mar 26, 2018

@BurntSushi I don't think this should be closed until a new termcolor release is made and the rust lock file is updated.

@alexcrichton
Copy link
Member

Oh awesome thanks so much for the fix here @ehuss! @BurntSushi mind publishing a new version of termcolor so we can test out the fix?

@alexcrichton alexcrichton reopened this Mar 26, 2018
@BurntSushi
Copy link
Member

Oh, errm, yeah I didn't mean to close this. I always forget that github does cross repo closes. In any case, termcolor 0.3.6 is on crates.io!

@alexcrichton
Copy link
Member

Awesome, thanks!

@ehuss want to send a PR I can r+? No worries if you're busy though!

@ehuss
Copy link
Contributor

ehuss commented Mar 26, 2018

@alexcrichton sure! I'd like to do a build first, but I'll post it soon!

ehuss added a commit to ehuss/rust that referenced this issue Mar 27, 2018
kennytm added a commit to kennytm/rust that referenced this issue Mar 27, 2018
Fix diagnostic colors on Windows 10 console.

This updates termcolor to pick up BurntSushi/ripgrep#867.

Fixes rust-lang#49322.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants