Skip to content

Commit

Permalink
cmd/errtrace: Best-effort detection of tty stdin using char device
Browse files Browse the repository at this point in the history
Follow-up to #78 and #82.

When stdin is implicitly used, we wait for 200ms for input before
printing a message to stderr to notify the user that we're reading
from stdin to avoid confusion. This added some complexity in the read
path, as well as the testing because of the background goroutine that
waits for a timeout before printing to stderr.

This is an alternative approach that does a best-effort detection of a
TTY by checking if stdin is a character device.
  • Loading branch information
prashantv committed Jan 9, 2024
1 parent 0110cc8 commit 9d4abff
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 91 deletions.
45 changes: 12 additions & 33 deletions cmd/errtrace/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ package main
import (
"bytes"
"encoding/json"
"errors"
"flag"
"fmt"
"go/ast"
Expand All @@ -48,7 +47,6 @@ import (
"sort"
"strconv"
"strings"
"time"

"braces.dev/errtrace"
)
Expand Down Expand Up @@ -560,8 +558,6 @@ func (cmd *mainCmd) processFile(r fileRequest) error {
return errtrace.Wrap(err)
}

var _stdinWait = 200 * time.Millisecond

func (cmd *mainCmd) readFile(r fileRequest) ([]byte, error) {
if r.Filepath != "-" {
return errtrace.Wrap2(os.ReadFile(r.Filename))
Expand All @@ -571,39 +567,22 @@ func (cmd *mainCmd) readFile(r fileRequest) ([]byte, error) {
return nil, errtrace.Wrap(fmt.Errorf("can't use -w with stdin"))
}

if !r.ImplicitStdin {
return errtrace.Wrap2(io.ReadAll(cmd.Stdin))
}

// If we're reading from stdin because there were no other arguments,
// wait a short time for the first read.
// If there's nothing, print a warning and continue waiting.
firstRead := make(chan struct{})
go func(firstRead <-chan struct{}, wait time.Duration) {
select {
case <-firstRead:
case <-time.After(wait):
cmd.log.Println("reading from stdin; use '-h' for help")
if r.ImplicitStdin {
// Calling without no args reads from stdin, but this is not obvious
// so print usage information to stderr, if we think stdin is a TTY.
// Best-effort check for a TTY by looking for a character device.
type statter interface {
Stat() (os.FileInfo, error)
}
}(firstRead, _stdinWait)

var buff bytes.Buffer
bs := make([]byte, 1024)
for {
n, err := cmd.Stdin.Read(bs)
buff.Write(bs[:n])
if err != nil {
if errors.Is(err, io.EOF) {
err = nil
if st, ok := cmd.Stdin.(statter); ok {
if fi, err := st.Stat(); err == nil &&
fi.Mode()&os.ModeCharDevice == os.ModeCharDevice {
cmd.log.Println("reading from stdin; use '-h' for help")
}
return buff.Bytes(), errtrace.Wrap(err)
}

if firstRead != nil {
close(firstRead)
firstRead = nil
}
}

return errtrace.Wrap2(io.ReadAll(cmd.Stdin))
}

type walker struct {
Expand Down
112 changes: 54 additions & 58 deletions cmd/errtrace/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"strings"
"sync"
"testing"
"time"

"braces.dev/errtrace"
"braces.dev/errtrace/internal/diff"
Expand Down Expand Up @@ -722,98 +721,95 @@ func TestGoListFilesBadJSON(t *testing.T) {
}

func TestStdinNoInputMessage(t *testing.T) {
// Verify that if there's no input on implicit stdin,
// we print a message to stderr to help the user.
defer func(old time.Duration) { _stdinWait = old }(_stdinWait)
_stdinWait = 10 * time.Millisecond // make the test run faster

tests := []struct {
name string
delay time.Duration // before writing
stdin func(testing.TB) io.Reader
args []string
wantStderr string
}{
{
name: "no delay",
delay: 0,
name: "stdin is a file",
stdin: func(t testing.TB) io.Reader {
f, err := os.Open("testdata/golden/noop.go")
if err != nil {
t.Fatal(err)
}
return f
},
},
{
name: "delay",
delay: 20 * time.Millisecond,
name: "stdin is a pipe",
stdin: func(t testing.TB) io.Reader {
r, w, err := os.Pipe()
if err != nil {
t.Fatal(err)
}

go func() {
w.WriteString("package foo")

Check failure on line 749 in cmd/errtrace/main_test.go

View workflow job for this annotation

GitHub Actions / Lint

Error return value of `w.WriteString` is not checked (errcheck)
w.Close()

Check failure on line 750 in cmd/errtrace/main_test.go

View workflow job for this annotation

GitHub Actions / Lint

Error return value of `w.Close` is not checked (errcheck)
}()

t.Cleanup(func() {
r.Close()

Check failure on line 754 in cmd/errtrace/main_test.go

View workflow job for this annotation

GitHub Actions / Lint

Error return value of `r.Close` is not checked (errcheck)
w.Close()

Check failure on line 755 in cmd/errtrace/main_test.go

View workflow job for this annotation

GitHub Actions / Lint

Error return value of `w.Close` is not checked (errcheck)
})

return r
},
},
{
name: "implicit stdin with a char device",
stdin: func(t testing.TB) io.Reader {
return fakeTerminal{strings.NewReader("package foo")}
},
wantStderr: "reading from stdin; use '-h' for help\n",
},
{
name: "explicit stdin with delay",
args: []string{"-"},
delay: 20 * time.Millisecond,
name: "explicit stdin with a char device",
stdin: func(t testing.TB) io.Reader {
return fakeTerminal{strings.NewReader("package foo")}
},
args: []string{"-"},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// mainCmd.Run is blocking so this has to be in a goroutine.
stdin, stdinw := io.Pipe()
done := make(chan struct{})
go func() {
defer close(done)

if tt.delay > 0 {
time.Sleep(20 * time.Millisecond)
}
if _, err := io.WriteString(stdinw, "package foo\n"); err != nil {
t.Error(err)
}

if err := stdinw.Close(); err != nil {
t.Error(err)
}
}()

var stderr lockedBuffer
var stderr bytes.Buffer
exitCode := (&mainCmd{
Stdin: stdin,
Stdin: tt.stdin(t),
Stdout: io.Discard,
Stderr: &stderr,
}).Run(tt.args)

if want := 0; exitCode != want {
t.Errorf("exit code = %d, want %d", exitCode, want)
}
<-done

// errtrace's goroutine may still be writing to stderr.
// Try a few times before giving up.
var gotStderr string
for i := 0; i < 10; i++ {
gotStderr = stderr.String()
if gotStderr != tt.wantStderr {
time.Sleep(100 * time.Millisecond)
continue
}
}

if want, got := tt.wantStderr, gotStderr; got != want {
if want, got := tt.wantStderr, stderr.String(); got != want {
t.Errorf("stderr = %q, want %q", got, want)
}
})
}
}

type lockedBuffer struct {
mu sync.RWMutex
buf bytes.Buffer
type fakeTerminal struct {
io.Reader
}

func (ft fakeTerminal) Stat() (os.FileInfo, error) {
return charDeviceFileInfo{}, nil
}

func (b *lockedBuffer) Write(p []byte) (int, error) {
b.mu.Lock()
defer b.mu.Unlock()
return errtrace.Wrap2(b.buf.Write(p))
type charDeviceFileInfo struct {
// embed so we implement the interface.
// unimplemented methods will panic.
os.FileInfo
}

func (b *lockedBuffer) String() string {
b.mu.RLock()
defer b.mu.RUnlock()
return b.buf.String()
func (fi charDeviceFileInfo) Mode() os.FileMode {
return os.ModeDevice | os.ModeCharDevice
}

func indent(s string) string {
Expand Down

0 comments on commit 9d4abff

Please sign in to comment.