Tighten up parsing to be less ambiguous, follow GNU conventions

3 files changed, 124 insertions, 94 deletions

diff --git a/README.md b/README.md
index d16389b..714d202 100644
--- a/README.md
+++ b/README.md
@@ -112,30 +112,24 @@ flag set.
 ```
 --flag    // boolean flags only
 --flag=x
---flag x  // non-boolean flags only
 ```
 
-The last form is not permitted for boolean flags because the
-meaning of the command
-
-```
-cmd --flag *
-```
-
-will change if there is a file called 0, false, etc.  You must
-use the --flag=false form to turn off a boolean flag.
-
 Unlike the flag package, a single dash before an option means something
 different than a double dash. Single dashes signify a series of shorthand
 letters for flags. All but the last shorthand letter must be boolean flags.
 
 ```
--f          // f must be boolean
--abc        // all flags must be boolean
--abcn=1234
--abcn 1234  // n must be non-boolean
--abcn1234   // n must be non-boolean
--Ifile      // I must be non-boolean
+// boolean flags
+-f
+-abc
+
+// non-boolean flags
+-n 1234
+-Ifile
+
+// mixed
+-abcs "hello"
+-abcn1234
 ```
 
 Flag parsing stops after the terminator "--". Unlike the flag package,
@@ -143,7 +137,8 @@ flags can be interspersed with arguments anywhere on the command line
 before this terminator.
 
 Integer flags accept 1234, 0664, 0x1234 and may be negative.
-Boolean flags may be 1, 0, t, f, true, false, TRUE, FALSE, True, False.
+Boolean flags (in their long form) accept 1, 0, t, f, true, false,
+TRUE, FALSE, True, False.
 Duration flags accept any input valid for time.ParseDuration.
 
 ## More info
diff --git a/flag.go b/flag.go
index 42f2308..06edf20 100644
--- a/flag.go
+++ b/flag.go
@@ -64,29 +64,27 @@
 	Command line flag syntax:
 		--flag    // boolean flags only
 		--flag=x
-		--flag x  // non-boolean flags only
-	The last form is not permitted for boolean flags because the
-	meaning of the command
-		cmd --flag *
-	will change if there is a file called 0, false, etc.  You must
-	use the --flag=false form to turn off a boolean flag.
 
 	Unlike the flag package, a single dash before an option means something
 	different than a double dash. Single dashes signify a series of shorthand
 	letters for flags. All but the last shorthand letter must be boolean flags.
-		-f          // f must be boolean
-		-abc        // all flags must be boolean
-		-abcn=1234
-		-abcn 1234  // n must be non-boolean
-		-abcn1234   // n must be non-boolean
-		-Ifile      // I must be non-boolean
+		// boolean flags
+		-f
+		-abc
+		// non-boolean flags
+		-n 1234
+		-Ifile
+		// mixed
+		-abcs "hello"
+		-abcn1234
 
 	Flag parsing stops after the terminator "--". Unlike the flag package,
 	flags can be interspersed with arguments anywhere on the command line
 	before this terminator.
 
 	Integer flags accept 1234, 0664, 0x1234 and may be negative.
-	Boolean flags may be 1, 0, t, f, true, false, TRUE, FALSE, True, False.
+	Boolean flags (in their long form) accept 1, 0, t, f, true, false,
+	TRUE, FALSE, True, False.
 	Duration flags accept any input valid for time.ParseDuration.
 
 	The default set of command-line flags is controlled by
@@ -105,6 +103,7 @@ import (
 	"os"
 	"sort"
 	"strconv"
+	"strings"
 	"time"
 )
 
@@ -902,6 +901,19 @@ func (f *FlagSet) usage() {
 	}
 }
 
