Make colorizer respect AppSettings::ColorAlways / ColorChoice::Always

[abysssol]

AppSettings::ColorAlways currently appears to have no effect.
This was an issue in the past (see #1400), but was fixed in pr #1402; however, it seems to have been broken again.

I have tested the altered colorizer in a local project, and it seems to have the correct behavior.
No lints are triggered. All tests have passed.

This should fix issue #1400 ... again.

[epage]

I did some digging and it looks like #1402 was lost when transitioning from v2 to v3. The commit shows up in v2-masters history but not in masters.

I feel like colored support in the rust ecosystem lends itself to these kinds of problems. I'm working on an exploration of it to better understand what might help make things better.

[abysssol]

I don't have much experience in the matter, but the colored crate (which I have used recently) seems to provide a user-friendly api. By default, it automatically detects whether the output is to a terminal or pipe, and disables color accordingly. It also has functions to force colored output, or force no color (using static variables).
I'm not sure if it would be insufficient for this project somehow, but it seems like it would make this type of problem far less likely, since it already handles most of the coloring logic.
Of course, at this point in development it would be more trouble than it's worth to port to it from termcolor, but it seems to me that it would make errors of this type in any projects that use it a non-issue (thus fixing color support in the rust ecosystem).
Again, I don't have much experience, so if I'm wrong, and there are some other important factors that I don't know about, please let me know, I'd love to learn more.

[epage]

I don't have much experience in the matter, but the colored crate (which I have used recently) seems to provide a user-friendly api. By default, it automatically detects whether the output is to a terminal or pipe, and disables color accordingly. It also has functions to force colored output, or force no color (using static variables).
I'm not sure if it would be insufficient for this project somehow, but it seems like it would make this type of problem far less likely, since it already handles most of the coloring logic.
Of course, at this point in development it would be more trouble than it's worth to port to it from termcolor, but it seems to me that it would make errors of this type in any projects that use it a non-issue (thus fixing color support in the rust ecosystem).
Again, I don't have much experience, so if I'm wrong, and there are some other important factors that I don't know about, please let me know, I'd love to learn more.

My interest isn't in any particular styling crate (there are a lot of different API trade offs) but in providing the tooling to ensure the user and the styling crates provide a good experience.

For example, while colored does a lot of things right, it only does tty detection on stdout and applies that to both stdout and stderr.

On another level of the problem, as an end user of clap and env_logger, you have to know the implementation details of the styling libraries to know when to override them yourselv because their logic is incomplete.

Like I said, its a mess which is why I'm looking into it. I'll have something posted to the rust-cli org soon-ish.

[epage]

@AbyssAbuse see rust-cli/team#15 (comment)