Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Required flags #155

Merged
merged 11 commits into from
Aug 2, 2019
47 changes: 35 additions & 12 deletions app.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,19 +116,30 @@ func (a *App) Run(arguments []string) (err error) {
set.SetOutput(ioutil.Discard)
err = set.Parse(arguments[1:])
nerr := normalizeFlags(a.Flags, set)
if nerr != nil {
fmt.Fprintln(a.Writer, nerr)
context := NewContext(a, set, set)
cerr := checkRequiredFlags(a.Flags, set)

context := NewContext(a, set, set)

// Define here so it closes over the above variables
showErrAndHelp := func(err error) {
fmt.Fprintln(a.Writer, err)
fmt.Fprintln(a.Writer)
ShowAppHelp(context)
fmt.Fprintln(a.Writer)
}

if nerr != nil {
showErrAndHelp(nerr)
return nerr
}
context := NewContext(a, set, set)

if cerr != nil {
showErrAndHelp(cerr)
return cerr
}

if err != nil {
fmt.Fprintf(a.Writer, "Incorrect Usage.\n\n")
ShowAppHelp(context)
fmt.Fprintln(a.Writer)
showErrAndHelp(fmt.Errorf("Incorrect Usage."))
return err
}

Expand Down Expand Up @@ -204,22 +215,34 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) {
set.SetOutput(ioutil.Discard)
err = set.Parse(ctx.Args().Tail())
nerr := normalizeFlags(a.Flags, set)
cerr := checkRequiredFlags(a.Flags, set)

context := NewContext(a, set, ctx.globalSet)

if nerr != nil {
fmt.Fprintln(a.Writer, nerr)
// Define here so it closes over the above variables
showErrAndHelp := func(err error) {
fmt.Fprintln(a.Writer, err)
fmt.Fprintln(a.Writer)
if len(a.Commands) > 0 {
ShowSubcommandHelp(context)
} else {
ShowCommandHelp(ctx, context.Args().First())
}
fmt.Fprintln(a.Writer)
fmt.Fprintln(a.Writer, err)
}

if nerr != nil {
showErrAndHelp(nerr)
return nerr
}

if cerr != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be nice to DRY this up by combining with the above conditional. The only thing that differs is which error you print.

showErrAndHelp(cerr)
return cerr
}

if err != nil {
fmt.Fprintf(a.Writer, "Incorrect Usage.\n\n")
ShowSubcommandHelp(context)
showErrAndHelp(err)
return err
}

Expand Down
25 changes: 19 additions & 6 deletions command.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,21 +88,31 @@ func (c Command) Run(ctx *Context) error {
err = set.Parse(ctx.Args().Tail())
}

if err != nil {
fmt.Fprint(ctx.App.Writer, "Incorrect Usage.\n\n")
// Define here so it closes over the above variables
showErrAndHelp := func(err error) {
fmt.Fprintln(ctx.App.Writer, err)
fmt.Fprintln(ctx.App.Writer)
ShowCommandHelp(ctx, c.Name)
fmt.Fprintln(ctx.App.Writer)
}

if err != nil {
showErrAndHelp(fmt.Errorf("Incorrect Usage."))
return err
}

nerr := normalizeFlags(c.Flags, set)
if nerr != nil {
fmt.Fprintln(ctx.App.Writer, nerr)
fmt.Fprintln(ctx.App.Writer)
ShowCommandHelp(ctx, c.Name)
fmt.Fprintln(ctx.App.Writer)
showErrAndHelp(nerr)
return nerr
}

cerr := checkRequiredFlags(c.Flags, set)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here with respect to DRYing.

if cerr != nil {
showErrAndHelp(cerr)
return cerr
}

context := NewContext(ctx.App, set, ctx.globalSet)

if checkCommandCompletions(context, c.Name) {
Expand Down Expand Up @@ -156,5 +166,8 @@ func (c Command) startApp(ctx *Context) error {
app.Action = helpSubcommand.Action
}

// set the writer to the original App's writer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a separate bug that I noticed when I tried to integrate the latest code with our codebase. The Writer wasn't copied to the sub command, so subcommands got a new os.Stdout Writer even though I set the App's Writer to a bytes.Buffer to capture output in my tests.

app.Writer = ctx.App.Writer

return app.RunAsSubcommand(ctx)
}
25 changes: 25 additions & 0 deletions context.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package cli
import (
"errors"
"flag"
"fmt"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -337,3 +338,27 @@ func normalizeFlags(flags []Flag, set *flag.FlagSet) error {
}
return nil
}

func checkRequiredFlags(flags []Flag, set *flag.FlagSet) error {
// If the help flag is included then none of the other flags are required.
for _, f := range flags {
if f.getName() == "help" {
return nil
}
}

visited := make(map[string]bool)
set.Visit(func(f *flag.Flag) {
visited[f.Name] = true
})

for _, f := range flags {
if f.IsRequired() {
key := strings.Split(f.getName(), ",")[0]
if !visited[key] {
return fmt.Errorf("Required flag %s not set", f.getName())
}
}
}
return nil
}