From 97b8b5707cdbd8f51357bc846eb85f7887226504 Mon Sep 17 00:00:00 2001 From: Taran Pelkey Date: Fri, 28 Aug 2026 19:20:59 -0500 Subject: [PATCH] Resolve flags on demand instead of mutating App.Flags App.Setup() appended GlobalFlags, HelpFlag and VersionFlag onto App.Flags, and Command.Run() likewise reassigned c.Flags. Mutating the user's own slice made the effective flag set depend on whether Setup() had run, and made repeated runs of the same App accumulate flags. Introduce App.resolveFlags() and reuse Command.resolveFlags() to compute the effective set at the point of use, leaving App.Flags and Command.Flags as the caller wrote them. Also removes Command.NoGlobalFlags: global flags are now resolved uniformly for every command. Note that the help output now lists --version before the global flags and --help last, matching the new resolution order; the example tests are updated accordingly. --- app.go | 47 ++++++++++++++++++++--------------------- app_test.go | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++-- command.go | 28 ++++++++----------------- help.go | 5 +---- help_test.go | 23 -------------------- 5 files changed, 90 insertions(+), 72 deletions(-) diff --git a/app.go b/app.go index 6ebfa19adb..1341281c2c 100644 --- a/app.go +++ b/app.go @@ -6,6 +6,7 @@ import ( "io/ioutil" "os" "path/filepath" + "slices" "sort" "time" ) @@ -46,6 +47,9 @@ type App struct { HideVersion bool // Populate on app startup, only gettable through method Categories() categories CommandCategories + // Set when the app is run via RunAsSubcommand; the version flag belongs to + // the top-level app only + runningAsSubcommand bool // An action to execute when the bash-completion flag is set BashComplete BashCompleteFunc // An action to execute before any subcommands are run, but after the context is ready @@ -75,7 +79,7 @@ type App struct { // GlobalHideHelpCommand hides the help command for the app, all commands, and all subcommands GlobalHideHelpCommand bool // GlobalFlags are flags that can be used by the app, any command, or any - // subcommand, unless command.NoGlobalFlags is true + // subcommand GlobalFlags []Flag // Execute this function if the proper command cannot be found @@ -160,22 +164,10 @@ func (a *App) Setup() { } a.Commands = newCmds - // make the app-wide GlobalFlags usable by the app itself - for _, fl := range a.GlobalFlags { - a.appendFlag(fl) - } - if a.Command(helpCommand.Name) == nil { if !a.hideHelpCommand() { a.Commands = append(a.Commands, helpCommand) } - if !a.hideHelp() && (HelpFlag != BoolFlag{}) { - a.appendFlag(HelpFlag) - } - } - - if !a.HideVersion { - a.appendFlag(VersionFlag) } a.categories = CommandCategories{} @@ -207,14 +199,15 @@ func (a *App) Run(arguments []string) (err error) { shellComplete, arguments := checkShellCompleteFlag(a, arguments) // parse flags - set, err := flagSet(a.Name, a.Flags) + flags := a.resolveFlags() + set, err := flagSet(a.Name, flags) if err != nil { return err } set.SetOutput(ioutil.Discard) err = set.Parse(arguments[1:]) - nerr := normalizeFlags(a.Flags, set) + nerr := normalizeFlags(flags, set) context := NewContext(a, set, nil) if nerr != nil { fmt.Fprintln(a.Writer, nerr) @@ -305,9 +298,6 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { if !a.hideHelpCommand() { a.Commands = append(a.Commands, helpCommand) } - if !a.hideHelp() && (HelpFlag != BoolFlag{}) { - a.appendFlag(HelpFlag) - } } } @@ -320,15 +310,18 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { } a.Commands = newCmds + a.runningAsSubcommand = true + // parse flags - set, err := flagSet(a.Name, a.Flags) + flags := a.resolveFlags() + set, err := flagSet(a.Name, flags) if err != nil { return err } set.SetOutput(ioutil.Discard) err = set.Parse(ctx.Args().Tail()) - nerr := normalizeFlags(a.Flags, set) + nerr := normalizeFlags(flags, set) context := NewContext(a, set, ctx) if nerr != nil { @@ -448,7 +441,7 @@ func (a *App) VisibleCommands() []Command { // VisibleFlags returns a slice of the Flags with Hidden=false func (a *App) VisibleFlags() []Flag { - return visibleFlags(a.Flags) + return visibleFlags(a.resolveFlags()) } func (a *App) hasFlag(flag Flag) bool { @@ -470,10 +463,16 @@ func (a *App) errWriter() io.Writer { return a.ErrWriter } -func (a *App) appendFlag(flag Flag) { - if !a.hasFlag(flag) { - a.Flags = append(a.Flags, flag) +func (a *App) resolveFlags() []Flag { + flags := slices.Clone(a.Flags) + if !a.runningAsSubcommand && !a.HideVersion { + flags = append(flags, VersionFlag) + } + flags = append(flags, a.GlobalFlags...) + if !a.hideHelp() && (HelpFlag != BoolFlag{}) { + flags = append(flags, HelpFlag) } + return flags } // hideHelp reports whether the built-in help flag should be hidden for this diff --git a/app_test.go b/app_test.go index b38927d87a..0da939c1a0 100644 --- a/app_test.go +++ b/app_test.go @@ -141,8 +141,8 @@ func ExampleApp_Run_appHelp() { // // GLOBAL FLAGS: // --name value a name to say (default: "bob") - // --help, -h show help // --version, -v print the version + // --help, -h show help } func ExampleApp_Run_commandHelp() { @@ -197,8 +197,8 @@ func ExampleApp_Run_noAction() { // help, h Shows a list of commands or help for one command // // GLOBAL FLAGS: - // --help, -h show help // --version, -v print the version + // --help, -h show help } func ExampleApp_Run_subcommandNoAction() { @@ -1259,6 +1259,61 @@ func TestApp_Run_Version(t *testing.T) { } } +func TestApp_Run_SubcommandHasNoVersionFlag(t *testing.T) { + buf := new(bytes.Buffer) + + app := NewApp() + app.Name = "boom" + app.Version = "0.1.0" + app.Writer = buf + app.HelpWriter = buf + app.Commands = []Command{{ + Name: "foo", + Subcommands: []Command{{Name: "bar"}}, + }} + + if err := app.Run([]string{"boom", "foo", "--help"}); err != nil { + t.Error(err) + } + + // The version flag belongs to the top-level app only, so the subcommand + // help must not advertise a flag the subcommand cannot parse. + if output := buf.String(); strings.Contains(output, "--version") { + t.Errorf("want subcommand help to omit --version, got: \n%q", output) + } + + if err := app.Run([]string{"boom", "foo", "--version"}); err == nil { + t.Error("want --version to be rejected by the subcommand, got no error") + } +} + +func TestApp_Run_VersionFlagAfterSubcommand(t *testing.T) { + buf := new(bytes.Buffer) + + app := NewApp() + app.Name = "boom" + app.Version = "0.1.0" + app.Writer = buf + app.HelpWriter = buf + app.Commands = []Command{{ + Name: "foo", + Subcommands: []Command{{Name: "bar"}}, + }} + + if err := app.Run([]string{"boom", "foo", "bar"}); err != nil { + t.Error(err) + } + + buf.Reset() + if err := app.Run([]string{"boom", "--version"}); err != nil { + t.Error(err) + } + + if output := buf.String(); !strings.Contains(output, "0.1.0") { + t.Errorf("want version to contain %q, did not: \n%q", "0.1.0", output) + } +} + func TestApp_Run_Categories(t *testing.T) { app := NewApp() app.Name = "categories" diff --git a/command.go b/command.go index 3bf7a0452d..74a27fc056 100644 --- a/command.go +++ b/command.go @@ -43,8 +43,6 @@ type Command struct { Subcommands Commands // List of flags to parse Flags []Flag - // Do not append the app-wide App.GlobalFlags to this command's flags - NoGlobalFlags bool // Treat all flags as normal arguments if true SkipFlagParsing bool // Skip argument reordering which attempts to move flags before arguments, @@ -121,18 +119,8 @@ func (c Command) Run(ctx *Context) (err error) { return c.startApp(ctx) } - // combine the command flags with any app-wide GlobalFlags - c.Flags = c.resolveFlags(ctx) - - if !c.hideHelp(ctx) && (HelpFlag != BoolFlag{}) { - // append help to flags - c.Flags = append( - c.Flags, - HelpFlag, - ) - } - - set, err := flagSet(c.Name, c.Flags) + flags := c.resolveFlags(ctx) + set, err := flagSet(c.Name, flags) if err != nil { return err } @@ -190,7 +178,7 @@ func (c Command) Run(ctx *Context) (err error) { err = set.Parse(ctx.Args().Tail()) } - nerr := normalizeFlags(c.Flags, set) + nerr := normalizeFlags(flags, set) if nerr != nil { fmt.Fprintln(ctx.App.Writer, nerr) fmt.Fprintln(ctx.App.Writer) @@ -308,7 +296,7 @@ func (c Command) startApp(ctx *Context) error { // set the flags and commands app.Commands = c.Subcommands - app.Flags = c.resolveFlags(ctx) + app.Flags = c.Flags app.HideHelp = c.hideHelp(ctx) app.HideHelpCommand = c.hideHelpCommand(ctx) @@ -396,10 +384,12 @@ func (c Command) resolveAction(ctx *Context) ActionFunc { } func (c Command) resolveFlags(ctx *Context) []Flag { - if c.NoGlobalFlags { - return c.Flags + flags := slices.Concat(c.Flags, ctx.App.GlobalFlags) + if !c.hideHelp(ctx) && (HelpFlag != BoolFlag{}) { + // append help to flags + flags = append(flags, HelpFlag) } - return slices.Concat(c.Flags, ctx.App.GlobalFlags) + return flags } // VisibleFlags returns a slice of the Flags with Hidden=false diff --git a/help.go b/help.go index 2b1c041e80..ce0c431156 100644 --- a/help.go +++ b/help.go @@ -198,10 +198,7 @@ func ShowCommandHelp(ctx *Context, command string) error { c.EnableHistory = defaultEnableHistory } - // expose global flags to the help template, honoring NoGlobalFlags - if !c.NoGlobalFlags { - c.globalFlags = ctx.App.GlobalFlags - } + c.globalFlags = ctx.App.GlobalFlags if c.HasName(command) { if c.CustomHelpTemplate != "" { diff --git a/help_test.go b/help_test.go index 84d2709cd2..2104f53103 100644 --- a/help_test.go +++ b/help_test.go @@ -342,29 +342,6 @@ FLAGS: t.Errorf("expected output to exclude hidden global flag; got: %q", output.String()) } }) - - t.Run("excluded when NoGlobalFlags", func(t *testing.T) { - app := &App{ - GlobalFlags: globalFlags, - Commands: []Command{ - { - Name: "frobbly", - HelpName: "foo frobbly", - Action: func(*Context) error { return nil }, - NoGlobalFlags: true, - CustomHelpTemplate: customTemplate, - }, - }, - } - - output := &bytes.Buffer{} - app.HelpWriter = output - app.Run([]string{"foo", "help", "frobbly"}) - - if strings.Contains(output.String(), "--config") { - t.Errorf("expected output to exclude global flag --config; got: %q", output.String()) - } - }) } func TestShowAppHelp_HiddenCommand(t *testing.T) {