Skip to content

Commit

Permalink
Merge pull request #1987 from dearchap/pos_args_flags_mix
Browse files Browse the repository at this point in the history 
Fix:(issue_976) All positional args and flags to be mixed in any order
  • Loading branch information
@dearchap
dearchap authored Oct 19, 2024
2 parents 86d5470 + b34e3bd commit 5053ec7
Show file tree
Hide file tree
Showing 6 changed files with 106 additions and 54 deletions.
89 changes: 69 additions & 20 deletions command.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -591,24 +591,13 @@ func (cmd *Command) Run(ctx context.Context, osArgs []string) (deferErr error) {
hasDefault := cmd.DefaultCommand != ""
isFlagName := checkStringSliceIncludes(name, cmd.FlagNames())

var (
isDefaultSubcommand = false
defaultHasSubcommands = false
)

if hasDefault {
dc := cmd.Command(cmd.DefaultCommand)
defaultHasSubcommands = len(dc.Commands) > 0
for _, dcSub := range dc.Commands {
if checkStringSliceIncludes(name, dcSub.Names()) {
isDefaultSubcommand = true
break
}
}
tracef("using default command=%[1]q (cmd=%[2]q)", cmd.DefaultCommand, cmd.Name)
}

if isFlagName || (hasDefault && (defaultHasSubcommands && isDefaultSubcommand)) {
if isFlagName || hasDefault {
argsWithDefault := cmd.argsWithDefaultCommand(args)
tracef("using default command args=%[1]q (cmd=%[2]q)", argsWithDefault, cmd.Name)
if !reflect.DeepEqual(args, argsWithDefault) {
subCmd = cmd.Command(argsWithDefault.First())
}
Expand Down Expand Up @@ -656,7 +645,7 @@ func (cmd *Command) Run(ctx context.Context, osArgs []string) (deferErr error) {
deferErr = cmd.handleExitCoder(ctx, err)
}

tracef("returning deferErr (cmd=%[1]q)", cmd.Name)
tracef("returning deferErr (cmd=%[1]q) %[2]q", cmd.Name, deferErr)
return deferErr
}

Expand Down Expand Up @@ -802,14 +791,74 @@ func (cmd *Command) parseFlags(args Args) (Args, error) {
}

tracef("parsing flags iteratively tail=%[1]q (cmd=%[2]q)", args.Tail(), cmd.Name)
defer tracef("done parsing flags (cmd=%[1]q)", cmd.Name)

if err := parseIter(cmd.flagSet, cmd, args.Tail(), cmd.Root().shellCompletion); err != nil {
return cmd.Args(), err
}
rargs := args.Tail()
posArgs := []string{}
for {
tracef("rearrange:1 (cmd=%[1]q) %[2]q", cmd.Name, rargs)
for {
tracef("rearrange:2 (cmd=%[1]q) %[2]q %[3]q", cmd.Name, posArgs, rargs)

// no more args to parse. Break out of inner loop
if len(rargs) == 0 {
break
}

if strings.TrimSpace(rargs[0]) == "" {
break
}

// stop parsing once we see a "--"
if rargs[0] == "--" {
posArgs = append(posArgs, rargs...)
cmd.parsedArgs = &stringSliceArgs{posArgs}
return cmd.parsedArgs, nil
}

tracef("done parsing flags (cmd=%[1]q)", cmd.Name)
// let flagset parse this
if rargs[0][0] == '-' {
break
}

tracef("rearrange-3 (cmd=%[1]q) check %[2]q", cmd.Name, rargs[0])

// if there is a command by that name let the command handle the
// rest of the parsing
if cmd.Command(rargs[0]) != nil {
posArgs = append(posArgs, rargs...)
cmd.parsedArgs = &stringSliceArgs{posArgs}
return cmd.parsedArgs, nil
}

posArgs = append(posArgs, rargs[0])

// if this is the sole argument then
// break from inner loop
if len(rargs) == 1 {
rargs = []string{}
break
}

rargs = rargs[1:]
}
if err := parseIter(cmd.flagSet, cmd, rargs, cmd.Root().shellCompletion); err != nil {
posArgs = append(posArgs, cmd.flagSet.Args()...)
tracef("returning-1 (cmd=%[1]q) args %[2]q", cmd.Name, posArgs)
cmd.parsedArgs = &stringSliceArgs{posArgs}
return cmd.parsedArgs, err
}
tracef("rearrange-4 (cmd=%[1]q) check %[2]q", cmd.Name, cmd.flagSet.Args())
rargs = cmd.flagSet.Args()
if len(rargs) == 0 || strings.TrimSpace(rargs[0]) == "" || rargs[0] == "-" {
break
}
}

return cmd.Args(), nil
posArgs = append(posArgs, cmd.flagSet.Args()...)
tracef("returning-2 (cmd=%[1]q) args %[2]q", cmd.Name, posArgs)
cmd.parsedArgs = &stringSliceArgs{posArgs}
return cmd.parsedArgs, nil
}

// Names returns the names including short names and aliases.
Expand Down
62 changes: 31 additions & 31 deletions command_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,10 @@ func TestCommandFlagParsing(t *testing.T) {
}{
// Test normal "not ignoring flags" flow
{testArgs: []string{"test-cmd", "-break", "blah", "blah"}, skipFlagParsing: false, useShortOptionHandling: false, expectedErr: "flag provided but not defined: -break"},
{testArgs: []string{"test-cmd", "blah", "blah"}, skipFlagParsing: true, useShortOptionHandling: false}, // Test SkipFlagParsing without any args that look like flags
{testArgs: []string{"test-cmd", "blah", "-break"}, skipFlagParsing: true, useShortOptionHandling: false}, // Test SkipFlagParsing with random flag arg
{testArgs: []string{"test-cmd", "blah", "-help"}, skipFlagParsing: true, useShortOptionHandling: false}, // Test SkipFlagParsing with "special" help flag arg
{testArgs: []string{"test-cmd", "blah", "-h"}, skipFlagParsing: false, useShortOptionHandling: true}, // Test UseShortOptionHandling
{testArgs: []string{"test-cmd", "blah", "blah"}, skipFlagParsing: true, useShortOptionHandling: false}, // Test SkipFlagParsing without any args that look like flags
{testArgs: []string{"test-cmd", "blah", "-break"}, skipFlagParsing: true, useShortOptionHandling: false}, // Test SkipFlagParsing with random flag arg
{testArgs: []string{"test-cmd", "blah", "-help"}, skipFlagParsing: true, useShortOptionHandling: false}, // Test SkipFlagParsing with "special" help flag arg
{testArgs: []string{"test-cmd", "blah", "-h"}, skipFlagParsing: false, useShortOptionHandling: true, expectedErr: "No help topic for 'blah'"}, // Test UseShortOptionHandling
}

for _, c := range cases {
Expand Down Expand Up @@ -212,9 +212,9 @@ func TestParseAndRunShortOpts(t *testing.T) {
{testArgs: &stringSliceArgs{v: []string{"test", "-acf", "-invalid"}}, expectedErr: "flag provided but not defined: -invalid"},
{testArgs: &stringSliceArgs{v: []string{"test", "--invalid"}}, expectedErr: "flag provided but not defined: -invalid"},
{testArgs: &stringSliceArgs{v: []string{"test", "-acf", "--invalid"}}, expectedErr: "flag provided but not defined: -invalid"},
{testArgs: &stringSliceArgs{v: []string{"test", "-acf", "arg1", "-invalid"}}, expectedArgs: &stringSliceArgs{v: []string{"arg1", "-invalid"}}},
{testArgs: &stringSliceArgs{v: []string{"test", "-acf", "arg1", "--invalid"}}, expectedArgs: &stringSliceArgs{v: []string{"arg1", "--invalid"}}},
{testArgs: &stringSliceArgs{v: []string{"test", "-acfi", "not-arg", "arg1", "-invalid"}}, expectedArgs: &stringSliceArgs{v: []string{"arg1", "-invalid"}}},
{testArgs: &stringSliceArgs{v: []string{"test", "-acf", "arg1", "-invalid"}}, expectedErr: "flag provided but not defined: -invalid"},
{testArgs: &stringSliceArgs{v: []string{"test", "-acf", "arg1", "--invalid"}}, expectedErr: "flag provided but not defined: -invalid"},
{testArgs: &stringSliceArgs{v: []string{"test", "-acfi", "not-arg", "arg1", "-invalid"}}, expectedErr: "flag provided but not defined: -invalid"},
{testArgs: &stringSliceArgs{v: []string{"test", "-i", "ivalue"}}, expectedArgs: &stringSliceArgs{v: []string{}}},
{testArgs: &stringSliceArgs{v: []string{"test", "-i", "ivalue", "arg1"}}, expectedArgs: &stringSliceArgs{v: []string{"arg1"}}},
{testArgs: &stringSliceArgs{v: []string{"test", "-i"}}, expectedErr: "flag needs an argument: -i"},
Expand Down Expand Up @@ -639,9 +639,9 @@ func TestCommand_Command(t *testing.T) {
}

var defaultCommandTests = []struct {
cmdName string
defaultCmd string
expected bool
cmdName string
defaultCmd string
errNotExpected bool
}{
{"foobar", "foobar", true},
{"batbaz", "foobar", true},
Expand All @@ -650,8 +650,8 @@ var defaultCommandTests = []struct {
{"", "foobar", true},
{"", "", true},
{" ", "", false},
{"bat", "batbaz", false},
{"nothing", "batbaz", false},
{"bat", "batbaz", true},
{"nothing", "batbaz", true},
{"nothing", "", false},
}

Expand All @@ -668,7 +668,7 @@ func TestCommand_RunDefaultCommand(t *testing.T) {
}

err := cmd.Run(buildTestContext(t), []string{"c", test.cmdName})
if test.expected {
if test.errNotExpected {
assert.NoError(t, err)
} else {
assert.Error(t, err)
Expand All @@ -678,10 +678,10 @@ func TestCommand_RunDefaultCommand(t *testing.T) {
}

var defaultCommandSubCommandTests = []struct {
cmdName string
subCmd string
defaultCmd string
expected bool
cmdName string
subCmd string
defaultCmd string
errNotExpected bool
}{
{"foobar", "", "foobar", true},
{"foobar", "carly", "foobar", true},
Expand All @@ -693,14 +693,14 @@ var defaultCommandSubCommandTests = []struct {
{"", "jimbob", "foobar", true},
{"", "j", "foobar", true},
{"", "carly", "foobar", true},
{"", "jimmers", "foobar", true},
{"", "jimmers", "foobar", false},
{"", "jimmers", "", true},
{" ", "jimmers", "foobar", false},
{"", "", "", true},
{" ", "", "", false},
{" ", "j", "", false},
{"bat", "", "batbaz", false},
{"nothing", "", "batbaz", false},
{"bat", "", "batbaz", true},
{"nothing", "", "batbaz", true},
{"nothing", "", "", false},
{"nothing", "j", "batbaz", false},
{"nothing", "carly", "", false},
Expand All @@ -726,7 +726,7 @@ func TestCommand_RunDefaultCommandWithSubCommand(t *testing.T) {
}

err := cmd.Run(buildTestContext(t), []string{"c", test.cmdName, test.subCmd})
if test.expected {
if test.errNotExpected {
assert.NoError(t, err)
} else {
assert.Error(t, err)
Expand All @@ -736,10 +736,10 @@ func TestCommand_RunDefaultCommandWithSubCommand(t *testing.T) {
}

var defaultCommandFlagTests = []struct {
cmdName string
flag string
defaultCmd string
expected bool
cmdName string
flag string
defaultCmd string
errNotExpected bool
}{
{"foobar", "", "foobar", true},
{"foobar", "-c derp", "foobar", true},
Expand All @@ -754,14 +754,14 @@ var defaultCommandFlagTests = []struct {
{"", "--carly=derp", "foobar", true},
{"", "-j", "foobar", true},
{"", "-j", "", true},
{" ", "-j", "foobar", false},
{" ", "-j", "foobar", true},
{"", "", "", true},
{" ", "", "", false},
{" ", "-j", "", false},
{"bat", "", "batbaz", false},
{"nothing", "", "batbaz", false},
{"bat", "", "batbaz", true},
{"nothing", "", "batbaz", true},
{"nothing", "", "", false},
{"nothing", "--jimbob", "batbaz", false},
{"nothing", "--jimbob", "batbaz", true},
{"nothing", "--carly", "", false},
}

Expand Down Expand Up @@ -810,7 +810,7 @@ func TestCommand_RunDefaultCommandWithFlags(t *testing.T) {
appArgs = append(appArgs, test.cmdName)

err := cmd.Run(buildTestContext(t), appArgs)
if test.expected {
if test.errNotExpected {
assert.NoError(t, err)
} else {
assert.Error(t, err)
Expand Down Expand Up @@ -1735,7 +1735,7 @@ func TestCommand_CommandNotFound(t *testing.T) {

_ = cmd.Run(buildTestContext(t), []string{"command", "foo"})

assert.Equal(t, 1, counts.CommandNotFound, 1)
assert.Equal(t, 1, counts.CommandNotFound)
assert.Equal(t, 0, counts.SubCommand)
assert.Equal(t, 1, counts.Total)
}
Expand Down
2 changes: 2 additions & 0 deletions examples_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,7 @@ func ExampleCommand_Run_sliceValues() {
&cli.FloatSliceFlag{Name: "float64Slice"},
&cli.IntSliceFlag{Name: "intSlice"},
},
HideHelp: true,
Action: func(ctx context.Context, cmd *cli.Command) error {
for i, v := range cmd.FlagNames() {
fmt.Printf("%d-%s %#v\n", i, v, cmd.Value(v))
Expand Down Expand Up @@ -454,6 +455,7 @@ func ExampleCommand_Run_mapValues() {
Flags: []cli.Flag{
&cli.StringMapFlag{Name: "stringMap"},
},
HideHelp: true,
Action: func(ctx context.Context, cmd *cli.Command) error {
for i, v := range cmd.FlagNames() {
fmt.Printf("%d-%s %#v\n", i, v, cmd.StringMap(v))
Expand Down
2 changes: 2 additions & 0 deletions help.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ func helpCommandAction(ctx context.Context, cmd *Command) error {
args := cmd.Args()
firstArg := args.First()

tracef("doing help for cmd %[1]q with args %[2]q", cmd, args)

// This action can be triggered by a "default" action of a command
// or via cmd.Run when cmd == helpCmd. So we have following possibilities
//
Expand Down
4 changes: 2 additions & 2 deletions help_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ func Test_helpCommand_Action_ErrorIfNoTopic(t *testing.T) {
flagSet: flag.NewFlagSet("test", 0),
}

_ = cmd.flagSet.Parse([]string{"foo"})
_ = cmd.Run(context.Background(), []string{"foo", "bar"})

err := helpCommandAction(context.Background(), cmd)
require.Error(t, err, "expected error from helpCommandAction()")
Expand Down Expand Up @@ -295,7 +295,7 @@ func Test_helpSubcommand_Action_ErrorIfNoTopic(t *testing.T) {
cmd := &Command{
flagSet: flag.NewFlagSet("test", 0),
}
_ = cmd.flagSet.Parse([]string{"foo"})
_ = cmd.Run(context.Background(), []string{"foo", "bar"})

err := helpCommandAction(context.Background(), cmd)
require.Error(t, err, "expected error from helpCommandAction(), but got nil")
Expand Down
1 change: 0 additions & 1 deletion parse.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
)

type iterativeParser interface {
newFlagSet() (*flag.FlagSet, error)
useShortOptionHandling() bool
}

Expand Down

0 comments on commit 5053ec7

Please sign in to comment.