Refactor to remove Airbrussh::Colors in favor of SSHKit::Color about airbrussh HOT 2 CLOSED

I'm not sure - my gut feeling is that it's simpler not to add the colors in the first place if they are not needed, but perhaps this has downsides I haven't thought of.

One difference is that in the SSHKit approach, colors wouldn't be stripped from output of other commands called by SSHKit. I can't work out if this is a benefit or disadvantage. If I understand correctly, Airbrussh's config.color = false option strips the colors from the output of commands run by SSHKit. This could be useful if users want the formatter to do this but I'm not sure if that is a common use case.

The changes I made recently continue to allow SSHKit users to force colors to be on, but do not allow users to force them to be off. Since these haven't been released I'm not sure if there are use cases where users want to be able to force the colors to be off even though their output has a tty associated with it. Personally I would be in favour of adding support for the Airbrussh color config options to SSHKit. This was disussed a bit here: capistrano/sshkit#245, but things have moved on quite a bit, so maybe worth discussing again.

If we did add that new option, then in a future major release I would like to remove this colorize override in the SimpleTextFormatter. This would leave a single method - format_message on the SimpleTextFormatter. I think this could be reimplemented as 2 config options on the Pretty formatter:

1. Whether to show the command UUID
2. Whether to show the log level prefix (eg INFO, WARN etc)

The SimpleTextFormatter output is the same as the pretty formatter, but with each of these components omitted.

I realise all of this isn't really helping with the mission of making Airbrussh the default Capistrano formatter, but I think, if it makes sense, it would be good if Airbrussh could be configured by using the same options as SSHKit.

If we wanted to reuse the SSHKit color support, then one approach would be to use the colorize method from the abstract formatter, but without adding support for config.color = :true | :false | :auto, this would be a retrograde step for Airbrussh I guess.

I'm not sure if this helps at all? :(

from airbrussh.

I don't think this refactor is necessary. Sharing the Colors implementation between SSHKit and Capistrano further couples the internals of the two gems, with not much benefit.

from airbrussh.