Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pointer-embedded fields are not supported #240

Open
purpleidea opened this issue Mar 4, 2024 · 3 comments
Open

pointer-embedded fields are not supported #240

purpleidea opened this issue Mar 4, 2024 · 3 comments

Comments

@purpleidea
Copy link

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
Copy link
Author

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
Copy link
Author

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: purpleidea/mgmt@9527d0d lib, cli: Move cli struct tags and embed this datastructure (this name, and sha1 is valid unless I rebase or merge)

@alexflint
Copy link
Owner

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.

@alexflint alexflint changed the title fields are not supported pointer-embedded fields are not supported Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants