Accessible color config boilerplate, colors update

This commit is focused on incorporating cli/go-gh accessible colors configuration setting into GitHub CLI experience along with new color function to supersede ColorScheme.Gray and ColorScheme.Grayf.

Originally, I was considering having all use of ColorScheme.Gray and ColorScheme.Grayf fallback to the new muted logic if accessible colors were enabled, however I decided not being that it exceeds the acceptance criteria.  This means that every command using ColorScheme.Gray needs to be updated to use ColorScheme.Muted
This commit is contained in:
Andy Feller 2025-03-31 11:19:51 -04:00
parent e30244686c
commit 346fab212b
7 changed files with 135 additions and 30 deletions

View file

@ -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) {

View file

@ -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.

View file

@ -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 {

View file

@ -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
}

View file

@ -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...))
}

View file

@ -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),
},

View file

@ -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()