jessevdk / go-flags

go command line option parser
http://godoc.org/github.com/jessevdk/go-flags
BSD 3-Clause "New" or "Revised" License
2.6k stars 309 forks source link

scan() doesn't handle tabs as whitespace in struct tag `....` field #176

Closed jboelter closed 8 years ago

jboelter commented 8 years ago

I formatted my options like below using tabs and it took a bit to figure out what I did wrong. The scan code only skips spaces for whitespace when parsing the struct tag. Support for tabs (or any whitespace?) in the struct tags would be nice. Let me know if you want a pull request or take the snippet below.

var opts struct {
    Verbose          bool   `short:"v"  long:"verbose"                                                          description:"Show verbose debug information"`
    Local            bool   `short:"l"  long:"local"                                                            description:"execute locally"`
    Port             int    `short:"p"  long:"port"         default:"80"                                        description:"port to listen for HTTP"`
}
func (x *multiTag) scan() (map[string][]string, error) {

...
        // Skip whitespace
        for i < len(v) && (v[i] == ' ' || v[i] == '\t') {
            i++
        }
jessevdk commented 8 years ago

go-flags follows the go tags spec which does not allow newlines or tabs in tags. I prefer for go-flags to be up to spec so it's easier to explain.

jboelter commented 8 years ago

Edit: my bad; vet was not actually running. The implementation make sense. Seems like the language spec is more open than the reflect implementation of get tag per this thread: https://groups.google.com/forum/#!searchin/golang-nuts/tabs$20in$20struct$20tags$20/golang-nuts/V1s_qrjDCBQ/7yBOr8JPLQMJ

There's a comment in https://golang.org/src/reflect/type.go that indicates StructTags are separated by spaces.

-- end edit

The spec seems to indicate that any string_lit is a valid struct tag. It compiles and doesn't generate any lint or vet warnings. Is there somewhere else this is spec'd?

https://golang.org/ref/spec#Struct_types

StructType     = "struct" "{" { FieldDecl ";" } "}" .
FieldDecl      = (IdentifierList Type | AnonymousField) [ Tag ] .
AnonymousField = [ "*" ] TypeName .
Tag            = string_lit .

https://golang.org/ref/spec#string_lit

string_lit             = raw_string_lit | interpreted_string_lit .
raw_string_lit         = "`" { unicode_char | newline } "`" .
interpreted_string_lit = `"` { unicode_value | byte_value } `"` .
jessevdk commented 8 years ago

Sorry, I should have been more precise. I was talking about the key/value tag parsing done by the go stdlib: https://golang.org/pkg/reflect/#StructTag