Skip to content

Disable color outputs using NO_COLOR env var#9357

Merged
brandond merged 3 commits intok3s-io:masterfrom
rishinair11:master
Mar 5, 2024
Merged

Disable color outputs using NO_COLOR env var#9357
brandond merged 3 commits intok3s-io:masterfrom
rishinair11:master

Conversation

@rishinair11
Copy link
Copy Markdown
Contributor

@rishinair11 rishinair11 commented Feb 6, 2024

Proposed Changes

Setting this environment variable will not wrap the text in ANSI color codes, so that we can print a raw output.
This helps us a lot when piping the output to a text file, ANSI color codes make it difficult to read through the output in a text editor.

Types of Changes

Introduced a new environment variable called NO_COLOR.
NO_COLOR=1 will disable the wrapping of output text in ANSI color codes.
Default functionality will be maintained if the env var is not set at all.

Verification

Testing

Ran this suit of tests:

  • Default usage:
    Screenshot 2024-02-29 at 16 53 31
  • Usage with NO_COLOR env var:
    Screenshot 2024-02-29 at 16 53 56
    NO_COLOR can be set to whatever.

Linked Issues

User-Facing Change

To enable raw output for the `check-config` subcommand, you may now set NO_COLOR=1

Further Comments

Let me know if the variable name is not suitable, or any other questions/discussions. Will be happy to refactor my solution if needed.

Setting this environment variable will not wrap the text in color ANSI code, so that we can print a raw output.

Signed-off-by: Rishikesh Nair <alienware505@gmail.com>
Comment thread contrib/util/check-config.sh Outdated
printf '\033['"$codes"'m'
}
wrap_color() {
if [ $RAW_OUTPUT -eq 1 ]; then
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems fine, but an even more minimal change might be to make color() return early if this env var is set?

Also, I think NO_COLOR is a more widely supported variable to use for this. If it is set to anything other than an empty string, color output should be suppressed.

Copy link
Copy Markdown
Contributor Author

@rishinair11 rishinair11 Feb 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that makes sense.
I changed RAW_OUTPUT -> NO_COLOR and changed the behaviour to check if NO_COLOR is set anything other than an empty string, color output will be suppressed.
e21f1d7

I'll also update the PR name and description to reflect these new changes.

Also, if NO_COLOR is empty, output will be colored, otherwise not colored.

Signed-off-by: Rishikesh Nair <alienware505@gmail.com>
@rishinair11 rishinair11 changed the title Disable color outputs using RAW_OUTPUT env var Disable color outputs using NO_COLOR env var Feb 29, 2024
@rishinair11 rishinair11 requested a review from brandond February 29, 2024 11:31
Comment thread contrib/util/check-config.sh Outdated
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 41.93%. Comparing base (57482a1) to head (e21f1d7).
Report is 48 commits behind head on master.

❗ Current head e21f1d7 differs from pull request most recent head 91b948f. Consider uploading reports for the commit 91b948f to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9357      +/-   ##
==========================================
- Coverage   45.69%   41.93%   -3.76%     
==========================================
  Files         154      154              
  Lines       16623    13523    -3100     
==========================================
- Hits         7596     5671    -1925     
+ Misses       7814     6735    -1079     
+ Partials     1213     1117      -96     
Flag Coverage Δ
e2etests ?
inttests 38.49% <ø> (+0.91%) ⬆️
unittests 15.96% <ø> (+0.54%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Co-authored-by: Brad Davidson <brad@oatmail.org>
Signed-off-by: Rishikesh Nair <42700059+rishinair11@users.noreply.github.com>
@rishinair11
Copy link
Copy Markdown
Contributor Author

Thank you for the review @brandond.
Is there something more I have to do to get this merged? Let me know, thank you.

@brandond
Copy link
Copy Markdown
Member

brandond commented Mar 5, 2024

No, you are set. We are working through some issues with scheduling work for the next release cycle, so this may remain on hold for a bit longer.

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.

3 participants