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

Print colored output only to a tty #1452

Merged
merged 1 commit into from
Dec 15, 2014

Conversation

dohse
Copy link

@dohse dohse commented Nov 27, 2014

Color output only if standard and error output are ttys.

@dohse dohse force-pushed the color-only-tty-output branch 3 times, most recently from 76018ac to b8f9233 Compare December 1, 2014 22:56
@boneskull
Copy link
Contributor

@dohse I can't imagine this works in a browser context.

@dohse
Copy link
Author

dohse commented Dec 15, 2014

The idea is from the reporter base. Does mocha.js run in browser context, but none of the reporters?

@dohse dohse force-pushed the color-only-tty-output branch from b8f9233 to d683ad2 Compare December 15, 2014 11:19
@dohse
Copy link
Author

dohse commented Dec 15, 2014

I changed this PR to only overwrite base reporter defaults when a color option is specified

@dasilvacontin
Copy link
Contributor

@dohse, cool! Does checking for undefined/null in Mocha.prototype.useColors make sense? (and not setting anything in that case)

@dohse
Copy link
Author

dohse commented Dec 15, 2014

Yeah, it makes sense to set no option.

Color output only if standard and error output are ttys.
@dohse dohse force-pushed the color-only-tty-output branch from d683ad2 to 3bc92de Compare December 15, 2014 11:32
@dohse
Copy link
Author

dohse commented Dec 15, 2014

Ok, changed it to only set useColors in options if a flag is present.

@dasilvacontin
Copy link
Contributor

@dohse, about reporters being used in the browser or not, I'm not sure right now. I've never used mocha in the browser, and I should look into that.

@dohse
Copy link
Author

dohse commented Dec 15, 2014

I doubt that reporters relying on the base reporter are running in the browser as they are already using tty.istty(). That might be different for the mocha main module.

@boneskull
Copy link
Contributor

Hmm, sorry, I was confused about what I was looking at.

boneskull pushed a commit that referenced this pull request Dec 15, 2014
Print colored output only to a tty
@boneskull boneskull merged commit 8b88b3b into mochajs:master Dec 15, 2014
@dohse
Copy link
Author

dohse commented Dec 15, 2014

@boneskull I originally implemented the tty check in lib/mocha.js so your comment was right. The changed solution should address that.

Thanks for merging!

@dohse dohse deleted the color-only-tty-output branch December 15, 2014 16:43
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.

3 participants