Fix help used as command returns errors when required flags are missing

[jaimem88]

What type of PR is this?

• bug

What this PR does / why we need it:

Using app help or app h without providing required flags causes app.Run() to return an errRequiredFlags error. This does not happend if using -help or -h.

Added a function to check if any of the arguments matches HelpFlag.Names() to stop early and show the app help.

Which issue(s) this PR fixes:

Fixes https://github.com/urfave/cli/issues/1247

Special notes for your reviewer:

I'm new to the code base so perhaps there is an easier way to check if the command matches help or h before calling checkRequiredFlags. However, context.Command.Name is empty just before checkRequiredFlags so I couldn't just check that first.

Testing

Added test Test_Help_Required_Flags_Does_Not_Error to help_test.go

Release Notes

(REQUIRED)

Fixed https://github.com/urfave/cli/issues/1247

[codefromthecrypt]

will be great to see this landed as it is surprising behavior without a fix. @jaimem88 let me know if you ran out of steam!

[codefromthecrypt]

I think this also hits when help is default. Ex. if your app is named "foo", foo and foo help will trigger the unnecessarily arg validation

[dearchap]

Issue not present in latest releases. Closing.

[dearchap]

I dont think this is a trivial fix. Consider the following code with urfave:main(without this PR fix)

package main

import (
	"fmt"
	"os"

	"github.com/urfave/cli/v2"
)

func printer(s, action string) error {
	fmt.Printf("%s %s\n", s, action)
	return nil
}

func main() {
	cli := &cli.App{
		Name: "test",
		Flags: []cli.Flag{
			&cli.StringFlag{
				Name:     "flag1",
				Required: true,
			},
			&cli.StringFlag{
				Name:     "flag2",
				Required: true,
			},
		},
		Commands: []*cli.Command{
			{
				Name:  "list",
				Usage: "list all clusters",
				Before: func(ctx *cli.Context) error {
					return printer("list", "before")
				},
				Flags: []cli.Flag{
					&cli.StringFlag{
						Name:     "filter",
						Required: true,
					},
				},
				After: func(ctx *cli.Context) error {
					return printer("list", "after")
				},
				Action: func(_ *cli.Context) error {
					return printer("list", "action")
				},
				Subcommands: []*cli.Command{
					{
						Name:  "subcommand1",
						Usage: "Some subcommand1",
						Before: func(ctx *cli.Context) error {
							return printer("subc1", "before")
						},
						After: func(ctx *cli.Context) error {
							return printer("subc1", "after")
						},
						Action: func(_ *cli.Context) error {
							return printer("subc1", "action")
						},
					},
				},
			},
			// ... other misc. commands that all require a token
		},
	}
	cli.Run(os.Args)
}

Invoking as

$ go run main.go --flag1 aa --flag2 bb  list --filter ss subcommand1
list before
subc1 before
subc1 action
subc1 after
list after

$ go run main.go --flag1 aa --flag2 bb  list --filter ss help
list before
NAME:
   test list - list all clusters

USAGE:
   test list command [command options] [arguments...]

COMMANDS:
   subcommand1  Some subcommand1
   help, h      Shows a list of commands or help for one command

OPTIONS:
   --filter value  
   --help, -h      show help (default: false)
list after

Without required flags for list

$ go run main.go --flag1 aa --flag2 bb  list help
NAME:
   test list - list all clusters

USAGE:
   test list command [command options] [arguments...]

COMMANDS:
   subcommand1  Some subcommand1
   help, h      Shows a list of commands or help for one command

OPTIONS:
   --filter value  
   --help, -h      show help (default: false)

and then finally without required flags for app itself

$ go run main.go  list 
NAME:
   test - A new cli application

USAGE:
   test [global options] command [command options] [arguments...]

COMMANDS:
   list     list all clusters
   help, h  Shows a list of commands or help for one command

GLOBAL OPTIONS:
   --flag1 value  
   --flag2 value  
   --help, -h     show help (default: false)

Now with this PR we would see the following

$ go run main.go --flag1 aa --flag2 bb  list --filter ss help
NAME:
   test - A new cli application

USAGE:
   test [global options] command [command options] [arguments...]

COMMANDS:
   list     list all clusters
   help, h  Shows a list of commands or help for one command

GLOBAL OPTIONS:
   --flag1 value  
   --flag2 value  
   --help, -h     show help (default: false)

$ go run main.go --flag1 aa --flag2 bb  list help
NAME:
   test - A new cli application

USAGE:
   test [global options] command [command options] [arguments...]

COMMANDS:
   list     list all clusters
   help, h  Shows a list of commands or help for one command

GLOBAL OPTIONS:
   --flag1 value  
   --flag2 value  
   --help, -h     show help (default: false)

$ go run main.go --flag1 aa --flag2 bb  list 
NAME:
   test list - list all clusters

USAGE:
   test list command [command options] [arguments...]

COMMANDS:
   subcommand1  Some subcommand1
   help, h      Shows a list of commands or help for one command

OPTIONS:
   --filter value  
   --help, -h      show help (default: false)

$ go run main.go list 
NAME:
   test - A new cli application

USAGE:
   test [global options] command [command options] [arguments...]

COMMANDS:
   list     list all clusters
   help, h  Shows a list of commands or help for one command

GLOBAL OPTIONS:
   --flag1 value  
   --flag2 value  
   --help, -h     show help (default: false)

So the help is extremely inconsistent. The given PR would work only for one level of subcommands but not further since the correct context for the leaf subcommand(and its App) has not been created yet.

[dearchap]

No movement for this for a long time and fix isn't sufficient. Closing for now

[dearchap]

Closing. Will revisit when new v3 parser is introduced.