spf13 / cobra

A Commander for modern Go CLI interactions
https://cobra.dev
Apache License 2.0
37.71k stars 2.83k forks source link

Flags don't contain inherited flags #412

Open n10v opened 7 years ago

n10v commented 7 years ago

As in the docs said:

// Flags returns the complete FlagSet that applies
// to this command (local and persistent declared here and by all parents).
func (c *Command) Flags() *flag.FlagSet

But consider this script:

package main

import (
    "fmt"

    "github.com/spf13/cobra"
    "github.com/spf13/pflag"
)

var parent = &cobra.Command{
    Run: func(*cobra.Command, []string) {
    },
}
var child = &cobra.Command{
    Run: func(*cobra.Command, []string) {
    },
}

func init() {
    parent.PersistentFlags().Bool("1st-persistent-parent-flag", true, "")
    parent.PersistentFlags().Bool("2st-persistent-parent-flag", true, "")
    child.Flags().Bool("1st-child-flag", true, "")
    child.Flags().Bool("2st-child-flag", true, "")
    parent.AddCommand(child)

    fmt.Println("child.Flags().VisitAll() before child.LocalFlags():")
    child.Flags().VisitAll(func(f *pflag.Flag) {
        fmt.Println(f.Name)
    })

    child.LocalFlags()

    fmt.Println()
    fmt.Println("child.Flags().VisitAll() after child.LocalFlags():")
    child.Flags().VisitAll(func(f *pflag.Flag) {
        fmt.Println(f.Name)
    })
}

func main() {
    fmt.Println()
    fmt.Printf("parent.Execute(): %v", parent.Execute())
}

Output:

child.Flags().VisitAll() before child.LocalFlags():
1st-child-flag
2st-child-flag

child.Flags().VisitAll() after child.LocalFlags():
1st-child-flag
1st-persistent-parent-flag
2st-child-flag
2st-persistent-parent-flag

parent.Execute(): <nil>

Only after the calling child.LocalFlags() user can see persistent flags in child.Flags(). I think, it's not obvious behaviour.

P.S. Instead of child.LocalFlags() you can use child.InheritedFlags() or another function that calls c.mergePersistentFlags() and you will see the same effect.

github-actions[bot] commented 4 years ago

This issue is being marked as stale due to a long period of inactivity

ghostsquad commented 4 years ago

this seems like a "gotcha" that would be hard to discover/debug.

n10v commented 4 years ago

Maybe just call mergePersistentFlags in VisitAll 🤔

andig commented 1 year ago

It seems the same (if I'm not mistaken) applies for PersistentFlags. A persistent flag should be available on the child commands too, but isn't in my tests.