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

Color on Windows #21

Closed
jibsen opened this issue Feb 1, 2016 · 5 comments
Closed

Color on Windows #21

jibsen opened this issue Feb 1, 2016 · 5 comments

Comments

@jibsen
Copy link
Collaborator

jibsen commented Feb 1, 2016

On Windows, we should probably try to detect if the command window supports ANSI escapes, and default to not using color if we are not sure. The situation is a bit tricky, as different console emulators set different environment variables.

  • Some programs (like ConEmu) set the environment variable ANSICON (related to https://github.com/adoxa/ansicon, which also appears to set CLICOLOR)
  • Some set TERM (mostly bash, but for instance MSYS2 bash returns 0 for isatty(), explained here)

Most importantly though, the default command prompt does not support ANSI colors and returns true for isatty().

@nemequ
Copy link
Owner

nemequ commented Feb 1, 2016

I'm okay with b342b1b, but please use _getenv_s or _dupenv_s (I would probably use the former with a small stack based buffer, but whatever) to avoid that CRT warning.

I guess it would be possible to use SetConsoleTextAttribute to set colors on Windows, but TBH I'm not sure it's worth it. I don't think anyone takes the default command prompt seriously, and if conemu supports colors that's good enough for me.

@nemequ
Copy link
Owner

nemequ commented Feb 1, 2016

SetConsoleTextAttribute wouldn't work for people who are using --color always but redirecting the output (like we do to colorize AppVeyor's output), so it's probably better to avoid that.

Also, since you only want to check that ANSICON != NULL, you don't even need a buffer. Something like this should work:

if (isatty(fileno(stream))) {
  size_t ansicon_size;
  getenv_s(&ansicon_size, NULL, 0, "ANSICON");
  return ansicon_size != 0;
}
return false;

@jibsen
Copy link
Collaborator Author

jibsen commented Feb 1, 2016

I've updated the branch, but getenv_s is an extension to C11, so now some of the GCC builds break.

Quite oddly, only the mingw builds appear to break.

@nemequ
Copy link
Owner

nemequ commented Feb 1, 2016

Only mingw breaks because the getenv call is in guarded by an #if defined(_WIN32). I think everywhere else it's probably okay to assume that ttys support color. AFAIK Windows is the only platform which implements Annex K, but they're also the only ones who complain about getenv…

I pushed a fix which will just call the standard getenv on mingw, getenv_s on non-mingw windows, and still just use isatty everywhere else.

@nemequ nemequ closed this as completed Feb 1, 2016
@jibsen
Copy link
Collaborator Author

jibsen commented Feb 2, 2016

Ah, of course -- thanks.

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

2 participants