Add support for NO_COLOR environment variable #143
Conversation
djmattyg007
force-pushed
the
supprt_nocolor_env
branch
from
3360bc9 to
cde3bfb
Compare
|
I've updated the PR to resolve the issue building on windows. |
src/cli/main.rs
Outdated
| match mode { | ||
| Some(ansi::Mode::Ansi8Bit) => { | ||
| if global_matches.subcommand_name() != Some("paint") | ||
| && global_matches.subcommand_name() != Some("colorcheck") |
There was a problem hiding this comment.
I like that you moved the long print command to a new function. But why split the if into a match+if?
There was a problem hiding this comment.
I'm very new to rust, so I'm not really sure of how best to go about improving this. It's just my inexperience.
I believe you can match against multiple values, but I'm not sure how to include a negation in a match block, which I think you would need to replicate the above check. Would you be able to let me know what I'm missing?
There was a problem hiding this comment.
Ah, you had to change it because mode is now an Option<…>. But I think you could still simply compare to Some(ansi::Mode::Ansi8Bit):
if mode == Some(ansi::Mode::Ansi8Bit)
&& global_matches.subcommand_name() != Some("paint")
&& global_matches.subcommand_name() != Some("colorcheck")There's not necessarily a need for a match if it's a simple comparison. It would be different if you want to match on Some(_) or Some(mode) and then decide based on the mode.
|
This looks good, thank you for your contribution! |
djmattyg007
force-pushed
the
supprt_nocolor_env
branch
2 times, most recently
from
78801c0 to
d8d0459
Compare
djmattyg007
force-pushed
the
supprt_nocolor_env
branch
from
d8d0459 to
517e9f5
Compare
|
@sharkdp Thank you for your guidance 🙏 I've updated the PR, hopefully it's good to go 🙂 |
src/ansi.rs
Outdated
| use std::env; | ||
| let env_nocolor = env::var_os("NO_COLOR"); | ||
| match env_nocolor { | ||
| Some(val) => None, |
There was a problem hiding this comment.
This can be Some(_) => None to silence the clippy warnings.
There was a problem hiding this comment.
I've updated the PR with this change
This implements the NO_COLOR environment variable standard, as per https://no-color.org/
djmattyg007
force-pushed
the
supprt_nocolor_env
branch
from
517e9f5 to
767fac4
Compare
This implements the NO_COLOR environment variable standard, as per
https://no-color.org/