Displays current value as "Default" on Subcommand Positional Value when Extra
donatj opened this issue · 6 comments
When using positional values, if I accidentally add an extra argument the "default" is shown as whatever I entered already for the existing positional arguments.
You can see from the following example program the "id" field shows the executed "bad" value as the default.
package main
import "github.com/integrii/flaggy"
func main() {
var id string
getCmd := flaggy.NewSubcommand("get")
getCmd.Description = "Get from id"
getCmd.AddPositionalValue(&id, "id", 1, true, "ID")
flaggy.AttachSubcommand(getCmd, 1)
flaggy.Parse()
}
$ ./main get bad extra
get - Get from id
Usage:
get [id]
Positional Variables:
id (Required) - ID (default: bad)
Flags:
--version Displays the program version string.
-h --help Displays help with available flag, subcommand, and positional value parameters.
Unexpected argument: extra
Thanks! I will look into getting this patched up. I appreciate the clear report with example code, too.
I am going to think on this one a little more before I make any changes... The code works like this:
- When its time to show help:
- loop over every flag and positional
- look at the current value of the assignment variable
- set that value as the default value
- display help
This obviously does not work when flaggy exits in the middle of parsing flags... Some of the assignment vars may have been set and the default values are long gone.
I considered snapshotting all variables before parsing any subcommand, but subcommands can be nested, so it is possible that the subcommand snapshots a variable that has already been assigned from a parent subcommand.
It seems like the best way to solve this is to write a recursive crawler that crawls down all possible paths including their flags and positionals in order to build a snapshot of all the default values. This way, when help does fire, we can go back and look up what things were before anything was parsed. This could be a pretty serious feature though, and will likely impact performance and code complexity.
Thinkin...
lol - honestly it's no skin off my back, I just noticed it when I typo'd a command but it doesn't really hurt me any.
I wouldn't want it to needlessly overcomplicate the code 😄
Yeah, I don't want to either. I need to solve it simply. I can't leave the feature broken, though. I will eventually get to this some weekend when I have downtime and thinking comes easy.
This took some careful thought, but simply saving the original state the first time a variable is parsed, then using that in help output solves the problem. No crazy recursion required. This is now in the unstable branch for longer periods of testing.