search

cli / go-gh

A Go module for interacting with gh and the GitHub API from the command line.
https://pkg.go.dev/github.com/cli/go-gh/v2
MIT License
322 stars 45 forks source link

Expose IsColorForced #117

Closed heaths closed 1 year ago

heaths commented 1 year ago

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 term which 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.

heaths commented 1 year ago

@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 iostreams from cli/cli - it's powerful - but ended up doing something similar with heaths/go-console that I've been using in pretty much all my CLI-related modules.

As for the name, whatever you guys like is fine. :) I just what was there public. Alternatively, what about just wrapping both into an IsColorEnabled e.g.,

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.

mislav commented 1 year ago

Alternatively, what about just wrapping both into an IsColorEnabled

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.

I was hoping you'd port iostreams from cli/cli - it's powerful

The term package is an extraction from iostreams but I intentionally didn't want to export everything from iostreams because it's too large, unwieldy, and conflates too many concepts under a single level of abstraction.