-
-
Notifications
You must be signed in to change notification settings - Fork 47
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
Disable color output in pipes #33
Conversation
Hey, it should be fine, but let me investigate some dependents to make sure it's not depended on. I'll add some extra |
Ok, the conditions have to be tweaked a little bit, but I'll take care of it. You're right that we should match the default |
Codecov Report
@@ Coverage Diff @@
## master #33 +/- ##
===========================================
- Coverage 100.00% 98.70% -1.30%
===========================================
Files 2 2
Lines 151 155 +4
Branches 28 30 +2
===========================================
+ Hits 151 153 +2
- Misses 0 2 +2
Continue to review full report at Codecov.
|
I wanted to keep my pretty formatting inside GitHub Actions 😇 Taking your original example, along with these changes: $ node app.js > log.txt
#=> NO COLORS
$ node app.js
#=> COLORS
$ FORCE_COLOR=1 node app.js > log.txt
#=> COLORS
$ FORCE_COLOR=0 node app.js > log.txt
#=> NO COLORS @ai If you can confirm that it also looks correct to you, then this is ready to go. |
Do we enable or disable color output on GitHub Action? GitHub Action user can call tool with a pipe as well. |
Right now it's enabled on GH because I thought |
LGTM. Thanks for fixing the tests 👍 |
Now have bash tests in place for detections with & without TTY sessions. Sorry for notification noise; wanted it all in together. |
Maintains CI colorization; users can disable with: * `uvu --no-color` * `NODE_DISABLE_COLORS=1 uvu` Related: - lukeed/kleur#33
Most of color output libraries like
chalk
,supports-color
, orcolorette
do not enable color output in pipes.it is useful for cases like:
Also, color marks could force
grep
to have unexpected results.@lukeed What do you think if we will add the same
process.stdout.isTTY
check tokleur
as well?