Skip to content
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

Add node v9 color depth support #99

Closed
wants to merge 1 commit into from
Closed

Conversation

Qix-
Copy link
Member

@Qix- Qix- commented Jul 12, 2019

Closes #98.

No need to use .hasColors() as it's a small wrapper around .getColorDepth(), which is supported in an older version of Node anyway - so it's a win win.

@Qix- Qix- requested a review from sindresorhus July 12, 2019 18:29
@sindresorhus
Copy link
Member

I'm pretty lukewarm about this. The downside of using stream.getColorDepth() is that we have no control over it. The Node.js core team is known for making a lot of bad decisions. Additionally, the behavior of stream.getColorDepth() can change between Node.js versions, which can lead to weird inconsistencies and bugs for users.

@Qix-
Copy link
Member Author

Qix- commented Aug 27, 2019

"Depth" in color terminology always means the number of bits. Usually it means number of bits per channel, but Node.js has made their own decisions about that...

I don't foresee that changing much. The benefit here is that we're no longer relying on environment variables, and custom TTY implementations can now indicate their support for colors.

Literally right after writing that last comment I snooped around some of the PRs adding support for this. I totally agree, what a nightmare - little to no thought or discussion went into adding this feature into core node.js.

Really sad state of affairs they've got going on there.

Closing. The color depth method is a timebomb.

@Qix- Qix- closed this Aug 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve the detections
2 participants