Repository navigation
fix: treat "-vn=" in a short option group as an empty value - #2454
Conversation
|
| assert.Equal(t, []string{"positional"}, args) | ||
| }) | ||
|
|
||
| t.Run("-vn= in a short option group sets empty string", func(t *testing.T) { |
coilysiren
left a comment
There was a problem hiding this comment.
still approved - just housekeeping
With short option handling, the value after "=" was passed to every bool flag in the group, not only to the last flag. "-vn=foo" failed with `invalid value "foo" for flag -v`, and "-vx=false" set both v and x to false. Only the last flag of a group can take a value, so set each bool flag before it to true and pass the value to the last flag only. Fixes: 94ba4f7 ("Fix:(issue_2066) Remove dependency on golang flag") Assisted-by: claude:claude-opus-5-5 Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
Juneezee
left a comment
There was a problem hiding this comment.
LGTM, thanks! This is the same !valFromEqual check the single flag path already has:
Line 176 in 335d72b
I checked the table in the description against main and this branch, and the two new subtests fail on main.
While testing this I found a related bug that is already on main: the value after = is passed to every bool flag in the group, not only to the last flag. With the command from the description, app -vn=foo fails with invalid value "foo" for flag -v: parse error:
Lines 238 to 245 in bb59353
It is in the same loop, so I pushed a fix with a test to this branch in 0cd8330.
What type of PR is this?
What this PR does / why we need it:
#2297 (for #2293) made
--name=set the flag to an empty string instead of reading the next argument as its value. The short option group path inparseFlagswas not changed, so withUseShortOptionHandlingthe same=behaves differently once the flag is the last member of a group:nbeforenafterapp -n= positional"", args[positional]app -vn= positional"positional", args[]"", args[positional]app -vn=flag needs an argument: n""command_parse.go: the last flag of a short option group only reads the next argument when there was no=in the token, which is the same!valFromEqualcheck the single flag path already has.command_test.go: two subtests inTestFlagEqualsEmptyValue, one for-vn= positionaland one for-vn=at the end of the args.Which issue(s) this PR fixes:
None filed. I found this while reading the short option code next to the #2297 change and couldn't find an existing issue or PR about it.
Testing
main(the positional is consumed as the value;flag needs an argument: n) and pass with the change.make generate vet test gfmrunpasses.make generaterewrites a few doc links ingodoc-current.txtwith my local Go 1.24, onmaintoo, so I left that file alone.Release Notes