Make command tests pass

command.go

@@ -3,7 +3,6 @@ package cli
 import (
 	"flag"
 	"fmt"
-	"io/ioutil"
 	"sort"
 	"strings"
 )
@@ -99,29 +98,7 @@ func (c *Command) Run(ctx *Context) (err error) {
 		c.UseShortOptionHandling = true
 	}
 
-	//if ctx.App.EnableShellCompletion {
+	set, err := c.parseFlags(ctx.Args())
-	//	c.appendFlag(GenerateCompletionFlag)
-	//}
-
-	set, err := flagSet(c.Name, c.Flags)
-	if err != nil {
-		return err
-	}
-	set.SetOutput(ioutil.Discard)
-
-	if c.SkipFlagParsing {
-		err = set.Parse(append([]string{"--"}, ctx.Args().Tail()...))
-	} else {
-		err = set.Parse(ctx.Args().Tail())
-	}
-
-	nerr := normalizeFlags(c.Flags, set)
-	if nerr != nil {
-		_, _ = fmt.Fprintln(ctx.App.Writer, nerr)
-		_, _ = fmt.Fprintln(ctx.App.Writer)
-		_ = ShowCommandHelp(ctx, c.Name)
-		return nerr
-	}
 
 	context := NewContext(ctx.App, set, ctx)
 	context.Command = c
