Skip to content

Commit aaa9e41

Browse files
committed
fix: /dev/null was triggering colour output
os.ModeCharDevice matches /dev/null too, so redirecting there silently turned on colourised (and much slower) output. Use isatty instead, like fatih/color already does internally - no new dependency needed. Tests cover a regular file, /dev/null, pipes and a real tty.
1 parent 8b5af06 commit aaa9e41

3 files changed

Lines changed: 106 additions & 4 deletions

File tree

cmd/utils.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"os"
88
"strings"
99

10+
"github.com/mattn/go-isatty"
1011
"github.com/mikefarah/yq/v4/pkg/yqlib"
1112
"github.com/spf13/cobra"
1213
)
@@ -45,9 +46,10 @@ func initCommand(cmd *cobra.Command, args []string) (string, []string, error) {
4546
}
4647

4748
func setupColors() {
48-
fileInfo, _ := os.Stdout.Stat()
49-
50-
if forceColor || (!forceNoColor && (fileInfo.Mode()&os.ModeCharDevice) != 0) {
49+
fd := os.Stdout.Fd()
50+
// os.Stdout can be a regular file, pipe, or other character device such
51+
// as /dev/null - only a real terminal should get colourised output.
52+
if forceColor || (!forceNoColor && (isatty.IsTerminal(fd) || isatty.IsCygwinTerminal(fd))) {
5153
colorsEnabled = true
5254
}
5355
}

cmd/utils_test.go

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@ package cmd
33
import (
44
"fmt"
55
"os"
6+
"path/filepath"
7+
"runtime"
68
"strings"
9+
"syscall"
710
"testing"
811

912
"github.com/mikefarah/yq/v4/pkg/yqlib"
@@ -964,6 +967,103 @@ func TestSetupColors(t *testing.T) {
964967
}
965968
}
966969

970+
// terminalCase is a file/device to test setupColors against, paired with
971+
// whether it should be treated as an interactive terminal - so what does
972+
// and doesn't get colourised output is visible in a single place.
973+
type terminalCase struct {
974+
open func(t *testing.T) *os.File
975+
isTerm bool
976+
}
977+
978+
func terminalTestCases() map[string]terminalCase {
979+
return map[string]terminalCase{
980+
"regular file": {isTerm: false, open: func(t *testing.T) *os.File {
981+
f, err := os.CreateTemp(t.TempDir(), "isterminal")
982+
if err != nil {
983+
t.Fatalf("failed to create temp file: %v", err)
984+
}
985+
return f
986+
}},
987+
"/dev/null": {isTerm: false, open: func(t *testing.T) *os.File {
988+
f, err := os.OpenFile(os.DevNull, os.O_RDWR, 0644)
989+
if err != nil {
990+
t.Fatalf("failed to open %s: %v", os.DevNull, err)
991+
}
992+
return f
993+
}},
994+
"unnamed pipe, read end": {isTerm: false, open: func(t *testing.T) *os.File {
995+
r, w, err := os.Pipe()
996+
if err != nil {
997+
t.Fatalf("failed to create pipe: %v", err)
998+
}
999+
w.Close()
1000+
return r
1001+
}},
1002+
"unnamed pipe, write end (e.g. piped to a pager)": {isTerm: false, open: func(t *testing.T) *os.File {
1003+
r, w, err := os.Pipe()
1004+
if err != nil {
1005+
t.Fatalf("failed to create pipe: %v", err)
1006+
}
1007+
r.Close()
1008+
return w
1009+
}},
1010+
"named pipe (FIFO)": {isTerm: false, open: func(t *testing.T) *os.File {
1011+
if runtime.GOOS == "windows" {
1012+
t.Skip("named pipes not supported by this test on windows")
1013+
}
1014+
path := filepath.Join(t.TempDir(), "fifo")
1015+
if err := syscall.Mkfifo(path, 0600); err != nil {
1016+
t.Fatalf("failed to create named pipe: %v", err)
1017+
}
1018+
// O_RDWR avoids blocking on open: a FIFO opened only for reading
1019+
// (or only writing) blocks until a peer opens the other end.
1020+
f, err := os.OpenFile(path, os.O_RDWR, os.ModeNamedPipe)
1021+
if err != nil {
1022+
t.Fatalf("failed to open named pipe: %v", err)
1023+
}
1024+
return f
1025+
}},
1026+
"controlling terminal (/dev/tty)": {isTerm: true, open: func(t *testing.T) *os.File {
1027+
f, err := os.OpenFile("/dev/tty", os.O_RDWR, 0)
1028+
if err != nil {
1029+
t.Skipf("no controlling terminal available in this environment: %v", err)
1030+
}
1031+
return f
1032+
}},
1033+
}
1034+
}
1035+
1036+
func TestSetupColorsForFileKind(t *testing.T) {
1037+
for name, tc := range terminalTestCases() {
1038+
t.Run(name, func(t *testing.T) {
1039+
f := tc.open(t)
1040+
defer f.Close()
1041+
1042+
originalStdout := os.Stdout
1043+
originalForceColor := forceColor
1044+
originalForceNoColor := forceNoColor
1045+
originalColorsEnabled := colorsEnabled
1046+
defer func() {
1047+
os.Stdout = originalStdout
1048+
forceColor = originalForceColor
1049+
forceNoColor = originalForceNoColor
1050+
colorsEnabled = originalColorsEnabled
1051+
}()
1052+
1053+
os.Stdout = f
1054+
forceColor = false
1055+
forceNoColor = false
1056+
colorsEnabled = false
1057+
1058+
setupColors()
1059+
1060+
if colorsEnabled != tc.isTerm {
1061+
t.Errorf("setupColors() colorsEnabled = %v, want %v for %s", colorsEnabled, tc.isTerm, name)
1062+
}
1063+
})
1064+
}
1065+
}
1066+
9671067
func TestLoadSplitFileExpression(t *testing.T) {
9681068
// Create a temporary file with expression content
9691069
tempFile, err := os.CreateTemp("", "split")

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ require (
1313
github.com/hashicorp/hcl/v2 v2.24.0
1414
github.com/jinzhu/copier v0.4.0
1515
github.com/magiconair/properties v1.18.11
16+
github.com/mattn/go-isatty v0.0.20
1617
github.com/pelletier/go-toml/v2 v2.4.3
1718
github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e
1819
github.com/spf13/cobra v1.10.2
@@ -32,7 +33,6 @@ require (
3233
github.com/google/go-cmp v0.6.0 // indirect
3334
github.com/inconshreveable/mousetrap v1.1.0 // indirect
3435
github.com/mattn/go-colorable v0.1.14 // indirect
35-
github.com/mattn/go-isatty v0.0.20 // indirect
3636
github.com/mitchellh/go-wordwrap v1.0.1 // indirect
3737
golang.org/x/sync v0.22.0 // indirect
3838
golang.org/x/sys v0.47.0 // indirect

0 commit comments

Comments
 (0)