cmd/errtrace: Best-effort detection of tty stdin using char device (#84)

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.

cmd/errtrace/main.go

@@ -32,7 +32,6 @@ package main
 import (
 	"bytes"
 	"encoding/json"
-	"errors"
 	"flag"
 	"fmt"
 	"go/ast"
@@ -48,7 +47,6 @@ import (
 	"sort"
 	"strconv"
 	"strings"
-	"time"
 
 	"braces.dev/errtrace"
 )
@@ -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))
@@ -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 {
+		// Running with no args reads from stdin, but this is not obvious
+		// so print a usage hint 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 {

cmd/errtrace/main_test.go

@@ -17,7 +17,6 @@ import (
 	"strings"
 	"sync"
 	"testing"
-	"time"
 
 	"braces.dev/errtrace"
 	"braces.dev/errtrace/internal/diff"
@@ -722,98 +721,94 @@ 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: "stdin is a pipe",
+			stdin: func(t testing.TB) io.Reader {
+				r, w, err := os.Pipe()
+				if err != nil {
+					t.Fatal(err)
+				}
+
+				go func() {
+					if _, err := w.WriteString("package foo"); err != nil {
+						t.Errorf("failed to write to stdin as pipe: %v", err)
+					}
+					if err := w.Close(); err != nil {
+						t.Errorf("failed to close stdin pipe: %v", err)
+					}
+				}()
+
+				return r
+			},
 		},
 		{
-			name:       "delay",
-			delay:      20 * time.Millisecond,
+			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 {