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.
cli · BagToad · Apr 16, 2025 · Apr 11, 2025 · Apr 14, 2025 · Apr 15, 2025
commit f283d6d11ce1c3d15c3cf55a93d2c1a6fa1fcdd6
@@ -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,35 +308,37 @@ 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 + " "
 	}
 
 	sp.Start()
 	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()

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