-
Notifications
You must be signed in to change notification settings - Fork 371
Extend UnknownFlags to UnknownFlagsHandling and introduce PassUnknownFlagToArgs (#337)
#440
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -137,10 +137,29 @@ const ( | |
| PanicOnError | ||
| ) | ||
|
|
||
| // UnknownFlagsHandling decides how to handle unknown flags | ||
| type UnknownFlagsHandling int | ||
|
|
||
| const ( | ||
| // ErrorOnUnknownFlag will return an error if an unknown flag is found | ||
| ErrorOnUnknownFlag UnknownFlagsHandling = iota | ||
| // IgnoreUnknownFlag will ignore unknown flags and continue parsing rest of the flags | ||
| IgnoreUnknownFlag | ||
| // PassUnknownFlagToArgs will treat unknown flags as non-flag arguments. | ||
| // Combined shorthand flags mixed with known ones and unknown ones results | ||
| // combined flags only with unknown ones. | ||
| // E.g. -fghi results -gh if only `f` and `i` are known. | ||
| PassUnknownFlagToArgs | ||
| ) | ||
|
|
||
| // ParseErrorsAllowlist defines the parsing errors that can be ignored | ||
| type ParseErrorsAllowlist struct { | ||
| // UnknownFlags will ignore unknown flags errors and continue parsing rest of the flags | ||
| // Deprecated: Use UnknownFlagsHandling instead | ||
| UnknownFlags bool | ||
|
|
||
| // UnknownFlagsHandling decides how to handle unknown flags. Defaults to UnknownFlagsHandlingErrorOnUnknown. | ||
| UnknownFlagsHandling UnknownFlagsHandling | ||
| } | ||
|
|
||
| // ParseErrorsWhitelist defines the parsing errors that can be ignored. | ||
|
|
@@ -337,6 +356,35 @@ func (f *FlagSet) HasAvailableFlags() bool { | |
| return false | ||
| } | ||
|
|
||
| // getUnknownFlagsHandling returns the UnknownFlagsHandling value, | ||
| // considering deprecated ParseErrorsWhitelist and UnknownFlags field | ||
| // After removing ParseErrorsWhitelist, this function can be simplified | ||
| // and moved to ParseErrorsAllowlist.getUnknownFlagsHandling() | ||
| func (f *FlagSet) getUnknownFlagsHandling() UnknownFlagsHandling { | ||
| // first check ParseErrorsAllowlist: | ||
| // if UnknownFlagsHandling is set, use it | ||
| if f.ParseErrorsAllowlist.UnknownFlagsHandling != ErrorOnUnknownFlag { | ||
| return f.ParseErrorsAllowlist.UnknownFlagsHandling | ||
| } | ||
|
|
||
| if f.ParseErrorsAllowlist.UnknownFlags { | ||
| return IgnoreUnknownFlag | ||
| } | ||
|
|
||
| // then, check deprecated ParseErrorsWhitelist: | ||
| // if UnknownFlagsHandling is set, use it | ||
| if f.ParseErrorsWhitelist.UnknownFlagsHandling != ErrorOnUnknownFlag { | ||
| return f.ParseErrorsAllowlist.UnknownFlagsHandling | ||
| } | ||
|
|
||
| if f.ParseErrorsWhitelist.UnknownFlags { | ||
| return IgnoreUnknownFlag | ||
| } | ||
|
|
||
| // Otherwise, return the default value | ||
| return ErrorOnUnknownFlag | ||
| } | ||
|
|
||
| // VisitAll visits the command-line flags in lexicographical order or | ||
| // in primordial order if f.SortFlags is false, calling fn for each. | ||
| // It visits all flags, even those not set. | ||
|
|
@@ -988,6 +1036,17 @@ func stripUnknownFlagValue(args []string) []string { | |
| return nil | ||
| } | ||
|
|
||
| // errUnknownFlag is used for internal unknown flag handling. | ||
| type unknownFlagError struct { | ||
| // UnknownFlags is flags that are unknown and unprocessed. | ||
| // It depends on the context whether this has a prefix like '-' or '--'. | ||
| UnknownFlags string | ||
| } | ||
|
|
||
| func (e *unknownFlagError) Error() string { | ||
| return fmt.Sprintf("unknown flag: %v", e.UnknownFlags) | ||
| } | ||
|
|
||
| func (f *FlagSet) parseLongArg(s string, args []string, fn parseFunc) (a []string, err error) { | ||
| a = args | ||
| name := s[2:] | ||
|
|
@@ -999,22 +1058,25 @@ func (f *FlagSet) parseLongArg(s string, args []string, fn parseFunc) (a []strin | |
| split := strings.SplitN(name, "=", 2) | ||
| name = split[0] | ||
| flag, exists := f.formal[f.normalizeFlagName(name)] | ||
| unknownFlagsHandling := f.getUnknownFlagsHandling() | ||
|
|
||
| if !exists { | ||
| switch { | ||
| case name == "help": | ||
| f.usage() | ||
| return a, ErrHelp | ||
| case f.ParseErrorsWhitelist.UnknownFlags: | ||
| fallthrough | ||
| case f.ParseErrorsAllowlist.UnknownFlags: | ||
| case unknownFlagsHandling == IgnoreUnknownFlag: | ||
| // --unknown=unknownval arg ... | ||
| // we do not want to lose arg in this case | ||
| if len(split) >= 2 { | ||
| return a, nil | ||
| } | ||
|
|
||
| return stripUnknownFlagValue(a), nil | ||
| case unknownFlagsHandling == PassUnknownFlagToArgs: | ||
| return a, &unknownFlagError{ | ||
| UnknownFlags: s, | ||
| } | ||
| default: | ||
| err = f.fail(&NotExistError{name: name, messageType: flagUnknownFlagMessage}) | ||
| return | ||
|
|
@@ -1060,14 +1122,14 @@ func (f *FlagSet) parseSingleShortArg(shorthands string, args []string, fn parse | |
|
|
||
| flag, exists := f.shorthands[c] | ||
| if !exists { | ||
| unknownFlagsHandling := f.getUnknownFlagsHandling() | ||
|
|
||
| switch { | ||
| case c == 'h': | ||
| f.usage() | ||
| err = ErrHelp | ||
| return | ||
| case f.ParseErrorsWhitelist.UnknownFlags: | ||
| fallthrough | ||
| case f.ParseErrorsAllowlist.UnknownFlags: | ||
| case unknownFlagsHandling == IgnoreUnknownFlag: | ||
| // '-f=arg arg ...' | ||
| // we do not want to lose arg in this case | ||
| if len(shorthands) > 2 && shorthands[1] == '=' { | ||
|
|
@@ -1077,6 +1139,20 @@ func (f *FlagSet) parseSingleShortArg(shorthands string, args []string, fn parse | |
|
|
||
| outArgs = stripUnknownFlagValue(outArgs) | ||
| return | ||
| case unknownFlagsHandling == PassUnknownFlagToArgs: | ||
| // '-f=arg': pass all the argument | ||
| if len(shorthands) > 2 && shorthands[1] == '=' { | ||
| outShorts = "" | ||
| err = &unknownFlagError{ | ||
| UnknownFlags: shorthands, | ||
| } | ||
| return | ||
| } | ||
| // '-fgh': pass only the first switch | ||
| err = &unknownFlagError{ | ||
| UnknownFlags: shorthands[0:1], | ||
| } | ||
| return | ||
|
Comment on lines
+1144
to
+1155
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, differs in the next call. For an unknown flag,
Both In the next |
||
| default: | ||
| err = f.fail(&NotExistError{ | ||
| name: string(c), | ||
|
|
@@ -1127,14 +1203,31 @@ func (f *FlagSet) parseSingleShortArg(shorthands string, args []string, fn parse | |
| func (f *FlagSet) parseShortArg(s string, args []string, fn parseFunc) (a []string, err error) { | ||
| a = args | ||
| shorthands := s[1:] | ||
| var errUnknownFlagAll *unknownFlagError | ||
|
|
||
| // "shorthands" can be a series of shorthand letters of flags (e.g. "-vvv"). | ||
| for len(shorthands) > 0 { | ||
| shorthands, a, err = f.parseSingleShortArg(shorthands, args, fn) | ||
| if err != nil { | ||
| return | ||
| if errUnknownFlag, ok := err.(*unknownFlagError); ok { | ||
| // this means f.ParseErrorsAllowlist.UnknownFlagsHandling is set to UnknownFlagsHandlingPassUnknownToArgs | ||
| if errUnknownFlagAll == nil { | ||
| errUnknownFlagAll = &unknownFlagError{ | ||
| UnknownFlags: "-", | ||
| } | ||
| } | ||
|
|
||
| errUnknownFlagAll.UnknownFlags = errUnknownFlagAll.UnknownFlags + | ||
| errUnknownFlag.UnknownFlags | ||
| err = nil | ||
| } else { | ||
| return | ||
| } | ||
| } | ||
| } | ||
| if errUnknownFlagAll != nil { | ||
| err = errUnknownFlagAll | ||
| } | ||
|
|
||
| return | ||
| } | ||
|
|
@@ -1164,7 +1257,13 @@ func (f *FlagSet) parseArgs(args []string, fn parseFunc) (err error) { | |
| args, err = f.parseShortArg(s, args, fn) | ||
| } | ||
| if err != nil { | ||
| return | ||
| if errUnknownFlag, ok := err.(*unknownFlagError); ok { | ||
| // this means f.ParseErrorsAllowlist.UnknownFlagsHandling is set to UnknownFlagsHandlingPassUnknownToArgs | ||
| f.args = append(f.args, errUnknownFlag.UnknownFlags) | ||
| err = nil | ||
| } else { | ||
| return | ||
| } | ||
| } | ||
| } | ||
| return | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why the previous case calls
stripUnknownFlagValueand this is one does not.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can consider the case of
--unknown somevalue.--unknownis an unknown flag, and we cannot reliably tell whethersomevalueis a flag value or a positional argument.With
PassUnknownFlagToArgs, we do not need to classifysomevalue; we simply leave the argument stream intact and let the caller receive everything as positional args. So we don't need to callstripUnknownFlagValue().With
IgnoreUnknownFlag, the existing behavior treats the next token as a flag value (when it does not look like a flag) and callsstripUnknownFlagValue()to strip it. This behavior is inherited from the existingIgnoreUnknownFlagimplementation.