From f1ce6804e359be23bcd00e9032bfd49720f5874e Mon Sep 17 00:00:00 2001 From: Mike Fridman Date: Thu, 27 Aug 2026 14:05:18 +0200 Subject: [PATCH] fix xflag delimiter and flag value parsing --- xflag/parse.go | 67 +++++++++++++++++++++++---------------------- xflag/parse_test.go | 47 +++++++++++++++++++++++++++---- 2 files changed, 76 insertions(+), 38 deletions(-) diff --git a/xflag/parse.go b/xflag/parse.go index 0bbcea5..2059a2d 100644 --- a/xflag/parse.go +++ b/xflag/parse.go @@ -14,46 +14,38 @@ import ( // // This is a bit unfortunate, but most users nowadays consuming CLI tools expect this behavior. func ParseToEnd(f *flag.FlagSet, arguments []string) error { - if err := f.Parse(arguments); err != nil { - return err - } - if f.NArg() == 0 { - return nil - } + arguments, trailingArgs := splitAtDelimiter(arguments) var args []string - remainingArgs := f.Args() - for i := 0; i < len(remainingArgs); i++ { - arg := remainingArgs[i] - // If the arg looks like a flag, parses like a flag, and quacks like a flag, then it - // probably is a flag. - // - // Note, there's an edge cases here which we EXPLICITLY do not handle, and quite honestly - // 99.999% of the time you wouldn't build a CLI with this behavior. + parseOnce := true + for parseOnce || len(arguments) > 0 { + parseOnce = false + // If the next argument looks like a flag, parses like a flag, and quacks like a flag, + // then it probably is a flag. Let the standard parser make that determination. When it + // instead stops at a positional argument, preserve that argument and resume parsing after + // it on the next iteration. // - // If you want to treat an unknown flag as a positional argument. For example: + // There is one edge case here which we EXPLICITLY do not handle, and quite honestly + // 99.999% of the time you wouldn't build a CLI with this behavior: treating an unknown flag + // as a positional argument. For example: // // $ ./cmd --valid=true arg1 --unknown-flag=foo arg2 // - // Right now, this will trigger an error. But *some* users might want that unknown flag to - // be treated as a positional argument. It's trivial to add this behavior, by using VisitAll - // to iterate over all defined flags (regardless if they are set), and then checking if the - // flag is in the map of known flags. - if len(arg) > 1 && arg[0] == '-' { - // If we encounter a "--", treat all subsequent arguments as positional. The "--" itself - // is stripped, consistent with the standard library's behavior. - if arg == "--" { - args = append(args, remainingArgs[i+1:]...) - break - } - if err := f.Parse(remainingArgs[i:]); err != nil { - return err - } - remainingArgs = f.Args() - i = -1 // Reset to handle newly parsed arguments. - continue + // This triggers an error. Some users might want the unknown flag to be treated as a + // positional argument instead. That behavior could be added by using VisitAll to collect + // the defined flags before deciding whether to pass a flag-looking argument to Parse. + if err := f.Parse(arguments); err != nil { + return err + } + + arguments = f.Args() + if len(arguments) == 0 { + break } - args = append(args, arg) + + args = append(args, arguments[0]) + arguments = arguments[1:] } + args = append(args, trailingArgs...) if len(args) > 0 { // Use "--" as a sentinel to set the FlagSet's internal args field without unsafe // reflection. When flag.Parse encounters "--" it stops processing and stores the remaining @@ -62,3 +54,12 @@ func ParseToEnd(f *flag.FlagSet, arguments []string) error { } return nil } + +func splitAtDelimiter(arguments []string) (before, after []string) { + for i, arg := range arguments { + if arg == "--" { + return arguments[:i], arguments[i+1:] + } + } + return arguments, nil +} diff --git a/xflag/parse_test.go b/xflag/parse_test.go index 5b2a747..f422767 100644 --- a/xflag/parse_test.go +++ b/xflag/parse_test.go @@ -15,6 +15,13 @@ func TestParseToEnd(t *testing.T) { require.NoError(t, ParseToEnd(fs, []string{})) require.False(t, *debugP) require.Equal(t, 0, fs.NFlag()) + require.True(t, fs.Parsed()) + }) + t.Run("only double dash terminator", func(t *testing.T) { + fs := flag.NewFlagSet("name", flag.ContinueOnError) + require.NoError(t, ParseToEnd(fs, []string{"--"})) + require.Empty(t, fs.Args()) + require.True(t, fs.Parsed()) }) t.Run("no args", func(t *testing.T) { fs := flag.NewFlagSet("name", flag.ContinueOnError) @@ -115,6 +122,11 @@ func TestParseToEnd(t *testing.T) { require.Error(t, err) require.Equal(t, err.Error(), "flag provided but not defined: -some-unknown-flag") }) + t.Run("missing flag value after positional argument", func(t *testing.T) { + fs, _ := newFlagset() + err := ParseToEnd(fs, []string{"arg1", "--flag1"}) + require.EqualError(t, err, "flag needs an argument: -flag1") + }) t.Run("only positional args", func(t *testing.T) { fs, c := newFlagset() err := ParseToEnd(fs, []string{"arg1", "arg2", "arg3"}) @@ -152,6 +164,14 @@ func TestParseToEnd(t *testing.T) { require.Equal(t, "value2", c.flag2) require.Equal(t, []string{"arg1", "arg2", "arg3"}, fs.Args()) }) + t.Run("flag-looking value after positional argument", func(t *testing.T) { + fs, c := newFlagset() + err := ParseToEnd(fs, []string{"arg1", "--flag1", "--flag3", "arg2"}) + require.NoError(t, err) + require.Equal(t, "--flag3", c.flag1) + require.False(t, c.flag3) + require.Equal(t, []string{"arg1", "arg2"}, fs.Args()) + }) t.Run("standalone dash is positional", func(t *testing.T) { fs, c := newFlagset() args := []string{"--flag1=value1", "-", "arg1"} @@ -162,15 +182,32 @@ func TestParseToEnd(t *testing.T) { }) t.Run("flags after double dash terminator", func(t *testing.T) { fs, c := newFlagset() - // The initial f.Parse consumes --flag1 and stops at "--", leaving ["--flag3"] as remaining - // args (the "--" is stripped by std lib). The loop then parses --flag3 as a flag, - // collecting zero positional args. args := []string{"--flag1=value1", "--", "--flag3"} err := ParseToEnd(fs, args) require.NoError(t, err) require.Equal(t, "value1", c.flag1) - require.True(t, c.flag3) - require.Equal(t, 0, fs.NArg()) + require.False(t, c.flag3) + require.Equal(t, []string{"--flag3"}, fs.Args()) + }) + t.Run("double dash terminator before flags", func(t *testing.T) { + fs, c := newFlagset() + err := ParseToEnd(fs, []string{"--", "--flag3"}) + require.NoError(t, err) + require.False(t, c.flag3) + require.Equal(t, []string{"--flag3"}, fs.Args()) + }) + t.Run("unknown flag after double dash terminator", func(t *testing.T) { + fs, _ := newFlagset() + err := ParseToEnd(fs, []string{"arg1", "--", "--unknown", "arg2"}) + require.NoError(t, err) + require.Equal(t, []string{"arg1", "--unknown", "arg2"}, fs.Args()) + }) + t.Run("second double dash is positional", func(t *testing.T) { + fs, c := newFlagset() + err := ParseToEnd(fs, []string{"--", "--", "--flag3"}) + require.NoError(t, err) + require.False(t, c.flag3) + require.Equal(t, []string{"--", "--flag3"}, fs.Args()) }) t.Run("duplicate flags last wins", func(t *testing.T) { fs, c := newFlagset()