tty: add IsStdoutTerminal, StderrTerminalWidth, and IsInteractiveTerminal#1931
Merged
tty: add IsStdoutTerminal, StderrTerminalWidth, and IsInteractiveTerminal#1931
Conversation
… with tests Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
lpcox
March 15, 2026 00:01
View session
4 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the internal/tty helpers to provide additional terminal/TTY predicates and terminal width detection, enabling callers to make more reliable decisions about human-facing CLI output formatting.
Changes:
- Add
IsStdoutTerminal()as a stdout counterpart toIsStderrTerminal(). - Add
StderrTerminalWidth()to retrieve stderr terminal width viagolang.org/x/term. - Add
IsInteractiveTerminal()as a single predicate combining “stderr is a terminal” and “not running in a container”, plus corresponding tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/tty/tty.go | Adds new exported helpers for stdout terminal detection, stderr terminal width, and an “interactive terminal” predicate. |
| internal/tty/tty_test.go | Adds unit tests covering the new helpers and documents term.IsTerminal behavior for pipe FDs. |
Comments suppressed due to low confidence (2)
internal/tty/tty_test.go:97
- This CI-specific test assumes
CI/GITHUB_ACTIONSimplies a non-terminal stderr. That’s not guaranteed across CI providers or when running tests locally withCI=1, and can make the test fail spuriously. Consider checkingterm.IsTerminal(int(os.Stderr.Fd()))and skipping if stderr is actually a terminal, or directly asserting!IsStderrTerminal()before the interactive check.
if os.Getenv("CI") == "" && os.Getenv("GITHUB_ACTIONS") == "" {
t.Skip("Skipping CI-specific assertion: not running in a CI environment")
}
assert.False(t, IsInteractiveTerminal(), "should not be interactive in CI")
internal/tty/tty_test.go:69
TestStderrTerminalWidthassumes that ifterm.IsTerminal(stderr)is true thenStderrTerminalWidth()must succeed. The function’s contract is “true if successful”, butterm.GetSizecan legitimately fail (or return non-positive width) even whenIsTerminalis true (e.g., some CI/pseudo-TTYs, Windows/remote environments), which would make this test flaky. Consider asserting weaker invariants: whenokis true,widthmust be >0 and stderr must be a terminal; whenokis false,widthmust be 0 (and don’t fail solely becauseIsTerminalis true).
func TestStderrTerminalWidth(t *testing.T) {
width, ok := StderrTerminalWidth()
isTerminal := term.IsTerminal(int(os.Stderr.Fd()))
if isTerminal {
assert.True(t, ok, "should succeed when stderr is a terminal")
assert.Greater(t, width, 0, "terminal width should be positive")
} else {
assert.False(t, ok, "should fail when stderr is not a terminal")
assert.Equal(t, 0, width, "width should be 0 when not a terminal")
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+75
to
+80
| if os.Getenv("CI") == "" && os.Getenv("GITHUB_ACTIONS") == "" { | ||
| t.Skip("Skipping CI-specific assertion: not running in a CI environment") | ||
| } | ||
| width, ok := StderrTerminalWidth() | ||
| assert.False(t, ok, "should not detect terminal width in CI") | ||
| assert.Equal(t, 0, width, "width should be 0 in CI") |
This was referenced Mar 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the improvement opportunities from the go-fan module review of
golang.org/x/term(#issue).Changes
internal/tty/tty.goIsStdoutTerminal()— symmetric wrapper toIsStderrTerminal(), useful for detecting piped stdout in future CLI output formattingStderrTerminalWidth()— returns terminal width viaterm.GetSize; returns(0, false)when stderr is not a terminal or width cannot be determinedIsInteractiveTerminal()— unified signal combiningIsStderrTerminal() && !IsRunningInContainer(); provides a single predicate for "output is definitely going to a human terminal"internal/tty/tty_test.goIsStdoutTerminal(agreement withterm.IsTerminal)TestTermIsTerminal_PipeIsNotTerminal— documents that pipe fds return false fromterm.IsTerminal, validating the invariant both wrappers rely onStderrTerminalWidth— verifies width>0 only when stderr is a terminal, and 0/false in CIIsInteractiveTerminal— verifies it equalsIsStderrTerminal() && !IsRunningInContainer(), and is false in CISecurity Summary
No security vulnerabilities introduced. CodeQL: 0 alerts.