Rework color detection #87
Conversation
picocolors.js
Outdated
| argv = p.argv || [], | ||
| env = p.env || {}, | ||
| out = p.stdout || {} | ||
| let ne = v => v != null && String(v).length > 0 |
There was a problem hiding this comment.
Do we really need such elaborate check?
v is guaranteed to be undefined or String, so this could be changed to simple v => !!v.
And in this case we don't need this function at all.
Just use !!env.NO_COLOR instead of ne(env.NO_COLOR).
There was a problem hiding this comment.
See #62 (comment). The key is to keep treating empty string value as falsy value.
There was a problem hiding this comment.
If I understood @lukeed comment correctly 😅
There was a problem hiding this comment.
> !!""
false
> !!"1"
true
> !!undefined
false
There was a problem hiding this comment.
I see your point. I tried to recall this behavior but testing it on numbers !!0 tripped me over.
picocolors.js
Outdated
| (require != null && require("tty").isatty(1) && env.TERM !== "dumb") || | ||
| "CI" in env) | ||
| p.platform === "win32" || | ||
| (out.isTTY && env.TERM !== "dumb") || |
There was a problem hiding this comment.
out is used only once.
Could be shorter to write
(p.stdout || {}).isTTY && ...
There was a problem hiding this comment.
Good point, missed that
| @@ -1,12 +1,13 @@ | |||
| let argv = process.argv || [], | |||
| env = process.env | |||
| let p = process || {}, | |||
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Oh actually using globalThis would make this library not work in older Node.js version :/
Closes #41, #44, #62, #67, #71, #80, #83.