alexflint / go-arg

Struct-based argument parsing in Go
https://pkg.go.dev/github.com/alexflint/go-arg
BSD 2-Clause "Simplified" License
2.04k stars 100 forks source link

subcommands must be pointers to structs but args is a pointer to interface #236

Closed purpleidea closed 8 months ago

purpleidea commented 8 months ago

Firstly, this is a great library. All of the other parsing libraries have these big and clunky API's and this is doing the simple, correct thing. I love it.

I decided to try to port https://github.com/purpleidea/mgmt/ away from urfave/cli to use this package instead. I'm also trying to clean up the way we do CLI parsing in mgmt because it's very clunky due to my earlier inexperience, and it is a good time to move to a new internal API at the same as I switch libraries.

I'd like to be able to have an internal API that let's someone pass in a one of the "specification" structs and then it runs the parser. Unfortunately (but maybe necessarily, I'm not sure) the go-arg parser requires that it sees a direct struct, instead of a struct stored as an interface somewhere.

I chose interface{} because this is the same input type as is used in the signature for NewParser or MustParse.

When you have this indirection you get the error:

subcommands must be pointers to structs but args is a pointer to interface

I think it would be a good idea to relax this if possible, but I didn't spend long enough digging through the reflection code.

In the meantime, as a workaround, I'm having the consumer of the API call NewParser and passing through the *arg.Parser type. I'll publish my full code as soon as I get it to a workable place.

Full example to show how I hit this is below:

package main

import (
    "fmt"
    "os"

    "github.com/alexflint/go-arg"
)

var args struct {
    Argv0 string `arg:"positional"`

    Iter  int
    Debug *bool `arg:"-d" arg:"--debug" help:"turn on debug mode"`

    SomeInt *int `arg:"-i" arg:"--int" help:"some integer"`
}

func main() {
    fmt.Printf("hello\n")

    var st interface{} = args // triggers the issue if it's an interface{}

    //arg.MustParse(&args)
    p, err := arg.NewParser(arg.Config{
    }, &st)
    if err != nil {
        fmt.Printf("FAIL1\n")
        panic(err) // subcommands must be pointers to structs but args is a pointer to interface
    }

    if err := p.Parse(os.Args); err != nil {
        fmt.Printf("FAIL2\n")
        panic(err)
    }

    fmt.Printf("args: %+v\n", args)
    if d := args.Debug; d != nil {
        fmt.Printf("args.Debug: %+v\n", *d)
    }
    if i := args.SomeInt; i != nil {
        fmt.Printf("args.SomeInt: %+v\n", *i)
    }
}

Thanks again for the great library!

alexflint commented 8 months ago

It may work if you do it like this:

var st interface{} = &args
p, err := arg.NewParser(arg.Config{}, st)

The reason it has to be like this is that in your original code, var st interface{} = args actually makes a copy of the args struct. You can confirm this in the following snippet: https://go.dev/play/p/jNeOsi5lAvE. If you pass a copy of a struct in to go-arg, then any values that go-arg populated won't be visible in the original struct. I ran into this a few times and was extremely confused before I figured it out, so I added the explicit check.

purpleidea commented 8 months ago

Oh geez, I'm so tunnel-visioned, thanks for the pointer (pun intended). Yeah that definitely works. Thank you!

(I'm almost done my port, which I'll publish shortly after it's all polished!)