Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Don't show colors if stdout is not a tty #50

Merged
merged 2 commits into from Aug 27, 2011

Conversation

Projects
None yet
2 participants
Contributor

tpope commented Aug 26, 2011

This prevents raw color codes popping up if the output of turn is piped to a pager or redirected to a file, and is generally considered to be a best practice for command line tools. It also prevents them from showing up if turn is invoked inside ruby backticks, which has the unfortunate side effect of breaking the tests that looks for colors. I'm open to suggestions of how to fix this.

Contributor

trans commented Aug 27, 2011

The tests in question could override this and temporarily force colorization to come out.

Contributor

tpope commented Aug 27, 2011

Correct me if I'm missing something here, but since we are basically testing the color conditional itself, and since the test suite spawns turn in a subshell, there's not a good way to override code running there, correct? I mean I could pass in COLOR=force_if_stdout_tty or something, but then the actual logic gets convoluted.

It seems like it would be easier to just switch the COLORIZE constant to a colorize? method, and unit test that method, no?

To be clear, these are the tests in question.

Contributor

tpope commented Aug 27, 2011

I pushed up an example of how that might look.

Contributor

trans commented Aug 27, 2011

That looks good. Did the tests run okay?

Contributor

tpope commented Aug 27, 2011

Everything's green. Or gray if you redirect to a file. :)

Contributor

tpope commented Aug 27, 2011

I can squash the two to one commit if you like.

Contributor

trans commented Aug 27, 2011

Na, it's okay. I just click the big green Merge button to my left :-)

trans added a commit that referenced this pull request Aug 27, 2011

Merge pull request #50 from tpope/stdout_tty
Don't show colors if stdout is not tty and covert COLORIZE constant into colorize? method.

@trans trans merged commit 7a69822 into turn-project:master Aug 27, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment