Add group flags

[knqyf263]

Description

Add group flags

Close https://github.com/spf13/cobra/issues/1327

Example

main.go

package main

import (
	"fmt"

	"github.com/spf13/cobra"
)

func main() {
	var output string
	var rootCmd = &cobra.Command{
		Use:   "root [sub]",
		Short: "My root command",
		Run: func(cmd *cobra.Command, args []string) {
			fmt.Printf("Output: %s\n", output)
		},
	}
	rootCmd.Flags().Int("replicas", 0, "replicas")
	rootCmd.NamedFlags("cache").String("dir", "", "cache dir")
	rootCmd.NamedFlags("cache").Duration("max-age", 0, "cache ttl")
	rootCmd.NamedFlags("result").StringVarP(&output, "output", "o", "", "output format")
	rootCmd.NamedFlags("result").StringP("filename", "f", "", "filename")

	rootCmd.SetArgs([]string{"--output", "dummy.txt"})
	rootCmd.Execute()
	fmt.Println()
	rootCmd.SetArgs([]string{"--help"})
	rootCmd.Execute()
}

$ go run main.go
Output: dummy.txt

My root command

Usage:
  root [sub] [flags]

Flags:
  -h, --help           help for root
      --replicas int   replicas

Cache Flags:
      --dir string         cache dir
      --max-age duration   cache ttl

Result Flags:
  -f, --filename string   filename
  -o, --output string     output format

[CLAassistant]

All committers have signed the CLA.

[knqyf263]

@marckhouzam I don't think this is the best implementation, but I hope this is a good starting point for discussion.

I borrowed an idea of NamedFlagSets from Kubernetes. https://github.com/kubernetes/component-base/blob/b5a495af30a7bb04642ce82f4816b47e75f78dbe/cli/flag/sectioned.go#L33-L41

[github-actions[bot]]

The Cobra project currently lacks enough contributors to adequately respond to all PRs. This bot triages issues and PRs according to the following rules:

• After 60d of inactivity, lifecycle/stale is applied. - After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied and the PR is closed. You can:
• Make a comment to remove the stale label and show your support. The 60 days reset. - If a PR has lifecycle/rotten and is closed, comment and ask maintainers if they'd be interseted in reopening.

[github-actions[bot]]

The Cobra project currently lacks enough contributors to adequately respond to all PRs. This bot triages issues and PRs according to the following rules:

• After 60d of inactivity, lifecycle/stale is applied. - After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied and the PR is closed. You can:
• Make a comment to remove the stale label and show your support. The 60 days reset. - If a PR has lifecycle/rotten and is closed, comment and ask maintainers if they'd be interested in reopening.

[EraYaN]

This would still be a very valuable feature and should not go stale.

[pedromotita]

Hi, is there any updates do this PR status? It's a really nice feature. A lot of projects would benefit from it

[knqyf263]

@pedromotita I wanted to kick off the discussion, but I've not received any response from maintainers.

[eiffel-fl]

Hi.

@pedromotita I wanted to kick off the discussion, but I've not received any response from maintainers.

I am not in maintainers heads, but you can try to publish this PR as ready for review and they may take a look (cost nothing to try by the way).

Best regards.

[marckhouzam]

Thanks @knqyf263 for your patience (and everyone else interested in this), I will start looking at this PR.

[marckhouzam]

There is a PR to support this in spf13/pflag directly: https://github.com/spf13/pflag/pull/339 I'm mentioning this as it can be a reference, however, there has been no movement in that project for years, so I think we should continue adding support in Cobra instead.

[marckhouzam]

@knqyf263 Although I think your API is very elegant, I'm concerned it complicates the code in Cobra and would make maintenance more difficult. I'm worried about creating yet another "type" of flags and then have: local flags, inherited flags and named flags.

The grouping of flags in the help does not affect their actual behaviour. So from a behavioural perspective we still have either local or inherited flags. I would prefer keeping those two groups and adding an independent concept of "flag group" on top of this.

For better or worse we already have an API for command groups (see doc here), so it may be good for users of Cobra to have a similar API for flag groups.

With that in mind, what would you think of an API that looked something like:

var rootCmd = &cobra.Command{
	Use:   "root [sub]",
	Short: "My root command",
	Run: func(cmd *cobra.Command, args []string) {
		fmt.Printf("Root command")
	},
}

// Create flags exactly like is currently done
rootCmd.Flags().Int("replicas", 0, "replicas")
rootCmd.Flags.()String("dir", "", "cache dir")
rootCmd.Flags().Duration("max-age", 0, "cache ttl")
rootCmd.PersistentFlags().StringVarP(&output, "output", "o", "", "output format")
rootCmd.PersistentFlags().StringP("filename", "f", "", "filename")

// Add new, separate knowledge for grouping flags in the help output
err = rootCmd.AddFlagHelpGroup(cobra.Group{ID: "group1", "Title": "cache"}, cobra.Group{ID: "group2", "Title": "result"})
err = rootCmd.AddFlagToHelpGroupID("dir", "group1")
err = rootCmd.AddFlagToHelpGroupID("max-age", "group1")
err = rootCmd.AddFlagToHelpGroupID("output", "group2")
err = rootCmd.AddFlagToHelpGroupID("filename", "group2")

Notice the use of the word "Help" to qualify the flag groups; I think we need something along those lines because Cobra has a concept of flag group for mutual exclusion and such things.

I admit I like your API better, but I am hoping that something along the lines of the above will be safer to implement. And it should handle global flags transparently, I'm hoping.

Hopefully this can start a discussion leading to something we all agree is useful.

[pedromotita]

@knqyf263 any thoughts on @marckhouzam API proposal? He has a fair point

Let us know if you're still willing to work on this PR. I'm new to the community, but would be very happy to help 😄

[pedromotita]

@marckhouzam I've been trying to wrap my head around a solution using your API suggestion. The main problem is that flag.FlagSet has no reference to cobra.Group.

Whereas dealing with Command we can use {{if (and (eq .GroupID $group.ID) ... }} in the template, we can't do that for flags... We could create a map do tie a Group.ID to a flag.FlagSet, but it's not a elegant solution at all...

The more I think about this feature, the more I get convinced part of it should be implemented in spf13/pflag package 😢

[marckhouzam]

I believe flags have an annotation field. Maybe you can use that to associate the group Id?

[pedromotita]

@marckhouzam I had not thought of that. Thanks!

I opened the PR #2117 with your API proposal and some test cases :)