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

Windows / Colors are displayed as codes #4722

Closed
AndyBitz opened this issue Nov 14, 2017 · 14 comments
Closed

Windows / Colors are displayed as codes #4722

AndyBitz opened this issue Nov 14, 2017 · 14 comments

Comments

@AndyBitz
Copy link

I'm on Windows 10 Build 16299.19

I've recently updated from cargo 0.20.0 to 0.22.0 and now I have the following issue.

Colors (especially green) are not displayed correctly under normal circumstances.
When I redirect stdout to a file everything seems to almost work fine.

cargo color bug

compiler error message

I've tried this with cargo 0.21.1 and had the same bug.
A VM with the same Windows Version did't have this problem.
An other device with an older windows version did't have this bug until I updated to the current version.

Could the recent cmd.exe change be the problem?

The nightly version has the same bug. Only 0.20.0 seems to work normal.
This is the same for cmd, powershell and hyper (the terminal emulator I use).

I've posted this on StackOverflow before.

@alexcrichton
Copy link
Member

Thanks for the report!

@BurntSushi you wouldn't happen to know what's going on here would you? We're using a pretty thin wrapper over the termcolor crate

@AndyBitz
Copy link
Author

Yes, termcolor seems to be the problem.
It replaced term after 0.20.0.

But I'm not sure why it only works when stdout is redirected.
It also seems to print wrong color codes when stderr is redirected to stdout.

cargo run --color always > output.txt 2>&1

output.txt
term

@alexcrichton
Copy link
Member

Do you have the TERM environment variable set? If so, do you know what its value is?

@AndyBitz
Copy link
Author

Ahhh. It is xterm-256color. Setting it to nothing solves the problem.

@alexcrichton
Copy link
Member

Ok that'd explain it! Do you know how come that variable is set? Should Cargo not be interpreting that environment variable?

@retep998
Copy link
Member

retep998 commented Nov 19, 2017

https://github.com/BurntSushi/ripgrep/blob/2c84825ccbe2d42e0648b56a7d3289c5f5bcb9e5/termcolor/src/lib.rs#L176-L182

If TERM is set, then it will forcefully attempt to use ANSI codes even if the stdout/stderr it is writing to is a windows console. This is clearly the wrong behavior as the correct behavior is to unconditionally use the windows console API as the way to perform colors when stdout/stderr is a windows console. Even if the color choice is set to ColorChoice::AlwaysAnsi, even if TERM is set, stick to the windows console API!

That said, if you are writing to the windows console and you successfully set ENABLE_VIRTUAL_TERMINAL_PROCESSING via SetConsoleMode (or it was already set via GetConsoleMode), then and only then are you allowed to use Microsoft's flavor of ANSI escape codes. The supported escape codes in this case depend on the version of Windows 10, not the TERM environment variable!

@AndyBitz
Copy link
Author

@alexcrichton Probably when working with hyper.js.

@retep998 so, termcolor needs to get fixed?

@alexcrichton
Copy link
Member

@AndyBitz hm I'm a little confused then as to what's going on, it sounds like you're using a terminal emulator which claims to support ansi escape codes but it doesn't?

@BurntSushi
Copy link
Member

I'm also confused. If you're setting TERM to something other than dumb, then you're declaring that your terminal supports ANSI colors.

@retep998
Copy link
Member

retep998 commented Nov 20, 2017

@BurntSushi If you're writing to the windows console, then you must disregard what TERM says. Unless the console has the ENABLE_VIRTUAL_TERMINAL_PROCESSING mode enabled, ANSI escape codes will never work. This is the chain of logic that should occur:

  • Call GetConsoleMode
  • If GetConsoleMode fails then use ansi escape codes based on TERM, the ColorChoice, and the name of the pipe.
  • If GetConsoleMode succeeds and the current modes already have ENABLE_VIRTUAL_TERMINAL_PROCESSING then use Microsoft flavor escape codes.
  • Otherwise call SetConsoleMode with the current modes or'd with ENABLE_VIRTUAL_TERMINAL_PROCESSING.
  • If SetConsoleMode succeeds then use Microsoft flavor escape codes.
  • Otherwise use the Windows console API for colors.

@AndyBitz
Copy link
Author

AndyBitz commented Nov 20, 2017

Sorry for the confusion, I meant that when I worked on hyper the TERM variable (probably) got set globally and was therefore available in every terminal I used which caused cargo to display wrong colors everywhere.

Also I found this in the Microsoft docs, which is as @retep998 says.

@jacobmorzinski
Copy link

My own experience: my TERM is set to msys (not set by me, but set by GitHub's old "git shell" CLI package). I haven't yet upgraded to GitHub's new desktop app, because the old "git shell" CLI package is stunningly useful and I don't want to break it. I do confirm that removing TERM restores colors to cargo's windows console output, so I'm glad that I found this thread.

@stale
Copy link

stale bot commented Sep 17, 2018

As there hasn't been any activity here in over 6 months I've marked this as stale and if no further activity happens for 7 days I will close it.

I'm a bot so this may be in error! If this issue should remain open, could someone (the author, a team member, or any interested party) please comment to that effect?

The team would be especially grateful if such a comment included details such as:

  • Is this still relevant?
  • If so, what is blocking it?
  • Is it known what could be done to help move this forward?

Thank you for contributing!

(The cargo team is currently evaluating the use of Stale bot, and using #6035 as the tracking issue to gather feedback.)

If you're reading this comment from the distant future, fear not if this was closed automatically. If you believe it's still an issue please leave a comment and a team member can reopen this issue. Opening a new issue is also acceptable!

@stale stale bot added the stale label Sep 17, 2018
@AndyBitz
Copy link
Author

I couldn't reproduce this with the latest version of cargo and rustc.
I'll assume that it got fixed and close the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants