Extend `UnknownFlags` to `UnknownFlagsHandling` and introduce `PassUnknownFlagToArgs` (#337) by ikedam · Pull Request #440 · spf13/pflag

ikedam · 2025-08-09T07:13:43Z

Close #337

Alternative to #338.

FlagSet.ParseErrorsAllowlist.UnknownFlagsHandling=PassUnknownFlagToArgs 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() results:
```
-xy -z --unknown-flag arg0 arg1 arg2
```

This request deprecates FlagSet.ParseErrorsAllowlist.UnknownFlags and extends it to FlagSet.ParseErrorsAllowlist.UnknownFlagsHandling.

FlagSet.ParseErrorsAllowlist.UnknownFlagsHandling = ErrorOnUnknownFlag (Default)
- Same to FlagSet.ParseErrorsAllowlist.UnknownFlags = false
- Treats unknown flags error.
FlagSet.ParseErrorsAllowlist.UnknownFlagsHandling = IgnoreUnknownFlag
- Same to FlagSet.ParseErrorsAllowlist.UnknownFlags = true
- Ignore unknown flags.
FlagSet.ParseErrorsAllowlist.UnknownFlagsHandling=PassUnknownFlagToArgs:
- Pass unknown flags to Args() (described above)

Backward compatibilities:

FlagSet.ParseErrorsAllowlist.UnknownFlagsHandling is not set (ErrorOnUnknownFlag) and FlagSet.ParseErrorsAllowlist.UnknownFlags = true:
- Same to FlagSet.ParseErrorsAllowlist.UnknownFlagsHandling = ErrorOnUnknownFlag
- Ignore unknown flags.
Also same for ParseErrorsWhitelist

ikedam · 2025-08-09T07:14:48Z

+	if f.Parsed() {
+		t.Fatal("f.Parse() = true before Parse")
+	}
+	f.ParseErrorsAllowlist.UnknownFlagsHandling = UnknownFlagsHandlingPassUnknownToArgs


This test code is same to https://github.com/spf13/pflag/pull/338/files#diff-7a7385a10188cdaa9ac18effc375607e65672f55084b8049b6d3f44a6ab70c51 except this line.

ikedam · 2025-08-09T07:17:24Z

+	// 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


Reeeally long names...
I worry names like ErrorOnUnknown would cause name conflict.

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.

Renamed in c0bdd63

tomasaschan

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.

tomasaschan · 2026-03-06T10:58:13Z

+	// 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


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 · 2026-03-06T11:01:44Z

+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


…nFlagsHandlingPassUnknownToArgs` (spf13#337) * Conflicted with spf13#446

ikedam · 2026-03-06T17:02:37Z

First, rebased and resolved the conflict.
Conflicted with #446 .

Needs to support both ParseErrorsAllowlist and ParseErrorsWhitelist, and resolved as d1d7aee .
The conflict was left unresolved in the previous commit and fixed in this one so reviewers can see exactly how it was resolved.

* `UnknownFlagsHandlingErrorOnUnknown` to `ErrorOnUnknownFlag` and so on

tomasaschan

I'm sorry this took so long to get to reviewing. I have a couple of questions, but given this behavior is opt-in I think it would be OK to merge this to make it easier to test in real applications, and roll forward when more edge cases pop up.

Let me know if you prefer a merge now or to address those questions first.

tomasaschan · 2026-05-28T06:53:38Z

 			return stripUnknownFlagValue(a), nil
+		case unknownFlagsHandling == PassUnknownFlagToArgs:
+			return a, &unknownFlagError{


I don't understand why the previous case calls stripUnknownFlagValue and this is one does not.

We can consider the case of --unknown somevalue.
--unknown is an unknown flag, and we cannot reliably tell whether somevalue is a flag value or a positional argument.

With PassUnknownFlagToArgs, we do not need to classify somevalue; we simply leave the argument stream intact and let the caller receive everything as positional args. So we don't need to call stripUnknownFlagValue().

With IgnoreUnknownFlag, the existing behavior treats the next token as a flag value (when it does not look like a flag) and calls stripUnknownFlagValue() to strip it. This behavior is inherited from the existing IgnoreUnknownFlag implementation.

tomasaschan · 2026-05-28T06:55:42Z

+			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


What about -fg=g were both f and g are shorthands, but only g accept a value? Or -fgh (same)?

Same here, differs in the next call.

For an unknown flag, parseShortArg only need to choose between:

strip only the first letter, or

strip all the argument

Both -fg=g and -fgh are the former case: only strips the first letter (f) as an unknown flag, and pass the rest (g=g or gh).
The rest will be handled in the next parseSingleShortArg() call.

In the next parseSingleShortArg(), g=g results the latter case, and gh results the former case.

ikedam commented Aug 9, 2025

View reviewed changes

ikedam mentioned this pull request Aug 9, 2025

Add PassUnknownFlagsToArgs to allow get unknown flags via Args() #338

Closed

tomasaschan added this to the 1.1 milestone Aug 15, 2025

tomasaschan reviewed Mar 6, 2026

View reviewed changes

ikedam added 2 commits March 7, 2026 01:44

Extend UnknownFlags to UnknownFlagsHandling and introduce `Unknow…

2cd30f1

…nFlagsHandlingPassUnknownToArgs` (spf13#337) * Conflicted with spf13#446

Handle both ParseErrorsAllowlist and ParseErrorsWhitelist spf13#337

d1d7aee

ikedam force-pushed the feature/337_PreserveUnknownFlagsEnum branch from f16d4d7 to d1d7aee Compare March 6, 2026 16:56

Rename symbols to shorter ones spf13#337

c0bdd63

* `UnknownFlagsHandlingErrorOnUnknown` to `ErrorOnUnknownFlag` and so on

ikedam changed the title ~~Extend UnknownFlags to UnknownFlagsHandling and introduce UnknownFlagsHandlingPassUnknownToArgs (#337)~~ Extend UnknownFlags to UnknownFlagsHandling and introduce PassUnknownFlagToArgs (#337) Mar 6, 2026

ikedam requested a review from tomasaschan March 6, 2026 17:10

tomasaschan approved these changes May 28, 2026

View reviewed changes

tomasaschan merged commit 9912b67 into spf13:master Jun 6, 2026
1 check passed

Conversation

ikedam commented Aug 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomasaschan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ikedam commented Mar 6, 2026

Uh oh!

tomasaschan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ikedam commented Aug 9, 2025 •

edited

Loading