feat: inline xflag package and improve ParseToEnd

go.mod

@@ -2,10 +2,7 @@ module github.com/pressly/cli
 
 go 1.21.0
 
-require (
-	github.com/mfridman/xflag v0.1.0
-	github.com/stretchr/testify v1.11.1
-)
+require github.com/stretchr/testify v1.11.1
 
 require (
 	github.com/davecgh/go-spew v1.1.1 // indirect

go.sum

@@ -1,7 +1,5 @@
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
-github.com/mfridman/xflag v0.1.0 h1:TWZrZwG1QklFX5S4j1vxfF1sZbZeZSGofMwPMLAF29M=
-github.com/mfridman/xflag v0.1.0/go.mod h1:/483ywM5ZO5SuMVjrIGquYNE5CzLrj5Ux/LxWWnjRaE=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U=

parse.go

@@ -10,7 +10,7 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/mfridman/xflag"
+	"github.com/pressly/cli/xflag"
 )
 
 // Parse traverses the command hierarchy and parses arguments. It returns an error if parsing fails

xflag/doc.go

@@ -0,0 +1,5 @@
+// Package xflag extends the standard library's flag package to support parsing flags interleaved
+// with positional arguments. By default, Go's flag package stops parsing flags at the first
+// non-flag argument, which is unintuitive for most CLI users. This package provides [ParseToEnd] as
+// a drop-in replacement that handles flags anywhere in the argument list.
+package xflag

xflag/parse.go

@@ -0,0 +1,64 @@
+package xflag
+
+import (
+	"flag"
+)
+
+// ParseToEnd is a drop-in replacement for flag.Parse. It improves upon the standard behavior by
+// parsing flags even when they are interspersed with positional arguments. This overcomes Go's
+// default limitation of stopping flag parsing upon encountering the first positional argument. For
+// more details, see:
+//
+//   - https://github.com/golang/go/issues/4513
+//   - https://github.com/golang/go/issues/63138
+//
+// 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
+	}
+	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.
+		//
+		// If you want to treat 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
+		}
+		args = append(args, arg)
+	}
+	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
+		// arguments as positional args, which is exactly what we need.
+		return f.Parse(append([]string{"--"}, args...))
+	}
+	return nil
+}

xflag/parse_test.go

