Hook ignored on nodes tagged with "embed"

[mkmik]

In the following example I created a struct called Common with an AfterApply hook.
When this structure is embedded using Go struct embedding, the AfterApply hook gets invoked as per Go method discovery rules.

However, if the struct is embedded using the kong embedding (via the embed tag), the hook is not invoked.

I think that would be useful since this allows multiple structures with common flags to be embedded and have each of their hooks invoked. This is currently not possible with Go embedding since it doesn't work if more than one structure (including the containing struct) defines the hook method.

package main

import (
	"fmt"

	"github.com/alecthomas/kong"
)

type Common struct {
	Foo bool `help:"FOO"`
}

func (*Common) AfterApply() error {
	fmt.Printf("called Common.AfterApply\n")
	return nil
}

var CLI struct {
	Rm struct {
		Common

		Force     bool `help:"Force removal."`
		Recursive bool `help:"Recursively remove files."`

		Paths []string `arg name:"path" help:"Paths to remove." type:"path"`
	} `cmd help:"Remove files."`

	Ls struct {
		C Common `embed:""`

		Paths []string `arg optional name:"path" help:"Paths to list." type:"path"`
	} `cmd help:"List paths."`
}

func main() {
	ctx := kong.Parse(&CLI)
	switch ctx.Command() {
	case "rm <path>":
	case "ls":
	default:
		panic(ctx.Command())
	}
}

[alecthomas]

I'm not sure about the semantics or usefulness of this. What are the use cases exactly?

Regarding semantics, normally for hooks, a kong.Context is passed through for each matching node, eg. flag, command or arg. But for an embedded container there is no such node. It could pass each node that the container owns, but then it would be called multiple times. Alternatively it could be called once with the first one, but that would be confusing.

The final option is to call the embed callbacks with no context at all, which would work but is it useful?

[mkmik]

Perhaps I'm holding it wrong. This is my use case:

I have flag that takes a file path. I want this flag to default to foo.txt if the file foo.txt exists, otherwise I want it to default to "".

I want this flag to be shared by multiple commands and I don't want each command to do extra work to deal with this flag otherwise I'll forget to call this logic eventually in some command.

[alecthomas]

Ah. In this case I think you want to create your own flag type and implement one of the hook methods on it. eg.

type MaybeFile string

func (m *MaybeFile) BeforeApply(...) error {
  // ...
}

[alecthomas]

FYI there is also some question over whether the existingfile directive should have this exact behaviour, as in some circumstances it can make it a bit less than useful to require the file exist.

[mkmik]

Yes I know I can use a custom type but I hoped I could not force this custom type to the body of the commands.

[mkmik]

Also, with flag types the logic can only handle one flag at a time

[alecthomas]

Also, with flag types the logic can only handle one flag at a time

Right, but you only referred to a single flag originally.

That said, I can see that it would be useful. I think the simplest solution would be to support hooks but there would be no context directly associated with the embedded struct.

[mkmik]

I don't understand why can't those structs get the context. I guess usually there will be multiple instances of them, since they will be declared as values. Sure, they could be defined as pointers and they could point to the same instance, but I'm not sure why somebody would end up doing that.

[lrstanley]

I think I'm running into the same issue. My usecase might be a bit different, where I'm embedding a struct that has closely-related flags, and I'd like the logic to be at the struct level. For example:

type VersionFlag struct {
	Enabled     bool `short:"v" env:"-" name:"version" help:"prints version information and exits"`
	EnabledJSON bool `name:"version-json" env:"-" help:"prints version information in JSON format and exits"`
}

func (v VersionFlag) BeforeApply([...]) error {
	// [...]
	return nil
}

type CLI struct {
    Foo string `etc`

    Version VersionFlag `embed:""`
}

Ideally, I don't want these flags to be a prefixed parent flag, but I also don't want to have a BeforeApply or similar for each individual flag.

The primary reasoning for structuring it like this is that I have a library I currently use with another cli library, and I'd like to move over to Kong. That library has pluggable functionality, like:

• Auto-version info, incl. pulling build flags, deps, etc.
• Bundled logger with pre-configured flags.
• Flags for generating markdown of the tool usage (flags, env vars, commands, etc).
• Flags for outputting a json-version of the tool usage as well.

Also looked at plugins, but I wasn't able to get plugins to work in the above scenario either (would be very nice just to pass in a plugin that has hooks setup).

[dio]

• Flags for generating markdown of the tool usage (flags, env vars, commands, etc).
• Flags for outputting a json-version of the tool usage as well.

@lrstanley I'm also looking at this, probably a similar approach taken by: https://github.com/alecthomas/mango-kong can be used (?).

[mitar]

It seems this also makes custom validators not run on nested structs: https://go.dev/play/p/ROe5n4bp1AD

I think semantics for validators is clearer than for hooks?

[renom]

Will it be fixed?

[renom]

The Validate() and hooks are not working on the root struct (CLI in the examples above) either.

[artemklevtsov]

Faced with the same issue. I tried to encapsulate some flags with try for reuse and include it with embed tag. Validate for this struct not works.

type TestCmd struct {
	Query QueryFlags `embed:""`
}

type LogQueryFlags struct {
	Source      string
}

func (f *QueryFlags) Validate() error {
	fmt.Println("!!!")
}