Disable color for no_tty about amazing_print HOT 9 CLOSED

I agree that we shouldn't extend the String class. We could use the Rainbow gem and refine String just for AP's use like described here. Then @ddarbyson can do the same in his scripts if needed. To detect whether the TTY supports colors, use tty-color.

from amazing_print.

Seems like this removing the monkey patches would also close #36

from amazing_print.

Hi @ddarbyson and thanks for using AmazingPrint and asking your question here!

There's two ways that colors can get set. With Ruby 2.7+ IRB supports colorization natively. This can be disabled with

IRB.conf[:USE_COLORIZE] = true

See https://docs.ruby-lang.org/en/2.7.0/IRB.html for more info

When colors are off in IRB but on in AmazingPrint, we can turn them off for a single output providing the plain: true option to ap

We see that nil has color again because we only turned off colors for formatting the string and not the return value of the ap command itself.

from amazing_print.

Hey, @HarlemSquirrel thanks for getting back to me.

I should clarify my intention. We are not using IRB and Amazing Print in the traditional sense. For us, AP has become a staple for writing programs so we require the module always.

But doing this conflicts with the String mixin you have.

https://github.com/amazing-print/amazing_print/blob/master/lib/amazing_print/core_ext/string.rb

By design, AP is injecting colour methods directly into the String class (i.e. "my string".red). You can see, if we are using AP as a staple gem, then these assumed methods actually conflict and assume too much.

For example, this conflicts with other code which handles the colour of the string. In this case, we opted to drop our previous gems which handle colour in favour of AP String mixins. I suppose that is okay, however, when using our program in none tty, there is no control over the colour.

So I would propose 2 changes

1. Don't assume anything should be mixed into standard Ruby classes unless specified
2. Allow the string.rb mixing to detect no tty and do not use colour

from amazing_print.

@ddarbyson We are doing some detection for tty here

I also agree that we should shouldn't mix in to standard library. For colorizing output there's the more robust rainbow gem that can add those mix ins.

@paddor what do you think?

I'm happy to review a PR to refactor how we colorize here but I don't have the time to dig in myself at the moment.

from amazing_print.

hi @HarlemSquirrel I investigated the colourize? method and I do see ENV['TERM'] control, however, it has no effect when using the colours methods provided by the String mixin in such as "mystringexample".red

Having said that, .red is not influenced by ENV['TERM'] and therefore using the colour mixins provided by AP String class still has coloured output in none tty sessions - which produces unescaped character sequences in email or log files; that's why i'm looking to turn off colour.

To reiterate, in our case, we use AP String mixin for colour, b/c there's no way of turning it off, and there's no point in introducing another gem to handle colour.

What would be ideal for AP is to introduce two improvements:

1. Have AP colour String mixin optional
2. Have AP colour String be influenced by ENV['TERM']

from amazing_print.

I really like the idea and appreciate the thought you guys have put into it. It really does solve all my issues at once and furthermore, our developments will benefit from the Rainbow gem.

I would be more than happy to test once it's ready.

from amazing_print.

@ddarbyson I currently don’t have a lot of spare time. If possible, could you provide a PR that does this? I guess that PR would remove the coloring methods and do something like using Rainbow wherever colors are used.

from amazing_print.

See: #91

from amazing_print.