@@ -0,0 +1,206 @@
+package xflag
+
+import (
+	"flag"
+	"io"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestParseToEnd(t *testing.T) {
+	t.Run("empty", func(t *testing.T) {
+		fs := flag.NewFlagSet("name", flag.ContinueOnError)
+		debugP := fs.Bool("debug", false, "debug mode")
+		require.NoError(t, ParseToEnd(fs, []string{}))
+		require.False(t, *debugP)
+		require.Equal(t, 0, fs.NFlag())
+	})
+	t.Run("no args", func(t *testing.T) {
+		fs := flag.NewFlagSet("name", flag.ContinueOnError)
+		debugP := fs.Bool("debug", false, "debug mode")
+		err := ParseToEnd(fs, []string{"--debug", "true"})
+		require.NoError(t, err)
+		require.Equal(t, 1, fs.NFlag())
+		require.True(t, *debugP)
+	})
+	t.Run("before with args", func(t *testing.T) {
+		fs := flag.NewFlagSet("name", flag.ContinueOnError)
+		debugP := fs.Bool("debug", false, "debug mode")
+		err := ParseToEnd(fs, []string{"--debug=true", "arg1", "arg2"})
+		require.NoError(t, err)
+		require.True(t, *debugP)
+		require.Equal(t, 1, fs.NFlag())
+		require.Equal(t, []string{"arg1", "arg2"}, fs.Args())
+	})
+	t.Run("after with args", func(t *testing.T) {
+		fs := flag.NewFlagSet("name", flag.ContinueOnError)
+		debugP := fs.Bool("debug", false, "debug mode")
+		err := ParseToEnd(fs, []string{"arg1", "arg2", "--debug"})
+		require.NoError(t, err)
+		require.True(t, *debugP)
+
+		f := fs.Lookup("debug")
+		require.Equal(t, "true", f.Value.String())
+		require.Equal(t, 1, fs.NFlag())
+		require.Equal(t, []string{"arg1", "arg2"}, fs.Args())
+	})
+	t.Run("before and after with args", func(t *testing.T) {
+		fs, c := newFlagset()
+		args := []string{
+			"--flag1=value1",
+			"--flag3=true",
+			"arg1",
+			"arg2",
+			"--flag2=value2",
+			"--flag4=false",
+			"arg3",
+		}
+		err := ParseToEnd(fs, args)
+		require.NoError(t, err)
+		require.Equal(t, config{
+			flag1: "value1",
+			flag3: true,
+			flag2: "value2",
+			flag4: false,
+		}, *c)
+		require.Equal(t, []string{"arg1", "arg2", "arg3"}, fs.Args())
+		require.Equal(t, 4, fs.NFlag())
+	})
+	t.Run("break", func(t *testing.T) {
+		fs, c := newFlagset()
+		args := []string{
+			"--flag1=value1",
+			"--flag3=true",
+			"arg1",
+			"arg2",
+			"--flag2=value2",
+			"--flag4=false",
+			"arg3",
+			"--",
+			"arg4",
+			"arg5",
+			"--flag4=true", // This is now a positional argument no matter what.
+		}
+		err := ParseToEnd(fs, args)
+		require.NoError(t, err)
+		require.Equal(t, config{
+			flag1: "value1",
+			flag3: true,
+			flag2: "value2",
+			flag4: false,
+		}, *c)
+		require.Equal(t, []string{"arg1", "arg2", "arg3", "arg4", "arg5", "--flag4=true"}, fs.Args())
+		require.Equal(t, 4, fs.NFlag())
+	})
+	t.Run("unknown flag before", func(t *testing.T) {
+		fs, _ := newFlagset()
+		args := []string{
+			"--flag1=value1",
+			"--some-unknown-flag=foo", // This gets treated as a flag.
+			"arg1",
+		}
+		err := ParseToEnd(fs, args)
+		require.Error(t, err)
+		require.Equal(t, err.Error(), "flag provided but not defined: -some-unknown-flag")
+	})
+	t.Run("unknown flag after", func(t *testing.T) {
+		fs, _ := newFlagset()
+		args := []string{
+			"--flag1=value1",
+			"arg1",
+			"--some-unknown-flag=foo", // This gets treated as a flag.
+		}
+		err := ParseToEnd(fs, args)
+		require.Error(t, err)
+		require.Equal(t, err.Error(), "flag provided but not defined: -some-unknown-flag")
+	})
+	t.Run("only positional args", func(t *testing.T) {
+		fs, c := newFlagset()
+		err := ParseToEnd(fs, []string{"arg1", "arg2", "arg3"})
+		require.NoError(t, err)
+		// All flags should retain defaults.
+		require.Equal(t, config{flag1: "asdf", flag2: "qwerty", flag3: false, flag4: true}, *c)
+		require.Equal(t, 0, fs.NFlag())
+		require.Equal(t, []string{"arg1", "arg2", "arg3"}, fs.Args())
+	})
+	t.Run("single dash flag syntax", func(t *testing.T) {
+		fs, c := newFlagset()
+		args := []string{
+			"-flag1=value1",
+			"arg1",
+			"-flag3",
+			"arg2",
+		}
+		err := ParseToEnd(fs, args)
+		require.NoError(t, err)
+		require.Equal(t, "value1", c.flag1)
+		require.True(t, c.flag3)
+		require.Equal(t, []string{"arg1", "arg2"}, fs.Args())
+	})
+	t.Run("space separated flags interleaved", func(t *testing.T) {
+		fs, c := newFlagset()
+		args := []string{
+			"arg1",
+			"--flag1", "value1",
+			"arg2",
+			"--flag2", "value2",
+			"arg3",
+		}
+		err := ParseToEnd(fs, args)
+		require.NoError(t, err)
+		require.Equal(t, "value1", c.flag1)
+		require.Equal(t, "value2", c.flag2)
+		require.Equal(t, []string{"arg1", "arg2", "arg3"}, fs.Args())
+	})
+	t.Run("standalone dash is positional", func(t *testing.T) {
+		fs, c := newFlagset()
+		args := []string{"--flag1=value1", "-", "arg1"}
+		err := ParseToEnd(fs, args)
+		require.NoError(t, err)
+		require.Equal(t, "value1", c.flag1)
+		require.Equal(t, []string{"-", "arg1"}, fs.Args())
+	})
+	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())
+	})
+	t.Run("duplicate flags last wins", func(t *testing.T) {
+		fs, c := newFlagset()
+		args := []string{
+			"--flag1=first",
+			"arg1",
+			"--flag1=second",
+		}
+		err := ParseToEnd(fs, args)
+		require.NoError(t, err)
+		require.Equal(t, "second", c.flag1)
+		require.Equal(t, []string{"arg1"}, fs.Args())
+	})
+}
+
+type config struct {
+	flag1 string
+	flag2 string
+	flag3 bool
+	flag4 bool
+}
+
+func newFlagset() (*flag.FlagSet, *config) {
+	fs := flag.NewFlagSet("name", flag.ContinueOnError)
+	fs.SetOutput(io.Discard)
+	c := new(config)
+	fs.StringVar(&c.flag1, "flag1", "asdf", "flag1 usage")
+	fs.StringVar(&c.flag2, "flag2", "qwerty", "flag2 usage")
+	fs.BoolVar(&c.flag3, "flag3", false, "flag3 usage")
+	fs.BoolVar(&c.flag4, "flag4", true, "flag4 urage")
+	return fs, c
+}