Merge pull request #10709 from cli/andyfeller/table-datetime-contrast-github-cli-832

Ensure table datetime columns have thematic, customizable muted text
This commit is contained in:
Andy Feller 2025-04-02 17:20:15 -04:00 committed by GitHub
commit b52c131d4c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 163 additions and 55 deletions

View file

@ -99,11 +99,12 @@ func listRun(opts *ListOptions) error {
}
client := api.NewClientFromHTTP(httpClient)
cs := opts.IO.ColorScheme()
opts.IO.StartProgressIndicator()
result, err := shared.GetCaches(client, repo, shared.GetCachesOptions{Limit: opts.Limit, Sort: opts.Sort, Order: opts.Order, Key: opts.Key, Ref: opts.Ref})
opts.IO.StopProgressIndicator()
if err != nil {
return fmt.Errorf("%s Failed to get caches: %w", opts.IO.ColorScheme().FailureIcon(), err)
return fmt.Errorf("%s Failed to get caches: %w", cs.FailureIcon(), err)
}
if len(result.ActionsCaches) == 0 && opts.Exporter == nil {
@ -130,11 +131,11 @@ func listRun(opts *ListOptions) error {
tp := tableprinter.New(opts.IO, tableprinter.WithHeader("ID", "KEY", "SIZE", "CREATED", "ACCESSED"))
for _, cache := range result.ActionsCaches {
tp.AddField(opts.IO.ColorScheme().Cyan(fmt.Sprintf("%d", cache.Id)))
tp.AddField(cs.Cyanf("%d", cache.Id))
tp.AddField(cache.Key)
tp.AddField(humanFileSize(cache.SizeInBytes))
tp.AddTimeField(opts.Now, cache.CreatedAt, opts.IO.ColorScheme().Gray)
tp.AddTimeField(opts.Now, cache.LastAccessedAt, opts.IO.ColorScheme().Gray)
tp.AddTimeField(opts.Now, cache.CreatedAt, cs.Muted)
tp.AddTimeField(opts.Now, cache.LastAccessedAt, cs.Muted)
tp.EndRow()
}

View file

@ -152,7 +152,7 @@ func (a *App) List(ctx context.Context, opts *listOptions, exporter cmdutil.Expo
case false:
nameColor = cs.Yellow
case true:
nameColor = cs.Gray
nameColor = cs.Muted
}
tp.AddField(formattedName, tableprinter.WithColor(nameColor))
@ -172,7 +172,7 @@ func (a *App) List(ctx context.Context, opts *listOptions, exporter cmdutil.Expo
if err != nil {
return fmt.Errorf("error parsing date %q: %w", c.CreatedAt, err)
}
tp.AddTimeField(time.Now(), ct, cs.Gray)
tp.AddTimeField(time.Now(), ct, cs.Muted)
if hasNonProdVSCSTarget {
tp.AddField(c.VSCSTarget)

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

@ -206,7 +206,7 @@ func printTable(io *iostreams.IOStreams, gists []shared.Gist, filter *regexp.Reg
tableprinter.WithColor(highlightFilesFunc(&gist)),
)
tp.AddField(visibility, tableprinter.WithColor(visColor))
tp.AddTimeField(time.Now(), gist.UpdatedAt, cs.Gray)
tp.AddTimeField(time.Now(), gist.UpdatedAt, cs.Muted)
tp.EndRow()
}

View file

@ -487,8 +487,8 @@ func Test_listRun(t *testing.T) {
},
wantOut: heredoc.Docf(`
%[1]s[0;4;39mID %[1]s[0m %[1]s[0;4;39mDESCRIPTION %[1]s[0m %[1]s[0;4;39mFILES %[1]s[0m %[1]s[0;4;39mVISIBILITY%[1]s[0m %[1]s[0;4;39mUPDATED %[1]s[0m
1234 %[1]s[0;30;43mocto%[1]s[0m%[1]s[0;1;39m match in the description%[1]s[0m 1 file %[1]s[0;32mpublic %[1]s[0m %[1]s[38;5;242mabout 6 hours ago%[1]s[m
2345 %[1]s[0;1;39mmatch in the file name %[1]s[0m %[1]s[0;30;43m2 files%[1]s[0m %[1]s[0;31msecret %[1]s[0m %[1]s[38;5;242mabout 6 hours ago%[1]s[m
1234 %[1]s[0;30;43mocto%[1]s[0m%[1]s[0;1;39m match in the description%[1]s[0m 1 file %[1]s[0;32mpublic %[1]s[0m %[1]s[38;5;242mabout 6 hours ago%[1]s[0m
2345 %[1]s[0;1;39mmatch in the file name %[1]s[0m %[1]s[0;30;43m2 files%[1]s[0m %[1]s[0;31msecret %[1]s[0m %[1]s[38;5;242mabout 6 hours ago%[1]s[0m
`, "\x1b"),
},
{
@ -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, iostreams.NoTheme)
cs := iostreams.NewColorScheme(tt.color, false, false, false, iostreams.NoTheme)
matched := false
got, err := highlightMatch(tt.input, regex, &matched, cs.Blue, cs.Highlight)

View file

@ -78,7 +78,7 @@ func listRun(opts *ListOptions) error {
t.AddField(gpgKey.Emails.String())
t.AddField(gpgKey.KeyID)
t.AddField(gpgKey.PublicKey, tableprinter.WithTruncate(truncateMiddle))
t.AddTimeField(now, gpgKey.CreatedAt, cs.Gray)
t.AddTimeField(now, gpgKey.CreatedAt, cs.Muted)
expiresAt := gpgKey.ExpiresAt.Format(time.RFC3339)
if t.IsTTY() {
if gpgKey.ExpiresAt.IsZero() {
@ -87,7 +87,7 @@ func listRun(opts *ListOptions) error {
expiresAt = gpgKey.ExpiresAt.Format("2006-01-02")
}
}
t.AddField(expiresAt, tableprinter.WithColor(cs.Gray))
t.AddField(expiresAt, tableprinter.WithColor(cs.Muted))
t.EndRow()
}

View file

@ -224,7 +224,7 @@ func listRun(opts *ListOptions) error {
if !isTTY {
table.AddField(shared.PrStateWithDraft(&pr))
}
table.AddTimeField(opts.Now(), pr.CreatedAt, cs.Gray)
table.AddTimeField(opts.Now(), pr.CreatedAt, cs.Muted)
table.EndRow()
}
err = table.Render()

View file

@ -88,7 +88,7 @@ func listRun(opts *ListOptions) error {
}
table := tableprinter.New(opts.IO, tableprinter.WithHeader("Title", "Type", "Tag name", "Published"))
iofmt := opts.IO.ColorScheme()
cs := opts.IO.ColorScheme()
for _, rel := range releases {
title := text.RemoveExcessiveWhitespace(rel.Name)
if title == "" {
@ -100,13 +100,13 @@ func listRun(opts *ListOptions) error {
var badgeColor func(string) string
if rel.IsLatest {
badge = "Latest"
badgeColor = iofmt.Green
badgeColor = cs.Green
} else if rel.IsDraft {
badge = "Draft"
badgeColor = iofmt.Red
badgeColor = cs.Red
} else if rel.IsPrerelease {
badge = "Pre-release"
badgeColor = iofmt.Yellow
badgeColor = cs.Yellow
}
table.AddField(badge, tableprinter.WithColor(badgeColor))
@ -116,7 +116,7 @@ func listRun(opts *ListOptions) error {
if rel.PublishedAt.IsZero() {
pubDate = rel.CreatedAt
}
table.AddTimeField(time.Now(), pubDate, iofmt.Gray)
table.AddTimeField(time.Now(), pubDate, cs.Muted)
table.EndRow()
}
err = table.Render()

View file

@ -90,7 +90,7 @@ func listRun(opts *ListOptions) error {
}
t.AddField(sshType)
t.AddField(deployKey.Key, tableprinter.WithTruncate(truncateMiddle))
t.AddTimeField(now, deployKey.CreatedAt, cs.Gray)
t.AddTimeField(now, deployKey.CreatedAt, cs.Muted)
t.EndRow()
}

View file

@ -181,7 +181,7 @@ func listRun(opts *ListOptions) error {
totalMatchCount := len(listResult.Repositories)
for _, repo := range listResult.Repositories {
info := repoInfo(repo)
infoColor := cs.Gray
infoColor := cs.Muted
if repo.IsPrivate {
infoColor = cs.Yellow
@ -195,7 +195,7 @@ func listRun(opts *ListOptions) error {
tp.AddField(repo.NameWithOwner, tableprinter.WithColor(cs.Bold))
tp.AddField(text.RemoveExcessiveWhitespace(repo.Description))
tp.AddField(info, tableprinter.WithColor(infoColor))
tp.AddTimeField(opts.Now(), *t, cs.Gray)
tp.AddTimeField(opts.Now(), *t, cs.Muted)
tp.EndRow()
}

View file

@ -176,7 +176,7 @@ func listRun(opts *ListOptions) error {
tp.AddField(string(run.Event))
tp.AddField(fmt.Sprintf("%d", run.ID), tableprinter.WithColor(cs.Cyan))
tp.AddField(run.Duration(opts.now).String())
tp.AddTimeField(opts.now, run.StartedTime(), cs.Gray)
tp.AddTimeField(opts.now, run.StartedTime(), cs.Muted)
tp.EndRow()
}

View file

@ -89,11 +89,11 @@ func listRun(opts *ListOptions) error {
t.AddField(id)
t.AddField(sshKey.Key, tableprinter.WithTruncate(truncateMiddle))
t.AddField(sshKey.Type)
t.AddTimeField(now, sshKey.CreatedAt, cs.Gray)
t.AddTimeField(now, sshKey.CreatedAt, cs.Muted)
} else {
t.AddField(sshKey.Title)
t.AddField(sshKey.Key)
t.AddTimeField(now, sshKey.CreatedAt, cs.Gray)
t.AddTimeField(now, sshKey.CreatedAt, cs.Muted)
t.AddField(id)
t.AddField(sshKey.Type)
}

View file

@ -30,32 +30,36 @@ 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")
gray256 = func(t string) string {
return fmt.Sprintf("\x1b[%d;5;%dm%s\x1b[m", 38, 242, t)
return fmt.Sprintf("\x1b[%d;5;%dm%s\x1b[0m", 38, 242, t)
}
)
// 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,31 @@ 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)
case DarkTheme:
return darkThemeMuted(t)
default:
return 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 +142,7 @@ func (c *ColorScheme) GreenBold(t string) string {
return greenBold(t)
}
// Use Muted instead for thematically contrasting color.
func (c *ColorScheme) Gray(t string) string {
if !c.enabled {
return t
@ -123,6 +153,7 @@ func (c *ColorScheme) Gray(t string) string {
return gray(t)
}
// 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),
},
@ -175,3 +175,67 @@ func TestTableHeader(t *testing.T) {
})
}
}
func TestMuted(t *testing.T) {
reset := "\x1b[0m"
gray4bit := "\x1b[0;90m"
gray8bit := "\x1b[38;5;242m"
brightBlack4bit := "\x1b[0;90m"
dimBlack4bit := "\x1b[0;2;37m"
tests := []struct {
name string
cs *ColorScheme
input string
expected string
}{
{
name: "when color is disabled but accessible colors are disabled, text is not stylized",
cs: NewColorScheme(false, false, false, false, NoTheme),
input: "this should not be stylized",
expected: "this should not be stylized",
},
{
name: "when 4-bit color is enabled but accessible colors are disabled, legacy 4-bit gray color is used",
cs: NewColorScheme(true, false, false, false, NoTheme),
input: "this should be 4-bit gray",
expected: fmt.Sprintf("%sthis should be 4-bit gray%s", gray4bit, reset),
},
{
name: "when 8-bit color is enabled but accessible colors are disabled, legacy 8-bit gray color is used",
cs: NewColorScheme(true, true, false, false, NoTheme),
input: "this should be 8-bit gray",
expected: fmt.Sprintf("%sthis should be 8-bit gray%s", gray8bit, reset),
},
{
name: "when 24-bit color is enabled but accessible colors are disabled, legacy 8-bit gray color is used",
cs: NewColorScheme(true, true, true, false, NoTheme),
input: "this should be 8-bit gray",
expected: fmt.Sprintf("%sthis should be 8-bit gray%s", gray8bit, reset),
},
{
name: "when 4-bit color is enabled and theme is dark, 4-bit light color is used",
cs: NewColorScheme(true, true, true, true, DarkTheme),
input: "this should be 4-bit dim black",
expected: fmt.Sprintf("%sthis should be 4-bit dim black%s", dimBlack4bit, reset),
},
{
name: "when 4-bit color is enabled and theme is light, 4-bit dark color is used",
cs: NewColorScheme(true, true, true, true, LightTheme),
input: "this should be 4-bit bright black",
expected: fmt.Sprintf("%sthis should be 4-bit bright black%s", brightBlack4bit, reset),
},
{
name: "when 4-bit color is enabled but no theme, 4-bit default color is used",
cs: NewColorScheme(true, true, true, true, NoTheme),
input: "this should have no explicit color",
expected: "this should have no explicit color",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.expected, tt.cs.Muted(tt.input))
})
}
}

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