Repository navigation
Conversation
Since v3.2.0 an argument such as "-4" is treated as the first positional argument, and flag parsing stops there, whenever it does not start with a letter. That keeps negative numbers out of flag parsing, but it also means a flag named "4" can no longer be given as "-4", which worked in v3.1 and v2. Only stop at such an argument when it does not name a defined flag.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
What this PR does / why we need it:
A flag whose name starts with a digit can't be given with a single dash any more. With
&BoolFlag{Name: "4"}, runningapp -4 operandleaves the flag unset, andcmd.Args()comes back as["-4", "operand"]. Flag parsing also stops at-4, so any flags after it are ignored as well.--4still works. Digit flags are fairly common in real tools (ssh -4/-6,curl -0,ls -1), and getopt and the POSIX utility syntax guidelines treat-4as an option when the utility defines one.#2088 (released in v3.2.0) added the cause: any single-dash argument whose first character isn't a letter ends flag parsing, so that negative numbers such as
-10reach positional arguments. That check comes before flag lookup, so it also catches flags that really are defined. Before that, v3.1.1 and v2 (both on the standardflagpackage) parsed-4as the flag.command_parse.go: stop parsing at a single-dash argument that doesn't start with a letter only when it doesn't name a defined flag. A new helper,startsWithAppliedFlag, checks the name before any=. WithUseShortOptionHandlingit also checks the group's first character, so-46sets both4and6. Undefined arguments like-5still become positional, so negative-number arguments behave as before.command_test.go: addTestCommand_DigitFlag. It covers-4,-4 -6 -level high, a name that starts with a digit (-2fa=on), a short option group (-46), and a control case showing that an undefined-5still ends flag parsing.Which issue(s) this PR fixes:
None filed. I found this on current main while comparing single-dash parsing with v3.1. I didn't find a matching issue or a competing PR.
Special notes for your reviewer:
If a flag named
1exists,-1now always means that flag and is never a negative-number positional. getopt and v3.1 behave the same way, and only commands that define such a flag are affected.Testing
go test -run TestCommand_DigitFlag .: the first 4 cases fail on main (192ed0b) and pass with this change. The control case passes on both.-4and-4vset the flags, and on v3.2.0 both are left as positional arguments. On v2-maint-4sets the flag.go test ./...,make generate,make vet,make test,make test GFLAGS='--tags urfave_cli_no_template',make check-binary-size,make diffcheck,make v3diff,gofmt -l .,goimports -l .andgolangci-lint run(v2 latest) pass. Generated API documentation is unchanged.make gfmrun(gfmrun not installed) and Windows/Linux CI.Release Notes