diff --git a/README.md b/README.md index f4e7667..ae992d3 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`), **one SQLite driver** + (`github.com/schollz/progressbar/v3`), `golang.org/x/term` to tell + whether stderr is a terminal, **one SQLite driver** (`modernc.org/sqlite`, pure Go, so builds keep cgo disabled), and `golang.org/x/sys` for `flock(2)` (the scan lock, see "Database"). `github.com/spf13/viper` is permitted if configuration-file support @@ -589,10 +590,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 b07d9a1..95fefb7 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) + - warn about and skip symlink, socket, FIFO, device and `.zfs` operands, keeping the records beneath them (2026-10-03, https://git.eeqj.de/sneak/sfdupes/issues/9) diff --git a/go.mod b/go.mod index 5a13062..cf8c37c 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( github.com/schollz/progressbar/v3 v3.19.1 github.com/spf13/cobra v1.10.2 golang.org/x/sys v0.46.0 + golang.org/x/term v0.44.0 modernc.org/sqlite v1.54.0 ) @@ -19,7 +20,6 @@ require ( github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect github.com/rivo/uniseg v0.4.7 // indirect github.com/spf13/pflag v1.0.9 // 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..363759a --- /dev/null +++ b/progress_test.go @@ -0,0 +1,161 @@ +package main + +import ( + "os" + "path/filepath" + "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 + +//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 through a run of warnings with no items between them, as when the +// walk meets many unreadable paths, for several of the spinner's +// redraws: every warning must land on a line of its own, never inside a +// redraw. +// +//nolint:paralleltest // captureStderr replaces the process-wide os.Stderr +func TestProgressWarningsOnOwnLines(t *testing.T) { + stderr := captureStderr(t) + p := newWalkSpinner(t) + + // No pause between warnings: one written straight to stderr is + // garbled only if a redraw lands while it is being written. + issued := 0 + for start := time.Now(); time.Since(start) < spinnerIdle; issued++ { + p.warnf("warning") + } + + // 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. + shown := 0 + + for line := range strings.SplitSeq(stderr(), "\n") { + if !strings.Contains(line, "warning") { + continue + } + + shown++ + + if text := line[strings.LastIndex(line, "\r")+1:]; text != "warning" { + t.Errorf("terminal shows %q, want %q", text, "warning") + } + } + + if shown != issued { + t.Errorf("%d warning lines, want %d", shown, issued) + } +} + +// 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) + } +}