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

pointer-embedded fields are not supported #240

Open purpleidea opened 8 months ago

purpleidea commented 8 months ago

I was doing some lovely refactoring and I was able to take my main tool's config struct, separate it out, decorate it with the arg struct tags, and then embed it both in my main struct and also in a new struct which also included some fields for sub commands.

I then got the following error:

RunArgs.Config: *lib.Config fields are not supported

The struct looks approximately like this:

type RunArgs struct {

    *lib.Config // embedded config XXX: this line breaks things!

    RunEmpty *cliUtil.EmptyArgs `arg:"subcommand:empty" help:"run empty payload"`
    RunLang  *cliUtil.LangArgs  `arg:"subcommand:lang" help:"run lang (mcl) payload"`
}

In the lib package:

type Config struct {
    // Program is the name of this program, usually set at compile time.
    Program string

    // Version is the version of this program, usually set at compile time.
    Version string

    Hostname *string `arg:"--hostname" help:"hostname to use"`

    // ...
}

It seems we don't support the embedded struct? Either this is a "feature we need to add" or there's some gotcha I don't know about. In either case, I'd appreciate your feedback, and would appreciate a patch even more!

purpleidea commented 8 months ago

Playing with this some more, it seems I am noticing that the issue is the *struct{} (pointer to a struct).

I assume this is because the parser wouldn't have anywhere to "put" the data...

I suppose I can workaround this by leaving out the pointer... Although I guess creating a struct there would be convenient too if it's possible.

LMK, and in the worst case scenario I can create a docs patch for this if you'd like one.

Cheers

purpleidea commented 8 months ago

PS: For anyone following at home, I've updated my WIP branch which is here: https://github.com/purpleidea/mgmt/tree/feat/go-arg

The interesting commit is here: https://github.com/purpleidea/mgmt/commit/9527d0dcbdfc9919a0fd5db1440c95b2128dd231 lib, cli: Move cli struct tags and embed this datastructure (this name, and sha1 is valid unless I rebase or merge)

alexflint commented 8 months ago

Yeah, it's as you say - the library supports directly-embedded structs but not pointer-embedded structs.

In order to support pointer-embedded structs, we would have to decide under what conditions to allocate and initialize the pointer. We might do that always no matter what, or only if an option is provided that should go in to the pointer-embedded struct, or perhaps whenever the pointer-embedded struct contains fields that the library recognizes as command line arguments.

I'd be open to a PR that just always initializes pointer-embedded structs.