Add PassUnknownFlagsToArgs to allow get unknown flags via Args()

[ikedam]

Close #337

FlagSet.ParseErrorsWhitelist.UnknownFlags=true behaves like this:

• Input:

  -xayb -c -z --known-flag known-flag-value --unknown-flag arg0 arg1 arg2
  

  • -x, -y, -z and --unknown-flag are unknown
• Args():

  arg1 arg2
  

This request adds FlagSet.ParseErrorsWhitelist.PassUnknownFlagsToArgs, and setting it to true results following behavior:

• Args() with FlagSet.ParseErrorsWhitelist.PassUnknownFlagsToArgs=true:

  -xy -z --unknown-flag arg0 arg1 arg2
  

Any unknown and unprocessed flags are passed to Args().

[CLAassistant]

All committers have signed the CLA.

[jippi]

@spf13 👋 Any chance of this landing? I have exactly this use-case as well :)

If it needs work, I would be happy to take it on

[tomasaschan]

In general I think this looks OK, but there's a conflict after merging #295 that needs to be resolved by rebasing.

[ikedam]

@tomasaschan Thanks for reviewing!

I introduced your code suggestion, rebased onto origin/main and resolved conflicts.

Tests pass and all ParseErrorsWhitelist (including ones in code comments) are changed to ParseErrorsAllowlist

vscode ➜ /workspaces/pflag (feature/337_PreserveUnknownFlags) $ go test ./...
ok      github.com/spf13/pflag  0.008s
vscode ➜ /workspaces/pflag (feature/337_PreserveUnknownFlags) $ git grep ParseErrorsWhitelist
vscode ➜ /workspaces/pflag (feature/337_PreserveUnknownFlags) $ 

[tomasaschan]

I didn't spot this before, but putting this branch here instead of in its own case means users will have to specify both this option and UnknownFlags: true for this to have any effect. Is that intentional?

Either way, it seems to me like this PR introduces a new mode to handle unknown flags where there are already two. Before this change, depending on the value of UnknownFlags an unknown flag would either cause an error or be ignored (but consumed). Now, it can also be ignored but not consumed (and instead passed to Args), but controlling this with two boolean feature flags results in an API where there's a combination of values that doesn't make sense.

Maybe it would be better to change UnknownFlags to an enum with constants for ErrorOnUnknown, IgnoreUnknown and PassUnknownToArgs instead? I realize this means the change is breaking, but we have a bunch of ideas lined up that would require breaking changes, so I think it's time to start thinking about what the path to 2.0 looks like anyway. Would you be open to this idea?

[ikedam]

That makes sense.

I believe switching to enum can be achieved preserving backward compatibility. Created #440 . But that requires complicated codes for backward compatibility, and it's OK to wait for 2.0.