+func (f *FlagSet) setFlag(flag *Flag, value string, origArg string) error {
+	if err := flag.Value.Set(value); err != nil {
+		return f.failf("invalid argument %q for %s: %v", value, origArg, err)
+	}
+	// mark as visited for Visit()
+	if f.actual == nil {
+		f.actual = make(map[string]*Flag)
+	}
+	f.actual[flag.Name] = flag
+
+	return nil
+}
+
 func (f *FlagSet) parseArgs(args []string) error {
 	for len(args) > 0 {
 		s := args[0]
@@ -911,9 +923,6 @@ func (f *FlagSet) parseArgs(args []string) error {
 			continue
 		}
 
-		var flag *Flag = nil
-		has_value := false
-		value := ""
 		if s[1] == '-' {
 			if len(s) == 2 { // "--" terminates the flags
 				f.args = append(f.args, args...)
@@ -923,17 +932,10 @@ func (f *FlagSet) parseArgs(args []string) error {
 			if len(name) == 0 || name[0] == '-' || name[0] == '=' {
 				return f.failf("bad flag syntax: %s", s)
 			}
-			// check for = argument to flag
-			for i := 1; i < len(name); i++ { // equals cannot be first
-				if name[i] == '=' {
-					value = name[i+1:]
-					has_value = true
-					name = name[0:i]
-					break
-				}
-			}
+			split := strings.SplitN(name, "=", 2)
+			name = split[0]
 			m := f.formal
-			_, alreadythere := m[name] // BUG
+			flag, alreadythere := m[name] // BUG
 			if !alreadythere {
 				if name == "help" { // special case for nice help message.
 					f.usage()
@@ -941,12 +943,21 @@ func (f *FlagSet) parseArgs(args []string) error {
 				}
 				return f.failf("unknown flag: --%s", name)
 			}
-			flag = m[name]
+			if len(split) == 1 {
+				if _, ok := flag.Value.(*boolValue); !ok {
+					return f.failf("flag needs an argument: %s", s)
+				}
+				f.setFlag(flag, "true", s)
+			} else {
+				if err := f.setFlag(flag, split[1], s); err != nil {
+					return err
+				}
+			}
 		} else {
 			shorthands := s[1:]
 			for i := 0; i < len(shorthands); i++ {
 				c := shorthands[i]
-				_, alreadythere := f.shorthands[c]
+				flag, alreadythere := f.shorthands[c]
 				if !alreadythere {
 					if c == 'h' { // special case for nice help message.
 						f.usage()
@@ -954,53 +965,26 @@ func (f *FlagSet) parseArgs(args []string) error {
 					}
 					return f.failf("unknown shorthand flag: %q in -%s", c, shorthands)
 				}
-				flag = f.shorthands[c]
-				if i == len(shorthands)-1 {
-					break
+				if _, ok := flag.Value.(*boolValue); ok {
+					f.setFlag(flag, "true", s)
+					continue
 				}
-				if shorthands[i+1] == '=' {
-					value = shorthands[i+2:]
-					has_value = true
+				if i < len(shorthands)-1 {
+					if err := f.setFlag(flag, shorthands[i+1:], s); err != nil {
+						return err
+					}
 					break
 				}
-				if fv, ok := flag.Value.(*boolValue); ok {
-					fv.Set("true")
-				} else {
-					value = shorthands[i+1:]
-					has_value = true
-					break
+				if len(args) == 0 {
+					return f.failf("flag needs an argument: %q in -%s", c, shorthands)
 				}
-			}
-		}
-
-		// we have a flag, possibly with included =value argument
-		if fv, ok := flag.Value.(*boolValue); ok { // special case: doesn't need an arg
-			if has_value {
-				if err := fv.Set(value); err != nil {
-					f.failf("invalid boolean value %q for %s: %v", value, s, err)
+				if err := f.setFlag(flag, args[0], s); err != nil {
+					return err
 				}
-			} else {
-				fv.Set("true")
-			}
-		} else {
-			// It must have a value, which might be the next argument.
-			if !has_value && len(args) > 0 {
-				// value is the next arg
-				has_value = true
-				value = args[0]
 				args = args[1:]
+				break // should be unnecessary
 			}
-			if !has_value {
-				return f.failf("flag needs an argument: %s", s)
-			}
-			if err := flag.Value.Set(value); err != nil {
-				return f.failf("invalid value %q for %s: %v", value, s, err)
-			}
-		}
-		if f.actual == nil {
-			f.actual = make(map[string]*Flag)
 		}
-		f.actual[flag.Name] = flag
 	}
 	return nil
 }
diff --git a/flag_test.go b/flag_test.go
index 7649946..9caa8c4 100644
--- a/flag_test.go
+++ b/flag_test.go
@@ -111,6 +111,7 @@ func testParse(f *FlagSet, t *testing.T) {
 	}
 	boolFlag := f.Bool("bool", false, "bool value")
 	bool2Flag := f.Bool("bool2", false, "bool2 value")
+	bool3Flag := f.Bool("bool3", false, "bool3 value")
 	intFlag := f.Int("int", 0, "int value")
 	int64Flag := f.Int64("int64", 0, "int64 value")
 	uintFlag := f.Uint("uint", 0, "uint value")
@@ -122,13 +123,14 @@ func testParse(f *FlagSet, t *testing.T) {
 	args := []string{
 		"--bool",
 		"--bool2=true",
-		"--int", "22",
-		"--int64", "0x23",
-		"--uint", "24",
-		"--uint64", "25",
-		"--string", "hello",
-		"--float64", "2718e28",
-		"--duration", "2m",
+		"--bool3=false",
+		"--int=22",
+		"--int64=0x23",
+		"--uint=24",
+		"--uint64=25",
+		"--string=hello",
+		"--float64=2718e28",
+		"--duration=2m",
 		extra,
 	}
 	if err := f.Parse(args); err != nil {
@@ -143,6 +145,9 @@ func testParse(f *FlagSet, t *testing.T) {
 	if *bool2Flag != true {
 		t.Error("bool2 flag should be true, is ", *bool2Flag)
 	}
+	if *bool3Flag != false {
+		t.Error("bool3 flag should be false, is ", *bool2Flag)
+	}
 	if *intFlag != 22 {
 		t.Error("int flag should be 22, is ", *intFlag)
 	}
@@ -171,6 +176,52 @@ func testParse(f *FlagSet, t *testing.T) {
 	}
 }
 
+func TestShorthand(t *testing.T) {
+	f := NewFlagSet("shorthand", ContinueOnError)
+	if f.Parsed() {
+		t.Error("f.Parse() = true before Parse")
+	}
+	boolaFlag := f.BoolP("boola", "a", false, "bool value")
+	boolbFlag := f.BoolP("boolb", "b", false, "bool2 value")
+	boolcFlag := f.BoolP("boolc", "c", false, "bool3 value")
+	stringFlag := f.StringP("string", "s", "0", "string value")
+	extra := "interspersed-argument"
+	notaflag := "--i-look-like-a-flag"
+	args := []string{
+		"-ab",
+		extra,
+		"-cs",
+		"hello",
+		"--",
+		notaflag,
+	}
+	if err := f.Parse(args); err != nil {
+		t.Fatal(err)
+	}
+	if !f.Parsed() {
+		t.Error("f.Parse() = false after Parse")
+	}
+	if *boolaFlag != true {
+		t.Error("boola flag should be true, is ", *boolaFlag)
+	}
+	if *boolbFlag != true {
+		t.Error("boolb flag should be true, is ", *boolbFlag)
+	}
+	if *boolcFlag != true {
+		t.Error("boolc flag should be true, is ", *boolcFlag)
+	}
+	if *stringFlag != "hello" {
+		t.Error("string flag should be `hello`, is ", *stringFlag)
+	}
+	if len(f.Args()) != 2 {
+		t.Error("expected one argument, got", len(f.Args()))
+	} else if f.Args()[0] != extra {
+		t.Errorf("expected argument %q got %q", extra, f.Args()[0])
+	} else if f.Args()[1] != notaflag {
+		t.Errorf("expected argument %q got %q", notaflag, f.Args()[1])
+	}
+}
+
 func TestParse(t *testing.T) {
 	ResetForTesting(func() { t.Error("bad parse") })
 	testParse(CommandLine(), t)
@@ -196,8 +247,8 @@ func TestUserDefined(t *testing.T) {
 	var flags FlagSet
 	flags.Init("test", ContinueOnError)
 	var v flagVar
-	flags.Var(&v, "v", "usage")
-	if err := flags.Parse([]string{"--v", "1", "--v", "2", "--v=3"}); err != nil {
+	flags.VarP(&v, "v", "v", "usage")
+	if err := flags.Parse([]string{"--v=1", "-v2", "-v", "3"}); err != nil {
 		t.Error(err)
 	}
 	if len(v) != 3 {