Tighten up parsing to be less ambiguous, follow GNU conventions
diff --git a/README.md b/README.md index d16389b..714d202 100644 --- a/README.md +++ b/README.md
@@ -112,30 +112,24 @@ ``` --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 @@ 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 @@ "os" "sort" "strconv" + "strings" "time" ) @@ -902,6 +901,19 @@ } } +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 @@ 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 @@ 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 @@ } 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 @@ } return f.failf("unknown shorthand flag: %q in -%s", c, shorthands) } - flag = f.shorthands[c] - if i == len(shorthands)-1 { + if _, ok := flag.Value.(*boolValue); ok { + f.setFlag(flag, "true", s) + continue + } + if i < len(shorthands)-1 { + if err := f.setFlag(flag, shorthands[i+1:], s); err != nil { + return err + } break } - if shorthands[i+1] == '=' { - value = shorthands[i+2:] - has_value = true - break + if len(args) == 0 { + return f.failf("flag needs an argument: %q in -%s", c, shorthands) } - if fv, ok := flag.Value.(*boolValue); ok { - fv.Set("true") - } else { - value = shorthands[i+1:] - has_value = true - break + if err := f.setFlag(flag, args[0], s); err != nil { + return err } - } - } - - // 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) - } - } 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:] - } - 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) + break // should be unnecessary } } - 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 @@ } 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 @@ 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 @@ 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 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 @@ 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 {