From ecd369e62284c45f90df134a9a936d71f42508da Mon Sep 17 00:00:00 2001 From: Gerald Kaszuba Date: Tue, 22 May 2018 23:18:08 +1000 Subject: [PATCH] Kong tag PR fixes (#12) * Removed bubbling errors in favour of panic * Added a test for Parse with a bad build and an arg. * Removed is() function in favour of a switch with strings * Collect key and value in parseCSV, reducing complexity. * Fix in global to not use parser instance on error. --- build.go | 6 +--- global.go | 8 +++-- global_test.go | 31 ++++++++++++++++++ kong.go | 7 ++-- tag.go | 87 +++++++++++++++++++++++--------------------------- 5 files changed, 80 insertions(+), 59 deletions(-) create mode 100644 global_test.go diff --git a/build.go b/build.go index 2e70d9d..b3299cc 100644 --- a/build.go +++ b/build.go @@ -61,11 +61,7 @@ func buildNode(v reflect.Value, seenFlags map[string]bool, cmd bool) *Node { name = strings.ToLower(dashedString(ft.Name)) } - tag, err := parseTag(fv, ft.Tag.Get("kong")) - if err != nil { - fail("%s", err) - } - + tag := parseTag(fv, ft.Tag.Get("kong")) decoder := DecoderForField(tag.Type, ft) if !cmd { diff --git a/global.go b/global.go index 72544ca..a9fd899 100644 --- a/global.go +++ b/global.go @@ -1,11 +1,15 @@ package kong -import "os" +import ( + "os" +) // Parse constructs a new parser and parses the default command-line. func Parse(cli interface{}, options ...Option) string { parser, err := New(cli, options...) - parser.FatalIfErrorf(err) + if err != nil { + panic(err) + } cmd, err := parser.Parse(os.Args[1:]) parser.FatalIfErrorf(err) return cmd diff --git a/global_test.go b/global_test.go new file mode 100644 index 0000000..fdc5336 --- /dev/null +++ b/global_test.go @@ -0,0 +1,31 @@ +package kong + +import ( + "os" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestParseHandlingBadBuild(t *testing.T) { + var cli struct { + Enabled bool `kong:"unknown"` + } + + args := os.Args + defer func() { + os.Args = args + }() + + os.Args = []string{os.Args[0], "hi"} + + defer func() { + if r := recover(); r != nil { + require.Equal(t, Error{msg: "unknown is an unknown kong key"}, r) + } + }() + + Parse(&cli, ExitFunction(func(_ int) { panic("exiting") })) + + require.Fail(t, "we were expecting a panic") +} diff --git a/kong.go b/kong.go index 1afd862..3df6c09 100644 --- a/kong.go +++ b/kong.go @@ -51,6 +51,7 @@ func New(ast interface{}, options ...Option) (*Kong, error) { for _, option := range options { option(k) } + return k, nil } @@ -75,11 +76,7 @@ func (k *Kong) Parse(args []string) (command string, err error) { } func (k *Kong) Errorf(format string, args ...interface{}) { - if k.Model != nil { - fmt.Fprintf(os.Stderr, k.Model.Name+": "+format, args...) - } else { - fmt.Fprintf(os.Stderr, format, args...) - } + fmt.Fprintf(os.Stderr, k.Model.Name+": "+format, args...) } func (k *Kong) FatalIfErrorf(err error, args ...interface{}) { diff --git a/tag.go b/tag.go index acdb5c0..ef10f99 100644 --- a/tag.go +++ b/tag.go @@ -1,7 +1,6 @@ package kong import ( - "fmt" "reflect" "strings" "unicode/utf8" @@ -21,19 +20,21 @@ type Tag struct { Short rune } -func parseCSV(s string) ([]string, error) { - num := 0 - parts := []string{} - current := []rune{} +func parseCSV(s string) map[string]string { + d := map[string]string{} + + key := []rune{} + value := []rune{} + quotes := false + inKey := true add := func() { - parts = append(parts, string(current)) - current = []rune{} - num++ + d[string(key)] = string(value) + key = []rune{} + value = []rune{} + inKey = true } - quotes := false - runes := []rune(s) for idx := 0; idx < len(runes); idx++ { r := runes[idx] @@ -48,6 +49,10 @@ func parseCSV(s string) ([]string, error) { add() continue } + if r == '=' && inKey { + inKey = false + continue + } if r == '\\' { if next == '\'' { idx++ @@ -59,72 +64,60 @@ func parseCSV(s string) ([]string, error) { if next == ',' || eof { continue } - return parts, fmt.Errorf("%v has an unexpected char at pos %v", s, idx) + fail("%v has an unexpected char at pos %v", s, idx) } else { quotes = true continue } } - current = append(current, r) + if inKey { + key = append(key, r) + } else { + value = append(value, r) + } } if quotes { - return parts, fmt.Errorf("%v is not quoted properly", s) + fail("%v is not quoted properly", s) } add() - return parts, nil + return d } -func parseTag(fv reflect.Value, s string) (*Tag, error) { +func parseTag(fv reflect.Value, s string) *Tag { t := &Tag{} if s == "" { - return t, nil + return t } - parts, err := parseCSV(s) - if err != nil { - return t, err - } - - for _, part := range parts { - is := func(m string) bool { return part == m } - value := func(m string) (string, bool) { - split := strings.SplitN(part, "=", 2) - if split[0] != m { - return "", false - } - if len(split) == 1 { - return "", true - } - return split[1], true - } - - if is("cmd") { + for k, v := range parseCSV(s) { + switch k { + case "cmd": t.Cmd = true - } else if is("arg") { + case "arg": t.Arg = true - } else if is("required") { + case "required": t.Required = true - } else if is("optional") { + case "optional": t.Optional = true - } else if v, ok := value("default"); ok { + case "default": t.Default = v - } else if v, ok := value("help"); ok { + case "help": t.Help = v - } else if v, ok := value("type"); ok { + case "type": t.Type = v - } else if v, ok := value("placeholder"); ok { + case "placeholder": t.Placeholder = v - } else if v, ok := value("env"); ok { + case "env": t.Env = v - } else if v, ok := value("rune"); ok { + case "rune": t.Short, _ = utf8.DecodeRuneInString(v) if t.Short == utf8.RuneError { t.Short = 0 } - } else { - return t, fmt.Errorf("%v is an unknown kong key", part) + default: + fail("%v is an unknown kong key", k) } } @@ -132,5 +125,5 @@ func parseTag(fv reflect.Value, s string) (*Tag, error) { t.Placeholder = strings.ToUpper(dashedString(fv.Type().Name())) } - return t, nil + return t }