v3: yield the version flag's -v alias to a user-defined flag#2330
Open
c-tonneslan wants to merge 1 commit into
Open
v3: yield the version flag's -v alias to a user-defined flag#2330c-tonneslan wants to merge 1 commit into
c-tonneslan wants to merge 1 commit into
Conversation
The default version flag carries -v as an alias. A user-defined flag
that also wants -v (canonical case: --verbose / -v) wasn't actually
broken at parse time: -v ended up setting the user flag. But
checkVersion went through cmd.Bool, which resolves aliases, so it
asked for the value of "v" and got back the user flag's value. The
end result: -v was silently treated as "print version and exit".
Two changes, both narrow:
- During root setup, drop any aliases from the local copy of the
version flag that are already claimed by a user-defined flag's
name or alias. Keeps the user flag the sole owner of the
short form.
- checkVersion now looks for the actual version flag attached to
the command (matching against the canonical primary name from
the global VersionFlag) and checks its IsSet directly, so it
can't be fooled by a same-named alias on a different flag.
Fixes urfave#2229.
Signed-off-by: Charlie Tonneslan <cst0520@gmail.com>
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
1 participant
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.
Fixes #2229.
The default version flag carries `-v` as an alias. A user-defined flag that also wants `-v` (canonical case: `--verbose / -v`) wasn't actually broken at parse time: `-v` ended up setting the user flag. But `checkVersion` went through `cmd.Bool`, which resolves aliases, so it asked for the value of `"v"` and got back the user flag's value. The end result: passing `-v` was silently treated as "print version and exit."
Two narrow changes:
Added `TestVersionFlagYieldsAliasToUserFlag` covering the reporter's reproducer.