Add simplest TTY detection for Colorize

[makenowjust]

Simplest version of #3925.

It renames @on to @enabled because Colorize::Object has #on method also, but @on is not related and its name does not make a sense.

[bcardiff]

@makenowjust would you mind adding a doc sentence regarding that by default colorize is enabled if both STDOUT and STDERR are tty or if Colorize.enabled= / #toggle is explicitly called?

Apologies for the delay on this. 🙏

If you won't be able let us know and I will pick it from here.

[bcardiff]

I think this class_property should belong to module Colorize. I found it misleading to be inside the generic class.

[makenowjust]

Sorry. It's my mistake. I fixed it and added doc comment to Colorize.enabled?.

[makenowjust]

What? https://travis-ci.org/crystal-lang/crystal/jobs/214889120

[asterite]

Just a question: what if I want to send output to a file and later show it colorized in another way but still recognizing the color codes? For example like what travis does (stores colored output and probably formats it in the web with JS). I don't understand why the Colorize should be enabled or disabled according to one possible output (STDOUT and STDERR). I understand that this is the most probable output, but this will lead to a lot of confusion and unexpected behaviour.

I'd say to revert this merge, but I'd like to know others' opinions.

[asterite]

For example:

# foo.cr
require "colorize"

# I want to write "hello" in red to a file
File.write("file.txt", "hello".colorize.red)

puts "Done!"

Now:

$ bin/crystal foo.cr
Done!
$ cat file.txt
hello # in red
$ bin/crystal foo.cr > another.txt
Done!
$ cat file.txt
hello # without colors

Totally unexpected behaviour.

Colorize shouldn't be tied to STDOUT/STDERR.

[bcardiff]

This PR change the default setting from enabled to enabled if TTY.
And tidy up some names.

Colorize was already global and not sensible to the targeted IO. I agree it is an issue it should be addressed (for example in playground you can send the log with ansi colors despite the tty). But I see that as a separate issue.

[asterite]

For example this is with Ruby's colorize:

require "colorize"

puts "hello".colorize(:red)

It outputs "hello" in red to the console. But if we redirect the output to a file with ruby foo.rb > file.txt and then cat file.txt we still get colors. And I don't think it's good to assume that just because we send the output to a file or some other device we don't want colors (for example using cat and having colors if very convenient).

@makenowjust What do you think about this?

[bew]

I agree with @asterite 's last comment, colorize should not assume anything, but it should be easy for the user to disable the colors on demand.
We could have an helper like Colorize.on_tty_only! that checks stdout/err and disable colors when it's not a tty.

A use case would be to have a big colored output, and pipe it to less:

$ ./some_big_colored_output | less -R

The -R will interpret the colors and display them in the less pager..

[makenowjust]

@asterite said:

(for example using cat and having colors if very convenient).

But I use cat for removing colors sometime (i.e. brew install foo | cat). Expected behavior is different between peoples. This makes harder colorize problem.

However I think I shouldn't have changed default behavior (backward compatibility is important and current behavior is difficult to understand). @bew's Colorize.on_tty_only! seems good idea. Thank you @bew.