From 6355e54e3cc1becd3a454ea5f0508f27f64b849a Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Fri, 21 Mar 2025 11:51:03 -0400 Subject: [PATCH] Ensure table headers are thematically contrasting This commit refactors the color format around table headers to ensure the GitHub CLI uses thematically appropriate colors based on dark background, light background, or no color at all. In order to do so, `ColorScheme` needs information from the terminal about the background appearance (dark, light, none) to determine appropriate muted color. --- internal/tableprinter/table_printer.go | 2 +- pkg/cmd/gist/list/list_test.go | 2 +- pkg/iostreams/color.go | 61 +++++++++++++++++--------- pkg/iostreams/color_test.go | 16 +++---- pkg/iostreams/iostreams.go | 2 +- 5 files changed, 51 insertions(+), 32 deletions(-) diff --git a/internal/tableprinter/table_printer.go b/internal/tableprinter/table_printer.go index e1454170f..69b22be12 100644 --- a/internal/tableprinter/table_printer.go +++ b/internal/tableprinter/table_printer.go @@ -80,7 +80,7 @@ func NewWithWriter(w io.Writer, isTTY bool, maxWidth int, cs *iostreams.ColorSch tp.AddHeader( upperCasedHeaders, WithPadding(paddingFunc), - WithColor(cs.LightGrayUnderline), + WithColor(cs.TableHeader), ) } diff --git a/pkg/cmd/gist/list/list_test.go b/pkg/cmd/gist/list/list_test.go index 0acfae109..870f58147 100644 --- a/pkg/cmd/gist/list/list_test.go +++ b/pkg/cmd/gist/list/list_test.go @@ -694,7 +694,7 @@ func Test_highlightMatch(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - cs := iostreams.NewColorScheme(tt.color, false, false) + cs := iostreams.NewColorScheme(tt.color, false, false, iostreams.NoTheme) matched := false got, err := highlightMatch(tt.input, regex, &matched, cs.Blue, cs.Highlight) diff --git a/pkg/iostreams/color.go b/pkg/iostreams/color.go index c8d48168f..f97d1f93e 100644 --- a/pkg/iostreams/color.go +++ b/pkg/iostreams/color.go @@ -9,34 +9,44 @@ import ( ) const ( + NoTheme = "none" + DarkTheme = "dark" + LightTheme = "light" highlightStyle = "black:yellow" ) +// Special cases like darkTableHeader / lightTableHeader are necessary when using color and modifiers +// (bold, underline, dim) because ansi.ColorFunc requires a foreground color and resets formats. var ( - magenta = ansi.ColorFunc("magenta") - cyan = ansi.ColorFunc("cyan") - red = ansi.ColorFunc("red") - yellow = ansi.ColorFunc("yellow") - blue = ansi.ColorFunc("blue") - green = ansi.ColorFunc("green") - gray = ansi.ColorFunc("black+h") - lightGrayUnderline = ansi.ColorFunc("white+du") - bold = ansi.ColorFunc("default+b") - cyanBold = ansi.ColorFunc("cyan+b") - greenBold = ansi.ColorFunc("green+b") - highlightStart = ansi.ColorCode(highlightStyle) - highlight = ansi.ColorFunc(highlightStyle) + magenta = ansi.ColorFunc("magenta") + cyan = ansi.ColorFunc("cyan") + red = ansi.ColorFunc("red") + yellow = ansi.ColorFunc("yellow") + blue = ansi.ColorFunc("blue") + green = ansi.ColorFunc("green") + gray = ansi.ColorFunc("black+h") + bold = ansi.ColorFunc("default+b") + cyanBold = ansi.ColorFunc("cyan+b") + greenBold = ansi.ColorFunc("green+b") + highlightStart = ansi.ColorCode(highlightStyle) + highlight = ansi.ColorFunc(highlightStyle) + darkTableHeader = ansi.ColorFunc("white+du") + lightTableHeader = ansi.ColorFunc("black+hu") gray256 = func(t string) string { return fmt.Sprintf("\x1b[%d;5;%dm%s\x1b[m", 38, 242, t) } ) -func NewColorScheme(enabled, is256enabled bool, trueColor bool) *ColorScheme { +// NewColorScheme initializes color logic based on provided terminal capabilities. +// Logic dealing with terminal theme detected, such as whether color is enabled, 8-bit color supported, true color supported, +// and terminal theme detected. +func NewColorScheme(enabled, is256enabled, trueColor bool, theme string) *ColorScheme { return &ColorScheme{ enabled: enabled, is256enabled: is256enabled, hasTrueColor: trueColor, + theme: theme, } } @@ -44,6 +54,7 @@ type ColorScheme struct { enabled bool is256enabled bool hasTrueColor bool + theme string } func (c *ColorScheme) Enabled() bool { @@ -115,13 +126,6 @@ func (c *ColorScheme) Grayf(t string, args ...interface{}) string { return c.Gray(fmt.Sprintf(t, args...)) } -func (c *ColorScheme) LightGrayUnderline(t string) string { - if !c.enabled { - return t - } - return lightGrayUnderline(t) -} - func (c *ColorScheme) Magenta(t string) string { if !c.enabled { return t @@ -254,3 +258,18 @@ func (c *ColorScheme) HexToRGB(hex string, x string) string { b, _ := strconv.ParseInt(hex[4:6], 16, 64) return fmt.Sprintf("\033[38;2;%d;%d;%dm%s\033[0m", r, g, b, x) } + +func (c *ColorScheme) TableHeader(t string) string { + if !c.enabled { + return t + } + + switch c.theme { + case DarkTheme: + return darkTableHeader(t) + case LightTheme: + return lightTableHeader(t) + default: + return t + } +} diff --git a/pkg/iostreams/color_test.go b/pkg/iostreams/color_test.go index 59fea53ed..9c84f72e1 100644 --- a/pkg/iostreams/color_test.go +++ b/pkg/iostreams/color_test.go @@ -19,28 +19,28 @@ func TestColorFromRGB(t *testing.T) { hex: "fc0303", text: "red", wants: "\033[38;2;252;3;3mred\033[0m", - cs: NewColorScheme(true, true, true), + cs: NewColorScheme(true, true, true, NoTheme), }, { name: "no truecolor", hex: "fc0303", text: "red", wants: "red", - cs: NewColorScheme(true, true, false), + cs: NewColorScheme(true, true, false, NoTheme), }, { name: "no color", hex: "fc0303", text: "red", wants: "red", - cs: NewColorScheme(false, false, false), + cs: NewColorScheme(false, false, false, NoTheme), }, { name: "invalid hex", hex: "fc0", text: "red", wants: "red", - cs: NewColorScheme(false, false, false), + cs: NewColorScheme(false, false, false, NoTheme), }, } @@ -63,28 +63,28 @@ func TestHexToRGB(t *testing.T) { hex: "fc0303", text: "red", wants: "\033[38;2;252;3;3mred\033[0m", - cs: NewColorScheme(true, true, true), + cs: NewColorScheme(true, true, true, NoTheme), }, { name: "no truecolor", hex: "fc0303", text: "red", wants: "red", - cs: NewColorScheme(true, true, false), + cs: NewColorScheme(true, true, false, NoTheme), }, { name: "no color", hex: "fc0303", text: "red", wants: "red", - cs: NewColorScheme(false, false, false), + cs: NewColorScheme(false, false, false, NoTheme), }, { name: "invalid hex", hex: "fc0", text: "red", wants: "red", - cs: NewColorScheme(false, false, false), + cs: NewColorScheme(false, false, false, NoTheme), }, } diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index 2bc712a3c..6c12d7911 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -366,7 +366,7 @@ func (s *IOStreams) TerminalWidth() int { } func (s *IOStreams) ColorScheme() *ColorScheme { - return NewColorScheme(s.ColorEnabled(), s.ColorSupport256(), s.HasTrueColor()) + return NewColorScheme(s.ColorEnabled(), s.ColorSupport256(), s.HasTrueColor(), s.TerminalTheme()) } func (s *IOStreams) ReadUserFile(fn string) ([]byte, error) {