ff icon indicating copy to clipboard operation
ff copied to clipboard

Feature request: multi-level flags

Open josharian opened this issue 3 years ago • 13 comments

I often find myself repeating code like this:

var verbose bool

func init() {
	all := []*flag.FlagSet{ /* list of FlagSets for different sub-commands and sub-sub-commands */ }
	for _, fs := range all {
		fs.BoolVar(&verbose, "v", false, "print debug information")
	}
}

That way:

$ cmd -v sub sub1
$ cmd sub -v sub1
$ cmd sub sub1 -v

all do the same thing. (And it's not just -v. I find it frustrating as a user to have to figure out at which level a flag goes, so I generally want to be able to put them anywhere.)

I'd like it if ff made this easier to accomplish.

I'm not sure what the API would look like but it'd be nice if there was a way to say "these flags can be set by any subcommand of this command".

Or maybe always do that and complain if there are any collisions? (That will also help avoid confusing UX in clients for which some flag means different things depending on where on the command line you add it.)

josharian avatar Feb 24 '21 19:02 josharian

ff or ffcli?

peterbourgon avatar Feb 24 '21 20:02 peterbourgon

Sorry, ffcli.

josharian avatar Feb 24 '21 20:02 josharian

Right. So, the approach is basically a visitor pattern — a function which accepts a *flag.FlagSet and registers the global flags within it. It's demonstrated in the objectctl example. RegisterFlags is the visitor

// RegisterFlags registers the flag fields into the provided flag.FlagSet. This
// helper function allows subcommands to register the root flags into their
// flagsets, creating "global" flags that can be passed after any subcommand at
// the commandline.
func (c *Config) RegisterFlags(fs *flag.FlagSet) {
	fs.StringVar(&c.Token, "token", "", "secret token for object API")
	fs.BoolVar(&c.Verbose, "v", false, "log verbose output")
}

which is invoked on the e.g. create subcommand like this

	fs := flag.NewFlagSet("objectctl create", flag.ExitOnError)
	fs.BoolVar(&cfg.overwrite, "overwrite", false, "overwrite existing object, if it exists")
	rootConfig.RegisterFlags(fs)

	return &ffcli.Command{
		Name:       "create",
		ShortUsage: "objectctl create [flags] <key> <value data...>",
		ShortHelp:  "Create or overwrite an object",
		FlagSet:    fs,
		Exec:       cfg.Exec,
	}

and makes the global flag values accessible like this

// Exec function for this command.
func (c *Config) Exec(ctx context.Context, args []string) error {

	. . .

	if c.rootConfig.Verbose {
		fmt.Fprintf(c.out, "create %q OK\n", key)
	}

Does this serve the purpose for you?

peterbourgon avatar Feb 24 '21 20:02 peterbourgon

Let me try migrating to v3 and playing with it a bit. One of the things I appreciate about ffcli is that most of the setup is declarative (I use top level vars). It looks like this'll require more glue code, like my init statement approach above, which I was hoping to avoid. I was rather drawn to automatic propagation. But I'll see how that approach goes, thanks.

josharian avatar Feb 27 '21 20:02 josharian

This or something like it is useful in ~every ffcli CLI. I'd love to find a way to make it easier — without expanding the surface area of ffcli.Command, which is already too wide. I wonder if a helper function or type could do the job.

peterbourgon avatar Feb 28 '21 23:02 peterbourgon

@josharian Where'd you land on this one? What could I have given you to make it better?

peterbourgon avatar Apr 07 '21 06:04 peterbourgon

I got distracted. :( I still suspect I’d like automatic propagation best, but I haven’t spent any time digging or thinking about it yet.

josharian avatar Apr 07 '21 18:04 josharian

Concretely, automatic propagation would mean defining or marking specific flags in a Command's FlagSet such that they're automatically registered in the FlagSets of all Subcommands, transitively?

peterbourgon avatar Apr 07 '21 19:04 peterbourgon

Yes. Or not even requiring marking, and just doing it.

Note that one way to implement is to try all flag sets of all ancestors, as opposed to registering additional flags. I believe that the only user-visible difference is whether -h shows all flags, including ancestor flags, or just flags specific to that subcommand. I lean weakly towards just subcommand-specific flags. The other advantage to this implementation approach is that it makes it clear what happens when an ancestor and a subcommand both define the same flag: the subcommand takes precedence. (The other reasonable approach is failure.)

josharian avatar Apr 07 '21 20:04 josharian

Understood. I think it would be surprising if this behavior was the default, but it may make sense as something to opt-in to.

peterbourgon avatar Apr 07 '21 21:04 peterbourgon

This is a solid idea and if one could explicitly mark a flag as "inheritable" then it would simplify a lot of the boilerplate and glue code.

An effective way to surface this to users may be to display 2 flag sections:

USAGE
  gh run <command> [flags]

AVAILABLE COMMANDS
  cancel:      Cancel a workflow run
  ...

FLAGS
  -R, --repo [HOST/]OWNER/REPO   Select another repository using the [HOST/]OWNER/REPO format

INHERITED FLAGS
  --help   Show help for command

Example from gh CLI using cobra.

mfridman avatar Apr 07 '23 03:04 mfridman

One way might be to have a Command accept more than one FlagSet.

peterbourgon avatar Apr 08 '23 19:04 peterbourgon

I hope this will be addressed by #113.

peterbourgon avatar Jul 20 '23 23:07 peterbourgon