Skip to content

config: respect NO_COLOR specification#778

Merged
lorenzo merged 1 commit intohadolint:masterfrom
m-ildefons:no-color-spec
Mar 3, 2022
Merged

config: respect NO_COLOR specification#778
lorenzo merged 1 commit intohadolint:masterfrom
m-ildefons:no-color-spec

Conversation

@m-ildefons
Copy link
Copy Markdown
Member

  • Decide whether to print ANSI color codes or not based on the
    presence, not the value of the NO_COLOR environment variable

The specification provided by https://no-color.org regarding the
interpretation of the NO_COLOR environment variable specifically state
that the value does not matter, only whether it is set or not should
change program behavior.
Therefore, Hadolint should respect that and ignore the value.

How to verify it

Assuming no other settings are in place, NO_COLOR= hadolint Dockerfile should result in uncolored output just like NO_COLOR=true hadolint Dockerfile and NO_COLOR=n hadolint Dockerfile. However unset NO_COLOR; hadolint Dockerfile should give colored output.

@lorenzo
Copy link
Copy Markdown
Member

lorenzo commented Feb 28, 2022

the test fails on windows for some weird reason

@m-ildefons
Copy link
Copy Markdown
Member Author

Hi,
yeah I've seen it, it's not weird, but I'm not sure how to resolve it. The problem is as follows:
no-color.org specifies that the value of $NO_COLOR is irrelevant and only whether it is set or not counts. However on unixoids an environment variable can be set to the empty string and count as set, but on Windows setting an environment variable to the empty string is the same as it being unset. Therefore on Windows, the expected outcome of setting $NO_COLOR to the empty string is color in the output whereas on unixoids there should be no color in the output in that case.

Maybe I can query what platform Hadolint is running on and adjust the tests behavior accordingly? The case where the setting is applied because the environment variable is set to the empty string only needs to be tested on unixoids because on Windows that is "impossible ©️ ™️".

@lorenzo
Copy link
Copy Markdown
Member

lorenzo commented Mar 1, 2022

@m-ildefons yeah, I think detecting the platform during the test and skipping that one on windows is best

- Decide whether to print ANSI color codes or not based on the
  _presence_, not the value of the NO_COLOR environment variable

The specification provided by https://no-color.org regarding the
interpretation of the NO_COLOR environment variable specifically state
that the value does not matter, only whether it is set or not should
change program behavior.
Therefore, Hadolint should respect that and ignore the value.

There is a platform specific distinction in the test suite because on
Windows it is expected that setting an environment variable to the empty
string is equal to unsetting it, in contrast to POSIX platforms where an
environment variable can be set to the empty string and still count as
set. With $NO_COLOR it only matters if it is set, not what it is set to,
the case on POSIX where it is set to the empty string should be
respected.
@lorenzo
Copy link
Copy Markdown
Member

lorenzo commented Mar 3, 2022

great, thanks again!

@lorenzo lorenzo merged commit b8a1f71 into hadolint:master Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants