Merge pull request #55 from integrii/#47

Fixes for unexpected flags not crashing out properly. Fixes #47

.gitignore

@@ -12,3 +12,5 @@
 
 # Project-local glide cache, RE: https://github.com/Masterminds/glide/issues/736
 .glide/
+
+.idea/

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 {

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)
@@ -285,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)

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)
@@ -108,6 +114,8 @@ 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"}
 
+	flaggy.DebugMode = true
+
 	var boolT bool
 	var intT int
 	var testN string
@@ -151,15 +159,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)
 	}

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) {

parsedValue.go

@@ -0,0 +1,23 @@
+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 {
+	if len(key) == 0 && len(value) == 0 {
+		panic("cant add parsed value with no key or value")
+	}
+	return parsedValue{
+		Key:          key,
+		Value:        value,
+		IsPositional: isPositional,
+	}
+}

parser.go

@@ -4,18 +4,20 @@ 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.
 	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
@@ -46,8 +48,89 @@ 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)
-	return p.parse(p, args, 0)
+
+	debugPrint("Kicking off parsing with args:", args)
+	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()
+		debugPrint("parsedValues:", parsedValues)
+		argsNotParsed := findArgsNotInParsedValues(args, parsedValues)
+		if len(argsNotParsed) > 0 {
+			// flatten out unused args for our error message
+			var argsNotParsedFlat string
+			for _, a := range argsNotParsed {
+				argsNotParsedFlat = argsNotParsedFlat + " " + a
+			}
+			p.ShowHelpAndExit("Unknown arguments supplied: " + argsNotParsedFlat)
+		}
+	}
+
+	return nil
+}
+
+// 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
+	var skipNext bool
+	for _, a := range args {
+
+		// if the final argument (--) is seen, then we stop checking because all
+		// further values are trailing arguments.
+		if determineArgType(a) == argIsFinal {
+			return argsNotUsed
+		}
+
+		// allow for skipping the next arg when needed
+		if skipNext {
+			skipNext = false
+			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
+
+		// search all args for a corresponding parsed value
+		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
+				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 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)
+		}
+	}
+
+	return argsNotUsed
 }
 
 // ShowVersionAndExit shows the version of this parser
@@ -105,7 +188,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
 }

parser_test.go

@@ -4,6 +4,7 @@ import "testing"
 
 func TestDoubleParse(t *testing.T) {
 	ResetParser()
+	DefaultParser.ShowHelpOnUnexpected = false
 
 	err := DefaultParser.Parse()
 	if err != nil {