Can't enable colors back

[dex4er]

Please revert 0c4bd34

I run helm diff in the CI/CD pipeline and I miss the color output from helm diff now. Actually even running it through "grep" on the console with the colors were useful. Then this change is a regression for me.

Maybe you can introduce some addtional option like "--color" or "--force-color" instead?

[dex4er]

OK I see it was PR #240 that made this regression.

[mumoshu]

@dex4er Thanks for reporting! "Regression" sounds like a strong word though.

@misberner WDYT? I think adding another flag --color to force color might be a good idea. I'm not yet convinced this is a regression and the PR/commit can just be reverted.

[dex4er]

Oh I didn't realise there is modern flamewar between color and no-color. Now I found https://no-color.org/ with reasonable proposition that NO_COLOR envvar should disable ANSI codes.

Detection of the terminal is not enough unfortunately because I use helm diff explicitely in CI/CD context and it is detected as non-terminal and has TERM=dumb envvar set. Then seeing a colors in the report file is very useful to quickly verify the changes in the new deployments then I'm very satisfied with original helm diff's feature.

Well, "regression" looked like neutral word for me and it might be described as "feature which doesn't work in newer version anymore" 😄

[misberner]

It's a bit indirect, but you can get the old behavior via --no-color=false. Does this not work? It might seem a bit magic that the default for this flag is not a constant (i.e., there is no value you can assign that matches exactly the auto-sense behavior you get by omitting the flag), but I found that preferable over introducing a new flag or worse, removing the existing one.

See the third example in the picture in the original PR.

[dex4er]

I can confirm --no-color=false works. Looks maybe a little ackward but at least it produces correct output in CI/CD context. Thanks a lot!

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[z0rc]

Here is the thing, TERM=dumb is way of communicating that terminal does not understand almost any terminfo features, especially color controls, see https://invisible-island.net/ncurses/terminfo.src.html#tic-dumb. It's perfectly reasonable for application to not emit any colors when running under such env. Ultimately it's up to CI tool to set correct TERM when it's actually able to understand color control chars.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[mumoshu]

Thank you everyone for sharing valuable feedback!

When I wrote my first response in this thread, I didn't know that TERM=dumb was used that way. I now believe helm-diff should just disable colors when TERM=dumb. For @dex4er's case, it looks like your CI did set a correct TERM so I'd say it's definitely helm-diff's fault that it didn't respect it(to disable colors).

I'm going to create a PR shortly.