fix(spin): preserve color output when --show-output is given (#850)

go.mod

@@ -14,6 +14,7 @@ require (
 	github.com/charmbracelet/x/ansi v0.8.0
 	github.com/charmbracelet/x/editor v0.1.0
 	github.com/charmbracelet/x/term v0.2.1
+	github.com/charmbracelet/x/xpty v0.1.2
 	github.com/muesli/roff v0.1.0
 	github.com/muesli/termenv v0.15.3-0.20241211131612-0d230cb6eb15
 	github.com/rivo/uniseg v0.4.7
@@ -26,6 +27,10 @@ require (
 	github.com/atotto/clipboard v0.1.4 // indirect
 	github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect
 	github.com/aymerick/douceur v0.2.0 // indirect
+	github.com/charmbracelet/x/conpty v0.1.0 // indirect
+	github.com/charmbracelet/x/errors v0.0.0-20240508181413-e8d8b6e2de86 // indirect
+	github.com/charmbracelet/x/termios v0.1.1 // indirect
+	github.com/creack/pty v1.1.24 // indirect
 	github.com/dlclark/regexp2 v1.11.0 // indirect
 	github.com/dustin/go-humanize v1.0.1 // indirect
 	github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f // indirect

go.sum

@@ -32,12 +32,22 @@ github.com/charmbracelet/log v0.4.0 h1:G9bQAcx8rWA2T3pWvx7YtPTPwgqpk7D68BX21IRW8
 github.com/charmbracelet/log v0.4.0/go.mod h1:63bXt/djrizTec0l11H20t8FDSvA4CRZJ1KH22MdptM=
 github.com/charmbracelet/x/ansi v0.8.0 h1:9GTq3xq9caJW8ZrBTe0LIe2fvfLR/bYXKTx2llXn7xE=
 github.com/charmbracelet/x/ansi v0.8.0/go.mod h1:wdYl/ONOLHLIVmQaxbIYEC/cRKOQyjTkowiI4blgS9Q=
+github.com/charmbracelet/x/conpty v0.1.0 h1:4zc8KaIcbiL4mghEON8D72agYtSeIgq8FSThSPQIb+U=
+github.com/charmbracelet/x/conpty v0.1.0/go.mod h1:rMFsDJoDwVmiYM10aD4bH2XiRgwI7NYJtQgl5yskjEQ=
 github.com/charmbracelet/x/editor v0.1.0 h1:p69/dpvlwRTs9uYiPeAWruwsHqTFzHhTvQOd/WVSX98=
 github.com/charmbracelet/x/editor v0.1.0/go.mod h1:oivrEbcP/AYt/Hpvk5pwDXXrQ933gQS6UzL6fxqAGSA=
+github.com/charmbracelet/x/errors v0.0.0-20240508181413-e8d8b6e2de86 h1:JSt3B+U9iqk37QUU2Rvb6DSBYRLtWqFqfxf8l5hOZUA=
+github.com/charmbracelet/x/errors v0.0.0-20240508181413-e8d8b6e2de86/go.mod h1:2P0UgXMEa6TsToMSuFqKFQR+fZTO9CNGUNokkPatT/0=
 github.com/charmbracelet/x/exp/golden v0.0.0-20240815200342-61de596daa2b h1:MnAMdlwSltxJyULnrYbkZpp4k58Co7Tah3ciKhSNo0Q=
 github.com/charmbracelet/x/exp/golden v0.0.0-20240815200342-61de596daa2b/go.mod h1:wDlXFlCrmJ8J+swcL/MnGUuYnqgQdW9rhSD61oNMb6U=
 github.com/charmbracelet/x/term v0.2.1 h1:AQeHeLZ1OqSXhrAWpYUtZyX1T3zVxfpZuEQMIQaGIAQ=
 github.com/charmbracelet/x/term v0.2.1/go.mod h1:oQ4enTYFV7QN4m0i9mzHrViD7TQKvNEEkHUMCmsxdUg=
+github.com/charmbracelet/x/termios v0.1.1 h1:o3Q2bT8eqzGnGPOYheoYS8eEleT5ZVNYNy8JawjaNZY=
+github.com/charmbracelet/x/termios v0.1.1/go.mod h1:rB7fnv1TgOPOyyKRJ9o+AsTU/vK5WHJ2ivHeut/Pcwo=
+github.com/charmbracelet/x/xpty v0.1.2 h1:Pqmu4TEJ8KeA9uSkISKMU3f+C1F6OGBn8ABuGlqCbtI=
+github.com/charmbracelet/x/xpty v0.1.2/go.mod h1:XK2Z0id5rtLWcpeNiMYBccNNBrP2IJnzHI0Lq13Xzq4=
+github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s=
+github.com/creack/pty v1.1.24/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/dlclark/regexp2 v1.11.0 h1:G/nrcoOa7ZXlpoa/91N3X7mM3r8eIlMBBJZvsz/mxKI=

spin/command.go

@@ -48,22 +48,29 @@ func (o Options) Run() error {
 	// If the command succeeds, and we are printing output and we are in a TTY then push the STDOUT we got to the actual
 	// STDOUT for piping or other things.
 	//nolint:nestif
-	if m.status == 0 {
-		if o.ShowOutput {
-			// BubbleTea writes the View() to stderr.
-			// If the program is being piped then put the accumulated output in stdout.
-			if !isOutTTY {
-				_, err := os.Stdout.WriteString(m.stdout)
-				if err != nil {
-					return fmt.Errorf("failed to write to stdout: %w", err)
-				}
+	if m.err != nil {
+		if _, err := fmt.Fprintf(os.Stderr, "%s\n", m.err.Error()); err != nil {
+			return fmt.Errorf("failed to write to stdout: %w", err)
+		}
+		return exit.ErrExit(1)
+	} else if m.status == 0 {
+		var output string
+		if o.ShowOutput || (o.ShowStdout && o.ShowStderr) {
+			output = m.output
+		} else if o.ShowStdout {
+			output = m.stdout
+		} else if o.ShowStderr {
+			output = m.stderr
+		}
+		if output != "" {
+			if _, err := os.Stdout.WriteString(output); err != nil {
+				return fmt.Errorf("failed to write to stdout: %w", err)
 			}
 		}
 	} else if o.ShowError {
 		// Otherwise if we are showing errors and the command did not exit with a 0 status code then push all of the command
 		// output to the terminal. This way failed commands can be debugged.
-		_, err := os.Stdout.WriteString(m.output)
-		if err != nil {
+		if _, err := os.Stdout.WriteString(m.output); err != nil {
 			return fmt.Errorf("failed to write to stdout: %w", err)
 		}
 	}

spin/pty.go

@@ -0,0 +1,22 @@
+package spin
+
+import (
+	"os"
+
+	"github.com/charmbracelet/x/term"
+	"github.com/charmbracelet/x/xpty"
+)
+
+func openPty(f *os.File) (pty xpty.Pty, err error) {
+	width, height, err := term.GetSize(f.Fd())
+	if err != nil {
+		return nil, err
+	}
+
+	pty, err = xpty.NewPty(width, height)
+	if err != nil {
+		return nil, err
+	}
+
+	return pty, nil
+}

spin/spin.go

@@ -15,15 +15,18 @@
 package spin
 
 import (
+	"bytes"
+	"context"
 	"io"
 	"os"
 	"os/exec"
-	"strings"
+	"runtime"
 	"syscall"
 
 	"github.com/charmbracelet/bubbles/spinner"
 	tea "github.com/charmbracelet/bubbletea"
 	"github.com/charmbracelet/x/term"
+	"github.com/charmbracelet/x/xpty"
 )
 
 type model struct {
@@ -40,16 +43,19 @@ type model struct {
 	showStdout bool
 	showStderr bool
 	showError  bool
+	err        error
 }
 
 var (
-	bothbuf strings.Builder
-	outbuf  strings.Builder
-	errbuf  strings.Builder
+	bothbuf bytes.Buffer
+	outbuf  bytes.Buffer
+	errbuf  bytes.Buffer
 
 	executing *exec.Cmd
 )
 
+type errorMsg error
+
 type finishCommandMsg struct {
 	stdout string
 	stderr string
@@ -65,15 +71,51 @@ func commandStart(command []string) tea.Cmd {
 		}
 
 		executing = exec.Command(command[0], args...) //nolint:gosec
-		if term.IsTerminal(os.Stdout.Fd()) {
+		executing.Stdin = os.Stdin
+
+		isTerminal := term.IsTerminal(os.Stdout.Fd())
+
+		// NOTE(@andreynering): We had issues with Git Bash on Windows
+		// when it comes to handling PTYs, so we're falling back to
+		// to redirecting stdout/stderr as usual to avoid issues.
+		//nolint:nestif
+		if isTerminal && runtime.GOOS == "windows" {
 			executing.Stdout = io.MultiWriter(&bothbuf, &outbuf)
 			executing.Stderr = io.MultiWriter(&bothbuf, &errbuf)
+			_ = executing.Run()
+		} else if isTerminal {
+			stdoutPty, err := openPty(os.Stdout)
+			if err != nil {
+				return errorMsg(err)
+			}
+			defer stdoutPty.Close() //nolint:errcheck
+
+			stderrPty, err := openPty(os.Stderr)
+			if err != nil {
+				return errorMsg(err)
+			}
+			defer stderrPty.Close() //nolint:errcheck
+
+			if outUnixPty, isOutUnixPty := stdoutPty.(*xpty.UnixPty); isOutUnixPty {
+				executing.Stdout = outUnixPty.Slave()
+			}
+			if errUnixPty, isErrUnixPty := stderrPty.(*xpty.UnixPty); isErrUnixPty {
+				executing.Stderr = errUnixPty.Slave()
+			}
+
+			go io.Copy(io.MultiWriter(&bothbuf, &outbuf), stdoutPty) //nolint:errcheck
+			go io.Copy(io.MultiWriter(&bothbuf, &errbuf), stderrPty) //nolint:errcheck
+
+			if err = stdoutPty.Start(executing); err != nil {
+				return errorMsg(err)
+			}
+			_ = xpty.WaitProcess(context.Background(), executing)
 		} else {
 			executing.Stdout = os.Stdout
 			executing.Stderr = os.Stderr
+			_ = executing.Run()
 		}
-		executing.Stdin = os.Stdin
-		_ = executing.Run()
+
 		status := executing.ProcessState.ExitCode()
 		if status == -1 {
 			status = 1
@@ -103,6 +145,10 @@ func (m model) Init() tea.Cmd {
 }
 
 func (m model) View() string {
+	if m.quitting {
+		return ""
+	}
+
 	var out string
 	if m.showStderr {
 		out += errbuf.String()
@@ -111,10 +157,6 @@ func (m model) View() string {
 		out += outbuf.String()
 	}
 
-	if m.quitting {
-		return out
-	}
-
 	if !m.isTTY {
 		return m.title
 	}
@@ -129,7 +171,6 @@ func (m model) View() string {
 }
 
 func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
-	var cmd tea.Cmd
 	switch msg := msg.(type) {
 	case finishCommandMsg:
 		m.stdout = msg.stdout
@@ -143,8 +184,13 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 		case "ctrl+c":
 			return m, commandAbort
 		}
+	case errorMsg:
+		m.err = msg
+		m.quitting = true
+		return m, tea.Quit
 	}
 
+	var cmd tea.Cmd
 	m.spinner, cmd = m.spinner.Update(msg)
 	return m, cmd
 }