subcommands silently ignore app level flags

[lcowell]

This app (see below) has a limit flag and an info command. If I call the app like this:

./boom --limit 100 info

The limit flag is not set when I call the info action. If I move those flags to the command's definition, then I can call the app like this:

./boom info --limit 100

However, in my real world use case, flags like limit apply to most, if not all the commands.

I think flags declared at the app level, should be available to any command and flags declared at the command level, should only be available to that particular command. I think that if a flag is not accepted, the user should be informed of this.

Thanks for building this awesome tool! Please let me know if there's anything else I can do to help.

Here's the code I'm referring to:

package main

import (
    "fmt"
    "os"

    "github.com/codegangsta/cli"
)

func main() {
    app := cli.NewApp()
    app.Name = "boom"
    app.Usage = "make an explosive entrance"
    app.Flags = []cli.Flag{
        cli.IntFlag{
            Name: "limit, l",
        },
    }
    app.Commands = []cli.Command{
        {
            Name:      "info",
            ShortName: "i",
            Action: func(c *cli.Context) {
                limit := c.Int("limit")
                fmt.Println(limit)
            },
        },
    }

    app.Run(os.Args)
}

[jszwedko]

@lcowell you should be able to access limit via c.GlobalInt("limit"). I admit it is an idiosyncrasy of the API that you need to call a different method to get flags defined at a "higher" level.

[lcowell]

Thanks @jszwedko! This does the trick for me. I think README.md should have some information about global vs. non-global flags because the solution was pretty clear once I had the information.

[wwwdata]

I had this issue too and it confused me a lot. I would also susggest to remove the Global Methods completely and just pass the global options/flags to all sub commands plus any other additional options defined for sub command. You can define more for a sub command right? I didn't find out how to do that via the Readme.

[apriendeau]

I do think that Commands should inherit app flags so that if you do this for example:

./boom --verbose info

this should also work

./boom info --verbose

Its not a matter of access; Its a matter that the all the flags get checked when parsing them.

[pmoust]

I agree as well, even if the maintainers feel differently it should be documented in the README that the accessor is c.GlobalWhatever("name") for subcommands.

[jgautheron]

Indeed it's really not clear, thought initially that it was a bug, came here looking for an existing issue.
👍 for adding it in the README.

[dropwhile]

If maintainers wish to support this (parent flags always available to child contexts), then I think this could be pretty easily accomplished by adding a method similar to this...

// lookup flags via post-order walking (child first, then parent)
// returning the first match, or nil if no match
func lookupPostOrderFlagSet(name string, ctx *Context) *flag.FlagSet {
    if f := ctx.flagSet.Lookup(name); f != nil {
        return ctx.flagSet
    }
    return lookupGlobalFlagSet(name, ctx)
}

and then changing the non-global lookups to use the above function in a similar fashion to how the global* lookups current work. Ex:

func (c *Context) IntSlice(name string) []int {
    if fs := lookupPostOrderFlagSet(name, c); fs != nil {
        return lookupIntSlice(name, fs)
    }
    return nil
}

Another option would be to just add an additional set of lookup methods that behave this way:

// Looks up the value of a local int slice flag, if no int slice flag exists
// then parent contexts are checked recursively for the int slice flag.
// returns nil if no flag exists locally or in parent contexts
func (c *Context) AnyIntSlice(name string) []int {
    if fs := lookupPostOrderFlagSet(name, c); fs != nil {
        return lookupIntSlice(name, fs)
    }
    return nil
}

[jszwedko]

👍 rolling this into #385, but we'll target the behavior that @cactus describes for 2.0 since it isn't backwards compatible.