From b24708c6df5334116016aaa57cccc18fd807d5a6 Mon Sep 17 00:00:00 2001 From: Eric Greer Date: Wed, 30 Oct 2019 21:44:44 -0700 Subject: [PATCH 1/9] Fixes for unexpected flags not crashing out properly. Fixes #47 --- flaggy_test.go | 8 +------- main.go | 1 + subCommand.go | 10 +++++++++- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/flaggy_test.go b/flaggy_test.go index 5e10d32..83fc01c 100644 --- a/flaggy_test.go +++ b/flaggy_test.go @@ -106,7 +106,7 @@ func TestComplexNesting(t *testing.T) { } func TestParsePositionalsA(t *testing.T) { - inputLine := []string{"-t", "-i=3", "subcommand", "-n", "testN", "-j=testJ", "positionalA", "positionalB", "--testK=testK", "--", "trailingA", "trailingB"} + inputLine := []string{"-t", "subcommand", "-i", "3", "-n", "testN", "positionalA", "positionalB", "--", "trailingA", "trailingB"} var boolT bool var intT int @@ -151,15 +151,9 @@ func TestParsePositionalsA(t *testing.T) { if boolT != true { t.Fatal("Global bool flag -t was incorrect:", boolT) } - if testK != "testK" { - t.Fatal("Subcommand flag testK was incorrect:", testK) - } if testN != "testN" { t.Fatal("Subcommand flag testN was incorrect:", testN) } - if testJ != "testJ" { - t.Fatal("Subcommand flag testJ was incorrect:", testJ) - } if positionalA != "positionalA" { t.Fatal("Positional A was incorrect:", positionalA) } diff --git a/main.go b/main.go index b44cd91..7fa47f0 100644 --- a/main.go +++ b/main.go @@ -55,6 +55,7 @@ func ResetParser() { } else { DefaultParser = NewParser("default") } + DefaultParser.ShowHelpOnUnexpected = false } // Parse parses flags as requested in the default package parser diff --git a/subCommand.go b/subCommand.go index c0c1307..dbc3f2f 100644 --- a/subCommand.go +++ b/subCommand.go @@ -32,6 +32,10 @@ type Subcommand struct { // NewSubcommand creates a new subcommand that can have flags or PositionalFlags // added to it. The position starts with 1, not 0 func NewSubcommand(name string) *Subcommand { + if len(name) == 0 { + fmt.Fprintln(os.Stderr, "Error creating subcommand (NewSubcommand()). No subcommand name was specified.") + exitOrPanic(2) + } newSC := &Subcommand{ Name: name, } @@ -156,12 +160,16 @@ func (sc *Subcommand) parseAllFlagsFromArgs(p *Parser, args []string) ([]string, a = parseFlagToName(a) // parse flag into key and value and apply to subcommand flags key, val := parseArgWithValue(a) - _, err = setValueForParsers(key, val, p, sc) + foundFlag, err := setValueForParsers(key, val, p, sc) if err != nil { return []string{}, false, err } // if this flag type was found and not set, and the parser is set to show // Help when an unknown flag is found, then show Help and exit. + if !foundFlag && p.ShowHelpOnUnexpected { + p.ShowHelpWithMessage("Unexpected flag provided: " + key) + exitOrPanic(2) + } } } From e40f5ebb6116957403cf93e7d1bdc2f8160c3752 Mon Sep 17 00:00:00 2001 From: Eric Greer Date: Fri, 1 Nov 2019 23:29:49 -0700 Subject: [PATCH 2/9] WIP: unknown flag detection --- argumentParser.go | 2 +- flag_test.go | 7 ++++- flaggy_test.go | 4 ++- parser.go | 3 ++- subCommand.go | 65 +++++++++++++++++++++++++++++++++------------- subcommand_test.go | 2 +- 6 files changed, 60 insertions(+), 23 deletions(-) diff --git a/argumentParser.go b/argumentParser.go index 320e85e..34d00f9 100644 --- a/argumentParser.go +++ b/argumentParser.go @@ -5,7 +5,7 @@ package flaggy // The return values represent the key being set, and any errors // returned when setting the key, such as failures to convert the string // into the appropriate flag value. We stop assigning values as soon -// as we find a parser that accepts it. +// as we find a any parser that accepts it. func setValueForParsers(key string, value string, parsers ...ArgumentParser) (bool, error) { for _, p := range parsers { diff --git a/flag_test.go b/flag_test.go index 1b2c601..8c81f29 100644 --- a/flag_test.go +++ b/flag_test.go @@ -8,11 +8,16 @@ import ( "time" ) -// debugOff makes defers easier +// debugOff makes defers easier and turns off debug mode func debugOff() { DebugMode = false } +// debugOn turns on debug mode +func debugOn() { + DebugMode = true +} + func TestGlobs(t *testing.T) { for _, a := range os.Args { fmt.Println(a) diff --git a/flaggy_test.go b/flaggy_test.go index 83fc01c..58b4fb5 100644 --- a/flaggy_test.go +++ b/flaggy_test.go @@ -106,7 +106,9 @@ func TestComplexNesting(t *testing.T) { } func TestParsePositionalsA(t *testing.T) { - inputLine := []string{"-t", "subcommand", "-i", "3", "-n", "testN", "positionalA", "positionalB", "--", "trailingA", "trailingB"} + inputLine := []string{"-t", "-i=3", "subcommand", "-n", "testN", "-j=testJ", "positionalA", "positionalB", "--testK=testK", "--", "trailingA", "trailingB"} + + flaggy.DebugMode = true var boolT bool var intT int diff --git a/parser.go b/parser.go index 0663932..de75dfd 100644 --- a/parser.go +++ b/parser.go @@ -105,7 +105,8 @@ func (p *Parser) ShowHelpWithMessage(message string) { } } -// Disable show version with --version. It is enabled by default. +// DisableShowVersionWithVersion disables the showing of version information +// with --version. It is enabled by default. func (p *Parser) DisableShowVersionWithVersion() { p.ShowVersionWithVersionFlag = false } diff --git a/subCommand.go b/subCommand.go index dbc3f2f..53ffe78 100644 --- a/subCommand.go +++ b/subCommand.go @@ -43,11 +43,14 @@ func NewSubcommand(name string) *Subcommand { } // parseAllFlagsFromArgs parses the non-positional flags such as -f or -v=value -// out of the supplied args and returns the positional items in order. -func (sc *Subcommand) parseAllFlagsFromArgs(p *Parser, args []string) ([]string, bool, error) { +// out of the supplied args and returns the resulting positional items in order, +// all the flag names found (without values), a bool to indicate if help was +// requested, and any errors found during parsing +func (sc *Subcommand) parseAllFlagsFromArgs(p *Parser, args []string) ([]string, []string, bool, error) { var err error var positionalOnlyArguments []string + var flagNamesFound []string var helpRequested bool // indicates the user has supplied -h and we // should render help if we are the last subcommand @@ -127,6 +130,10 @@ func (sc *Subcommand) parseAllFlagsFromArgs(p *Parser, args []string) ([]string, positionalOnlyArguments = append(positionalOnlyArguments, a) case argIsFlagWithSpace: a = parseFlagToName(a) + + // track all flag names found during parsing + flagNamesFound = append(flagNamesFound, a) + // debugPrint("Arg", i, "is flag with space:", a) // parse next arg as value to this flag and apply to subcommand flags // if the flag is a bool flag, then we check for a following positional @@ -137,7 +144,7 @@ func (sc *Subcommand) parseAllFlagsFromArgs(p *Parser, args []string) ([]string, // if an error occurs, just return it and quit parsing if err != nil { - return []string{}, false, err + return []string{}, []string{}, false, err } // by default, we just assign the next argument to the value and continue continue @@ -153,31 +160,28 @@ func (sc *Subcommand) parseAllFlagsFromArgs(p *Parser, args []string) ([]string, } _, err = setValueForParsers(a, nextArg, p, sc) if err != nil { - return []string{}, false, err + return []string{}, []string{}, false, err } case argIsFlagWithValue: // debugPrint("Arg", i, "is flag with value:", a) a = parseFlagToName(a) + + // track all flag names found during parsing + flagNamesFound = append(flagNamesFound, a) + // parse flag into key and value and apply to subcommand flags key, val := parseArgWithValue(a) - foundFlag, err := setValueForParsers(key, val, p, sc) + _, err := setValueForParsers(key, val, p, sc) if err != nil { - return []string{}, false, err - } - // if this flag type was found and not set, and the parser is set to show - // Help when an unknown flag is found, then show Help and exit. - if !foundFlag && p.ShowHelpOnUnexpected { - p.ShowHelpWithMessage("Unexpected flag provided: " + key) - exitOrPanic(2) + return []string{}, []string{}, false, err } } - } - return positionalOnlyArguments, helpRequested, nil + return positionalOnlyArguments, flagNamesFound, helpRequested, nil } -// Parse causes the argument parser to parse based on the supplied []string. +// parse causes the argument parser to parse based on the supplied []string. // depth specifies the non-flag subcommand positional depth func (sc *Subcommand) parse(p *Parser, args []string, depth int) error { @@ -202,11 +206,36 @@ func (sc *Subcommand) parse(p *Parser, args []string, depth int) error { // Parse the normal flags out of the argument list and retain the positionals. // Apply the flags to the parent parser and the current subcommand context. // ./command -f -z subcommand someVar -b becomes ./command subcommand somevar - positionalOnlyArguments, helpRequested, err := sc.parseAllFlagsFromArgs(p, args) + positionalOnlyArguments, flagNames, helpRequested, err := sc.parseAllFlagsFromArgs(p, args) if err != nil { return err } + // if we are set to show help on unexpected flags, we should ensure that + // all flags that aren't positional arguments are found in the list of flag + // names + debugPrint("flags and positionals:", flagNames, positionalOnlyArguments) + if p.ShowHelpOnUnexpected { + + // check over all arguments specified to ensure they are all known as either + // a flag or a subcommand + for _, a := range args { + + // check if the arg matches any known flag name + if stringInSlice(flagNames, a) { + continue + } + + // check if the flag matches any known positional name + if stringInSlice(positionalOnlyArguments, a) { + continue + } + + // crash out if any argument is unknown as both a flag and positional + p.ShowHelpAndExit("Unknown argument: " + a) + } + } + // indicate that trailing arguments have been extracted, so that they aren't // appended a second time p.trailingArgumentsExtracted = true @@ -300,7 +329,7 @@ func (sc *Subcommand) parse(p *Parser, args []string, depth int) error { } // find any positionals that were not used on subcommands that were - // found and throw help (unknown argument) + // found and throw help (unknown argument) in the global parse or subcommand for _, pv := range p.PositionalFlags { if pv.Required && !pv.Found { p.ShowHelpWithMessage("Required global positional variable " + pv.Name + " not found at position " + strconv.Itoa(pv.Position)) @@ -631,7 +660,7 @@ func (sc *Subcommand) SetValueForKey(key string, value string) (bool, error) { // debugPrint("Looking to set key", key, "to value", value) // check for and assign flags that match the key for _, f := range sc.Flags { - // debugPrint("Evaluating string flag", f.ShortName, "==", key, "||", f.LongName, "==", key) + debugPrint("Evaluating string flag", f.ShortName, "==", key, "||", f.LongName, "==", key) if f.ShortName == key || f.LongName == key { // debugPrint("Setting string value for", key, "to", value) f.identifyAndAssignValue(value) diff --git a/subcommand_test.go b/subcommand_test.go index ca7f715..68f0f16 100644 --- a/subcommand_test.go +++ b/subcommand_test.go @@ -75,7 +75,7 @@ func TestSubcommandHidden(t *testing.T) { defer func() { r := recover() if r == nil { - t.Fatal("Expected crash instead of exit. Subcommand id was wrong") + t.Fatal("Expected crash instead of exit. Subcommand id was set to a blank") } }() flaggy.ResetParser() From fa6246b8114d289cf97fd5f4bed04568b8fb6b96 Mon Sep 17 00:00:00 2001 From: Eric Greer Date: Sat, 2 Nov 2019 22:09:13 -0700 Subject: [PATCH 3/9] detection for values not used in order to fix ShowHelpOnUnexpected --- .gitignore | 2 + main.go | 1 - parsedValue.go | 20 +++++++++ parser.go | 65 +++++++++++++++++++++++++++- subCommand.go | 104 +++++++++++++++++++++++++-------------------- subcommand_test.go | 39 +++++++++++++++++ 6 files changed, 181 insertions(+), 50 deletions(-) create mode 100644 parsedValue.go diff --git a/.gitignore b/.gitignore index a1338d6..f46854f 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,5 @@ # Project-local glide cache, RE: https://github.com/Masterminds/glide/issues/736 .glide/ + +.idea/ diff --git a/main.go b/main.go index 7fa47f0..b44cd91 100644 --- a/main.go +++ b/main.go @@ -55,7 +55,6 @@ func ResetParser() { } else { DefaultParser = NewParser("default") } - DefaultParser.ShowHelpOnUnexpected = false } // Parse parses flags as requested in the default package parser diff --git a/parsedValue.go b/parsedValue.go new file mode 100644 index 0000000..d7dca60 --- /dev/null +++ b/parsedValue.go @@ -0,0 +1,20 @@ +package flaggy + +// parsedValue represents a flag or subcommand that was parsed. Primairily used +// to account for all parsed values in order to determine if unknown values were +// passed to the root parser after all subcommands have been parsed. +type parsedValue struct { + Key string + Value string + IsPositional bool // indicates that this value was positional and not a key/value +} + +// newParsedValue creates and returns a new parsedValue struct with the +// supplied values set +func newParsedValue(key string, value string, isPositional bool) parsedValue { + return parsedValue{ + Key: key, + Value: value, + IsPositional: isPositional, + } +} diff --git a/parser.go b/parser.go index de75dfd..767c447 100644 --- a/parser.go +++ b/parser.go @@ -15,7 +15,7 @@ type Parser struct { Version string // the optional version of the parser. ShowHelpWithHFlag bool // display help when -h or --help passed ShowVersionWithVersionFlag bool // display the version when --version passed - ShowHelpOnUnexpected bool // display help when an unexpected flag is passed + ShowHelpOnUnexpected bool // display help when an unexpected flag or subcommand is passed TrailingArguments []string // everything after a -- is placed here HelpTemplate *template.Template // template for Help output trailingArgumentsExtracted bool // indicates that trailing args have been parsed and should not be appended again @@ -47,7 +47,68 @@ func (p *Parser) ParseArgs(args []string) error { } p.parsed = true // debugPrint("Kicking off parsing with args:", args) - return p.parse(p, args, 0) + err := p.parse(p, args, 0) + if err != nil { + return err + } + + // if we are set to crash on unexpected args, look for those here TODO + if p.ShowHelpOnUnexpected { + parsedValues := p.findAllParsedValues() + argsNotParsed := findArgsNotInParsedValues(args, parsedValues) + if len(argsNotParsed) > 0 { + // flatten out unused args for our error message + var argsNotParedFlat string + for _, a := range argsNotParsed { + argsNotParedFlat = argsNotParedFlat + " " + a + } + p.ShowHelpAndExit("Unknown arguments supplied: " + argsNotParedFlat) + } + } + + return nil +} + +// findArgsNotInParsedValues finds arguments not used in parsed values +func findArgsNotInParsedValues(args []string, parsedValues []parsedValue) []string { + var argsNotUsed []string + + var skipNext bool + for _, arg := range args { + // allow for skipping the next arg when needed + if skipNext { + skipNext = false + continue + } + + // indicates that we found this arg used in one of the parsed values. Used + // to indicate which values should be added to argsNotUsed. + var foundArgUsed bool + + // search all args for a corresponding parsed value + for _, pv := range parsedValues { + // this argumenet was a key + if pv.Key == arg { + foundArgUsed = true + // if the value is not a positional value and the parsed value had a + // value that was not blank, we skip the next value in the argument list + if !pv.IsPositional && len(pv.Value) > 0 { + skipNext = true + break + } + } + // this prevents excessive parsed values from being checked after we find + // the arg used for the first time + if foundArgUsed { + break + } + } + if !foundArgUsed { + argsNotUsed = append(argsNotUsed, arg) + } + } + + return argsNotUsed } // ShowVersionAndExit shows the version of this parser diff --git a/subCommand.go b/subCommand.go index 53ffe78..62a3a9b 100644 --- a/subCommand.go +++ b/subCommand.go @@ -23,10 +23,11 @@ type Subcommand struct { Subcommands []*Subcommand Flags []*Flag PositionalFlags []*PositionalValue - AdditionalHelpPrepend string // additional prepended message when Help is displayed - AdditionalHelpAppend string // additional appended message when Help is displayed - Used bool // indicates this subcommand was found and parsed - Hidden bool // indicates this subcommand should be hidden from help + ParsedValues []parsedValue // a list of values and positionals parsed + AdditionalHelpPrepend string // additional prepended message when Help is displayed + AdditionalHelpAppend string // additional appended message when Help is displayed + Used bool // indicates this subcommand was found and parsed + Hidden bool // indicates this subcommand should be hidden from help } // NewSubcommand creates a new subcommand that can have flags or PositionalFlags @@ -46,11 +47,10 @@ func NewSubcommand(name string) *Subcommand { // out of the supplied args and returns the resulting positional items in order, // all the flag names found (without values), a bool to indicate if help was // requested, and any errors found during parsing -func (sc *Subcommand) parseAllFlagsFromArgs(p *Parser, args []string) ([]string, []string, bool, error) { +func (sc *Subcommand) parseAllFlagsFromArgs(p *Parser, args []string) ([]string, bool, error) { var err error var positionalOnlyArguments []string - var flagNamesFound []string var helpRequested bool // indicates the user has supplied -h and we // should render help if we are the last subcommand @@ -128,25 +128,31 @@ func (sc *Subcommand) parseAllFlagsFromArgs(p *Parser, args []string) ([]string, // this positional argument into a slice of their own, so that // we can determine if its a subcommand or positional value later positionalOnlyArguments = append(positionalOnlyArguments, a) + // track this as a parsed value with the subcommand + sc.addParsedPositionalValue(a) case argIsFlagWithSpace: a = parseFlagToName(a) - // track all flag names found during parsing - flagNamesFound = append(flagNamesFound, a) - // debugPrint("Arg", i, "is flag with space:", a) // parse next arg as value to this flag and apply to subcommand flags // if the flag is a bool flag, then we check for a following positional // and skip it if necessary if flagIsBool(sc, p, a) { debugPrint(sc.Name, "bool flag", a, "next var is:", nextArg) + // set the value in this subcommand and its root parser _, err = setValueForParsers(a, "true", p, sc) // if an error occurs, just return it and quit parsing if err != nil { - return []string{}, []string{}, false, err + return []string{}, false, err } - // by default, we just assign the next argument to the value and continue + + // log all values parsed by this subcommand. We leave the value blank + // because the bool value had no explicit true or false supplied + sc.addParsedFlag(a, "") + + // we've found and set a standalone bool flag, so we move on to the next + // argument in the list of arguments continue } @@ -160,29 +166,46 @@ func (sc *Subcommand) parseAllFlagsFromArgs(p *Parser, args []string) ([]string, } _, err = setValueForParsers(a, nextArg, p, sc) if err != nil { - return []string{}, []string{}, false, err + return []string{}, false, err } + + // log all parsed values in the subcommand + sc.addParsedFlag(a, nextArg) case argIsFlagWithValue: // debugPrint("Arg", i, "is flag with value:", a) a = parseFlagToName(a) - // track all flag names found during parsing - flagNamesFound = append(flagNamesFound, a) - // parse flag into key and value and apply to subcommand flags key, val := parseArgWithValue(a) + + // set the value in this subcommand and its root parser _, err := setValueForParsers(key, val, p, sc) if err != nil { - return []string{}, []string{}, false, err + return []string{}, false, err } + + // log all values parsed by the subcommand + sc.addParsedFlag(key, val) } } - return positionalOnlyArguments, flagNamesFound, helpRequested, nil + return positionalOnlyArguments, helpRequested, nil +} + +// findAllParsedValues finds all values parsed by all subcommands and this +// subcommand and its children +func (sc *Subcommand) findAllParsedValues() []parsedValue { + parsedValues := sc.ParsedValues + for _, sc := range sc.Subcommands { + parsedValues = append(parsedValues, sc.findAllParsedValues()...) + } + return parsedValues } // parse causes the argument parser to parse based on the supplied []string. -// depth specifies the non-flag subcommand positional depth +// depth specifies the non-flag subcommand positional depth. A slice of flags +// and subcommands parsed is returned so that the parser can ultimately decide +// if there were any unexpected values supplied by the user func (sc *Subcommand) parse(p *Parser, args []string, depth int) error { debugPrint("- Parsing subcommand", sc.Name, "with depth of", depth, "and args", args) @@ -203,39 +226,15 @@ func (sc *Subcommand) parse(p *Parser, args []string, depth int) error { sc.ensureNoConflictWithBuiltinVersion() } - // Parse the normal flags out of the argument list and retain the positionals. - // Apply the flags to the parent parser and the current subcommand context. - // ./command -f -z subcommand someVar -b becomes ./command subcommand somevar - positionalOnlyArguments, flagNames, helpRequested, err := sc.parseAllFlagsFromArgs(p, args) + // Parse the normal flags out of the argument list and return the positionals + // (subcommands and positional values), along with the flags used. + // Then the flag values are applied to the parent parser and the current + // subcommand being parsed. + positionalOnlyArguments, helpRequested, err := sc.parseAllFlagsFromArgs(p, args) if err != nil { return err } - // if we are set to show help on unexpected flags, we should ensure that - // all flags that aren't positional arguments are found in the list of flag - // names - debugPrint("flags and positionals:", flagNames, positionalOnlyArguments) - if p.ShowHelpOnUnexpected { - - // check over all arguments specified to ensure they are all known as either - // a flag or a subcommand - for _, a := range args { - - // check if the arg matches any known flag name - if stringInSlice(flagNames, a) { - continue - } - - // check if the flag matches any known positional name - if stringInSlice(positionalOnlyArguments, a) { - continue - } - - // crash out if any argument is unknown as both a flag and positional - p.ShowHelpAndExit("Unknown argument: " + a) - } - } - // indicate that trailing arguments have been extracted, so that they aren't // appended a second time p.trailingArgumentsExtracted = true @@ -346,6 +345,17 @@ func (sc *Subcommand) parse(p *Parser, args []string, depth int) error { return nil } +// addParsedFlag makes it easy to append flag values parsed by the subcommand +func (sc *Subcommand) addParsedFlag(key string, value string) { + sc.ParsedValues = append(sc.ParsedValues, newParsedValue(key, value, false)) +} + +// addParsedPositionalValue makes it easy to append positionals parsed by the +// subcommand +func (sc *Subcommand) addParsedPositionalValue(value string) { + sc.ParsedValues = append(sc.ParsedValues, newParsedValue("", value, true)) +} + // FlagExists lets you know if the flag name exists as either a short or long // name in the (sub)command func (sc *Subcommand) FlagExists(name string) bool { diff --git a/subcommand_test.go b/subcommand_test.go index 68f0f16..4c65527 100644 --- a/subcommand_test.go +++ b/subcommand_test.go @@ -40,6 +40,45 @@ func TestFlagExists(t *testing.T) { } +// TestExitOnUnknownFlag tests that when an unknown flag is supplied and the +// ShowHelpOnUnexpected value is set, an error is thrown on unknown flags. +func TestExitOnUnknownFlag(t *testing.T) { + defer func() { + r := recover() + if r == nil { + t.Fatal("Expected crash on unknown flag") + } + }() + flaggy.DebugMode = true + defer debugOff() + var expectedFlag string + var expectedPositional string + flaggy.ResetParser() + flaggy.String(&expectedFlag, "f", "flag", "an expected positonal flag") + flaggy.AddPositionalValue(&expectedPositional, "positionalTest", 1, true, "A test positional value") + flaggy.ParseArgs([]string{"positionalHere", "-f", "flagHere", "unexpectedValue"}) +} + +// TestExitOnUnknownFlagWithValue tests that when an unknown flag with a value +// is supplied and the ShowHelpOnUnexpected value is set, an error is thrown on +// the unknown flags. +func TestExitOnUnknownFlagWithValue(t *testing.T) { + defer func() { + r := recover() + if r == nil { + t.Fatal("Expected crash on unknown flag with value") + } + }() + flaggy.DebugMode = true + defer debugOff() + var expectedFlag string + var expectedPositional string + flaggy.ResetParser() + flaggy.String(&expectedFlag, "f", "flag", "an expected positonal flag") + flaggy.AddPositionalValue(&expectedPositional, "positionalTest", 1, true, "A test positional value") + flaggy.ParseArgs([]string{"positionalHere", "-f", "flagHere", "--unexpectedValue=true"}) +} + // TestDoublePositional tests errors when two positionals are // specified at the same time func TestDoublePositional(t *testing.T) { From 98c2784c08cd756b204e0ec8752315f6aaac5f3e Mon Sep 17 00:00:00 2001 From: Eric Greer Date: Sat, 2 Nov 2019 22:11:54 -0700 Subject: [PATCH 4/9] more comments --- parser.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/parser.go b/parser.go index 767c447..b3f67fc 100644 --- a/parser.go +++ b/parser.go @@ -69,7 +69,9 @@ func (p *Parser) ParseArgs(args []string) error { return nil } -// findArgsNotInParsedValues finds arguments not used in parsed values +// findArgsNotInParsedValues finds arguments not used in parsed values. The +// incoming args should be in the order supplied by the user and should not +// include the invoked binary, which is normally the first thing in os.Args. func findArgsNotInParsedValues(args []string, parsedValues []parsedValue) []string { var argsNotUsed []string @@ -89,7 +91,7 @@ func findArgsNotInParsedValues(args []string, parsedValues []parsedValue) []stri for _, pv := range parsedValues { // this argumenet was a key if pv.Key == arg { - foundArgUsed = true + foundArgUsed = true // the arg was used in this parsedValues set // if the value is not a positional value and the parsed value had a // value that was not blank, we skip the next value in the argument list if !pv.IsPositional && len(pv.Value) > 0 { @@ -103,6 +105,9 @@ func findArgsNotInParsedValues(args []string, parsedValues []parsedValue) []stri break } } + + // if the arg was not used in any parsed values, then we add it to the slice + // of arguments not used if !foundArgUsed { argsNotUsed = append(argsNotUsed, arg) } From dfe8a7997442baaee26163cbab5427a7ce60dd35 Mon Sep 17 00:00:00 2001 From: Eric Greer Date: Sat, 2 Nov 2019 22:18:51 -0700 Subject: [PATCH 5/9] support for trailing args with show help on unexpected --- parser.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/parser.go b/parser.go index b3f67fc..373d4eb 100644 --- a/parser.go +++ b/parser.go @@ -77,6 +77,13 @@ func findArgsNotInParsedValues(args []string, parsedValues []parsedValue) []stri var skipNext bool for _, arg := range args { + + // if the final argument (--) is seen, then we stop checking because all + // further values are trailing arguments. + if determineArgType(arg) == argIsFinal { + return argsNotUsed + } + // allow for skipping the next arg when needed if skipNext { skipNext = false From 12f3813c827d26ec130b4019d67685bdb724d17d Mon Sep 17 00:00:00 2001 From: Eric Greer Date: Sat, 2 Nov 2019 23:32:12 -0700 Subject: [PATCH 6/9] all tests passing with unknown arg support --- flag_test.go | 2 +- flaggy_test.go | 10 ++++++++-- parsedValue.go | 3 +++ parser.go | 16 +++++++++++----- subCommand.go | 19 +++++++++++++++---- subcommand_test.go | 2 +- 6 files changed, 39 insertions(+), 13 deletions(-) diff --git a/flag_test.go b/flag_test.go index 8c81f29..5c2c7ae 100644 --- a/flag_test.go +++ b/flag_test.go @@ -290,7 +290,7 @@ func TestInputParsing(t *testing.T) { var maskSliceFlagExpected = []net.IPMask{net.IPMask([]byte{255, 255, 255, 255}), net.IPMask([]byte{255, 255, 255, 0})} // display help with all flags used - ShowHelp("Showing help from TestInputParsing test.") + ShowHelp("Showing help for test: " + t.Name()) // Parse arguments ParseArgs(inputArgs) diff --git a/flaggy_test.go b/flaggy_test.go index 58b4fb5..1de99fd 100644 --- a/flaggy_test.go +++ b/flaggy_test.go @@ -31,6 +31,9 @@ func TestTrailingArguments(t *testing.T) { // positional values intermixed with eachother. func TestComplexNesting(t *testing.T) { + flaggy.DebugMode = true + defer debugOff() + flaggy.ResetParser() var testA string @@ -47,11 +50,12 @@ func TestComplexNesting(t *testing.T) { flaggy.Bool(&testF, "f", "testF", "") + flaggy.AttachSubcommand(scA, 1) + scA.AddPositionalValue(&testA, "testA", 1, false, "") scA.AddPositionalValue(&testB, "testB", 2, false, "") scA.AddPositionalValue(&testC, "testC", 3, false, "") scA.AttachSubcommand(scB, 4) - flaggy.AttachSubcommand(scA, 1) scB.AddPositionalValue(&testD, "testD", 1, false, "") scB.AttachSubcommand(scC, 2) @@ -60,7 +64,9 @@ func TestComplexNesting(t *testing.T) { scD.AddPositionalValue(&testE, "testE", 1, true, "") - flaggy.ParseArgs([]string{"scA", "-f", "A", "B", "C", "scB", "D", "scC", "scD", "E"}) + args := []string{"scA", "-f", "A", "B", "C", "scB", "D", "scC", "scD", "E"} + t.Log(args) + flaggy.ParseArgs(args) if !testF { t.Log("testF", testF) diff --git a/parsedValue.go b/parsedValue.go index d7dca60..04ada32 100644 --- a/parsedValue.go +++ b/parsedValue.go @@ -12,6 +12,9 @@ type parsedValue struct { // newParsedValue creates and returns a new parsedValue struct with the // supplied values set func newParsedValue(key string, value string, isPositional bool) parsedValue { + if len(key) == 0 && len(value) == 0 { + panic("cant add parsed value with no key or value") + } return parsedValue{ Key: key, Value: value, diff --git a/parser.go b/parser.go index 373d4eb..8e83b15 100644 --- a/parser.go +++ b/parser.go @@ -46,7 +46,8 @@ func (p *Parser) ParseArgs(args []string) error { return errors.New("Parser.Parse() called twice on parser with name: " + " " + p.Name + " " + p.ShortName) } p.parsed = true - // debugPrint("Kicking off parsing with args:", args) + + debugPrint("Kicking off parsing with args:", args) err := p.parse(p, args, 0) if err != nil { return err @@ -55,6 +56,7 @@ func (p *Parser) ParseArgs(args []string) error { // if we are set to crash on unexpected args, look for those here TODO if p.ShowHelpOnUnexpected { parsedValues := p.findAllParsedValues() + debugPrint("parsedValues:", parsedValues) argsNotParsed := findArgsNotInParsedValues(args, parsedValues) if len(argsNotParsed) > 0 { // flatten out unused args for our error message @@ -74,13 +76,12 @@ func (p *Parser) ParseArgs(args []string) error { // include the invoked binary, which is normally the first thing in os.Args. func findArgsNotInParsedValues(args []string, parsedValues []parsedValue) []string { var argsNotUsed []string - var skipNext bool - for _, arg := range args { + for _, a := range args { // if the final argument (--) is seen, then we stop checking because all // further values are trailing arguments. - if determineArgType(arg) == argIsFinal { + if determineArgType(a) == argIsFinal { return argsNotUsed } @@ -90,6 +91,10 @@ func findArgsNotInParsedValues(args []string, parsedValues []parsedValue) []stri continue } + // strip flag slashes from incoming arguments so they match up with the + // keys from parsedValues. + arg := parseFlagToName(a) + // indicates that we found this arg used in one of the parsed values. Used // to indicate which values should be added to argsNotUsed. var foundArgUsed bool @@ -97,7 +102,8 @@ func findArgsNotInParsedValues(args []string, parsedValues []parsedValue) []stri // search all args for a corresponding parsed value for _, pv := range parsedValues { // this argumenet was a key - if pv.Key == arg { + // debugPrint(pv.Key, "==", arg) + if pv.Key == arg || (pv.IsPositional && pv.Value == arg) { foundArgUsed = true // the arg was used in this parsedValues set // if the value is not a positional value and the parsed value had a // value that was not blank, we skip the next value in the argument list diff --git a/subCommand.go b/subCommand.go index 62a3a9b..412be9d 100644 --- a/subCommand.go +++ b/subCommand.go @@ -65,7 +65,7 @@ func (sc *Subcommand) parseAllFlagsFromArgs(p *Parser, args []string) ([]string, // find all the normal flags (not positional) and parse them out for i, a := range args { - debugPrint("parsing arg", 1, a) + debugPrint("parsing arg:", a) // evaluate if there is a following arg to avoid panics var nextArgExists bool @@ -157,7 +157,7 @@ func (sc *Subcommand) parseAllFlagsFromArgs(p *Parser, args []string) ([]string, } skipNext = true - debugPrint(sc.Name, "NOT bool flag", a) + // debugPrint(sc.Name, "NOT bool flag", a) // if the next arg was not found, then show a Help message if !nextArgExists { @@ -193,10 +193,14 @@ func (sc *Subcommand) parseAllFlagsFromArgs(p *Parser, args []string) ([]string, } // findAllParsedValues finds all values parsed by all subcommands and this -// subcommand and its children +// subcommand and its child subcommands func (sc *Subcommand) findAllParsedValues() []parsedValue { parsedValues := sc.ParsedValues for _, sc := range sc.Subcommands { + // skip unused subcommands + if !sc.Used { + continue + } parsedValues = append(parsedValues, sc.findAllParsedValues()...) } return parsedValues @@ -212,6 +216,13 @@ func (sc *Subcommand) parse(p *Parser, args []string, depth int) error { // if a command is parsed, its used sc.Used = true + debugPrint("used subcommand", sc.Name, sc.ShortName) + if len(sc.Name) > 0 { + sc.addParsedPositionalValue(sc.Name) + } + if len(sc.ShortName) > 0 { + sc.addParsedPositionalValue(sc.ShortName) + } // as subcommands are used, they become the context of the parser. This helps // us understand how to display help based on which subcommand is being used @@ -670,7 +681,7 @@ func (sc *Subcommand) SetValueForKey(key string, value string) (bool, error) { // debugPrint("Looking to set key", key, "to value", value) // check for and assign flags that match the key for _, f := range sc.Flags { - debugPrint("Evaluating string flag", f.ShortName, "==", key, "||", f.LongName, "==", key) + // debugPrint("Evaluating string flag", f.ShortName, "==", key, "||", f.LongName, "==", key) if f.ShortName == key || f.LongName == key { // debugPrint("Setting string value for", key, "to", value) f.identifyAndAssignValue(value) diff --git a/subcommand_test.go b/subcommand_test.go index 4c65527..eb76278 100644 --- a/subcommand_test.go +++ b/subcommand_test.go @@ -569,7 +569,7 @@ func TestSCInputParsing(t *testing.T) { var maskSliceFlagExpected = []net.IPMask{net.IPMask([]byte{255, 255, 255, 255}), net.IPMask([]byte{255, 255, 255, 0})} // display help with all flags used - flaggy.ShowHelp("Showing help from TestInputParsing test.") + flaggy.ShowHelp("Showing help from TestSCInputParsing test.") // Parse arguments flaggy.ParseArgs(inputArgs) From 0fe18b08bc5469b4d5978f07eb89b743c2f11fcb Mon Sep 17 00:00:00 2001 From: Eric Greer Date: Tue, 5 Nov 2019 21:27:01 -0800 Subject: [PATCH 7/9] disabling unexpected show help on double parse test --- parser_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/parser_test.go b/parser_test.go index f9abc82..f8dac95 100644 --- a/parser_test.go +++ b/parser_test.go @@ -4,6 +4,7 @@ import "testing" func TestDoubleParse(t *testing.T) { ResetParser() + DefaultParser.ShowHelpOnUnexpected = false err := DefaultParser.Parse() if err != nil { From 123975490757e1b340ca7c7dad311b9d80d2335f Mon Sep 17 00:00:00 2001 From: Eric Greer Date: Tue, 5 Nov 2019 21:29:34 -0800 Subject: [PATCH 8/9] public functions for enabling help on unexpected params and disabling help on unexpected params --- main.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/main.go b/main.go index b44cd91..b242cb6 100644 --- a/main.go +++ b/main.go @@ -327,6 +327,18 @@ func exitOrPanic(code int) { os.Exit(code) } +// ShowHelpOnUnexpectedEnable enables the ShowHelpOnUnexpected behavior on the +// default parser. This causes unknown inputs to error out. +func ShowHelpOnUnexpectedEnable() { + DefaultParser.ShowHelpOnUnexpected = true +} + +// ShowHelpOnUnexpectedDisable disables the ShowHelpOnUnexpected behavior on the +// default parser. This causes unknown inputs to error out. +func ShowHelpOnUnexpectedDisable() { + DefaultParser.ShowHelpOnUnexpected = false +} + // AddPositionalValue adds a positional value to the main parser at the global // context func AddPositionalValue(assignmentVar *string, name string, relativePosition int, required bool, description string) { From 29183fd6cad639c95dc1a4a17a26c86f32dd3d41 Mon Sep 17 00:00:00 2001 From: Eric Greer Date: Tue, 5 Nov 2019 21:54:01 -0800 Subject: [PATCH 9/9] only add flags to parsed list if they ended up setting a value. all tests passing --- parser.go | 14 +++++++++----- subCommand.go | 23 ++++++++++++++--------- subcommand_test.go | 2 ++ 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/parser.go b/parser.go index 8e83b15..41ab76e 100644 --- a/parser.go +++ b/parser.go @@ -4,12 +4,14 @@ import ( "errors" "fmt" "os" + "strconv" "text/template" ) -// Parser represents the set of vars and subcommands we are expecting -// from our input args, and the parser than handles them all. +// Parser represents the set of flags and subcommands we are expecting +// from our input arguments. Parser is the top level struct responsible for +// parsing an entire set of subcommands and flags. type Parser struct { Subcommand Version string // the optional version of the parser. @@ -60,11 +62,11 @@ func (p *Parser) ParseArgs(args []string) error { argsNotParsed := findArgsNotInParsedValues(args, parsedValues) if len(argsNotParsed) > 0 { // flatten out unused args for our error message - var argsNotParedFlat string + var argsNotParsedFlat string for _, a := range argsNotParsed { - argsNotParedFlat = argsNotParedFlat + " " + a + argsNotParsedFlat = argsNotParsedFlat + " " + a } - p.ShowHelpAndExit("Unknown arguments supplied: " + argsNotParedFlat) + p.ShowHelpAndExit("Unknown arguments supplied: " + argsNotParsedFlat) } } @@ -103,7 +105,9 @@ func findArgsNotInParsedValues(args []string, parsedValues []parsedValue) []stri for _, pv := range parsedValues { // this argumenet was a key // debugPrint(pv.Key, "==", arg) + debugPrint(pv.Key + "==" + arg + " || (" + strconv.FormatBool(pv.IsPositional) + " && " + pv.Value + " == " + arg + ")") if pv.Key == arg || (pv.IsPositional && pv.Value == arg) { + debugPrint("Found matching parsed arg for " + pv.Key) foundArgUsed = true // the arg was used in this parsedValues set // if the value is not a positional value and the parsed value had a // value that was not blank, we skip the next value in the argument list diff --git a/subCommand.go b/subCommand.go index 412be9d..7f99e3e 100644 --- a/subCommand.go +++ b/subCommand.go @@ -49,7 +49,6 @@ func NewSubcommand(name string) *Subcommand { // requested, and any errors found during parsing func (sc *Subcommand) parseAllFlagsFromArgs(p *Parser, args []string) ([]string, bool, error) { - var err error var positionalOnlyArguments []string var helpRequested bool // indicates the user has supplied -h and we // should render help if we are the last subcommand @@ -130,7 +129,7 @@ func (sc *Subcommand) parseAllFlagsFromArgs(p *Parser, args []string) ([]string, positionalOnlyArguments = append(positionalOnlyArguments, a) // track this as a parsed value with the subcommand sc.addParsedPositionalValue(a) - case argIsFlagWithSpace: + case argIsFlagWithSpace: // a flag with a space. ex) -k v or --key value a = parseFlagToName(a) // debugPrint("Arg", i, "is flag with space:", a) @@ -140,7 +139,7 @@ func (sc *Subcommand) parseAllFlagsFromArgs(p *Parser, args []string) ([]string, if flagIsBool(sc, p, a) { debugPrint(sc.Name, "bool flag", a, "next var is:", nextArg) // set the value in this subcommand and its root parser - _, err = setValueForParsers(a, "true", p, sc) + valueSet, err := setValueForParsers(a, "true", p, sc) // if an error occurs, just return it and quit parsing if err != nil { @@ -149,7 +148,9 @@ func (sc *Subcommand) parseAllFlagsFromArgs(p *Parser, args []string) ([]string, // log all values parsed by this subcommand. We leave the value blank // because the bool value had no explicit true or false supplied - sc.addParsedFlag(a, "") + if valueSet { + sc.addParsedFlag(a, "") + } // we've found and set a standalone bool flag, so we move on to the next // argument in the list of arguments @@ -164,14 +165,16 @@ func (sc *Subcommand) parseAllFlagsFromArgs(p *Parser, args []string) ([]string, p.ShowHelpWithMessage("Expected a following arg for flag " + a + ", but it did not exist.") exitOrPanic(2) } - _, err = setValueForParsers(a, nextArg, p, sc) + valueSet, err := setValueForParsers(a, nextArg, p, sc) if err != nil { return []string{}, false, err } // log all parsed values in the subcommand - sc.addParsedFlag(a, nextArg) - case argIsFlagWithValue: + if valueSet { + sc.addParsedFlag(a, nextArg) + } + case argIsFlagWithValue: // a flag with an equals sign. ex) -k=v or --key=value // debugPrint("Arg", i, "is flag with value:", a) a = parseFlagToName(a) @@ -179,13 +182,15 @@ func (sc *Subcommand) parseAllFlagsFromArgs(p *Parser, args []string) ([]string, key, val := parseArgWithValue(a) // set the value in this subcommand and its root parser - _, err := setValueForParsers(key, val, p, sc) + valueSet, err := setValueForParsers(key, val, p, sc) if err != nil { return []string{}, false, err } // log all values parsed by the subcommand - sc.addParsedFlag(key, val) + if valueSet { + sc.addParsedFlag(a, val) + } } } diff --git a/subcommand_test.go b/subcommand_test.go index eb76278..50730ae 100644 --- a/subcommand_test.go +++ b/subcommand_test.go @@ -63,6 +63,8 @@ func TestExitOnUnknownFlag(t *testing.T) { // is supplied and the ShowHelpOnUnexpected value is set, an error is thrown on // the unknown flags. func TestExitOnUnknownFlagWithValue(t *testing.T) { + flaggy.ResetParser() + flaggy.ShowHelpOnUnexpectedEnable() defer func() { r := recover() if r == nil {