schollz-progressbar/PR.md

Title: Fix progress bar getting permanently stuck after an overshooting Add/Set call

Problem

Add64 (progressbar.go:724, reached by Add, Add64, Set and Set64) is meant to reject a call that would push the counter past max and leave the bar unchanged:

if p.state.currentNum < p.config.max {
    ...
    p.state.currentNum += num
}
...
if p.state.currentNum > p.config.max {
    return errors.New("current number exceeds max")
}

The bounds check runs after currentNum (and currentBytes) have already been mutated, so a single call that overshoots max is not rejected atomically: the error is returned, but the invalid, over-max state is kept. Once that happens the bar is stuck: currentNum never advances again, because the guard at the top of the function (currentNum < max) is now permanently false, and currentPercent/the saucer width stay frozen above 100%. Every subsequent Add or Set call, even ones that would otherwise be perfectly valid, keeps returning "current number exceeds max" forever, with no way to recover short of Reset().

Reproduced on a clean checkout with:

bar := New(100)
bar.Add(95)                  // currentNum=95, no error
bar.Add(10)                  // currentNum=105, err="current number exceeds max"
bar.Add(1)                   // currentNum still 105, same error again -- stuck forever

This is reachable through the public API with nothing exotic: any caller whose increments do not sum to exactly max (a very common case, e.g. an estimated total that is off by a few bytes, or a Set() racing another goroutine's Add()) will hit this on the final update, at 95-105% of the way through, and never see the bar reach 100% or accept further progress again.

Fix

Move the bounds check before the mutation, so an overshooting call is rejected as a whole and leaves currentNum/currentBytes/currentPercent exactly as they were before the call:

if !p.config.ignoreLength && p.state.currentNum+num > p.config.max {
    return errors.New("current number exceeds max")
}

The ignoreLength mode is left out of the new guard on purpose: it advances with (currentNum + num) % max, which by construction wraps and never exceeds max, so the check would never trigger there anyway; excluding it keeps that code path untouched. The old post-hoc check at the end of the function is now unreachable in the non-ignoreLength path (the mutation it used to guard can no longer produce an out-of-range value) and is removed rather than left as dead code.

Behavior change worth flagging: previously, once currentNum reached exactly max, further Add(n) calls with n > 0 were silently absorbed with no error (currentNum simply stopped advancing, though currentBytes kept drifting upward unnoticed). With this fix such calls now consistently return the same "current number exceeds max" error as any other overshoot, which matches what the error message already claims and is the only existing test's expectation (TestBar, which only asserts that an error is returned, not any particular resulting state).

Added TestAddExceedsMaxDoesNotCorruptState in progressbar_test.go, next to the existing TestBar overshoot test, asserting that a rejected Add leaves currentNum/currentPercent unchanged and that the bar keeps accepting valid progress afterwards.

Verification

• go build ./...: exit 0.
• go vet . (this package): exit 0. go vet ./... (whole repo) exits 1, but solely because of a pre-existing, unrelated issue in examples/download-unknown/main.go:14 ("using resp before checking for errors"), confirmed present before this change via git stash; not touched here.
• gofmt -l .: no output (clean).
• go test ./... -count=1 -v: exit 0, 33 top-level PASS, 0 FAIL, 0 SKIP.
• Counterfactual: reverting only the Add64 fix (keeping the new test) fails it with expected currentNum to stay at 95 after a rejected Add, got 105, matching the reproduction above; re-applying the fix restores a clean pass.

Flags for account owner

• The "silently absorbed at exactly max" -> "consistently rejected" behavior change described above is the one substantive semantic change beyond the corruption fix; flagging it explicitly in case the maintainer wants it called out in a changelog entry.
• Two unrelated open PRs (#235, #237) touch progressbar.go/progressbar_test.go in different functions (clearProgressBar/render visibility); checked their diffs, no line overlap with this change.