integrii / flaggy

Idiomatic Go input parsing with subcommands, positional values, and flags at any position. No required project or package layout and no external dependencies.
The Unlicense
856 stars 30 forks source link

Incorrect parsing of bool flags on subcommands #57

Closed freb closed 4 years ago

freb commented 4 years ago

Bool flaggs specified on the parser are handled differently than those set on the subcommand. I couldn't find any documentation on why this should be the case, so I'm assuming there is a bug.

On the parser, you're able to pass a flag like -f, and the variable passed into flagg.Bool() will be set as true. However, if you define -f on a subcommand, passing -f to the program errors with: Expected a following arg for flag f, but it did not exist.

I've created a minimal example: https://play.golang.org/p/ScB7PMv9rZ5

I tried tracking this down. I discovered that the subcommand passed to the flagIsBool function does not contain the bool flag defined on the subcommand.

ethanmoffat commented 4 years ago

I'm seeing this issue as well, with nested subcommands.

The following fails:

./app mainSubCommand subOperation --valueflag value --boolflag

The following works:

./app mainSubCommand subOperation --boolflag --valueflag value

When I stepped through, it looks like parseAllFlagsFromArgs calls flagIsBool here. The way parsing is structured, flag parsing occurs for all flags on the main app, then on the primary subcommand, then on the secondary subcommand (where it finally correctly detects that the flag is a bool flag).

The reason it doesn't detect the flag as a bool flag earlier is that it only uses the current subcommand's flags as an input set here, which is an empty set for the main app and primary subcommand (at least in my case).

Possible solutions for this:

  1. (workaround) Make the bool flag an option of the primary app so that the early return condition doesn't get triggered
    • I've tested with my case and this works, but with the unfortunate side-effect of adding an invalid flag to the help message of the main app
  2. Rework command parsing to not exit early if flagIsBool returns false
  3. Change the input sets in flagIsBool so that they account for all flags in all subcommands as part of the inputs
freb commented 4 years ago

Thanks for expanding on this. I had just run into the issue you describe but hadn't yet diagnosed it was related to boolean flags. In my case:

This failed:

./app mainSubCommand subOperation --string1 TEXT1 --boolflag=true --string2 TEXT2

But this worked:

./app mainSubCommand subOperation --string1 TEXT1 --boolflag=true --string2=TEXT2

But as you described, this also worked:

./app mainSubCommand subOperation --boolflag --string1 TEXT --string2 TEXT2
ethanmoffat commented 4 years ago

@freb I was able to fix it for myself (PR submitted), do you want to check if that fixes it for your case too?

freb commented 4 years ago

I tested it and it works for all my corner cases. Thanks, much appreciated!

integrii commented 4 years ago

You guys are the best - thanks for attacking this and using flaggy. The fixed has been merged so i am closing this out!