diff --git a/README.md b/README.md index d48f695..6227c7a 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 @@ -557,10 +558,16 @@ 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. On a terminal the + spinner-style displays also redraw on their own several times a + second, so their count and elapsed time stay current while a phase + waits for its next item. +- A warning printed during a phase always lands on a line of its own, + never inside the progress display. - `report` and `trees` modes need no progress display, only their stderr summaries. diff --git a/TODO.md b/TODO.md index c6de6e9..4125568 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,10 @@ # Completed Steps +- progress prints at once on a non-terminal, uses a real terminal test, + and prints warnings through a spinner instead of racing its redraw + (2026-10-03, https://git.eeqj.de/sneak/sfdupes/issues/13) + - `report` and `trees` open the database read-only, and `scan` leaves it out of WAL mode, so reading needs only read access (2026-10-03, closes https://git.eeqj.de/sneak/sfdupes/issues/8) 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 f351d47..e6ca868 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,23 @@ 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. On a TTY +// the library also redraws a spinner from its own goroutine, several +// times a second, so its count and elapsed time stay current while a +// pass waits for its next item. 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 +53,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), @@ -81,9 +93,7 @@ func newProgress(label string, total int64) *progress { ) } - p.bar = progressbar.NewOptions64(total, opts...) - - return p + return progressbar.NewOptions64(total, opts...) } // increment records one completed item and refreshes the display. @@ -113,11 +123,22 @@ func (p *progress) warnf(format string, args ...any) { return } + msg := escapePath(fmt.Sprintf(format, args...)) + + if p.bar != nil && p.total < 0 { + // The library also redraws a spinner from its own goroutine, so + // a direct write could land inside a redraw. The bar prints the + // warning itself, just before its next redraw. + _, _ = progressbar.Bprintln(p.bar, msg) + + return + } + if p.bar != nil { _ = p.bar.Clear() } - fmt.Fprintln(os.Stderr, escapePath(fmt.Sprintf(format, args...))) + fmt.Fprintln(os.Stderr, msg) } // finish terminates the pass's display. diff --git a/progress_test.go b/progress_test.go new file mode 100644 index 0000000..b3e757a --- /dev/null +++ b/progress_test.go @@ -0,0 +1,186 @@ +package main + +import ( + "os" + "path/filepath" + "slices" + "strings" + "testing" + "time" +) + +// spinnerIdle comfortably outlasts the 100ms interval at which the +// progressbar library redraws a spinner from its own goroutine. +const spinnerIdle = 500 * 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) + } +} + +// newWalkSpinner returns the walk pass's terminal display, writing to +// os.Stderr whether or not it is a terminal, and stops the library's +// redraws when the test ends. +func newWalkSpinner(t *testing.T) *progress { + t.Helper() + + p := &progress{ + label: "walk", total: -1, start: time.Now(), + bar: newBar("walk", -1), + } + t.Cleanup(p.finish) + + return p +} + +// TestProgressWarningsOnOwnLines drives the terminal display of the walk +// pass: every warning must land on a line of its own, never inside a +// redraw of the spinner. +// +//nolint:paralleltest // captureStderr replaces the process-wide os.Stderr +func TestProgressWarningsOnOwnLines(t *testing.T) { + stderr := captureStderr(t) + p := newWalkSpinner(t) + + for _, w := range []string{"first", "second", "third"} { + p.increment() + p.warnf("warning %s", w) + } + + p.increment() + + // The spinner prints the warnings at its next redraw. + time.Sleep(spinnerIdle) + + // A terminal shows each line as the text after its last carriage + // return. + var warnings []string + + for line := range strings.SplitSeq(stderr(), "\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) + } +} + +// TestSpinnerShowsCountAfterBurst checks that once a burst of items +// faster than the redraw limit is over, the walk display shows every +// item completed while it waits for the next one. +// +//nolint:paralleltest // captureStderr replaces the process-wide os.Stderr +func TestSpinnerShowsCountAfterBurst(t *testing.T) { + stderr := captureStderr(t) + p := newWalkSpinner(t) + + for range 50 { + p.increment() + } + + time.Sleep(spinnerIdle) + + // A terminal shows the last frame drawn. The library starts each + // frame with a carriage return and erases the previous one with + // spaces first. + var shown string + + for frame := range strings.SplitSeq(stderr(), "\r") { + if strings.TrimSpace(frame) != "" { + shown = frame + } + } + + if !strings.Contains(shown, "(50/-,") { + t.Errorf("terminal shows %q, want a count of 50", shown) + } +}