fixed support-colors condition orders, this should fix cygwin issues#154
fixed support-colors condition orders, this should fix cygwin issues#154RShadowhand wants to merge 1 commit intoMarak:masterfrom RShadowhand:master
Conversation
|
This fixes colors for Linux as well and for git for windows (bash). |
|
|
||
| if (/^screen|^xterm|^vt100|color|ansi|cygwin|linux/i.test(process.env.TERM)) { | ||
| return true; | ||
| if (process.env.TERM === 'dumb') { |
There was a problem hiding this comment.
Switching the dumb check and the other process.env.TERM check does not change anything, because they are exclusive. All it does it making this diff harder to read. I suggest you switch them back to minimize this diff.
There was a problem hiding this comment.
I think this was done on purpose since
if (/^screen|^xterm|^vt100|color|ansi|cygwin|linux/i.test(process.env.TERM)) {
is moved to below if ('COLORTERM' in process.env) {
and moved ontop of
if (process.stdout && !process.stdout.isTTY) {
But this change does something since it now works in windows, and Linux.
There was a problem hiding this comment.
The process.stdout check should not have been moved, see my other comment above. When you move it back, the remaining change is that the two process.env.TERM switch place, which does nothing. Which means there is nothing left this patch does right. Sorry to say that.
|
@paladox The reason this patch seems to fix it for you is because it returns true for For example, even if your terminal supports colors, if you're sending the output of your program to another program or to a file (so not to the terminal screen), then we typically expect plain text (without color codes). While this patch it enables colors in interactive terminals on Windows for you; it also enables colors everywhere else - which defeats the purpose of the |
|
I think we should close this in favor of updating supports-colors from the official supports-colors repo ( https://github.com/chalk/supports-color/blob/master/index.js ). That should resolve a lot of these issues. Planning that for a (soon) future release. |
Support-colors.js's condition order was a mess.
Changed to make it fit to logical rules, instead of randomness.