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 c465312412
commit 4c23398243

View File

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