@@ -132,7 +109,7 @@ func (c *Command) Run(ctx *Context) (err error) {
 	if err != nil {
 		if c.OnUsageError != nil {
 			err = c.OnUsageError(context, err, false)
-			HandleExitCoder(err)
+			context.App.handleExitCoder(context, err)
 			return err
 		}
 		_, _ = fmt.Fprintln(context.App.Writer, "Incorrect Usage:", err.Error())
@@ -155,7 +132,7 @@ func (c *Command) Run(ctx *Context) (err error) {
 		defer func() {
 			afterErr := c.After(context)
 			if afterErr != nil {
-				HandleExitCoder(err)
+				context.App.handleExitCoder(context, err)
 				if err != nil {
 					err = newMultiError(err, afterErr)
 				} else {
@@ -168,8 +145,8 @@ func (c *Command) Run(ctx *Context) (err error) {
 	if c.Before != nil {
 		err = c.Before(context)
 		if err != nil {
-			ShowCommandHelp(context, c.Name)
+			_ = ShowCommandHelp(context, c.Name)
-			HandleExitCoder(err)
+			context.App.handleExitCoder(context, err)
 			return err
 		}
 	}
@@ -182,7 +159,7 @@ func (c *Command) Run(ctx *Context) (err error) {
 	err = c.Action(context)
 
 	if err != nil {
-		HandleExitCoder(err)
+		context.App.handleExitCoder(context, err)
 	}
 	return err
 }
@@ -195,6 +172,33 @@ func (c *Command) useShortOptionHandling() bool {
 	return c.UseShortOptionHandling
 }
 
+func (c *Command) parseFlags(args Args) (*flag.FlagSet, error) {
+	if c.SkipFlagParsing {
+		set, err := c.newFlagSet()
+		if err != nil {
+			return nil, err
+		}
+
+		return set, set.Parse(append([]string{"--"}, args.Tail()...))
+	}
+
+	//if !c.SkipArgReorder {
+	//	args = reorderArgs(args)
+	//}
+
+	set, err := parseIter(c, args.Tail())
+	if err != nil {
+		return nil, err
+	}
+
+	err = normalizeFlags(c.Flags, set)
+	if err != nil {
+		return nil, err
+	}
+
+	return set, nil
+}
+
 // Names returns the names including short names and aliases.
 func (c *Command) Names() []string {
 	return append([]string{c.Name}, c.Aliases...)
@@ -240,6 +244,7 @@ func (c *Command) startApp(ctx *Context) error {
 	app.Compiled = ctx.App.Compiled
 	app.Writer = ctx.App.Writer
 	app.ErrWriter = ctx.App.ErrWriter
+	app.ExitErrHandler = ctx.App.ExitErrHandler
 	app.UseShortOptionHandling = ctx.App.UseShortOptionHandling
 
 	app.Categories = newCommandCategories()

command_test.go

@@ -13,38 +13,32 @@ func TestCommandFlagParsing(t *testing.T) {
 	cases := []struct {
 		testArgs               []string
 		skipFlagParsing        bool
-		skipArgReorder         bool
+		useShortOptionHandling bool
 		expectedErr            error
-		UseShortOptionHandling bool
 	}{
 		// Test normal "not ignoring flags" flow
-		{[]string{"test-cmd", "blah", "blah", "-break"}, false, false, errors.New("flag provided but not defined: -break"), false},
+		{testArgs: []string{"test-cmd", "-break", "blah", "blah"}, skipFlagParsing: false, useShortOptionHandling: false, expectedErr: errors.New("flag provided but not defined: -break")},
 
-		// Test no arg reorder
+		{testArgs: []string{"test-cmd", "blah", "blah"}, skipFlagParsing: true, useShortOptionHandling: false, expectedErr: nil},   // Test SkipFlagParsing without any args that look like flags
-		{[]string{"test-cmd", "blah", "blah", "-break"}, false, true, nil, false},
+		{testArgs: []string{"test-cmd", "blah", "-break"}, skipFlagParsing: true, useShortOptionHandling: false, expectedErr: nil}, // Test SkipFlagParsing with random flag arg
-		{[]string{"test-cmd", "blah", "blah", "-break", "ls", "-l"}, false, true, nil, true},
+		{testArgs: []string{"test-cmd", "blah", "-help"}, skipFlagParsing: true, useShortOptionHandling: false, expectedErr: nil},  // Test SkipFlagParsing with "special" help flag arg
-
+		{testArgs: []string{"test-cmd", "blah", "-h"}, skipFlagParsing: false, useShortOptionHandling: true, expectedErr: nil},     // Test UseShortOptionHandling
-		{[]string{"test-cmd", "blah", "blah"}, true, false, nil, false},   // Test SkipFlagParsing without any args that look like flags
-		{[]string{"test-cmd", "blah", "-break"}, true, false, nil, false}, // Test SkipFlagParsing with random flag arg
-		{[]string{"test-cmd", "blah", "-help"}, true, false, nil, false},  // Test SkipFlagParsing with "special" help flag arg
-		{[]string{"test-cmd", "blah"}, false, false, nil, true},           // Test UseShortOptionHandling
 	}
 
 	for _, c := range cases {
 		app := &App{Writer: ioutil.Discard}
 		set := flag.NewFlagSet("test", 0)
-		_ = set.Parse(c.testArgs)
+		set.Parse(c.testArgs)
 
 		context := NewContext(app, set, nil)
 
 		command := Command{
-			Name:                   "test-cmd",
+			Name:            "test-cmd",
-			Aliases:                []string{"tc"},
+			Aliases:         []string{"tc"},
-			Usage:                  "this is for testing",
+			Usage:           "this is for testing",
-			Description:            "testing",
+			Description:     "testing",
-			Action:                 func(_ *Context) error { return nil },
+			Action:          func(_ *Context) error { return nil },
-			SkipFlagParsing:        c.skipFlagParsing,
+			SkipFlagParsing: c.skipFlagParsing,
-			UseShortOptionHandling: c.UseShortOptionHandling,
 		}
 
 		err := command.Run(context)
@@ -56,23 +50,23 @@ func TestCommandFlagParsing(t *testing.T) {
 
 func TestParseAndRunShortOpts(t *testing.T) {
 	cases := []struct {
-		testArgs     []string
+		testArgs     args
 		expectedErr  error
-		expectedArgs []string
+		expectedArgs *args
 	}{
-		{[]string{"foo", "test", "-a"}, nil, []string{}},
+		{testArgs: args{"foo", "test", "-a"}, expectedErr: nil, expectedArgs: &args{}},
-		{[]string{"foo", "test", "-c", "arg1", "arg2"}, nil, []string{"arg1", "arg2"}},
+		{testArgs: args{"foo", "test", "-c", "arg1", "arg2"}, expectedErr: nil, expectedArgs: &args{"arg1", "arg2"}},
-		{[]string{"foo", "test", "-f"}, nil, []string{}},
+		{testArgs: args{"foo", "test", "-f"}, expectedErr: nil, expectedArgs: &args{}},
-		{[]string{"foo", "test", "-ac", "--fgh"}, nil, []string{}},
+		{testArgs: args{"foo", "test", "-ac", "--fgh"}, expectedErr: nil, expectedArgs: &args{}},
-		{[]string{"foo", "test", "-af"}, nil, []string{}},
+		{testArgs: args{"foo", "test", "-af"}, expectedErr: nil, expectedArgs: &args{}},
-		{[]string{"foo", "test", "-cf"}, nil, []string{}},
+		{testArgs: args{"foo", "test", "-cf"}, expectedErr: nil, expectedArgs: &args{}},
-		{[]string{"foo", "test", "-acf"}, nil, []string{}},
+		{testArgs: args{"foo", "test", "-acf"}, expectedErr: nil, expectedArgs: &args{}},
-		{[]string{"foo", "test", "-invalid"}, errors.New("flag provided but not defined: -invalid"), []string{}},
+		{testArgs: args{"foo", "test", "-invalid"}, expectedErr: errors.New("flag provided but not defined: -invalid"), expectedArgs: &args{}},
-		{[]string{"foo", "test", "-acf", "arg1", "-invalid"}, nil, []string{"arg1", "-invalid"}},
+		{testArgs: args{"foo", "test", "-acf", "arg1", "-invalid"}, expectedErr: nil, expectedArgs: &args{"arg1", "-invalid"}},
 	}
 
 	var args Args
-	cmd := Command{
+	cmd := &Command{
 		Name:        "test",
 		Usage:       "this is for testing",
 		Description: "testing",
@@ -90,7 +84,7 @@ func TestParseAndRunShortOpts(t *testing.T) {
 
 	for _, c := range cases {
 		app := NewApp()
-		app.Commands = []Command{cmd}
+		app.Commands = []*Command{cmd}
 
 		err := app.Run(c.testArgs)
 
@@ -292,53 +286,14 @@ func TestCommand_Run_SubcommandsCanUseErrWriter(t *testing.T) {
 	}
 }
 
-func TestCommandFlagReordering(t *testing.T) {
-	cases := []struct {
-		testArgs      []string
-		expectedValue string
-		expectedArgs  []string
-		expectedErr   error
-	}{
-		{[]string{"some-exec", "some-command", "some-arg", "--flag", "foo"}, "foo", []string{"some-arg"}, nil},
-		{[]string{"some-exec", "some-command", "some-arg", "--flag=foo"}, "foo", []string{"some-arg"}, nil},
-		{[]string{"some-exec", "some-command", "--flag=foo", "some-arg"}, "foo", []string{"some-arg"}, nil},
-	}
-
-	for _, c := range cases {
-		value := ""
-		var args Args
-		app := &App{
-			Commands: []*Command{
-				{
-					Name: "some-command",
-					Flags: []Flag{
-						&StringFlag{Name: "flag"},
-					},
-					Action: func(c *Context) error {
-						fmt.Printf("%+v\n", c.String("flag"))
-						value = c.String("flag")
-						args = c.Args()
-						return nil
-					},
-				},
-			},
-		}
-
-		err := app.Run(c.testArgs)
-		expect(t, err, c.expectedErr)
-		expect(t, value, c.expectedValue)
-		expect(t, args, c.expectedArgs)
-	}
-}
-
 func TestCommandSkipFlagParsing(t *testing.T) {
 	cases := []struct {
-		testArgs     []string
+		testArgs     args
-		expectedArgs []string
+		expectedArgs *args
 		expectedErr  error
 	}{
-		{[]string{"some-exec", "some-command", "some-arg", "--flag", "foo"}, []string{"some-arg", "--flag", "foo"}, nil},
+		{testArgs: args{"some-exec", "some-command", "some-arg", "--flag", "foo"}, expectedArgs: &args{"some-arg", "--flag", "foo"}, expectedErr: nil},
-		{[]string{"some-exec", "some-command", "some-arg", "--flag=foo"}, []string{"some-arg", "--flag=foo"}, nil},
+		{testArgs: args{"some-exec", "some-command", "some-arg", "--flag=foo"}, expectedArgs: &args{"some-arg", "--flag=foo"}, expectedErr: nil},
 	}
 
 	for _, c := range cases {