Implement more robust isTTY check#63
Conversation
|
@jorgebucaran What's up with GitHub actions? I see there is a CI flow defined, but it doesn't seem to be running. |
|
I can confirm this fixes the compatibility problems we have for pino transport |
|
Wow, thanks! So, is this Node's canonical way to detect if you are in a pipe? Other than not being available inside worker_threads, are there any disadvantages to |
It was configured to run against push only. It's fixed now. 🙆♂️ |
|
@jorgebucaran This discussion might be useful: nodejs/node#28386 |
|
@jorgebucaran is there an ETA for a release that would ship this? |
| @@ -1,12 +1,13 @@ | |||
| const tty = require('tty'); | |||
There was a problem hiding this comment.
Colorette is an ES module, so this should be changed to use import.
There was a problem hiding this comment.
will that be compatible with current dual publishing of CJS and ES versions? do you autoreplace?
There was a problem hiding this comment.
Nope, looks like the time to upgrade that script has finally come! I can take care of that after we merge this.
|
Thanks! I'll merge here, update the script and publish a new minor version shortly. |
|
Released as 1.4.0. |
|
@mcollina @kibertoad Is there any reason to check if import * as tty from "tty"
...
const isCompatibleTerminal =
tty && tty.isatty(1) && env.TERM && env.TERM !== "dumb" |
|
@jorgebucaran It's a node-specific thing. There were prior bug reports from people using colorette outside of Node, I wanted to safeguard against that from the get go. |
|
Gotcha, thanks. I still feel that this should be handled by the user's bundler / preprocessor (#64 (comment)). |
3 participants
As per pinojs/pino-pretty#220 (comment)