feat: respect NO_COLOR env variable #651
Conversation
|
Hey @susliko, thank you for working on this. However, this PR seems to make a lot more changes than just enabling As you can see in the mima run, this is causing some binary incompatibility issues. Could you perhaps pair this work back to the bare minimum needed to add |
susliko
force-pushed
the
respect-no-color
branch
from
0c29117 to
7999ae6
Compare
|
@valencik Thank you for taking a look!
Mima issues were solely caused by the removal of unused escapes from
These files had some string concatenations with direct inclusion of ansi codes. This PR refactors them to use |
susliko
force-pushed
the
respect-no-color
branch
from
7999ae6 to
3dd09a7
Compare
| String no_color = System.getenv("NO_COLOR"); | ||
| Boolean isNoColorEnvSet = no_color != null && no_color.equals("1"); | ||
| if (colorSequence == null || isNoColorEnvSet) return s; |
There was a problem hiding this comment.
| String no_color = System.getenv("NO_COLOR"); | |
| Boolean isNoColorEnvSet = no_color != null && no_color.equals("1"); | |
| if (colorSequence == null || isNoColorEnvSet) return s; | |
| if (colorSequence == null) return s; | |
| if ("1".equals(System.getenv("NO_COLOR"))) return s; |
| @@ -229,38 +229,33 @@ final class JUnitReporter( | |||
| e.getFileName().contains("file:/") | |||
| } | |||
| val canHighlight = !PlatformCompat.isNative | |||
| new StringBuilder() | |||
There was a problem hiding this comment.
why is this file changed? what does it have to do with NO_COLOR?
| val Reset = "\u001b[0m" | ||
| val Reversed = "\u001b[7m" | ||
| val Bold = "\u001b[1m" | ||
| val Faint = "\u001b[2m" | ||
| val RED = "\u001B[31m" | ||
| val YELLOW = "\u001B[33m" | ||
| // Foreground colors | ||
| val BLUE = "\u001B[34m" | ||
| val Magenta = "\u001B[35m" | ||
| val CYAN = "\u001B[36m" | ||
| val GREEN = "\u001B[32m" | ||
| val DarkGrey = "\u001B[90m" | ||
| val GREEN = "\u001B[32m" | ||
| val LightGreen = "\u001b[92m" | ||
| val LightRed = "\u001b[91m" | ||
| val Magenta = "\u001B[35m" | ||
| val RED = "\u001B[31m" | ||
| val YELLOW = "\u001B[33m" |
There was a problem hiding this comment.
why are these colors modified or reordered? what does it have to do with NO_COLOR?
| val isNoColorEnvSet = System.getenv("NO_COLOR") == "1" | ||
| if (colorSequence == null || isNoColorEnvSet) s |
There was a problem hiding this comment.
why is this implementation different from Ansi above? please don't read the env var if colorSequence is null, just like above.
| sb.append( | ||
| s" (${AnsiColors.LightRed}- obtained${AnsiColors.Reset}, ${AnsiColors.LightGreen}+ expected${AnsiColors.Reset})" | ||
| ).append("\n") | ||
| val obtained = AnsiColors.c("- obtained", AnsiColors.LightRed) | ||
| val expected = AnsiColors.c("+ expected", AnsiColors.LightGreen) | ||
| sb.append(s" ($obtained, $expected)").append("\n") |
There was a problem hiding this comment.
looks like you're changing output to use .c instead of explicit color manipulation. please do that in a separate PR.
As per https://no-color.org/ this change will disable ANSI colors if the NO_COLOR environment variable is set to any non-empty string. (If the environment variable is unset or set to the empty string then the default colors will still be used.) This builds on the work of scalameta#651 but attempts to minimize the size of the patch.
|
Thank you again for this contribution! |
Resolves #149
This PR makes use of the NO_COLOR environment variable, disabling all ansi escapes when
NO_COLOR=1.Example: