-
-
Notifications
You must be signed in to change notification settings - Fork 294
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support NO_COLOR #726
Support NO_COLOR #726
Conversation
Just a small UX improvement for some people (that includes me). ref.: https://no-color.org
# Handle NO_COLOR as well: | ||
if os.environment.get('NO_COLOR') is not None: | ||
args.nc = True | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you actually tried to run this code? The lint fails because there's no environment
in the os
module 😅
(…try getenv()
instead)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getenv()
is preferred to environ.get()
Welcome to “don’t write code when tired”
The repo owner prefers it when the pull-requests contain one commit, BTW. (You can soft-reset to the 1st one, then amend and force-push the feature branch to achieve that.) |
I'm not sure whether the fact that Buku (soon...) adheres to certain conventions should necessarily be documented as part of Buku itself. NO_COLOR is not a Buku feature. |
It will be once the pull-request is merged 😅 (And the fact that it exists elsewhere doesn't mean anything – if it affects the runtime behaviour it should be documented so that end users are actually aware of it.) |
It's OK to add a note in the Wiki that Buku supports the env var |
Thank you! |
Added a note here: https://github.com/jarun/buku/wiki/Customize-colors |
Just a small UX improvement for some people (that includes me). ref.: https://no-color.org