From ca2f2b18ca507db40c2b7a72f371be8e4fc98b32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 4 Sep 2020 21:32:58 +0200 Subject: [PATCH 1/5] Add support for CLICOLOR "standard" The CLICOLOR_FORCE environment variable can now be used to force color output even when stdout is redirected. https://bixense.com/clicolors/ --- pkg/cmd/root/root.go | 7 ++++++- pkg/iostreams/iostreams.go | 5 ++++- utils/color.go | 6 +++++- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 252636e52..fe2d23d8c 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -61,7 +61,12 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) *cobra.Command { GLAMOUR_STYLE: the style to use for rendering Markdown. See https://github.com/charmbracelet/glamour#styles - NO_COLOR: avoid printing ANSI escape sequences for color output. + NO_COLOR: set to any value to avoid printing ANSI escape sequences for color output. + + CLICOLOR: set to "0" to disable printing ANSI colors in output. + + CLICOLOR_FORCE: set to a value other than "0" to keep ANSI colors in output + even when the output is piped. `), }, } diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index e9b9c2220..bbaaecda4 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -106,11 +106,14 @@ func System() *IOStreams { stdoutIsTTY := isTerminal(os.Stdout) stderrIsTTY := isTerminal(os.Stderr) + envNoColor := os.Getenv("NO_COLOR") != "" || os.Getenv("CLICOLOR") == "0" + envForceColor := os.Getenv("CLICOLOR_FORCE") != "" && os.Getenv("CLICOLOR_FORCE") != "0" + io := &IOStreams{ In: os.Stdin, Out: colorable.NewColorable(os.Stdout), ErrOut: colorable.NewColorable(os.Stderr), - colorEnabled: os.Getenv("NO_COLOR") == "" && stdoutIsTTY, + colorEnabled: envForceColor || (!envNoColor && stdoutIsTTY), } // prevent duplicate isTerminal queries now that we know the answer diff --git a/utils/color.go b/utils/color.go index 123e0927c..034dd54d7 100644 --- a/utils/color.go +++ b/utils/color.go @@ -39,7 +39,11 @@ func makeColorFunc(color string) func(string) string { } func isColorEnabled() bool { - if os.Getenv("NO_COLOR") != "" { + if os.Getenv("CLICOLOR_FORCE") != "" && os.Getenv("CLICOLOR_FORCE") != "0" { + return true + } + + if os.Getenv("NO_COLOR") != "" || os.Getenv("CLICOLOR") == "0" { return false } From d38a0c5d842c78d6f17cbe8db29f4af4c231068b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 7 Sep 2020 12:50:12 +0200 Subject: [PATCH 2/5] Encapsulate checking color-controlling environment variables --- pkg/iostreams/color.go | 11 +++ pkg/iostreams/color_test.go | 145 ++++++++++++++++++++++++++++++++++++ pkg/iostreams/iostreams.go | 5 +- utils/color.go | 5 +- 4 files changed, 160 insertions(+), 6 deletions(-) create mode 100644 pkg/iostreams/color.go create mode 100644 pkg/iostreams/color_test.go diff --git a/pkg/iostreams/color.go b/pkg/iostreams/color.go new file mode 100644 index 000000000..c5e476791 --- /dev/null +++ b/pkg/iostreams/color.go @@ -0,0 +1,11 @@ +package iostreams + +import "os" + +func EnvColorDisabled() bool { + return os.Getenv("NO_COLOR") != "" || os.Getenv("CLICOLOR") == "0" +} + +func EnvColorForced() bool { + return os.Getenv("CLICOLOR_FORCE") != "" && os.Getenv("CLICOLOR_FORCE") != "0" +} diff --git a/pkg/iostreams/color_test.go b/pkg/iostreams/color_test.go new file mode 100644 index 000000000..90b3ae024 --- /dev/null +++ b/pkg/iostreams/color_test.go @@ -0,0 +1,145 @@ +package iostreams + +import ( + "os" + "testing" +) + +func TestEnvColorDisabled(t *testing.T) { + orig_NO_COLOR := os.Getenv("NO_COLOR") + orig_CLICOLOR := os.Getenv("CLICOLOR") + orig_CLICOLOR_FORCE := os.Getenv("CLICOLOR_FORCE") + t.Cleanup(func() { + os.Setenv("NO_COLOR", orig_NO_COLOR) + os.Setenv("CLICOLOR", orig_CLICOLOR) + os.Setenv("CLICOLOR_FORCE", orig_CLICOLOR_FORCE) + }) + + tests := []struct { + name string + NO_COLOR string + CLICOLOR string + CLICOLOR_FORCE string + want bool + }{ + { + name: "pristine env", + NO_COLOR: "", + CLICOLOR: "", + CLICOLOR_FORCE: "", + want: false, + }, + { + name: "NO_COLOR enabled", + NO_COLOR: "1", + CLICOLOR: "", + CLICOLOR_FORCE: "", + want: true, + }, + { + name: "CLICOLOR disabled", + NO_COLOR: "", + CLICOLOR: "0", + CLICOLOR_FORCE: "", + want: true, + }, + { + name: "CLICOLOR enabled", + NO_COLOR: "", + CLICOLOR: "1", + CLICOLOR_FORCE: "", + want: false, + }, + { + name: "CLICOLOR_FORCE has no effect", + NO_COLOR: "", + CLICOLOR: "", + CLICOLOR_FORCE: "1", + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + os.Setenv("NO_COLOR", tt.NO_COLOR) + os.Setenv("CLICOLOR", tt.CLICOLOR) + os.Setenv("CLICOLOR_FORCE", tt.CLICOLOR_FORCE) + + if got := EnvColorDisabled(); got != tt.want { + t.Errorf("EnvColorDisabled(): want %v, got %v", tt.want, got) + } + }) + } +} + +func TestEnvColorForced(t *testing.T) { + orig_NO_COLOR := os.Getenv("NO_COLOR") + orig_CLICOLOR := os.Getenv("CLICOLOR") + orig_CLICOLOR_FORCE := os.Getenv("CLICOLOR_FORCE") + t.Cleanup(func() { + os.Setenv("NO_COLOR", orig_NO_COLOR) + os.Setenv("CLICOLOR", orig_CLICOLOR) + os.Setenv("CLICOLOR_FORCE", orig_CLICOLOR_FORCE) + }) + + tests := []struct { + name string + NO_COLOR string + CLICOLOR string + CLICOLOR_FORCE string + want bool + }{ + { + name: "pristine env", + NO_COLOR: "", + CLICOLOR: "", + CLICOLOR_FORCE: "", + want: false, + }, + { + name: "NO_COLOR enabled", + NO_COLOR: "1", + CLICOLOR: "", + CLICOLOR_FORCE: "", + want: false, + }, + { + name: "CLICOLOR disabled", + NO_COLOR: "", + CLICOLOR: "0", + CLICOLOR_FORCE: "", + want: false, + }, + { + name: "CLICOLOR enabled", + NO_COLOR: "", + CLICOLOR: "1", + CLICOLOR_FORCE: "", + want: false, + }, + { + name: "CLICOLOR_FORCE enabled", + NO_COLOR: "", + CLICOLOR: "", + CLICOLOR_FORCE: "1", + want: true, + }, + { + name: "CLICOLOR_FORCE disabled", + NO_COLOR: "", + CLICOLOR: "", + CLICOLOR_FORCE: "0", + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + os.Setenv("NO_COLOR", tt.NO_COLOR) + os.Setenv("CLICOLOR", tt.CLICOLOR) + os.Setenv("CLICOLOR_FORCE", tt.CLICOLOR_FORCE) + + if got := EnvColorForced(); got != tt.want { + t.Errorf("EnvColorForced(): want %v, got %v", tt.want, got) + } + }) + } +} diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index bbaaecda4..6dc89cb57 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -106,14 +106,11 @@ func System() *IOStreams { stdoutIsTTY := isTerminal(os.Stdout) stderrIsTTY := isTerminal(os.Stderr) - envNoColor := os.Getenv("NO_COLOR") != "" || os.Getenv("CLICOLOR") == "0" - envForceColor := os.Getenv("CLICOLOR_FORCE") != "" && os.Getenv("CLICOLOR_FORCE") != "0" - io := &IOStreams{ In: os.Stdin, Out: colorable.NewColorable(os.Stdout), ErrOut: colorable.NewColorable(os.Stderr), - colorEnabled: envForceColor || (!envNoColor && stdoutIsTTY), + colorEnabled: EnvColorForced() || (!EnvColorDisabled() && stdoutIsTTY), } // prevent duplicate isTerminal queries now that we know the answer diff --git a/utils/color.go b/utils/color.go index 034dd54d7..0b276bf11 100644 --- a/utils/color.go +++ b/utils/color.go @@ -4,6 +4,7 @@ import ( "io" "os" + "github.com/cli/cli/pkg/iostreams" "github.com/mattn/go-colorable" "github.com/mgutz/ansi" ) @@ -39,11 +40,11 @@ func makeColorFunc(color string) func(string) string { } func isColorEnabled() bool { - if os.Getenv("CLICOLOR_FORCE") != "" && os.Getenv("CLICOLOR_FORCE") != "0" { + if iostreams.EnvColorForced() { return true } - if os.Getenv("NO_COLOR") != "" || os.Getenv("CLICOLOR") == "0" { + if iostreams.EnvColorDisabled() { return false } From a0e0f313634740d1d163d06cc8c13e60df2fe1a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 7 Sep 2020 12:57:02 +0200 Subject: [PATCH 3/5] Disable ANSI color in prompts when color is disabled --- cmd/gh/main.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 456195314..6920b8cc0 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -10,6 +10,7 @@ import ( "path" "strings" + surveyCore "github.com/AlecAivazis/survey/v2/core" "github.com/cli/cli/api" "github.com/cli/cli/command" "github.com/cli/cli/internal/config" @@ -43,6 +44,10 @@ func main() { cmdFactory := factory.New(command.Version) stderr := cmdFactory.IOStreams.ErrOut + if !cmdFactory.IOStreams.ColorEnabled() { + surveyCore.DisableColor = true + } + rootCmd := root.NewCmdRoot(cmdFactory, command.Version, command.BuildDate) expandedArgs := []string{} From 88aacc14f0e05a448e018bb9d00082c535d6871b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 16 Sep 2020 17:50:31 +0200 Subject: [PATCH 4/5] Fix gray color presentation across terminals We used to send the ANSI sequence for "bright black" when we wanted gray, but this color turns out to not be visible in some popular color schemes. Instead, when we detect a 256-color terminal, switch to displaying a color sequence for gray that is consistent and does not depend on terminal color scheme. --- pkg/iostreams/color.go | 31 ++++++++++++++++++++++++++++--- pkg/iostreams/iostreams.go | 8 +++++++- utils/color.go | 4 ++++ 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/pkg/iostreams/color.go b/pkg/iostreams/color.go index 92f4f31ae..2efe93c8b 100644 --- a/pkg/iostreams/color.go +++ b/pkg/iostreams/color.go @@ -1,7 +1,9 @@ package iostreams import ( + "fmt" "os" + "strings" "github.com/mgutz/ansi" ) @@ -15,6 +17,10 @@ var ( green = ansi.ColorFunc("green") gray = ansi.ColorFunc("black+h") bold = ansi.ColorFunc("default+b") + + gray256 = func(t string) string { + return fmt.Sprintf("\x1b[%d;5;%dm%s\x1b[m", 38, 242, t) + } ) func EnvColorDisabled() bool { @@ -25,12 +31,28 @@ func EnvColorForced() bool { return os.Getenv("CLICOLOR_FORCE") != "" && os.Getenv("CLICOLOR_FORCE") != "0" } -func NewColorScheme(enabled bool) *ColorScheme { - return &ColorScheme{enabled: enabled} +func Is256ColorSupported() bool { + term := os.Getenv("TERM") + colorterm := os.Getenv("COLORTERM") + + return strings.Contains(term, "256") || + strings.Contains(term, "24bit") || + strings.Contains(term, "truecolor") || + strings.Contains(colorterm, "256") || + strings.Contains(colorterm, "24bit") || + strings.Contains(colorterm, "truecolor") +} + +func NewColorScheme(enabled, is256enabled bool) *ColorScheme { + return &ColorScheme{ + enabled: enabled, + is256enabled: is256enabled, + } } type ColorScheme struct { - enabled bool + enabled bool + is256enabled bool } func (c *ColorScheme) Bold(t string) string { @@ -65,6 +87,9 @@ func (c *ColorScheme) Gray(t string) string { if !c.enabled { return t } + if c.is256enabled { + return gray256(t) + } return gray(t) } diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index f4ed4b032..968f06e06 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -26,6 +26,7 @@ type IOStreams struct { // the original (non-colorable) output stream originalOut io.Writer colorEnabled bool + is256enabled bool progressIndicatorEnabled bool progressIndicator *spinner.Spinner @@ -47,6 +48,10 @@ func (s *IOStreams) ColorEnabled() bool { return s.colorEnabled } +func (s *IOStreams) ColorSupport256() bool { + return s.is256enabled +} + func (s *IOStreams) SetStdinTTY(isTTY bool) { s.stdinTTYOverride = true s.stdinIsTTY = isTTY @@ -200,7 +205,7 @@ func (s *IOStreams) TerminalWidth() int { } func (s *IOStreams) ColorScheme() *ColorScheme { - return NewColorScheme(s.ColorEnabled()) + return NewColorScheme(s.ColorEnabled(), s.ColorSupport256()) } func System() *IOStreams { @@ -213,6 +218,7 @@ func System() *IOStreams { Out: colorable.NewColorable(os.Stdout), ErrOut: colorable.NewColorable(os.Stderr), colorEnabled: EnvColorForced() || (!EnvColorDisabled() && stdoutIsTTY), + is256enabled: Is256ColorSupported(), pagerCommand: os.Getenv("PAGER"), } diff --git a/utils/color.go b/utils/color.go index 0b276bf11..3e875acc4 100644 --- a/utils/color.go +++ b/utils/color.go @@ -1,6 +1,7 @@ package utils import ( + "fmt" "io" "os" @@ -33,6 +34,9 @@ func makeColorFunc(color string) func(string) string { cf := ansi.ColorFunc(color) return func(arg string) string { if isColorEnabled() { + if color == "black+h" && iostreams.Is256ColorSupported() { + return fmt.Sprintf("\x1b[%d;5;%dm%s\x1b[m", 38, 242, arg) + } return cf(arg) } return arg From b2de27c6241cba458c6177478ca5b3af5833fbc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 16 Sep 2020 17:52:25 +0200 Subject: [PATCH 5/5] Fix Survey's presentation of default values For default values for e.g. `Input` prompts, Survey uses the literal "white" color, which makes no sense on dark terminals and is literally invisible on light backgrounds. This overrides Survey to output a gray color for 256-color terminals and "default" for basic terminals. --- cmd/gh/main.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 71b5fd149..db329c3cd 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -46,6 +46,19 @@ func main() { stderr := cmdFactory.IOStreams.ErrOut if !cmdFactory.IOStreams.ColorEnabled() { surveyCore.DisableColor = true + } else { + // override survey's poor choice of color + surveyCore.TemplateFuncsWithColor["color"] = func(style string) string { + switch style { + case "white": + if cmdFactory.IOStreams.ColorSupport256() { + return fmt.Sprintf("\x1b[%d;5;%dm", 38, 242) + } + return ansi.ColorCode("default") + default: + return ansi.ColorCode(style) + } + } } rootCmd := root.NewCmdRoot(cmdFactory, command.Version, command.BuildDate)