Extend UnknownFlags to UnknownFlagsHandling and introduce PassUnknownFlagToArgs (#337)#440
Extend UnknownFlags to UnknownFlagsHandling and introduce PassUnknownFlagToArgs (#337)#440ikedam wants to merge 3 commits intospf13:masterfrom
UnknownFlags to UnknownFlagsHandling and introduce PassUnknownFlagToArgs (#337)#440Conversation
flag_test.go
Outdated
| if f.Parsed() { | ||
| t.Fatal("f.Parse() = true before Parse") | ||
| } | ||
| f.ParseErrorsAllowlist.UnknownFlagsHandling = UnknownFlagsHandlingPassUnknownToArgs |
There was a problem hiding this comment.
This test code is same to https://github.com/spf13/pflag/pull/338/files#diff-7a7385a10188cdaa9ac18effc375607e65672f55084b8049b6d3f44a6ab70c51 except this line.
flag.go
Outdated
| // UnknownFlagsHandlingErrorOnUnknown will return an error if an unknown flag is found | ||
| UnknownFlagsHandlingErrorOnUnknown UnknownFlagsHandling = iota | ||
| // UnknownFlagsHandlingIgnoreUnknown will ignore unknown flags and continue parsing rest of the flags | ||
| UnknownFlagsHandlingIgnoreUnknown | ||
| // UnknownFlagsHandlingPassUnknownToArgs 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. | ||
| UnknownFlagsHandlingPassUnknownToArgs |
There was a problem hiding this comment.
Reeeally long names...
I worry names like ErrorOnUnknown would cause name conflict.
There was a problem hiding this comment.
Maybe ErrorOnUnknownFlag? Along with IgnoreUnknownFlag and PassUnknownFlagToArgs. I don't think they can be made shorter than that without losing meaning, but I don't think we need to be overly cautious about naming conflicts here.
If you really want to guard against it, another variant is to namespace these somehow - either with a struct value where the enum values are fields on the struct, or by just moving them to an appropriately named subpackage.
tomasaschan
left a comment
tomasaschan
left a comment
There was a problem hiding this comment.
I like this version a lot better than #338.
(The passing-error-as-control-flow thing smells a little, but the parseLongArg method was a mess before this change, so that's not your fault... Something to look out for in 2.0, I guess.)
Needs a rebase (I haven't looked into why) and probably shouldn't be merged until we've decided we don't have anything else we want to get into 1.0 before cutting 1.1.
flag.go
Outdated
| // UnknownFlagsHandlingErrorOnUnknown will return an error if an unknown flag is found | ||
| UnknownFlagsHandlingErrorOnUnknown UnknownFlagsHandling = iota | ||
| // UnknownFlagsHandlingIgnoreUnknown will ignore unknown flags and continue parsing rest of the flags | ||
| UnknownFlagsHandlingIgnoreUnknown | ||
| // UnknownFlagsHandlingPassUnknownToArgs 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. | ||
| UnknownFlagsHandlingPassUnknownToArgs |
There was a problem hiding this comment.
Maybe ErrorOnUnknownFlag? Along with IgnoreUnknownFlag and PassUnknownFlagToArgs. I don't think they can be made shorter than that without losing meaning, but I don't think we need to be overly cautious about naming conflicts here.
If you really want to guard against it, another variant is to namespace these somehow - either with a struct value where the enum values are fields on the struct, or by just moving them to an appropriately named subpackage.
flag.go
Outdated
| func (a *ParseErrorsAllowlist) getUnknownFlagsHandling() UnknownFlagsHandling { | ||
| // if UnknownFlagsHandling is set, use it | ||
| if a.UnknownFlagsHandling != UnknownFlagsHandlingErrorOnUnknown { | ||
| return a.UnknownFlagsHandling | ||
| } | ||
|
|
||
| if a.UnknownFlags { | ||
| return UnknownFlagsHandlingIgnoreUnknown | ||
| } | ||
| return UnknownFlagsHandlingErrorOnUnknown |
ikedam
force-pushed
the
feature/337_PreserveUnknownFlagsEnum
branch
from
f16d4d7 to
d1d7aee
Compare
* `UnknownFlagsHandlingErrorOnUnknown` to `ErrorOnUnknownFlag` and so on
ikedam
changed the title
UnknownFlags to UnknownFlagsHandling and introduce UnknownFlagsHandlingPassUnknownToArgs (#337)UnknownFlags to UnknownFlagsHandling and introduce PassUnknownFlagToArgs (#337)
2 participants
Close #337
Alternative to #338.
FlagSet.ParseErrorsAllowlist.UnknownFlagsHandling=PassUnknownFlagToArgsbehaves like this:--unknown-flagare unknownThis request deprecates
FlagSet.ParseErrorsAllowlist.UnknownFlagsand extends it toFlagSet.ParseErrorsAllowlist.UnknownFlagsHandling.FlagSet.ParseErrorsAllowlist.UnknownFlagsHandling = ErrorOnUnknownFlag(Default)FlagSet.ParseErrorsAllowlist.UnknownFlags = falseFlagSet.ParseErrorsAllowlist.UnknownFlagsHandling = IgnoreUnknownFlagFlagSet.ParseErrorsAllowlist.UnknownFlags = trueFlagSet.ParseErrorsAllowlist.UnknownFlagsHandling=PassUnknownFlagToArgs:Args()(described above)Backward compatibilities:
FlagSet.ParseErrorsAllowlist.UnknownFlagsHandlingis not set (ErrorOnUnknownFlag) andFlagSet.ParseErrorsAllowlist.UnknownFlags = true:FlagSet.ParseErrorsAllowlist.UnknownFlagsHandling = ErrorOnUnknownFlagParseErrorsWhitelist