Skip to content

Return -1 from GetMax for a bar of unknown length - #248

Open
abo3losh1 wants to merge 2 commits into
schollz:mainfrom
abo3losh1:fix/getmax-unknown-length
Open

abo3losh1 wants to merge 2 commits into
schollz:mainfrom
abo3losh1:fix/getmax-unknown-length

Conversation

@abo3losh1

Copy link
Copy Markdown

For a bar of unknown length, GetMax and GetMax64 return the bar width, while State().Max says -1:

bar := progressbar.NewOptions(-1)
bar.GetMax()     // 40
bar.State().Max  // -1

progressbar.Default(-1).GetMax() // 10, its width

bar = progressbar.NewOptions(100)
bar.ChangeMax(-1)
bar.GetMax()     // 40

lengthUnknown() stores config.width in config.max so the spinner can use it, and the getters return that field as is. State() already checks ignoreLength and reports -1, so this makes GetMax64 do the same, and GetMax now calls GetMax64. Nothing inside the package calls the getters, so rendering is unchanged.

Added TestGetMaxUnknownLength, covering NewOptions(-1) and ChangeMax(-1) / ChangeMax(50). It fails before the change with GetMax() = 40, want -1. The rest of the suite is unchanged. On Windows, TestSpinnerClearOnFinish fails both with and without this change.

Related, but left out on purpose: AddMax on an unknown-length bar adds to the width (NewOptions(-1).AddMax(100) gives a max of 140), and its if p.config.max == -1 check never matches. It's not obvious what the right result is there, so I can do that as a follow-up if you want.

For an unknown length, lengthUnknown stores the bar width in config.max,
so GetMax and GetMax64 returned the width (40 by default) while State()
reported Max as -1. Return -1 from both getters in that case.
Copilot AI balanced review requested due to automatic review settings October 14, 2026 23:18

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants