cobra icon indicating copy to clipboard operation
cobra copied to clipboard

When persistent flag is shadowed, help text does not show shadowing flag

Open marckhouzam opened this issue 2 years ago • 4 comments

If I declare a flag --flag on the root command then declare the same local flag on a sub command, the local flag shadows the persistent one. This seems reasonable. However the help command does not respect this and gives priority to persistent flags.

Here is a program to illustrate:

package main

import (
	"fmt"

	"github.com/spf13/cobra"
)

type testOpts struct {
	root string
	sub   string
}

var opt testOpts

var root = &cobra.Command{
	Use: "prog",
	Run: func(cmd *cobra.Command, args []string) {
		fmt.Printf("root called with root flag %s\n", opt.root)
		fmt.Printf("root called with sub flag %s\n", opt.sub)
	},
}

var sub = &cobra.Command{
	Use: "sub",
	Run: func(cmd *cobra.Command, args []string) {
		fmt.Printf("sub called with root flag %s\n", opt.root)
		fmt.Printf("sub called with sub flag %s\n", opt.sub)
	},
}

func main() {
	root.AddCommand(sub)
        // Persistent flag named --flag
	root.PersistentFlags().StringVar(&opt.root, "flag", "defaultroot", "usage for root")
        // Local flag also named --flag: it will shadow the above flag
	sub.Flags().StringVar(&opt.sub, "flag", "defaultsub", "usage for sub")

	root.Execute()
}

First we check what happens to the flag behaviour:

$ go build -o prog overrideArg.go
$ ./prog --flag MARC
root cmd with root flag MARC and sub flag DEFAULTSUB
# here it is the persistent flag that gets set, as expected

$ ./prog sub --flag MARC
sub cmd with root flag DEFAULTROOT and sub flag MARC
# here it is the local shadowing flag that gets set

$ ./prog --flag MARC sub
sub cmd with root flag DEFAULTROOT and sub flag MARC
# here too it is the local shadowing flag that gets set

This seems right: the more specialized flag takes precedence.

I don't know if this is just by chance in how pflag behaves. I wasn't aware this was possible.

The problem now, is that the help does not respect this behaviour:

./prog help
Usage:
  prog [flags]
  prog [command]

Available Commands:
  completion  Generate the autocompletion script for the specified shell
  help        Help about any command
  sub

Flags:
      --flag string   usage for root (default "DEFAULTROOT")
  -h, --help          help for prog

Use "prog [command] --help" for more information about a command.

# As expected, the persistent flag is documented above but...

$ ./prog help sub
Usage:
  prog sub [flags]

Flags:
  -h, --help   help for sub

Global Flags:
      --flag string   usage for root (default "DEFAULTROOT")

# Here the shadowing flag is not shown.  The user therefore does not realize the behaviour may be different than for the global flag

marckhouzam avatar Mar 31 '22 19:03 marckhouzam

This problem was reported in kubectl where they shadow persistent flags (probably by mistake): https://github.com/kubernetes/kubernetes/issues/103769

I've traced the problem to

https://github.com/spf13/cobra/blob/master/command.go#L1502-L1506

where we determine the list of local flags for the help by checking if a flag is not part of the parent. That approach gives priority to persistent flag instead of to local flags.

marckhouzam avatar Mar 31 '22 19:03 marckhouzam

The Cobra project currently lacks enough contributors to adequately respond to all issues. 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 issue is closed. You can:
  • Make a comment to remove the stale label and show your support. The 60 days reset. - If an issue has lifecycle/rotten and is closed, comment and ask maintainers if they'd be interseted in reopening

github-actions[bot] avatar May 31 '22 00:05 github-actions[bot]

Still good

marckhouzam avatar May 31 '22 00:05 marckhouzam

The Cobra project currently lacks enough contributors to adequately respond to all issues. 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 issue is closed. You can:
  • Make a comment to remove the stale label and show your support. The 60 days reset. - If an issue has lifecycle/rotten and is closed, comment and ask maintainers if they'd be interseted in reopening

github-actions[bot] avatar Aug 01 '22 00:08 github-actions[bot]

Still an issue.

eddiezane avatar Aug 17 '22 16:08 eddiezane

/assign

brianpursley avatar Aug 17 '22 19:08 brianpursley