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

cli(color): fix color option #663

Merged
merged 1 commit into from
Oct 30, 2018
Merged

cli(color): fix color option #663

merged 1 commit into from
Oct 30, 2018

Conversation

jayjun
Copy link
Contributor

@jayjun jayjun commented Oct 30, 2018

What kind of change does this PR introduce?
Fix --color option to use supports-color properly.

Did you add tests for your changes?
No.

If relevant, did you update the documentation?
N/A

Summary
The --color option’s default is set to return

return require("supports-color").supportsColor

Added in supports-color 5.0.0 (chalk/supports-color#64), supportsColor is a function and not the test result. Because a function is truthy, colours are enabled irrespective of TTY and FORCE_COLOR settings.

return require("supports-color").stdout

is the correct way to use supports-color. It checks process.stdout.isTTY so there’s no need to do the same.

Does this PR introduce a breaking change?
No.

Other information
See #452 for the last attempted fix.

@jsf-clabot
Copy link

jsf-clabot commented Oct 30, 2018

CLA assistant check
All committers have signed the CLA.

@webpack-bot
Copy link

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

@jayjun jayjun changed the title cli(color): fix color option and document no-color cli(color): fix color option Oct 30, 2018
@evenstensberg
Copy link
Member

Guess this is fine..

@evenstensberg evenstensberg merged commit 7b339a5 into webpack:master Oct 30, 2018
@jayjun jayjun deleted the color branch October 31, 2018 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants