fix: --long= should not consume the next argument

[telemachus]

No description provided.

[telemachus]

I added a couple of tests to confirm that (1) for a boolean, --long= parses as true, (2) for other flags --long= parses as --long="", and (3) --long= does not consume the next argument from args.

In the parsing code, I tried to minimize changes, but while I was there I updated two comments to reflect that only long flags were at issue, and I changed len(args) <= 0 to len(args) == 0 since I don't think it's ever possible for a slice to have a negative length.

I considered adding a further test (like below) for --long="" with non-string flags, but I wasn't sure whether testing for such a specific error was too fiddly. Let me know if you think more tests are needed.

{
	Name:         "--flt= -a",
	Constructors: []fftest.Constructor{fftest.CoreConstructor},
	Args:         []string{`--flt=`, `-a`},
	Want:         fftest.Vars{WantParseErrorIs: strconv.ErrSyntax},
},

[ccoVeille]

The complexity of this code is outstanding

	if value == "" && !eqFound {
		switch {
		case f.isBoolFlag:
			value = "true" // `--foo` defaults to true
			if len(args) > 0 {
				if _, err := strconv.ParseBool(args[0]); err == nil {
					value = args[0] // `--foo false` should also work
					args = args[1:]
				}
			}
		case !f.isBoolFlag && len(args) > 0:
			value, args = args[0], args[1:]
		case !f.isBoolFlag && len(args) == 0:
			return nil, fmt.Errorf("missing value")
		default:
			panic("unreachable")
		}
	}

Unless I'm wrong, it could be this

	if value == "" && !eqFound {
		switch {
		case f.isBoolFlag:
			value = "true" // `--foo` defaults to true
			if len(args) > 0 {
				if _, err := strconv.ParseBool(args[0]); err == nil {
					value = args[0] // `--foo false` should also work
					args = args[1:]
				}
			}
		case len(args) > 0:
			value, args = args[0], args[1:]
		default:
			return nil, fmt.Errorf("missing value")
		}
	}

[telemachus]

You are not wrong. Thanks.

[ccoVeille]

There is also something strange with the code (the code that already exists)

Let's assume --foo expects a string

--foo leads to an error
--foo true leads to "true"
--foo=1 leads to "1"
--foo=false leads to "false"
--foo= leads to ""

But if --foo is a boolean

--foo false
--foo whatever
--foo 0
--foo 2
--foo=2

Leads to this

--foo=false
--foo whatever
--foo=false
--foo=true 2
an error

This behavior is strange to me, but I'm unsure how other libraries parsing flags do

[telemachus]

The difference between --foo 2 and --foo=2 is...not great. That is probably why flag in Go's standard library restricts the form --flag arg to non-boolean flags.

[ccoVeille]

Exactly

[telemachus]

I would be in favor of that here as well, but I think @peterbourgon wants to keep boolean parsing as is. (Also, that would probably be a very breaking API change now. Don't know how that plays with v4 being in beta.)