diff --git a/pkg/cli/add_interactive_orchestrator.go b/pkg/cli/add_interactive_orchestrator.go index ce002f2b642..2ca94e1cfb3 100644 --- a/pkg/cli/add_interactive_orchestrator.go +++ b/pkg/cli/add_interactive_orchestrator.go @@ -62,7 +62,7 @@ func RunAddInteractive(ctx context.Context, workflowSpecs []string, verbose bool } // Clear the screen for a fresh interactive experience - fmt.Fprint(os.Stderr, "\033[H\033[2J") + console.ClearScreen() // Step 1: Welcome message fmt.Fprintln(os.Stderr, "") diff --git a/pkg/console/console.go b/pkg/console/console.go index c43f8903fb2..8d7774ef3ad 100644 --- a/pkg/console/console.go +++ b/pkg/console/console.go @@ -33,9 +33,6 @@ type CompilerError struct { Hint string // Optional hint for fixing the error } -// ANSI escape sequences for terminal control -var clearScreenSequence = "\033[2J\033[H" // Clear screen and move cursor to home position - // isTTY checks if stdout is a terminal func isTTY() bool { return tty.IsStdoutTerminal() @@ -483,11 +480,11 @@ func RenderTableAsJSON(config TableConfig) (string, error) { return string(jsonBytes), nil } -// ClearScreen clears the terminal screen if stdout is a TTY +// ClearScreen clears the terminal screen if stderr is a TTY // Uses ANSI escape codes for cross-platform compatibility func ClearScreen() { - if isTTY() { - fmt.Print(clearScreenSequence) + if tty.IsStderrTerminal() { + fmt.Fprint(os.Stderr, ansiClearScreen) } } @@ -495,7 +492,7 @@ func ClearScreen() { // Uses ANSI escape codes: \r moves cursor to start, \033[K clears to end of line func ClearLine() { if tty.IsStderrTerminal() { - fmt.Fprint(os.Stderr, "\r\033[K") + fmt.Fprintf(os.Stderr, "%s%s", ansiCarriageReturn, ansiClearLine) } } diff --git a/pkg/console/spinner.go b/pkg/console/spinner.go index f1b95baf2a6..cde25b0ab59 100644 --- a/pkg/console/spinner.go +++ b/pkg/console/spinner.go @@ -83,7 +83,7 @@ func (m spinnerModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { // render manually prints the spinner frame (required when using WithoutRenderer) func (m spinnerModel) render() { if m.output != nil { - fmt.Fprintf(m.output, "\r\033[K%s %s", m.spinner.View(), m.message) + fmt.Fprintf(m.output, "%s%s%s %s", ansiCarriageReturn, ansiClearLine, m.spinner.View(), m.message) } } @@ -138,7 +138,7 @@ func (s *SpinnerWrapper) Stop() { s.mu.Unlock() s.program.Quit() s.wg.Wait() // Wait for the goroutine to complete - fmt.Fprint(os.Stderr, "\r\033[K") + fmt.Fprintf(os.Stderr, "%s%s", ansiCarriageReturn, ansiClearLine) } else { s.mu.Unlock() } @@ -153,7 +153,7 @@ func (s *SpinnerWrapper) StopWithMessage(msg string) { s.mu.Unlock() s.program.Quit() s.wg.Wait() // Wait for the goroutine to complete - fmt.Fprintf(os.Stderr, "\r\033[K%s\n", msg) + fmt.Fprintf(os.Stderr, "%s%s%s\n", ansiCarriageReturn, ansiClearLine, msg) } else { s.mu.Unlock() // Still print the message even if spinner wasn't running diff --git a/pkg/console/terminal.go b/pkg/console/terminal.go new file mode 100644 index 00000000000..48ce6d48086 --- /dev/null +++ b/pkg/console/terminal.go @@ -0,0 +1,36 @@ +package console + +import ( + "fmt" + "os" + + "github.com/github/gh-aw/pkg/tty" +) + +// ANSI escape sequences for terminal control +const ( + // ansiClearScreen clears the screen and moves cursor to home position + ansiClearScreen = "\033[H\033[2J" + + // ansiClearLine clears from cursor to end of line + ansiClearLine = "\033[K" + + // ansiCarriageReturn moves cursor to start of current line + ansiCarriageReturn = "\r" +) + +// MoveCursorUp moves cursor up n lines if stderr is a TTY. +// Uses ANSI escape code: \033[nA where n is the number of lines. +func MoveCursorUp(n int) { + if tty.IsStderrTerminal() { + fmt.Fprintf(os.Stderr, "\033[%dA", n) + } +} + +// MoveCursorDown moves cursor down n lines if stderr is a TTY. +// Uses ANSI escape code: \033[nB where n is the number of lines. +func MoveCursorDown(n int) { + if tty.IsStderrTerminal() { + fmt.Fprintf(os.Stderr, "\033[%dB", n) + } +} diff --git a/pkg/console/terminal_test.go b/pkg/console/terminal_test.go new file mode 100644 index 00000000000..3b643d84b0f --- /dev/null +++ b/pkg/console/terminal_test.go @@ -0,0 +1,173 @@ +//go:build !integration + +package console + +import ( + "bytes" + "io" + "os" + "testing" + + "github.com/stretchr/testify/assert" +) + +// captureStderr captures stderr output during function execution +func captureStderr(t *testing.T, fn func()) string { + t.Helper() + + // Save original stderr + oldStderr := os.Stderr + + // Create a pipe to capture stderr + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("Failed to create pipe: %v", err) + } + + // Replace stderr with the write end of the pipe + os.Stderr = w + + // Create a channel to receive the captured output + outputChan := make(chan string, 1) + + // Read from the pipe in a goroutine + go func() { + var buf bytes.Buffer + io.Copy(&buf, r) + outputChan <- buf.String() + }() + + // Execute the function + fn() + + // Close the write end and restore stderr + w.Close() + os.Stderr = oldStderr + + // Get the captured output + output := <-outputChan + r.Close() + + return output +} + +func TestMoveCursorUp(t *testing.T) { + tests := []struct { + name string + lines int + }{ + { + name: "move up 1 line", + lines: 1, + }, + { + name: "move up 5 lines", + lines: 5, + }, + { + name: "move up 0 lines", + lines: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + output := captureStderr(t, func() { + MoveCursorUp(tt.lines) + }) + + // In non-TTY environments, output should be empty + // We just ensure no panic occurs + assert.NotNil(t, output, "MoveCursorUp should not panic") + }) + } +} + +func TestMoveCursorDown(t *testing.T) { + tests := []struct { + name string + lines int + }{ + { + name: "move down 1 line", + lines: 1, + }, + { + name: "move down 5 lines", + lines: 5, + }, + { + name: "move down 0 lines", + lines: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + output := captureStderr(t, func() { + MoveCursorDown(tt.lines) + }) + + // In non-TTY environments, output should be empty + // We just ensure no panic occurs + assert.NotNil(t, output, "MoveCursorDown should not panic") + }) + } +} + +func TestTerminalCursorFunctionsNoTTY(t *testing.T) { + // This test verifies that in non-TTY environments (like CI/tests), + // no ANSI codes are emitted for cursor movement functions + + tests := []struct { + name string + fn func() + }{ + { + name: "MoveCursorUp", + fn: func() { MoveCursorUp(5) }, + }, + { + name: "MoveCursorDown", + fn: func() { MoveCursorDown(3) }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + output := captureStderr(t, tt.fn) + + // Since tests typically run in non-TTY, verify output is empty + // This ensures we properly respect TTY detection + if os.Getenv("CI") != "" || !isRealTerminal() { + assert.Empty(t, output, "%s should not output ANSI codes in non-TTY", tt.name) + } + }) + } +} + +// isRealTerminal checks if we're actually running in a terminal +// This is a helper to distinguish between test environments and real terminals +func isRealTerminal() bool { + // In test environments, stderr is typically redirected + fileInfo, err := os.Stderr.Stat() + if err != nil { + return false + } + // Check if stderr is a character device (terminal) + return (fileInfo.Mode() & os.ModeCharDevice) != 0 +} + +func TestTerminalCursorFunctionsDoNotPanic(t *testing.T) { + // Ensure all cursor movement functions can be called safely without panicking + // even in edge cases + + t.Run("all cursor functions", func(t *testing.T) { + assert.NotPanics(t, func() { + MoveCursorUp(0) + MoveCursorUp(100) + MoveCursorDown(0) + MoveCursorDown(100) + }, "Cursor movement functions should never panic") + }) +}