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.