From 3aadd51096931d55e0e32253660da46af2d1314d Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Fri, 11 Apr 2025 11:07:53 -0600 Subject: [PATCH 1/7] feat(iostreams): Option to opt out of spinners --- pkg/iostreams/iostreams.go | 18 +- .../iostreams_progress_indicator_test.go | 234 ++++++++++++++++++ 2 files changed, 249 insertions(+), 3 deletions(-) create mode 100644 pkg/iostreams/iostreams_progress_indicator_test.go diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index f5e3c2aee..8608978c5 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -8,6 +8,7 @@ import ( "os" "os/exec" "os/signal" + "slices" "strings" "sync" "time" @@ -294,9 +295,20 @@ func (s *IOStreams) StartProgressIndicatorWithLabel(label string) { return } - // https://github.com/briandowns/spinner#available-character-sets - dotStyle := spinner.CharSets[11] - sp := spinner.New(dotStyle, 120*time.Millisecond, spinner.WithWriter(s.ErrOut), spinner.WithColor("fgCyan")) + 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..."} + } else { + // https://github.com/briandowns/spinner#available-character-sets + // ⣾ ⣷ ⣽ ⣻ ⡿ + spinnerStyle = spinner.CharSets[11] + } + + sp := spinner.New(spinnerStyle, 120*time.Millisecond, spinner.WithWriter(s.ErrOut), spinner.WithColor("fgCyan")) if label != "" { sp.Prefix = label + " " } diff --git a/pkg/iostreams/iostreams_progress_indicator_test.go b/pkg/iostreams/iostreams_progress_indicator_test.go new file mode 100644 index 000000000..c7c374811 --- /dev/null +++ b/pkg/iostreams/iostreams_progress_indicator_test.go @@ -0,0 +1,234 @@ +//go:build !windows + +package iostreams + +import ( + "fmt" + "io" + "os" + "strings" + "testing" + "time" + + "github.com/Netflix/go-expect" + "github.com/creack/pty" + "github.com/hinshun/vt10x" + "github.com/stretchr/testify/require" +) + +func TestStartProgressIndicatorWithLabel(t *testing.T) { + osOut := os.Stdout + defer func() { os.Stdout = osOut }() + // Why do we need a channel in these tests to implement a timeout instead of + // relying on expect's timeout? + // + // Well, expect's timeout is based on the maximum time of a single read + // from the console. This works in cases like prompting where we block + // waiting for input because the console is not ready to be read. + // But in this case, we are not blocking waiting for input and stdout + // can be constantly read. This means the timeout will never be reached + // in the event of a expectation failure. + // To fix this, we need to implement our own timeout that is based + // specifically on the total time spent reading the console and waiting + // for the target string instead of the max time for a single read + // 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") + + done := make(chan error) + + go func() { + _, err := console.ExpectString("Working...") + done <- err + }() + + io.StartProgressIndicatorWithLabel("") + defer io.StopProgressIndicator() + + select { + case err := <-done: + require.NoError(t, err) + case <-time.After(2 * time.Second): + t.Fatal("Test timed out waiting for progress indicator") + } + }) + + 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") + + done := make(chan error) + + go func() { + _, err := console.ExpectString("⣾") + done <- err + }() + + io.StartProgressIndicatorWithLabel("") + defer io.StopProgressIndicator() + + select { + case err := <-done: + require.NoError(t, err) + case <-time.After(2 * time.Second): + t.Fatal("Test timed out waiting for progress indicator") + } + }) + + 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") + progressIndicatorLabel := "downloading happiness" + + done := make(chan error) + + go func() { + _, err := console.ExpectString("Working...") + require.NoError(t, err) + _, err = console.ExpectString(progressIndicatorLabel) + done <- err + }() + + io.StartProgressIndicatorWithLabel(progressIndicatorLabel) + defer io.StopProgressIndicator() + + select { + case err := <-done: + require.NoError(t, err) + case <-time.After(2 * time.Second): + t.Fatal("Test timed out waiting for progress indicator") + } + }) + + t.Run("progress indicator shows label and spinner", func(t *testing.T) { + console := newTestVirtualTerminal(t) + io := newTestIOStreams(t, console) + progressIndicatorLabel := "downloading happiness" + + done := make(chan error) + + go func() { + _, err := console.ExpectString(progressIndicatorLabel) + require.NoError(t, err) + _, err = console.ExpectString("⣾") + done <- err + }() + + io.StartProgressIndicatorWithLabel(progressIndicatorLabel) + defer io.StopProgressIndicator() + + select { + case err := <-done: + require.NoError(t, err) + case <-time.After(2 * time.Second): + t.Fatal("Test timed out waiting for progress indicator") + } + }) +} + +func newTestVirtualTerminal(t *testing.T) *expect.Console { + t.Helper() + + // Create a PTY and hook up a virtual terminal emulator + ptm, pts, err := pty.Open() + require.NoError(t, err) + + term := vt10x.New(vt10x.WithWriter(pts)) + + // Create a console via Expect that allows scripting against the terminal + consoleOpts := []expect.ConsoleOpt{ + expect.WithStdin(ptm), + expect.WithStdout(term), + expect.WithCloser(ptm, pts), + failOnExpectError(t), + failOnSendError(t), + expect.WithDefaultTimeout(time.Second), + } + + console, err := expect.NewConsole(consoleOpts...) + require.NoError(t, err) + t.Cleanup(func() { testCloser(t, console) }) + + return console +} + +func newTestIOStreams(t *testing.T, console *expect.Console) *IOStreams { + t.Helper() + + in := console.Tty() + out := console.Tty() + errOut := console.Tty() + + // Because the briandowns/spinner checks os.Stdout directly, + // we need this hack to trick it into allowing the spinner to print... + os.Stdout = out + + io := &IOStreams{ + In: in, + Out: out, + ErrOut: errOut, + term: fakeTerm{}, + } + io.progressIndicatorEnabled = true + return io +} + +// failOnExpectError adds an observer that will fail the test in a standardised way +// if any expectation on the command output fails, without requiring an explicit +// assertion. +// +// Use WithRelaxedIO to disable this behaviour. +func failOnExpectError(t *testing.T) expect.ConsoleOpt { + t.Helper() + return expect.WithExpectObserver( + func(matchers []expect.Matcher, buf string, err error) { + t.Helper() + + if err == nil { + return + } + + if len(matchers) == 0 { + t.Fatalf("Error occurred while matching %q: %s\n", buf, err) + } + + var criteria []string + for _, matcher := range matchers { + criteria = append(criteria, fmt.Sprintf("%q", matcher.Criteria())) + } + t.Fatalf("Failed to find [%s] in %q: %s\n", strings.Join(criteria, ", "), buf, err) + }, + ) +} + +// failOnSendError adds an observer that will fail the test in a standardised way +// if any sending of input fails, without requiring an explicit assertion. +// +// Use WithRelaxedIO to disable this behaviour. +func failOnSendError(t *testing.T) expect.ConsoleOpt { + t.Helper() + return expect.WithSendObserver( + func(msg string, n int, err error) { + t.Helper() + + if err != nil { + t.Fatalf("Failed to send %q: %s\n", msg, err) + } + if len(msg) != n { + t.Fatalf("Only sent %d of %d bytes for %q\n", n, len(msg), msg) + } + }, + ) +} + +// testCloser is a helper to fail the test if a Closer fails to close. +func testCloser(t *testing.T, closer io.Closer) { + t.Helper() + if err := closer.Close(); err != nil { + t.Errorf("Close failed: %s", err) + } +} From 3eba461afb2746576cd26ed391683494c4f48dd5 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Mon, 14 Apr 2025 15:07:11 -0600 Subject: [PATCH 2/7] 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. --- pkg/cmd/factory/default.go | 7 +++ pkg/cmd/factory/default_test.go | 50 +++++++++++++++++++ pkg/iostreams/iostreams.go | 33 ++++++++---- .../iostreams_progress_indicator_test.go | 18 +++---- 4 files changed, 88 insertions(+), 20 deletions(-) diff --git a/pkg/cmd/factory/default.go b/pkg/cmd/factory/default.go index 6286c999d..7a45efeb4 100644 --- a/pkg/cmd/factory/default.go +++ b/pkg/cmd/factory/default.go @@ -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 diff --git a/pkg/cmd/factory/default_test.go b/pkg/cmd/factory/default_test.go index c0275d1de..aff833d4c 100644 --- a/pkg/cmd/factory/default_test.go +++ b/pkg/cmd/factory/default_test.go @@ -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 diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index 8608978c5..07d7aa27f 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -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 + " " } diff --git a/pkg/iostreams/iostreams_progress_indicator_test.go b/pkg/iostreams/iostreams_progress_indicator_test.go index c7c374811..3cd6eaf65 100644 --- a/pkg/iostreams/iostreams_progress_indicator_test.go +++ b/pkg/iostreams/iostreams_progress_indicator_test.go @@ -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 } From f283d6d11ce1c3d15c3cf55a93d2c1a6fa1fcdd6 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Mon, 14 Apr 2025 19:30:05 -0600 Subject: [PATCH 3/7] feat(iostreams): textual progress indicator does not clear screen This bypasses the `spinner` package for the textual progress indicator for users with the spinner disabled out of concerns for accessibility, specifically with screen readers: - The `spinner` package will continuously re-draw the screen. I wasn't able to have this cause problems with my Mac screen reader, but it's nonetheless a concern that other screen readers may not handle this screen re-drawing well. - The `spinner` package clears any progress indicator messages from the screen when stopping the progress indicator or changing its label. This is a problem because it interrupts screen readers and leaves no way to recover what the loading message was by scrolling up in the terminal. NOTE: this new implementation still interrupts the screen reader when the a new label is printed, but it does not clear the screen. This makes the loading messages recoverable, at least. --- pkg/iostreams/iostreams.go | 47 +++++++++++-------- .../iostreams_progress_indicator_test.go | 24 ++++++++++ 2 files changed, 51 insertions(+), 20 deletions(-) diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index 07d7aa27f..cfc0da170 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -291,6 +291,11 @@ func (s *IOStreams) StartProgressIndicatorWithLabel(label string) { return } + if s.spinnerDisabled { + s.startTextualProgressIndicator(label) + return + } + s.progressIndicatorMu.Lock() defer s.progressIndicatorMu.Unlock() @@ -303,28 +308,12 @@ func (s *IOStreams) StartProgressIndicatorWithLabel(label string) { return } - var spinnerStyle []string - 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 - // ⣾ ⣷ ⣽ ⣻ ⡿ - spinnerStyle = spinner.CharSets[11] - } + // https://github.com/briandowns/spinner#available-character-sets + // ⣾ ⣷ ⣽ ⣻ ⡿ + spinnerStyle := spinner.CharSets[11] sp := spinner.New(spinnerStyle, 120*time.Millisecond, spinner.WithWriter(s.ErrOut), spinner.WithColor("fgCyan")) - if label != "" && !s.spinnerDisabled { + if label != "" { sp.Prefix = label + " " } @@ -332,6 +321,24 @@ func (s *IOStreams) StartProgressIndicatorWithLabel(label string) { s.progressIndicator = sp } +func (s *IOStreams) startTextualProgressIndicator(label string) { + s.progressIndicatorMu.Lock() + defer s.progressIndicatorMu.Unlock() + + // 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 + } + + fmt.Fprintf(s.ErrOut, "%s%s", s.ColorScheme().Cyan(label), "\n") +} + func (s *IOStreams) StopProgressIndicator() { s.progressIndicatorMu.Lock() defer s.progressIndicatorMu.Unlock() diff --git a/pkg/iostreams/iostreams_progress_indicator_test.go b/pkg/iostreams/iostreams_progress_indicator_test.go index 3cd6eaf65..42eb5e56a 100644 --- a/pkg/iostreams/iostreams_progress_indicator_test.go +++ b/pkg/iostreams/iostreams_progress_indicator_test.go @@ -123,6 +123,30 @@ func TestStartProgressIndicatorWithLabel(t *testing.T) { t.Fatal("Test timed out waiting for progress indicator") } }) + + t.Run("multiple indicators with GH_SPINNER_DISABLED shows current label", func(t *testing.T) { + console := newTestVirtualTerminal(t) + io := newTestIOStreams(t, console, true) + progressIndicatorLabel1 := "downloading happiness" + progressIndicatorLabel2 := "downloading sadness" + done := make(chan error) + go func() { + _, err := console.ExpectString(progressIndicatorLabel1 + "...") + require.NoError(t, err) + _, err = console.ExpectString(progressIndicatorLabel2 + "...") + done <- err + }() + io.StartProgressIndicatorWithLabel(progressIndicatorLabel1) + defer io.StopProgressIndicator() + io.StartProgressIndicatorWithLabel(progressIndicatorLabel2) + + select { + case err := <-done: + require.NoError(t, err) + case <-time.After(2 * time.Second): + t.Fatal("Test timed out waiting for progress indicator") + } + }) } func newTestVirtualTerminal(t *testing.T) *expect.Console { From 290e78c904dacc0ef7df727b13c138be251781ae Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 16 Apr 2025 11:05:36 -0600 Subject: [PATCH 4/7] test(iostreams): update test description --- pkg/iostreams/iostreams_progress_indicator_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/iostreams/iostreams_progress_indicator_test.go b/pkg/iostreams/iostreams_progress_indicator_test.go index 42eb5e56a..60d0ece91 100644 --- a/pkg/iostreams/iostreams_progress_indicator_test.go +++ b/pkg/iostreams/iostreams_progress_indicator_test.go @@ -124,7 +124,7 @@ func TestStartProgressIndicatorWithLabel(t *testing.T) { } }) - t.Run("multiple indicators with GH_SPINNER_DISABLED shows current label", func(t *testing.T) { + t.Run("multiple calls to start progress indicator with GH_SPINNER_DISABLED prints additional labels", func(t *testing.T) { console := newTestVirtualTerminal(t) io := newTestIOStreams(t, console, true) progressIndicatorLabel1 := "downloading happiness" From 056d292f2635395bdbdc79e9c95fee84bae61724 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 16 Apr 2025 11:13:58 -0600 Subject: [PATCH 5/7] doc(iostreams): comment behavior of textual progress indicator --- pkg/iostreams/iostreams.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index cfc0da170..ba2cc6b50 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -292,6 +292,10 @@ func (s *IOStreams) StartProgressIndicatorWithLabel(label string) { } if s.spinnerDisabled { + // If the spinner is disabled, simply print a + // textual progress indicator and return. + // This means that s.ProgressIndicator will be nil. + // See also: the comment on StopProgressIndicator() s.startTextualProgressIndicator(label) return } @@ -339,6 +343,9 @@ func (s *IOStreams) startTextualProgressIndicator(label string) { fmt.Fprintf(s.ErrOut, "%s%s", s.ColorScheme().Cyan(label), "\n") } +// StopProgressIndicator stops the progress indicator if it is running. +// Note that a textual progess indicator does not create a progress indicator, +// so this method is a no-op in that case. func (s *IOStreams) StopProgressIndicator() { s.progressIndicatorMu.Lock() defer s.progressIndicatorMu.Unlock() From 1dbb01a1d1d0117f75aeb1ae011bbe53c92969bf Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 16 Apr 2025 11:17:23 -0600 Subject: [PATCH 6/7] doc(help): add documentation for GH_SPINNER_DISABLED --- pkg/cmd/root/help_topic.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/cmd/root/help_topic.go b/pkg/cmd/root/help_topic.go index b85d64ca3..4b692777c 100644 --- a/pkg/cmd/root/help_topic.go +++ b/pkg/cmd/root/help_topic.go @@ -114,6 +114,9 @@ var HelpTopics = []helpTopic{ %[1]sGH_ACCESSIBLE_PROMPTER%[1]s (preview): set to a truthy value to enable prompts that are more compatible with speech synthesis and braille screen readers. + + %[1]sGH_SPINNER_DISABLED%[1]s: set to a truthy value to replace the spinner animation with + a textual progress indicator. `, "`"), }, { From dabb29bd361dc29a80b2f666271a28cc0a29f599 Mon Sep 17 00:00:00 2001 From: Kynan Ware <47394200+BagToad@users.noreply.github.com> Date: Wed, 16 Apr 2025 11:21:39 -0600 Subject: [PATCH 7/7] test(factory): remove needless nil check --- pkg/cmd/factory/default_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/factory/default_test.go b/pkg/cmd/factory/default_test.go index aff833d4c..5036a1dc1 100644 --- a/pkg/cmd/factory/default_test.go +++ b/pkg/cmd/factory/default_test.go @@ -470,10 +470,8 @@ func Test_ioStreams_spinnerDisabled(t *testing.T) { } 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) - } + for k, v := range tt.env { + t.Setenv(k, v) } f := New("1") io := ioStreams(f)