fix(iostreams): spinner env var is now in factory

- Move env var evaluation to the factory defaults.
- Use only the label for the spinner instead of adding
"working..." every time. "working..." remains the default
when no label is provided.
This commit is contained in:
Kynan Ware 2025-04-14 15:07:11 -06:00
parent 3aadd51096
commit 3eba461afb
4 changed files with 88 additions and 20 deletions

View file

@ -6,6 +6,7 @@ import (
"net/http"
"os"
"regexp"
"slices"
"time"
"github.com/cli/cli/v2/api"
@ -283,6 +284,12 @@ func ioStreams(f *cmdutil.Factory) *iostreams.IOStreams {
io.SetNeverPrompt(true)
}
ghSpinnerDisabledValue, ghSpinnerDisabledIsSet := os.LookupEnv("GH_SPINNER_DISABLED")
falseyValues := []string{"false", "0", "no", ""}
if ghSpinnerDisabledIsSet && !slices.Contains(falseyValues, ghSpinnerDisabledValue) {
io.SetSpinnerDisabled(true)
}
// Pager precedence
// 1. GH_PAGER
// 2. pager from config

View file

@ -432,6 +432,56 @@ func Test_ioStreams_prompt(t *testing.T) {
}
}
func Test_ioStreams_spinnerDisabled(t *testing.T) {
tests := []struct {
name string
spinnerDisabled bool
env map[string]string
}{
{
name: "default config",
spinnerDisabled: false,
},
{
name: "spinner disabled via GH_SPINNER_DISABLED env var = 0",
env: map[string]string{"GH_SPINNER_DISABLED": "0"},
spinnerDisabled: false,
},
{
name: "spinner disabled via GH_SPINNER_DISABLED env var = false",
env: map[string]string{"GH_SPINNER_DISABLED": "false"},
spinnerDisabled: false,
},
{
name: "spinner disabled via GH_SPINNER_DISABLED env var = no",
env: map[string]string{"GH_SPINNER_DISABLED": "no"},
spinnerDisabled: false,
},
{
name: "spinner enabled via GH_SPINNER_DISABLED env var = 1",
env: map[string]string{"GH_SPINNER_DISABLED": "1"},
spinnerDisabled: true,
},
{
name: "spinner enabled via GH_SPINNER_DISABLED env var = true",
env: map[string]string{"GH_SPINNER_DISABLED": "true"},
spinnerDisabled: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if tt.env != nil {
for k, v := range tt.env {
t.Setenv(k, v)
}
}
f := New("1")
io := ioStreams(f)
assert.Equal(t, tt.spinnerDisabled, io.GetSpinnerDisabled())
})
}
}
func Test_ioStreams_colorLabels(t *testing.T) {
tests := []struct {
name string

View file

@ -8,7 +8,6 @@ import (
"os"
"os/exec"
"os/signal"
"slices"
"strings"
"sync"
"time"
@ -79,7 +78,8 @@ type IOStreams struct {
pagerCommand string
pagerProcess *os.Process
neverPrompt bool
neverPrompt bool
spinnerDisabled bool
TempFileOverride *os.File
}
@ -274,6 +274,14 @@ func (s *IOStreams) SetNeverPrompt(v bool) {
s.neverPrompt = v
}
func (s *IOStreams) GetSpinnerDisabled() bool {
return s.spinnerDisabled
}
func (s *IOStreams) SetSpinnerDisabled(v bool) {
s.spinnerDisabled = v
}
func (s *IOStreams) StartProgressIndicator() {
s.StartProgressIndicatorWithLabel("")
}
@ -295,13 +303,20 @@ func (s *IOStreams) StartProgressIndicatorWithLabel(label string) {
return
}
spinnerDisabledValue, spinnerDisabledIsSet := os.LookupEnv("GH_SPINNER_DISABLED")
falseyValues := []string{"false", "0", "no", ""}
var spinnerStyle []string
if spinnerDisabledIsSet && !slices.Contains(falseyValues, spinnerDisabledValue) {
// This progress indicator must be readable by screen readers
spinnerStyle = []string{"Working..."}
if s.spinnerDisabled {
// Default label when spinner disabled is "Working..."
if label == "" {
label = "Working..."
}
// Add an ellipsis to the label if it doesn't already have one.
ellipsis := "..."
if !strings.HasSuffix(label, ellipsis) {
label = label + ellipsis
}
spinnerStyle = []string{label}
} else {
// https://github.com/briandowns/spinner#available-character-sets
// ⣾ ⣷ ⣽ ⣻ ⡿
@ -309,7 +324,7 @@ func (s *IOStreams) StartProgressIndicatorWithLabel(label string) {
}
sp := spinner.New(spinnerStyle, 120*time.Millisecond, spinner.WithWriter(s.ErrOut), spinner.WithColor("fgCyan"))
if label != "" {
if label != "" && !s.spinnerDisabled {
sp.Prefix = label + " "
}

View file

@ -34,8 +34,7 @@ func TestStartProgressIndicatorWithLabel(t *testing.T) {
// from the console.
t.Run("progress indicator respects GH_SPINNER_DISABLED is true", func(t *testing.T) {
console := newTestVirtualTerminal(t)
io := newTestIOStreams(t, console)
t.Setenv("GH_SPINNER_DISABLED", "true")
io := newTestIOStreams(t, console, true)
done := make(chan error)
@ -57,8 +56,7 @@ func TestStartProgressIndicatorWithLabel(t *testing.T) {
t.Run("progress indicator respects GH_SPINNER_DISABLED is false", func(t *testing.T) {
console := newTestVirtualTerminal(t)
io := newTestIOStreams(t, console)
t.Setenv("GH_SPINNER_DISABLED", "false")
io := newTestIOStreams(t, console, false)
done := make(chan error)
@ -80,16 +78,13 @@ func TestStartProgressIndicatorWithLabel(t *testing.T) {
t.Run("progress indicator with GH_SPINNER_DISABLED shows label", func(t *testing.T) {
console := newTestVirtualTerminal(t)
io := newTestIOStreams(t, console)
t.Setenv("GH_SPINNER_DISABLED", "true")
io := newTestIOStreams(t, console, true)
progressIndicatorLabel := "downloading happiness"
done := make(chan error)
go func() {
_, err := console.ExpectString("Working...")
require.NoError(t, err)
_, err = console.ExpectString(progressIndicatorLabel)
_, err := console.ExpectString(progressIndicatorLabel + "...")
done <- err
}()
@ -106,7 +101,7 @@ func TestStartProgressIndicatorWithLabel(t *testing.T) {
t.Run("progress indicator shows label and spinner", func(t *testing.T) {
console := newTestVirtualTerminal(t)
io := newTestIOStreams(t, console)
io := newTestIOStreams(t, console, false)
progressIndicatorLabel := "downloading happiness"
done := make(chan error)
@ -156,7 +151,7 @@ func newTestVirtualTerminal(t *testing.T) *expect.Console {
return console
}
func newTestIOStreams(t *testing.T, console *expect.Console) *IOStreams {
func newTestIOStreams(t *testing.T, console *expect.Console, spinnerDisabled bool) *IOStreams {
t.Helper()
in := console.Tty()
@ -174,6 +169,7 @@ func newTestIOStreams(t *testing.T, console *expect.Console) *IOStreams {
term: fakeTerm{},
}
io.progressIndicatorEnabled = true
io.SetSpinnerDisabled(spinnerDisabled)
return io
}