fix: address PR #24 review concerns

1. Replace os.Stderr with v.Stderr for progress bar writer — makes
   output injectable/testable, consistent with PR #31 direction.

2. Fix isTerminal() to accept io.Writer and check v.Stderr fd instead
   of os.Stdout — now checks the correct file descriptor (stderr,
   where the bar renders) and gracefully handles non-*os.File writers
   (returns false in tests).

3. Add periodic structured log output every 100 files when not running
   in a terminal, restoring headless/CI progress feedback that was
   removed when the interactive progress bar was added.

4. Apply same os.Stderr -> v.Stderr fix to the verify progress bar
   for consistency.
This commit is contained in:
user 2026-02-15 21:31:58 -08:00
parent 9f43dc1428
commit 50b3b38289

View File

@ -126,24 +126,26 @@ func (v *Vaultik) Restore(opts *RestoreOptions) error {
humanize.Bytes(uint64(totalBytes)),
)
// Create progress bar if output is a terminal
// Create progress bar if stderr is a terminal
isTTY := isTerminal(v.Stderr)
var bar *progressbar.ProgressBar
if isTerminal() {
if isTTY {
bar = progressbar.NewOptions64(
totalBytes,
progressbar.OptionSetDescription("Restoring"),
progressbar.OptionSetWriter(os.Stderr),
progressbar.OptionSetWriter(v.Stderr),
progressbar.OptionShowBytes(true),
progressbar.OptionShowCount(),
progressbar.OptionSetWidth(40),
progressbar.OptionThrottle(100*time.Millisecond),
progressbar.OptionOnCompletion(func() {
fmt.Fprint(os.Stderr, "\n")
fmt.Fprint(v.Stderr, "\n")
}),
progressbar.OptionSetRenderBlankState(true),
)
}
filesProcessed := 0
for _, file := range files {
if v.ctx.Err() != nil {
return v.ctx.Err()
@ -151,17 +153,33 @@ func (v *Vaultik) Restore(opts *RestoreOptions) error {
if err := v.restoreFile(v.ctx, repos, file, opts.TargetDir, identity, chunkToBlobMap, blobCache, result); err != nil {
log.Error("Failed to restore file", "path", file.Path, "error", err)
filesProcessed++
// Update progress bar even on failure
if bar != nil {
_ = bar.Add64(file.Size)
}
// Periodic structured log for non-terminal contexts (headless/CI)
if !isTTY && filesProcessed%100 == 0 {
log.Info("Restore progress",
"files", fmt.Sprintf("%d/%d", filesProcessed, len(files)),
"bytes_restored", humanize.Bytes(uint64(result.BytesRestored)),
)
}
continue
}
filesProcessed++
// Update progress bar
if bar != nil {
_ = bar.Add64(file.Size)
}
// Periodic structured log for non-terminal contexts (headless/CI)
if !isTTY && (filesProcessed%100 == 0 || filesProcessed == len(files)) {
log.Info("Restore progress",
"files", fmt.Sprintf("%d/%d", filesProcessed, len(files)),
"bytes_restored", humanize.Bytes(uint64(result.BytesRestored)),
)
}
}
if bar != nil {
@ -555,7 +573,7 @@ func (v *Vaultik) verifyRestoredFiles(
// Create progress bar if output is a terminal
var bar *progressbar.ProgressBar
if isTerminal() {
if isTerminal(v.Stderr) {
bar = progressbar.NewOptions64(
totalBytes,
progressbar.OptionSetDescription("Verifying"),
@ -663,7 +681,11 @@ func (v *Vaultik) verifyFile(
return bytesVerified, nil
}
// isTerminal returns true if stdout is a terminal
func isTerminal() bool {
return term.IsTerminal(int(os.Stdout.Fd()))
// isTerminal returns true if the given writer is connected to a terminal.
// Returns false if the writer does not expose a file descriptor (e.g. in tests).
func isTerminal(w io.Writer) bool {
if f, ok := w.(*os.File); ok {
return term.IsTerminal(int(f.Fd()))
}
return false
}