Version handling with NewParser + Parse fails

[stevemil00]

Hi -- it seems like if you're using arg.NewParser and then parser.Parse, somehow go-arg finds but does not seem to use the Version method defined on the Args struct.

Actually, the TL;DR here might be that using arg.NewParser and then parser.Parse, there's no way to get the version string displayed, and you always get an error back. But if you call arg.MustParse, or arg.NewParser plus parser.MustParse, that error gets caught internally and go-arg prints the version and exits (which seems legit 😄 ).

Let's start by looking at this:

package main

import "fmt"
import "github.com/alexflint/go-arg"

type Args struct {
    What string `arg:"-w,--what"`
}

func (a *Args) Version() string {
    return("first vers")
}

func main() {
    argh := &Args{}
    arg.MustParse(argh)
    fmt.Printf("%#v\n", argh)
}

So that's just using os.Args internally, and it works just fine:

Stevens-MacBook-Pro:argh smiller$ go run first.go --version
first vers

If we use the relatively new parser.MustParse method, things work just fine, too:

package main

import "fmt"
import "os"
import "github.com/alexflint/go-arg"

type Args struct {
    What string `arg:"-w,--what"`
}

func (a *Args) Version() string {
    return "second vers"
}

func main() {
    argh := &Args{}
    config := arg.Config{Program: "program", IgnoreEnv: false}
    parser, err := arg.NewParser(config, argh)
    if err != nil {
        panic(err)
    }
    parser.MustParse(os.Args[1:])
    fmt.Printf("%#v\n", argh)
}

which gets you:

Stevens-MacBook-Pro:argh smiller$ go run second.go --version
second vers

but if you use NewParser plus Parse, it blows up:

package main

import "fmt"
import "os"
import "github.com/alexflint/go-arg"

type Args struct {
    What string `arg:"-w,--what"`
}

func (a *Args) Version() string {
    return "vers"
}

func main() {
    argh := &Args{}
    config := arg.Config{Program: "program", IgnoreEnv: false}
    parser, err := arg.NewParser(config, argh)
    if err != nil {
        panic(err)
    }
    err = parser.Parse(os.Args[1:])
    if err != nil {
        panic(err)
    }
    fmt.Printf("%#v\n", argh)
}

and you get:

Stevens-MacBook-Pro:argh smiller$ go run third.go --version
panic: version requested by user

goroutine 1 [running]:
main.main()
    /var/tmp/argh/third.go:24 +0x154
exit status 2

I tried stepping through this in the debugger, and it seemed like NewParser was recognizing that there was a Version method, running it, and storing its value in .version. But then parse would run, and it would run into this:

    // determine if the current command has a version option spec
    var hasVersionOption bool
    for _, spec := range curCmd.specs {
        if spec.long == "version" {
            hasVersionOption = true
            break
        }
    }

and since there isn't a version field in Args, it would blow up at:

        // check for special --help and --version flags
        switch arg {
        case "-h", "--help":
            return ErrHelp
        case "--version":
            if !hasVersionOption && p.version != "" {
                return ErrVersion
            }
        }

Looking at the code in parser.MustParse, it is following that same code path, and the help/version handling above blows up in the same way, but that code catches the error, prints the version, and exits:

func (p *Parser) MustParse(args []string) {
    err := p.Parse(args)
    switch {
    case err == ErrHelp:
        p.WriteHelpForSubcommand(p.config.Out, p.subcommand...)
        p.config.Exit(0)
    case err == ErrVersion:
        fmt.Fprintln(p.config.Out, p.version)
        p.config.Exit(0)
    case err != nil:
        p.FailSubcommand(err.Error(), p.subcommand...)
    }
}

so if you do what the third bit of go does, call NewParser and then call Parse, that MustParse error interception never happens and there's no way to get the version string printed.

It'd be nice if this was somehow consistent with NewParser + MustParse or with the top-level MustParse, though now that I know what's going on (and now that I've upgraded to 1.51 😄 ) it's an easy change to work around it.

[alexflint]

Thanks for the detailed report @stevemil00

Sorry that this is not very intuitive, but the intended behavior of Parser.Parse is that it should return arg.ErrVersion when --version is provided on the command line. It is done this way so that "--help" and "--version" can terminate the processing of command line arguments as soon as they are encountered. If I was designing the library from scratch now, I wouldn't handle this by returning and error, but in the current implementation at the moment, that's how it works. The intention is that you catch these errors programmatically if you want to respond to --version and --help.

I just now added some notes to the README just now showing how to handle this all from outside the library, reproducing the exact same logic as MustParse. In your case, use code like this:

type args struct {
    Something string
}

func (args) Version() string {
    return "1.2.3"
}

func main() {
    var args args
    p, err := arg.NewParser(arg.Config{}, &args)
    if err != nil {
        log.Fatalf("there was an error in the definition of the Go struct: %v", err)
    }

    err = p.Parse(os.Args[1:])
    switch {
    case err == arg.ErrHelp: // found "--help" on command line
        p.WriteHelp(os.Stdout)
        os.Exit(0)
    case err == arg.ErrVersion: // found "--version" on command line
        fmt.Println(args.Version())
        os.Exit(0)
    case err != nil:
        fmt.Printf("error: %v\n", err)
        p.WriteUsage(os.Stdout)
        os.Exit(1)
    }

    fmt.Printf("got %q\n", args.Something)
}

See also the full discussion, and in particular the second snippet in the README here.

Thanks again for the details report! Closing for now but do please re-open if I missed anything.