diff --git a/internal/config/config.go b/internal/config/config.go index 476154d66..51116df6c 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -12,9 +12,11 @@ import ( o "github.com/cli/cli/v2/pkg/option" ghauth "github.com/cli/go-gh/v2/pkg/auth" ghConfig "github.com/cli/go-gh/v2/pkg/config" + xcolor "github.com/cli/go-gh/v2/pkg/x/color" ) const ( + accessibleColorsKey = xcolor.AccessibleColorsSetting aliasesKey = "aliases" browserKey = "browser" editorKey = "editor" @@ -108,6 +110,11 @@ func (c *cfg) Authentication() gh.AuthConfig { return &AuthConfig{cfg: c.cfg} } +func (c *cfg) AccessibleColors(hostname string) gh.ConfigEntry { + // Intentionally panic if there is no user provided value or default value (which would be a programmer error) + return c.GetOrDefault(hostname, accessibleColorsKey).Unwrap() +} + func (c *cfg) Browser(hostname string) gh.ConfigEntry { // Intentionally panic if there is no user provided value or default value (which would be a programmer error) return c.GetOrDefault(hostname, browserKey).Unwrap() @@ -532,6 +539,8 @@ aliases: http_unix_socket: # What web browser gh should use when opening URLs. If blank, will refer to environment. browser: +# Preference for accessible colors that can be customized. This is a global config that cannot be overridden by hostname. Supported values: enabled, disabled +accessible_colors: disabled ` type ConfigOption struct { @@ -602,6 +611,15 @@ var Options = []ConfigOption{ return c.Browser(hostname).Value }, }, + { + Key: accessibleColorsKey, + Description: "toggle preference for accessible colors that can be customized", + DefaultValue: "disabled", + AllowedValues: []string{"enabled", "disabled"}, + CurrentValue: func(c gh.Config, hostname string) string { + return c.AccessibleColors(hostname).Value + }, + }, } func HomeDirPath(subdir string) (string, error) { diff --git a/internal/gh/gh.go b/internal/gh/gh.go index 8e640c41a..4f3ece204 100644 --- a/internal/gh/gh.go +++ b/internal/gh/gh.go @@ -35,6 +35,8 @@ type Config interface { // Set provides primitive access for setting configuration values, optionally scoped by host. Set(hostname string, key string, value string) + // AccessibleColors returns the configured accessible colors preference, optionally scoped by host. + AccessibleColors(hostname string) ConfigEntry // Browser returns the configured browser, optionally scoped by host. Browser(hostname string) ConfigEntry // Editor returns the configured editor, optionally scoped by host. diff --git a/internal/gh/mock/config.go b/internal/gh/mock/config.go index 569af1fac..ecb8814b8 100644 --- a/internal/gh/mock/config.go +++ b/internal/gh/mock/config.go @@ -19,6 +19,9 @@ var _ gh.Config = &ConfigMock{} // // // make and configure a mocked gh.Config // mockedConfig := &ConfigMock{ +// AccessibleColorsFunc: func(hostname string) gh.ConfigEntry { +// panic("mock out the AccessibleColors method") +// }, // AliasesFunc: func() gh.AliasConfig { // panic("mock out the Aliases method") // }, @@ -71,6 +74,9 @@ var _ gh.Config = &ConfigMock{} // // } type ConfigMock struct { + // AccessibleColorsFunc mocks the AccessibleColors method. + AccessibleColorsFunc func(hostname string) gh.ConfigEntry + // AliasesFunc mocks the Aliases method. AliasesFunc func() gh.AliasConfig @@ -118,6 +124,11 @@ type ConfigMock struct { // calls tracks calls to the methods. calls struct { + // AccessibleColors holds details about calls to the AccessibleColors method. + AccessibleColors []struct { + // Hostname is the hostname argument value. + Hostname string + } // Aliases holds details about calls to the Aliases method. Aliases []struct { } @@ -190,6 +201,7 @@ type ConfigMock struct { Write []struct { } } + lockAccessibleColors sync.RWMutex lockAliases sync.RWMutex lockAuthentication sync.RWMutex lockBrowser sync.RWMutex @@ -207,6 +219,38 @@ type ConfigMock struct { lockWrite sync.RWMutex } +// AccessibleColors calls AccessibleColorsFunc. +func (mock *ConfigMock) AccessibleColors(hostname string) gh.ConfigEntry { + if mock.AccessibleColorsFunc == nil { + panic("ConfigMock.AccessibleColorsFunc: method is nil but Config.AccessibleColors was just called") + } + callInfo := struct { + Hostname string + }{ + Hostname: hostname, + } + mock.lockAccessibleColors.Lock() + mock.calls.AccessibleColors = append(mock.calls.AccessibleColors, callInfo) + mock.lockAccessibleColors.Unlock() + return mock.AccessibleColorsFunc(hostname) +} + +// AccessibleColorsCalls gets all the calls that were made to AccessibleColors. +// Check the length with: +// +// len(mockedConfig.AccessibleColorsCalls()) +func (mock *ConfigMock) AccessibleColorsCalls() []struct { + Hostname string +} { + var calls []struct { + Hostname string + } + mock.lockAccessibleColors.RLock() + calls = mock.calls.AccessibleColors + mock.lockAccessibleColors.RUnlock() + return calls +} + // Aliases calls AliasesFunc. func (mock *ConfigMock) Aliases() gh.AliasConfig { if mock.AliasesFunc == nil { diff --git a/pkg/cmd/factory/default.go b/pkg/cmd/factory/default.go index 923651487..598be0747 100644 --- a/pkg/cmd/factory/default.go +++ b/pkg/cmd/factory/default.go @@ -19,6 +19,7 @@ import ( "github.com/cli/cli/v2/pkg/cmd/extension" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" + xcolor "github.com/cli/go-gh/v2/pkg/x/color" ) var ssoHeader string @@ -292,6 +293,8 @@ func ioStreams(f *cmdutil.Factory) *iostreams.IOStreams { io.SetPager(pager.Value) } + io.SetAccessibleColorsEnabled(xcolor.IsAccessibleColorsEnabled()) + return io } diff --git a/pkg/iostreams/color.go b/pkg/iostreams/color.go index 07fdcd79f..3e2d08ac2 100644 --- a/pkg/iostreams/color.go +++ b/pkg/iostreams/color.go @@ -30,7 +30,9 @@ var ( greenBold = ansi.ColorFunc("green+b") highlightStart = ansi.ColorCode(highlightStyle) highlight = ansi.ColorFunc(highlightStyle) + darkThemeMuted = ansi.ColorFunc("white+d") darkThemeTableHeader = ansi.ColorFunc("white+du") + lightThemeMuted = ansi.ColorFunc("black+h") lightThemeTableHeader = ansi.ColorFunc("black+hu") noThemeTableHeader = ansi.ColorFunc("default+u") @@ -42,20 +44,22 @@ var ( // 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 { +func NewColorScheme(enabled, is256enabled, trueColor, accessibleColors bool, theme string) *ColorScheme { return &ColorScheme{ - enabled: enabled, - is256enabled: is256enabled, - hasTrueColor: trueColor, - theme: theme, + enabled: enabled, + is256enabled: is256enabled, + hasTrueColor: trueColor, + accessibleColors: accessibleColors, + theme: theme, } } type ColorScheme struct { - enabled bool - is256enabled bool - hasTrueColor bool - theme string + enabled bool + is256enabled bool + hasTrueColor bool + accessibleColors bool + theme string } func (c *ColorScheme) Enabled() bool { @@ -73,6 +77,29 @@ func (c *ColorScheme) Boldf(t string, args ...interface{}) string { return c.Bold(fmt.Sprintf(t, args...)) } +func (c *ColorScheme) Muted(t string) string { + // Fallback to previous logic if accessible colors preview is disabled. + if !c.accessibleColors { + return c.Gray(t) + } + + // Muted text is only stylized if color is enabled. + if !c.enabled { + return t + } + + switch c.theme { + case LightTheme: + return lightThemeMuted(t) + default: + return darkThemeMuted(t) + } +} + +func (c *ColorScheme) Mutedf(t string, args ...interface{}) string { + return c.Muted(fmt.Sprintf(t, args...)) +} + func (c *ColorScheme) Red(t string) string { if !c.enabled { return t @@ -113,6 +140,7 @@ func (c *ColorScheme) GreenBold(t string) string { return greenBold(t) } +// Deprecated: Use Muted instead for thematically contrasting color. func (c *ColorScheme) Gray(t string) string { if !c.enabled { return t @@ -123,6 +151,7 @@ func (c *ColorScheme) Gray(t string) string { return gray(t) } +// Deprecated: Use Mutedf instead for thematically contrasting color. func (c *ColorScheme) Grayf(t string, args ...interface{}) string { return c.Gray(fmt.Sprintf(t, args...)) } diff --git a/pkg/iostreams/color_test.go b/pkg/iostreams/color_test.go index b35c2eb73..a399e6d58 100644 --- a/pkg/iostreams/color_test.go +++ b/pkg/iostreams/color_test.go @@ -20,28 +20,28 @@ func TestColorFromRGB(t *testing.T) { hex: "fc0303", text: "red", wants: "\033[38;2;252;3;3mred\033[0m", - cs: NewColorScheme(true, true, true, NoTheme), + cs: NewColorScheme(true, true, true, false, NoTheme), }, { name: "no truecolor", hex: "fc0303", text: "red", wants: "red", - cs: NewColorScheme(true, true, false, NoTheme), + cs: NewColorScheme(true, true, false, false, NoTheme), }, { name: "no color", hex: "fc0303", text: "red", wants: "red", - cs: NewColorScheme(false, false, false, NoTheme), + cs: NewColorScheme(false, false, false, false, NoTheme), }, { name: "invalid hex", hex: "fc0", text: "red", wants: "red", - cs: NewColorScheme(false, false, false, NoTheme), + cs: NewColorScheme(false, false, false, false, NoTheme), }, } @@ -64,28 +64,28 @@ func TestHexToRGB(t *testing.T) { hex: "fc0303", text: "red", wants: "\033[38;2;252;3;3mred\033[0m", - cs: NewColorScheme(true, true, true, NoTheme), + cs: NewColorScheme(true, true, true, false, NoTheme), }, { name: "no truecolor", hex: "fc0303", text: "red", wants: "red", - cs: NewColorScheme(true, true, false, NoTheme), + cs: NewColorScheme(true, true, false, false, NoTheme), }, { name: "no color", hex: "fc0303", text: "red", wants: "red", - cs: NewColorScheme(false, false, false, NoTheme), + cs: NewColorScheme(false, false, false, false, NoTheme), }, { name: "invalid hex", hex: "fc0", text: "red", wants: "red", - cs: NewColorScheme(false, false, false, NoTheme), + cs: NewColorScheme(false, false, false, false, NoTheme), }, } @@ -109,61 +109,61 @@ func TestTableHeader(t *testing.T) { }{ { name: "when color is disabled, text is not stylized", - cs: NewColorScheme(false, false, false, NoTheme), + cs: NewColorScheme(false, false, false, true, NoTheme), input: "this should not be stylized", expected: "this should not be stylized", }, { name: "when 4-bit color is enabled but no theme, 4-bit default color and underline are used", - cs: NewColorScheme(true, false, false, NoTheme), + cs: NewColorScheme(true, false, false, true, NoTheme), input: "this should have no explicit color but underlined", expected: fmt.Sprintf("%sthis should have no explicit color but underlined%s", defaultUnderline, reset), }, { name: "when 4-bit color is enabled and theme is light, 4-bit dark color and underline are used", - cs: NewColorScheme(true, false, false, LightTheme), + cs: NewColorScheme(true, false, false, true, LightTheme), input: "this should have dark foreground color and underlined", expected: fmt.Sprintf("%sthis should have dark foreground color and underlined%s", brightBlackUnderline, reset), }, { name: "when 4-bit color is enabled and theme is dark, 4-bit light color and underline are used", - cs: NewColorScheme(true, false, false, DarkTheme), + cs: NewColorScheme(true, false, false, true, DarkTheme), input: "this should have light foreground color and underlined", expected: fmt.Sprintf("%sthis should have light foreground color and underlined%s", dimBlackUnderline, reset), }, { name: "when 8-bit color is enabled but no theme, 4-bit default color and underline are used", - cs: NewColorScheme(true, true, false, NoTheme), + cs: NewColorScheme(true, true, false, true, NoTheme), input: "this should have no explicit color but underlined", expected: fmt.Sprintf("%sthis should have no explicit color but underlined%s", defaultUnderline, reset), }, { name: "when 8-bit color is enabled and theme is light, 4-bit dark color and underline are used", - cs: NewColorScheme(true, true, false, LightTheme), + cs: NewColorScheme(true, true, false, true, LightTheme), input: "this should have dark foreground color and underlined", expected: fmt.Sprintf("%sthis should have dark foreground color and underlined%s", brightBlackUnderline, reset), }, { name: "when 8-bit color is true and theme is dark, 4-bit light color and underline are used", - cs: NewColorScheme(true, true, false, DarkTheme), + cs: NewColorScheme(true, true, false, true, DarkTheme), input: "this should have light foreground color and underlined", expected: fmt.Sprintf("%sthis should have light foreground color and underlined%s", dimBlackUnderline, reset), }, { name: "when 24-bit color is enabled but no theme, 4-bit default color and underline are used", - cs: NewColorScheme(true, true, true, NoTheme), + cs: NewColorScheme(true, true, true, true, NoTheme), input: "this should have no explicit color but underlined", expected: fmt.Sprintf("%sthis should have no explicit color but underlined%s", defaultUnderline, reset), }, { name: "when 24-bit color is enabled and theme is light, 4-bit dark color and underline are used", - cs: NewColorScheme(true, true, true, LightTheme), + cs: NewColorScheme(true, true, true, true, LightTheme), input: "this should have dark foreground color and underlined", expected: fmt.Sprintf("%sthis should have dark foreground color and underlined%s", brightBlackUnderline, reset), }, { name: "when 24-bit color is true and theme is dark, 4-bit light color and underline are used", - cs: NewColorScheme(true, true, true, DarkTheme), + cs: NewColorScheme(true, true, true, true, DarkTheme), input: "this should have light foreground color and underlined", expected: fmt.Sprintf("%sthis should have light foreground color and underlined%s", dimBlackUnderline, reset), }, diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index 6c12d7911..30981386b 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -70,8 +70,9 @@ type IOStreams struct { stderrTTYOverride bool stderrIsTTY bool - colorOverride bool - colorEnabled bool + colorOverride bool + colorEnabled bool + accessibleColorsEnabled bool pagerCommand string pagerProcess *os.Process @@ -366,7 +367,7 @@ func (s *IOStreams) TerminalWidth() int { } func (s *IOStreams) ColorScheme() *ColorScheme { - return NewColorScheme(s.ColorEnabled(), s.ColorSupport256(), s.HasTrueColor(), s.TerminalTheme()) + return NewColorScheme(s.ColorEnabled(), s.ColorSupport256(), s.HasTrueColor(), s.AccessibleColorsEnabled(), s.TerminalTheme()) } func (s *IOStreams) ReadUserFile(fn string) ([]byte, error) { @@ -391,6 +392,14 @@ func (s *IOStreams) TempFile(dir, pattern string) (*os.File, error) { return os.CreateTemp(dir, pattern) } +func (s *IOStreams) SetAccessibleColorsEnabled(enabled bool) { + s.accessibleColorsEnabled = enabled +} + +func (s *IOStreams) AccessibleColorsEnabled() bool { + return s.accessibleColorsEnabled +} + func System() *IOStreams { terminal := ghTerm.FromEnv()