diff --git a/README.md b/README.md index 6508cbe..376d8d0 100644 --- a/README.md +++ b/README.md @@ -113,7 +113,8 @@ Goals, in order: `sfdupes`. - Dependencies: standard library, `github.com/spf13/cobra` for the CLI, **one progress-bar library** - (`github.com/schollz/progressbar/v3`), and **one SQLite driver** + (`github.com/schollz/progressbar/v3`), `golang.org/x/term` to tell + whether stderr is a terminal, and **one SQLite driver** (`modernc.org/sqlite`, pure Go, so builds keep cgo disabled). `github.com/spf13/viper` is permitted if configuration-file support is ever needed, but is not currently used. No other third-party @@ -533,10 +534,13 @@ hash: [12345/98765] 12% |████ | 92 files/s elapsed 2:32 eta 17:54 Additional requirements: -- When stderr is not a TTY, do not emit ANSI redraws: print a plain - one-line progress update no more often than every 5 seconds instead. +- When stderr is not a terminal (a pipe, a file, `/dev/null`), do not + emit ANSI redraws: print a plain one-line progress update the moment + each phase starts, then no more often than every 5 seconds. - Progress updates are driven from the main goroutine and must be - non-blocking with respect to the worker pool. + non-blocking with respect to the worker pool. Nothing redraws in the + background (the spinner advances only as items complete), so a + warning printed during a phase always lands on its own line. - `report` and `trees` modes need no progress display, only their stderr summaries. diff --git a/TODO.md b/TODO.md index b540d3a..c0db823 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,9 @@ # Completed Steps +- progress prints at once on a non-terminal, uses a real terminal test, + and has a single writer to stderr (2026-10-03, + https://git.eeqj.de/sneak/sfdupes/issues/13) - stamp the git tag or short commit in a plain `docker build .` instead of `dev` (2026-10-02, branch `next`, closes https://git.eeqj.de/sneak/sfdupes/issues/67): `.dockerignore` now diff --git a/go.mod b/go.mod index 9b0faaa..66cc9f6 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.25.7 require ( github.com/schollz/progressbar/v3 v3.19.1 github.com/spf13/cobra v1.10.2 + golang.org/x/term v0.44.0 modernc.org/sqlite v1.54.0 ) @@ -19,7 +20,6 @@ require ( github.com/rivo/uniseg v0.4.7 // indirect github.com/spf13/pflag v1.0.9 // indirect golang.org/x/sys v0.46.0 // indirect - golang.org/x/term v0.44.0 // indirect modernc.org/libc v1.74.1 // indirect modernc.org/mathutil v1.7.1 // indirect modernc.org/memory v1.11.0 // indirect diff --git a/progress.go b/progress.go index ecb0585..2d8376e 100644 --- a/progress.go +++ b/progress.go @@ -6,6 +6,7 @@ import ( "time" "github.com/schollz/progressbar/v3" + "golang.org/x/term" ) // plainInterval is the minimum time between progress lines when stderr @@ -24,24 +25,21 @@ const percentScale = 100 // stderrIsTTY reports whether stderr is attached to a terminal. func stderrIsTTY() bool { - fi, err := os.Stderr.Stat() - if err != nil { - return false - } - - return fi.Mode()&os.ModeCharDevice != 0 + return term.IsTerminal(int(os.Stderr.Fd())) } // progress renders one scan pass's progress on stderr. On a TTY it // delegates to the progressbar library (spinner style when the total is // unknown, full bar with count/percent/rate/elapsed/ETA otherwise). When // stderr is not a TTY it emits no ANSI redraws: it prints a plain -// one-line update no more often than every plainInterval. +// one-line update as the pass starts, then no more often than every +// plainInterval. // -// All methods must be called from the main goroutine only. A nil -// *progress is a valid no-display receiver: every method is a no-op, -// so batched database flushes during the streaming pass can reuse the -// update-pass helpers without rendering anything. +// All methods must be called from the main goroutine only; nothing +// else writes to stderr while a pass runs. A nil *progress is a valid +// no-display receiver: every method is a no-op, so batched database +// flushes during the streaming pass can reuse the update-pass helpers +// without rendering anything. type progress struct { label string total int64 // -1 when unknown (walk pass) @@ -53,10 +51,22 @@ type progress struct { func newProgress(label string, total int64) *progress { p := &progress{label: label, total: total, start: time.Now()} - if !stderrIsTTY() { + if stderrIsTTY() { + p.bar = newBar(label, total) + return p } + // Print the zero state at once: the first item may take minutes, + // and a pass must never look hung. + p.last = p.start + fmt.Fprintln(os.Stderr, p.plainLine()) + + return p +} + +// newBar builds the TTY display for newProgress. +func newBar(label string, total int64) *progressbar.ProgressBar { opts := []progressbar.Option{ progressbar.OptionSetWriter(os.Stderr), progressbar.OptionSetDescription(label), @@ -78,12 +88,14 @@ func newProgress(label string, total int64) *progress { opts = append(opts, progressbar.OptionSetPredictTime(false), progressbar.OptionSpinnerType(walkSpinnerType), + // Advance the spinner only on Add. The library's default + // redraws it from its own goroutine every 100ms, which + // garbles a warning printed at the same moment. + progressbar.OptionSetSpinnerChangeInterval(0), ) } - p.bar = progressbar.NewOptions64(total, opts...) - - return p + return progressbar.NewOptions64(total, opts...) } // increment records one completed item and refreshes the display. diff --git a/progress_test.go b/progress_test.go new file mode 100644 index 0000000..50a8c0c --- /dev/null +++ b/progress_test.go @@ -0,0 +1,150 @@ +package main + +import ( + "os" + "path/filepath" + "slices" + "strings" + "testing" + "time" +) + +// spinnerIdle outlasts the 100ms interval at which the progressbar +// library redraws a spinner from its own goroutine unless told not to. +const spinnerIdle = 300 * time.Millisecond + +// captureStderr points os.Stderr at a file for the rest of the test and +// returns a function reading back everything written to it. +func captureStderr(t *testing.T) func() string { + t.Helper() + + path := filepath.Join(t.TempDir(), "stderr") + + f, err := os.Create(path) //nolint:gosec // test-controlled path + if err != nil { + t.Fatal(err) + } + + saved := os.Stderr + os.Stderr = f + + t.Cleanup(func() { + os.Stderr = saved + + _ = f.Close() + }) + + return func() string { + b, err := os.ReadFile(path) //nolint:gosec // test-controlled path + if err != nil { + t.Fatal(err) + } + + return string(b) + } +} + +//nolint:paralleltest // replaces the process-wide os.Stderr +func TestStderrIsTTYFalseForNonTerminals(t *testing.T) { + r, pipe, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + + regular, err := os.Create(filepath.Join(t.TempDir(), "stderr")) + if err != nil { + t.Fatal(err) + } + + devNull, err := os.OpenFile(os.DevNull, os.O_WRONLY, 0) + if err != nil { + t.Fatal(err) + } + + saved := os.Stderr + + t.Cleanup(func() { + os.Stderr = saved + + for _, f := range []*os.File{r, pipe, regular, devNull} { + _ = f.Close() + } + }) + + cases := map[string]*os.File{ + "a pipe": pipe, + "a regular file": regular, + os.DevNull: devNull, + } + + for name, f := range cases { + os.Stderr = f + + if stderrIsTTY() { + t.Errorf("stderrIsTTY() = true with stderr on %s", name) + } + } +} + +// TestNewProgressPrintsBeforeFirstItem checks that each pass shows its +// zero state the moment it starts when stderr is not a terminal, and +// that the next line still waits for plainInterval. +// +//nolint:paralleltest // captureStderr replaces the process-wide os.Stderr +func TestNewProgressPrintsBeforeFirstItem(t *testing.T) { + stderr := captureStderr(t) + + newProgress("walk", -1).increment() + newProgress("hash", 10).increment() + + want := "walk: 0 files, elapsed 0s\n" + + "hash: [0/10] 0% 0 files/s elapsed 0s eta ?\n" + if got := stderr(); got != want { + t.Errorf("stderr = %q, want %q", got, want) + } +} + +// TestProgressWarningsOnOwnLines drives the terminal display of the walk +// pass: every warning must land on a line of its own between redraws of +// the spinner, and nothing may redraw while no method is being called, +// or a warning could be garbled by a redraw from another goroutine. +// +//nolint:paralleltest // captureStderr replaces the process-wide os.Stderr +func TestProgressWarningsOnOwnLines(t *testing.T) { + stderr := captureStderr(t) + + p := &progress{ + label: "walk", total: -1, start: time.Now(), + bar: newBar("walk", -1), + } + + for _, w := range []string{"first", "second", "third"} { + p.increment() + p.warnf("warning %s", w) + } + + p.increment() + + before := stderr() + + time.Sleep(spinnerIdle) + + if after := stderr(); after != before { + t.Errorf("stderr written while idle: %q", after[len(before):]) + } + + // A terminal shows each line as the text after its last carriage + // return. + var warnings []string + + for line := range strings.SplitSeq(before, "\n") { + if strings.Contains(line, "warning") { + warnings = append(warnings, line[strings.LastIndex(line, "\r")+1:]) + } + } + + want := []string{"warning first", "warning second", "warning third"} + if !slices.Equal(warnings, want) { + t.Errorf("warning lines = %q, want %q", warnings, want) + } +}