Conversation
Since IsColorDisabled - based on env vars - is public, also expose IsColorForced so the gamut of GitHub CLI env vars for color overrides are available to callers.
samcoe
left a comment
There was a problem hiding this comment.
This seems reasonable to expose to me. My only thought is that I don't love the name. The only other option I could come up with was IsColorEnabledForcibly which is a bit verbose but I think more descriptive. Thoughts?
mislav
left a comment
There was a problem hiding this comment.
I'm up for adding this for symmetry with IsColorDisabled. However, I think that a typical caller should still instantiate a Term and then call t.IsColorEnabled() on it to get the “final” state of color enabledness after all the conditions have been applied.
BTW for mocking input and capturing output you could always wrap Term in an application-specific type and override In(), Out(), and ErrOut() streams there.
|
@mislav sure, but to mock whether it's a TTY - which only checks stdout (redirects can be separate, of course, for stdout and stderr), whether color is enabled, etc., you end up wrapping the whole thing. And if you want to add in theme a la charmbracelet there's even more work. When cli/go-gh was initially proposed, I was hoping you'd port As for the name, whatever you guys like is fine. :) I just what was there public. Alternatively, what about just wrapping both into an func IsColorEnabled() bool {
return os.Getenv("CLICOLOR_FORCE") != "" && os.Getenv("CLICOLOR_FORCE") != "0" ||
os.Getenv("NO_COLOR") == "" || os.Getenv("CLICOLOR") != "0")
}That would seem a lot easier for external callers: a single check that wraps the logic of cli/cli. |
I wanted to avoid wrapping too many checks in a single function call because that wouldn't be granular enough. For instance, color enabledness is not just a result of environment variables, but also whether stdout is a terminal.
The |
Since IsColorDisabled - based on env vars - is public, also expose IsColorForced so the gamut of GitHub CLI env vars for color overrides are available to callers.
This is desirable when using something other than
termwhich does not allow for mocking or otherwise capturing output. Since I use another library, access to APIs that encapsulate the various env vars the GitHub CLI (and extensions using this module) is ideal so my extensions can do a better job at matching the behavior of the CLI.