fix: Resolve flags on demand instead of mutating App.Flags, remove NoGlobalFlags

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

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"

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

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 != "" {

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) {