Merge pull request #1499 from dearchap/issue_557

Fix help results inconsistency

Author: dearchap

app_test.go

@@ -177,10 +177,13 @@ func ExampleApp_Run_commandHelp() {
 	//    greet describeit - use it to see a description
 	//
 	// USAGE:
-	//    greet describeit [arguments...]
+	//    greet describeit [command options] [arguments...]
 	//
 	// DESCRIPTION:
 	//    This is how we describe describeit the function
+	//
+	// OPTIONS:
+	//    --help, -h  show help (default: false)
 }
 
 func ExampleApp_Run_noAction() {

command.go

@@ -170,7 +170,7 @@ func (c *Command) Run(ctx *Context) (err error) {
 	}
 
 	if c.Action == nil {
-		c.Action = helpSubcommand.Action
+		c.Action = helpCommand.Action
 	}
 
 	cCtx.Command = c
@@ -284,7 +284,7 @@ func (c *Command) startApp(ctx *Context) error {
 	if c.Action != nil {
 		app.Action = c.Action
 	} else {
-		app.Action = helpSubcommand.Action
+		app.Action = helpCommand.Action
 	}
 	app.OnUsageError = c.OnUsageError
 
@@ -298,7 +298,12 @@ func (c *Command) startApp(ctx *Context) error {
 // VisibleFlagCategories returns a slice containing all the visible flag categories with the flags they contain
 func (c *Command) VisibleFlagCategories() []VisibleFlagCategory {
 	if c.flagCategories == nil {
-		return []VisibleFlagCategory{}
+		c.flagCategories = newFlagCategories()
+		for _, fl := range c.Flags {
+			if cf, ok := fl.(CategorizableFlag); ok {
+				c.flagCategories.AddFlag(cf.GetCategory(), cf)
+			}
+		}
 	}
 	return c.flagCategories.VisibleCategories()
 }

godoc-current.txt

@@ -82,12 +82,10 @@ DESCRIPTION:
 
 OPTIONS:{{range .VisibleFlagCategories}}
    {{if .Name}}{{.Name}}
-   {{end}}{{range .Flags}}{{.}}
-   {{end}}{{end}}{{else}}{{if .VisibleFlags}}
+   {{end}}{{range .Flags}}{{.}}{{end}}{{end}}{{else}}{{if .VisibleFlags}}
 
 OPTIONS:
-   {{range .VisibleFlags}}{{.}}
-   {{end}}{{end}}{{end}}
+   {{range .VisibleFlags}}{{.}}{{end}}{{end}}{{end}}
 `
     CommandHelpTemplate is the text template for the command help topic. cli.go
     uses text/template to render templates. You can render custom help text by
@@ -161,8 +159,7 @@ COMMANDS:{{range .VisibleCategories}}{{if .Name}}
      {{$s := join .Names ", "}}{{$s}}{{ $sp := subtract $cv (offset $s 3) }}{{ indent $sp ""}}{{wrap .Usage $cv}}{{end}}{{end}}{{end}}{{if .VisibleFlags}}
 
 OPTIONS:
-   {{range .VisibleFlags}}{{.}}
-   {{end}}{{end}}
+   {{range .VisibleFlags}}{{.}}{{end}}{{end}}
 `
     SubcommandHelpTemplate is the text template for the subcommand help topic.
     cli.go uses text/template to render templates. You can render custom help
@@ -568,6 +565,7 @@ type Command struct {
 	// cli.go uses text/template to render templates. You can
 	// render custom help text by setting this variable.
 	CustomHelpTemplate string
+
 	// Has unexported fields.
 }
     Command is a subcommand for a cli.App.
@@ -586,6 +584,13 @@ func (c *Command) Run(ctx *Context) (err error)
     Run invokes the command given the context, parses ctx.Args() to generate
     command-specific flags
 
+func (c *Command) VisibleCategories() []CommandCategory
+    VisibleCategories returns a slice of categories and commands that are
+    Hidden=false
+
+func (c *Command) VisibleCommands() []*Command
+    VisibleCommands returns a slice of the Commands with Hidden=false
+
 func (c *Command) VisibleFlagCategories() []VisibleFlagCategory
     VisibleFlagCategories returns a slice containing all the visible flag
     categories with the flags they contain

help.go

@@ -15,33 +15,59 @@ const (
 	helpAlias = "h"
 )
 
-var helpCommand = &Command{
+// this instance is to avoid recursion in the ShowCommandHelp which can
+// add a help command again
+var helpCommandDontUse = &Command{
 	Name:      helpName,
 	Aliases:   []string{helpAlias},
 	Usage:     "Shows a list of commands or help for one command",
 	ArgsUsage: "[command]",
-	Action: func(cCtx *Context) error {
-		args := cCtx.Args()
-		if args.Present() {
-			return ShowCommandHelp(cCtx, args.First())
-		}
-
-		_ = ShowAppHelp(cCtx)
-		return nil
-	},
 }
 
-var helpSubcommand = &Command{
+var helpCommand = &Command{
 	Name:      helpName,
 	Aliases:   []string{helpAlias},
 	Usage:     "Shows a list of commands or help for one command",
 	ArgsUsage: "[command]",
 	Action: func(cCtx *Context) error {
 		args := cCtx.Args()
-		if args.Present() {
-			return ShowCommandHelp(cCtx, args.First())
+		argsPresent := args.First() != ""
+		firstArg := args.First()
+
+		// This action can be triggered by a "default" action of a command
+		// or via cmd.Run when cmd == helpCmd. So we have following possibilities
+		//
+		// 1 $ app
+		// 2 $ app help
+		// 3 $ app foo
+		// 4 $ app help foo
+		// 5 $ app foo help
+
+		// Case 4. when executing a help command set the context to parent
+		// to allow resolution of subsequent args. This will transform
+		// $ app help foo
+		//     to
+		// $ app foo
+		// which will then be handled as case 3
+		if cCtx.Command.Name == helpName || cCtx.Command.Name == helpAlias {
+			cCtx = cCtx.parentContext
 		}
 
+		// Case 4. $ app hello foo
+		// foo is the command for which help needs to be shown
+		if argsPresent {
+			return ShowCommandHelp(cCtx, firstArg)
+		}
+
+		// Case 1 & 2
+		// Special case when running help on main app itself as opposed to indivdual
+		// commands/subcommands
+		if cCtx.parentContext.App == nil {
+			_ = ShowAppHelp(cCtx)
+			return nil
+		}
+
+		// Case 3, 5
 		return ShowSubcommandHelp(cCtx)
 	},
 }
@@ -212,9 +238,19 @@ func ShowCommandHelp(ctx *Context, command string) error {
 
 	for _, c := range ctx.App.Commands {
 		if c.HasName(command) {
+			if !ctx.App.HideHelpCommand && !c.HasName(helpName) && len(c.Subcommands) != 0 {
+				c.Subcommands = append(c.Subcommands, helpCommandDontUse)
+			}
+			if !ctx.App.HideHelp && HelpFlag != nil {
+				c.appendFlag(HelpFlag)
+			}
 			templ := c.CustomHelpTemplate
 			if templ == "" {
-				templ = CommandHelpTemplate
+				if len(c.Subcommands) == 0 {
+					templ = CommandHelpTemplate
+				} else {
+					templ = SubcommandHelpTemplate
+				}
 			}
 
 			HelpPrinter(ctx.App.Writer, templ, c)

help_test.go

@@ -186,7 +186,7 @@ func Test_helpSubcommand_Action_ErrorIfNoTopic(t *testing.T) {
 
 	c := NewContext(app, set, nil)
 
-	err := helpSubcommand.Action(c)
+	err := helpCommand.Action(c)
 
 	if err == nil {
 		t.Fatalf("expected error from helpCommand.Action(), but got nil")
@@ -248,7 +248,7 @@ func TestShowCommandHelp_HelpPrinter(t *testing.T) {
 				fmt.Fprint(w, "yo")
 			},
 			command:      "",
-			wantTemplate: SubcommandHelpTemplate,
+			wantTemplate: AppHelpTemplate,
 			wantOutput:   "yo",
 		},
 		{
@@ -333,7 +333,7 @@ func TestShowCommandHelp_HelpPrinterCustom(t *testing.T) {
 				fmt.Fprint(w, "yo")
 			},
 			command:      "",
-			wantTemplate: SubcommandHelpTemplate,
+			wantTemplate: AppHelpTemplate,
 			wantOutput:   "yo",
 		},
 		{
@@ -1357,10 +1357,13 @@ DESCRIPTION:
    and a description long
    enough to wrap in this test
    case
+
+OPTIONS:
+   --help, -h  show help (default: false)
 `
 
 	if output.String() != expected {
-		t.Errorf("Unexpected wrapping, got:\n%s\nexpected: %s",
+		t.Errorf("Unexpected wrapping, got:\n%s\nexpected:\n%s",
 			output.String(), expected)
 	}
 }
@@ -1426,7 +1429,6 @@ USAGE:
 
 OPTIONS:
    --help, -h  show help (default: false)
-   
 `
 
 	if output.String() != expected {

template.go

@@ -54,12 +54,10 @@ DESCRIPTION:
 
 OPTIONS:{{range .VisibleFlagCategories}}
    {{if .Name}}{{.Name}}
-   {{end}}{{range .Flags}}{{.}}
-   {{end}}{{end}}{{else}}{{if .VisibleFlags}}
+   {{end}}{{range .Flags}}{{.}}{{end}}{{end}}{{else}}{{if .VisibleFlags}}
 
 OPTIONS:
-   {{range .VisibleFlags}}{{.}}
-   {{end}}{{end}}{{end}}
+   {{range .VisibleFlags}}{{.}}{{end}}{{end}}{{end}}
 `
 
 // SubcommandHelpTemplate is the text template for the subcommand help topic.
@@ -80,8 +78,7 @@ COMMANDS:{{range .VisibleCategories}}{{if .Name}}
      {{$s := join .Names ", "}}{{$s}}{{ $sp := subtract $cv (offset $s 3) }}{{ indent $sp ""}}{{wrap .Usage $cv}}{{end}}{{end}}{{end}}{{if .VisibleFlags}}
 
 OPTIONS:
-   {{range .VisibleFlags}}{{.}}
-   {{end}}{{end}}
+   {{range .VisibleFlags}}{{.}}{{end}}{{end}}
 `
 
 var MarkdownDocTemplate = `{{if gt .SectionNum 0}}% {{ .App.Name }} {{ .SectionNum }}

testdata/godoc-v2.x.txt

@@ -82,12 +82,10 @@ DESCRIPTION:
 
 OPTIONS:{{range .VisibleFlagCategories}}
    {{if .Name}}{{.Name}}
-   {{end}}{{range .Flags}}{{.}}
-   {{end}}{{end}}{{else}}{{if .VisibleFlags}}
+   {{end}}{{range .Flags}}{{.}}{{end}}{{end}}{{else}}{{if .VisibleFlags}}
 
 OPTIONS:
-   {{range .VisibleFlags}}{{.}}
-   {{end}}{{end}}{{end}}
+   {{range .VisibleFlags}}{{.}}{{end}}{{end}}{{end}}
 `
     CommandHelpTemplate is the text template for the command help topic. cli.go
     uses text/template to render templates. You can render custom help text by
@@ -161,8 +159,7 @@ COMMANDS:{{range .VisibleCategories}}{{if .Name}}
      {{$s := join .Names ", "}}{{$s}}{{ $sp := subtract $cv (offset $s 3) }}{{ indent $sp ""}}{{wrap .Usage $cv}}{{end}}{{end}}{{end}}{{if .VisibleFlags}}
 
 OPTIONS:
-   {{range .VisibleFlags}}{{.}}
-   {{end}}{{end}}
+   {{range .VisibleFlags}}{{.}}{{end}}{{end}}
 `
     SubcommandHelpTemplate is the text template for the subcommand help topic.
     cli.go uses text/template to render templates. You can render custom help
@@ -568,6 +565,7 @@ type Command struct {
 	// cli.go uses text/template to render templates. You can
 	// render custom help text by setting this variable.
 	CustomHelpTemplate string
+
 	// Has unexported fields.
 }
     Command is a subcommand for a cli.App.
@@ -586,6 +584,13 @@ func (c *Command) Run(ctx *Context) (err error)
     Run invokes the command given the context, parses ctx.Args() to generate
     command-specific flags
 
+func (c *Command) VisibleCategories() []CommandCategory
+    VisibleCategories returns a slice of categories and commands that are
+    Hidden=false
+
+func (c *Command) VisibleCommands() []*Command
+    VisibleCommands returns a slice of the Commands with Hidden=false
+
 func (c *Command) VisibleFlagCategories() []VisibleFlagCategory
     VisibleFlagCategories returns a slice containing all the visible flag
     categories with the flags they contain