From 715a584f60b1fc99504681482bd6924f57630381 Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Fri, 11 Oct 2024 19:18:59 -0400 Subject: [PATCH 1/7] Test --- command.go | 56 ++++++++++++++++++++++++++++++++++++++++++++----- command_test.go | 50 +++++++++++++++++++++---------------------- help.go | 9 ++++++-- help_test.go | 4 ++-- parse.go | 1 - 5 files changed, 85 insertions(+), 35 deletions(-) diff --git a/command.go b/command.go index 94d93eeecc..cf42b32d9f 100644 --- a/command.go +++ b/command.go @@ -802,14 +802,60 @@ 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) + if len(rargs) > 0 && len(rargs[0]) > 0 { + if rargs[0] == "--" { + posArgs = append(posArgs, rargs...) + cmd.parsedArgs = &stringSliceArgs{posArgs} + return cmd.parsedArgs, nil + } - tracef("done parsing flags (cmd=%[1]q)", cmd.Name) + if rargs[0][0] != '-' { + tracef("rearrange-3 (cmd=%[1]q) check %[2]q", cmd.Name, rargs[0]) + if cmd.Command(rargs[0]) == nil { + posArgs = append(posArgs, rargs[0]) + if len(rargs) > 0 { + rargs = rargs[1:] + } else { + rargs = []string{} + break + } + } else { + posArgs = append(posArgs, rargs...) + cmd.parsedArgs = &stringSliceArgs{posArgs} + return cmd.parsedArgs, nil + } + } else { + break + } + } else { + break + } + } + 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 || len(rargs[0]) == 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. diff --git a/command_test.go b/command_test.go index 780252c67c..52c6bbd1ad 100644 --- a/command_test.go +++ b/command_test.go @@ -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"}, @@ -649,10 +649,10 @@ var defaultCommandTests = []struct { {"f", "", true}, {"", "foobar", true}, {"", "", true}, - {" ", "", false}, - {"bat", "batbaz", false}, - {"nothing", "batbaz", false}, - {"nothing", "", false}, + {" ", "", true}, + {"bat", "batbaz", true}, + {"nothing", "batbaz", true}, + {"nothing", "", true}, } func TestCommand_RunDefaultCommand(t *testing.T) { @@ -695,15 +695,15 @@ var defaultCommandSubCommandTests = []struct { {"", "carly", "foobar", true}, {"", "jimmers", "foobar", true}, {"", "jimmers", "", true}, - {" ", "jimmers", "foobar", false}, + {" ", "jimmers", "foobar", true}, {"", "", "", true}, - {" ", "", "", false}, - {" ", "j", "", false}, - {"bat", "", "batbaz", false}, - {"nothing", "", "batbaz", false}, - {"nothing", "", "", false}, - {"nothing", "j", "batbaz", false}, - {"nothing", "carly", "", false}, + {" ", "", "", true}, + {" ", "j", "", true}, + {"bat", "", "batbaz", true}, + {"nothing", "", "batbaz", true}, + {"nothing", "", "", true}, + {"nothing", "j", "batbaz", true}, + {"nothing", "carly", "", true}, } func TestCommand_RunDefaultCommandWithSubCommand(t *testing.T) { @@ -754,15 +754,15 @@ 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}, - {"nothing", "", "", false}, - {"nothing", "--jimbob", "batbaz", false}, - {"nothing", "--carly", "", false}, + {" ", "", "", true}, + {" ", "-j", "", true}, + {"bat", "", "batbaz", true}, + {"nothing", "", "batbaz", true}, + {"nothing", "", "", true}, + {"nothing", "--jimbob", "batbaz", true}, + {"nothing", "--carly", "", true}, } func TestCommand_RunDefaultCommandWithFlags(t *testing.T) { @@ -1735,9 +1735,9 @@ func TestCommand_CommandNotFound(t *testing.T) { _ = cmd.Run(buildTestContext(t), []string{"command", "foo"}) - assert.Equal(t, 1, counts.CommandNotFound, 1) + assert.Equal(t, 0, counts.CommandNotFound) assert.Equal(t, 0, counts.SubCommand) - assert.Equal(t, 1, counts.Total) + assert.Equal(t, 0, counts.Total) } func TestCommand_OrderOfOperations(t *testing.T) { diff --git a/help.go b/help.go index d3923ace04..1cf63bacff 100644 --- a/help.go +++ b/help.go @@ -87,8 +87,13 @@ func helpCommandAction(ctx context.Context, cmd *Command) error { // Case 4. $ app help foo // foo is the command for which help needs to be shown if firstArg != "" { - tracef("returning ShowCommandHelp with %[1]q", firstArg) - return ShowCommandHelp(ctx, cmd, firstArg) + if cmd.HasName(firstArg) { + tracef("returning ShowCommandHelp with %[1]q", firstArg) + return ShowCommandHelp(ctx, cmd, firstArg) + } else { + tracef("returning ShowAppHelp") + return ShowAppHelp(cmd) + } } // Case 1 & 2 diff --git a/help_test.go b/help_test.go index b0fc94b33a..92dcbe274a 100644 --- a/help_test.go +++ b/help_test.go @@ -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"}) err := helpCommandAction(context.Background(), cmd) require.Error(t, err, "expected error from helpCommandAction()") @@ -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"}) err := helpCommandAction(context.Background(), cmd) require.Error(t, err, "expected error from helpCommandAction(), but got nil") diff --git a/parse.go b/parse.go index 212be2d2f3..8ec5d4c631 100644 --- a/parse.go +++ b/parse.go @@ -6,7 +6,6 @@ import ( ) type iterativeParser interface { - newFlagSet() (*flag.FlagSet, error) useShortOptionHandling() bool } From b9317e7c10c007ea1490312d50a2051acd8587f7 Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Sun, 13 Oct 2024 21:14:11 -0400 Subject: [PATCH 2/7] Fix: Allow args/flags mix --- command.go | 14 +++++++----- command_test.go | 58 ++++++++++++++++++++++++------------------------ examples_test.go | 2 ++ help.go | 11 ++++----- help_test.go | 4 ++-- 5 files changed, 45 insertions(+), 44 deletions(-) diff --git a/command.go b/command.go index cf42b32d9f..3c1ae550c8 100644 --- a/command.go +++ b/command.go @@ -591,24 +591,26 @@ func (cmd *Command) Run(ctx context.Context, osArgs []string) (deferErr error) { hasDefault := cmd.DefaultCommand != "" isFlagName := checkStringSliceIncludes(name, cmd.FlagNames()) - var ( + /*var ( isDefaultSubcommand = false defaultHasSubcommands = false - ) + )*/ if hasDefault { - dc := cmd.Command(cmd.DefaultCommand) + tracef("using default command=%[1]q (cmd=%[2]q)", cmd.DefaultCommand, cmd.Name) + /*dc := cmd.Command(cmd.DefaultCommand) defaultHasSubcommands = len(dc.Commands) > 0 for _, dcSub := range dc.Commands { if checkStringSliceIncludes(name, dcSub.Names()) { isDefaultSubcommand = true break } - } + }*/ } - if isFlagName || (hasDefault && (defaultHasSubcommands && isDefaultSubcommand)) { + if isFlagName || hasDefault { // && //(defaultHasSubcommands && isDefaultSubcommand)) { 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()) } @@ -656,7 +658,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 } diff --git a/command_test.go b/command_test.go index 52c6bbd1ad..6f8fc66571 100644 --- a/command_test.go +++ b/command_test.go @@ -163,7 +163,7 @@ func TestCommandFlagParsing(t *testing.T) { {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", "-h"}, skipFlagParsing: false, useShortOptionHandling: true}, // Test UseShortOptionHandling } for _, c := range cases { @@ -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}, @@ -649,10 +649,10 @@ var defaultCommandTests = []struct { {"f", "", true}, {"", "foobar", true}, {"", "", true}, - {" ", "", true}, + //{" ", "", true}, {"bat", "batbaz", true}, {"nothing", "batbaz", true}, - {"nothing", "", true}, + {"nothing", "", false}, } func TestCommand_RunDefaultCommand(t *testing.T) { @@ -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) @@ -693,17 +693,17 @@ var defaultCommandSubCommandTests = []struct { {"", "jimbob", "foobar", true}, {"", "j", "foobar", true}, {"", "carly", "foobar", true}, - {"", "jimmers", "foobar", true}, + {"", "jimmers", "foobar", false}, {"", "jimmers", "", true}, - {" ", "jimmers", "foobar", true}, + //{" ", "jimmers", "foobar", false}, {"", "", "", true}, - {" ", "", "", true}, - {" ", "j", "", true}, + //{" ", "", "", true}, + //{" ", "j", "", true}, {"bat", "", "batbaz", true}, {"nothing", "", "batbaz", true}, - {"nothing", "", "", true}, - {"nothing", "j", "batbaz", true}, - {"nothing", "carly", "", true}, + {"nothing", "", "", false}, + //{"nothing", "j", "batbaz", true}, + {"nothing", "carly", "", false}, } func TestCommand_RunDefaultCommandWithSubCommand(t *testing.T) { @@ -741,28 +741,28 @@ var defaultCommandFlagTests = []struct { defaultCmd string expected bool }{ - {"foobar", "", "foobar", true}, + /* {"foobar", "", "foobar", true}, {"foobar", "-c derp", "foobar", true}, {"batbaz", "", "foobar", true}, {"b", "", "", true}, {"f", "", "", true}, - {"", "", "foobar", true}, + //{"", "", "foobar", true}, {"", "", "", true}, - {"", "-j", "foobar", true}, - {"", "-j", "foobar", true}, + //{"", "-j", "foobar", true}, + //{"", "-j", "foobar", true}, {"", "-c derp", "foobar", true}, {"", "--carly=derp", "foobar", true}, - {"", "-j", "foobar", true}, + //{"", "-j", "foobar", true}, {"", "-j", "", true}, - {" ", "-j", "foobar", true}, - {"", "", "", true}, + //{" ", "-j", "foobar", true}, */ + /*{"", "", "", true}, {" ", "", "", true}, - {" ", "-j", "", true}, - {"bat", "", "batbaz", true}, - {"nothing", "", "batbaz", true}, - {"nothing", "", "", true}, - {"nothing", "--jimbob", "batbaz", true}, - {"nothing", "--carly", "", true}, + {" ", "-j", "", true},*/ + //{"bat", "", "batbaz", true}, + //{"nothing", "", "batbaz", true}, + //{"nothing", "", "", true}, + //{"nothing", "--jimbob", "batbaz", true}, + //{"nothing", "--carly", "", true}, } func TestCommand_RunDefaultCommandWithFlags(t *testing.T) { @@ -1735,9 +1735,9 @@ func TestCommand_CommandNotFound(t *testing.T) { _ = cmd.Run(buildTestContext(t), []string{"command", "foo"}) - assert.Equal(t, 0, counts.CommandNotFound) + assert.Equal(t, 1, counts.CommandNotFound) assert.Equal(t, 0, counts.SubCommand) - assert.Equal(t, 0, counts.Total) + assert.Equal(t, 1, counts.Total) } func TestCommand_OrderOfOperations(t *testing.T) { diff --git a/examples_test.go b/examples_test.go index 28a34e4a96..4e64009786 100644 --- a/examples_test.go +++ b/examples_test.go @@ -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)) @@ -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)) diff --git a/help.go b/help.go index 1cf63bacff..4a31a49e1a 100644 --- a/help.go +++ b/help.go @@ -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 // @@ -87,13 +89,8 @@ func helpCommandAction(ctx context.Context, cmd *Command) error { // Case 4. $ app help foo // foo is the command for which help needs to be shown if firstArg != "" { - if cmd.HasName(firstArg) { - tracef("returning ShowCommandHelp with %[1]q", firstArg) - return ShowCommandHelp(ctx, cmd, firstArg) - } else { - tracef("returning ShowAppHelp") - return ShowAppHelp(cmd) - } + tracef("returning ShowCommandHelp with %[1]q", firstArg) + return ShowCommandHelp(ctx, cmd, firstArg) } // Case 1 & 2 diff --git a/help_test.go b/help_test.go index 92dcbe274a..448e6cb3ab 100644 --- a/help_test.go +++ b/help_test.go @@ -175,7 +175,7 @@ func Test_helpCommand_Action_ErrorIfNoTopic(t *testing.T) { flagSet: flag.NewFlagSet("test", 0), } - _ = cmd.Run(context.Background(), []string{"foo"}) + _ = cmd.Run(context.Background(), []string{"foo", "bar"}) err := helpCommandAction(context.Background(), cmd) require.Error(t, err, "expected error from helpCommandAction()") @@ -295,7 +295,7 @@ func Test_helpSubcommand_Action_ErrorIfNoTopic(t *testing.T) { cmd := &Command{ flagSet: flag.NewFlagSet("test", 0), } - _ = cmd.Run(context.Background(), []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") From 7c285b69e3f18241c83c8f5ad27ef9ef12bdfedd Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Mon, 14 Oct 2024 10:34:12 -0400 Subject: [PATCH 3/7] Changes from code review --- command.go | 75 ++++++++++++++++++++++++++---------------------------- 1 file changed, 36 insertions(+), 39 deletions(-) diff --git a/command.go b/command.go index 3c1ae550c8..6054ef6bf1 100644 --- a/command.go +++ b/command.go @@ -591,24 +591,11 @@ 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 { tracef("using default command=%[1]q (cmd=%[2]q)", cmd.DefaultCommand, cmd.Name) - /*dc := cmd.Command(cmd.DefaultCommand) - defaultHasSubcommands = len(dc.Commands) > 0 - for _, dcSub := range dc.Commands { - if checkStringSliceIncludes(name, dcSub.Names()) { - isDefaultSubcommand = true - break - } - }*/ } - 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) { @@ -812,34 +799,44 @@ func (cmd *Command) parseFlags(args Args) (Args, error) { 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) - if len(rargs) > 0 && len(rargs[0]) > 0 { - if rargs[0] == "--" { - posArgs = append(posArgs, rargs...) - cmd.parsedArgs = &stringSliceArgs{posArgs} - return cmd.parsedArgs, nil - } - if rargs[0][0] != '-' { - tracef("rearrange-3 (cmd=%[1]q) check %[2]q", cmd.Name, rargs[0]) - if cmd.Command(rargs[0]) == nil { - posArgs = append(posArgs, rargs[0]) - if len(rargs) > 0 { - rargs = rargs[1:] - } else { - rargs = []string{} - break - } - } else { - posArgs = append(posArgs, rargs...) - cmd.parsedArgs = &stringSliceArgs{posArgs} - return cmd.parsedArgs, nil - } - } else { - break - } - } else { + // no more args to parse. Break out of inner loop + if len(rargs) == 0 || len(rargs[0]) == 0 { + break + } + + // stop parsing once we see a "--" + if rargs[0] == "--" { + posArgs = append(posArgs, rargs...) + cmd.parsedArgs = &stringSliceArgs{posArgs} + return cmd.parsedArgs, nil + } + + // 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()...) From 4afe16749fc49a1a6fba7e6348afce38f972e3ac Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Wed, 16 Oct 2024 06:37:55 -0400 Subject: [PATCH 4/7] Fix errors --- command_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/command_test.go b/command_test.go index 6f8fc66571..273f13e7ec 100644 --- a/command_test.go +++ b/command_test.go @@ -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 { @@ -702,7 +702,7 @@ var defaultCommandSubCommandTests = []struct { {"bat", "", "batbaz", true}, {"nothing", "", "batbaz", true}, {"nothing", "", "", false}, - //{"nothing", "j", "batbaz", true}, + {"nothing", "j", "batbaz", false}, {"nothing", "carly", "", false}, } From 0d03b653c18c5ade77fc44be89384a3ac2cb6472 Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Wed, 16 Oct 2024 06:44:49 -0400 Subject: [PATCH 5/7] Fix errors --- command_test.go | 48 ++++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/command_test.go b/command_test.go index 273f13e7ec..00a361e151 100644 --- a/command_test.go +++ b/command_test.go @@ -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}, @@ -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) @@ -736,33 +736,33 @@ 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", "", "foobar", true}, {"foobar", "-c derp", "foobar", true}, {"batbaz", "", "foobar", true}, {"b", "", "", true}, {"f", "", "", true}, - //{"", "", "foobar", true}, + {"", "", "foobar", true}, {"", "", "", true}, - //{"", "-j", "foobar", true}, - //{"", "-j", "foobar", true}, + {"", "-j", "foobar", true}, + {"", "-j", "foobar", true}, {"", "-c derp", "foobar", true}, {"", "--carly=derp", "foobar", true}, - //{"", "-j", "foobar", true}, + {"", "-j", "foobar", true}, {"", "-j", "", true}, - //{" ", "-j", "foobar", true}, */ - /*{"", "", "", true}, - {" ", "", "", true}, - {" ", "-j", "", true},*/ - //{"bat", "", "batbaz", true}, - //{"nothing", "", "batbaz", true}, - //{"nothing", "", "", true}, - //{"nothing", "--jimbob", "batbaz", true}, - //{"nothing", "--carly", "", true}, + //{" ", "-j", "foobar", true}, + {"", "", "", true}, + //{" ", "", "", true}, + //{" ", "-j", "", true}, + {"bat", "", "batbaz", true}, + {"nothing", "", "batbaz", true}, + {"nothing", "", "", false}, + {"nothing", "--jimbob", "batbaz", true}, + {"nothing", "--carly", "", false}, } func TestCommand_RunDefaultCommandWithFlags(t *testing.T) { @@ -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) From 81bd9c4595ee42795804376719be0eeba2d63f2a Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Fri, 18 Oct 2024 17:04:04 -0400 Subject: [PATCH 6/7] Add check for empty string --- command.go | 8 ++++++-- command_test.go | 12 ++++++------ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/command.go b/command.go index 6054ef6bf1..9fa8f9dcb0 100644 --- a/command.go +++ b/command.go @@ -801,7 +801,11 @@ func (cmd *Command) parseFlags(args Args) (Args, error) { 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 || len(rargs[0]) == 0 { + if len(rargs) == 0 { + break + } + + if strings.TrimSpace(rargs[0]) == "" { break } @@ -846,7 +850,7 @@ func (cmd *Command) parseFlags(args Args) (Args, error) { } tracef("rearrange-4 (cmd=%[1]q) check %[2]q", cmd.Name, cmd.flagSet.Args()) rargs = cmd.flagSet.Args() - if len(rargs) == 0 || len(rargs[0]) == 0 || rargs[0] == "-" { + if len(rargs) == 0 || strings.TrimSpace(rargs[0]) == "" || rargs[0] == "-" { break } } diff --git a/command_test.go b/command_test.go index 00a361e151..373fc97257 100644 --- a/command_test.go +++ b/command_test.go @@ -695,10 +695,10 @@ var defaultCommandSubCommandTests = []struct { {"", "carly", "foobar", true}, {"", "jimmers", "foobar", false}, {"", "jimmers", "", true}, - //{" ", "jimmers", "foobar", false}, + {" ", "jimmers", "foobar", false}, {"", "", "", true}, - //{" ", "", "", true}, - //{" ", "j", "", true}, + {" ", "", "", false}, + {" ", "j", "", false}, {"bat", "", "batbaz", true}, {"nothing", "", "batbaz", true}, {"nothing", "", "", false}, @@ -754,10 +754,10 @@ var defaultCommandFlagTests = []struct { {"", "--carly=derp", "foobar", true}, {"", "-j", "foobar", true}, {"", "-j", "", true}, - //{" ", "-j", "foobar", true}, + {" ", "-j", "foobar", true}, {"", "", "", true}, - //{" ", "", "", true}, - //{" ", "-j", "", true}, + {" ", "", "", false}, + {" ", "-j", "", false}, {"bat", "", "batbaz", true}, {"nothing", "", "batbaz", true}, {"nothing", "", "", false}, From b34e3bd2e77d43ab0f0e54228119a595eb08b672 Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Fri, 18 Oct 2024 21:47:20 -0400 Subject: [PATCH 7/7] Fix tests --- command_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command_test.go b/command_test.go index 373fc97257..dd822c76f2 100644 --- a/command_test.go +++ b/command_test.go @@ -649,7 +649,7 @@ var defaultCommandTests = []struct { {"f", "", true}, {"", "foobar", true}, {"", "", true}, - //{" ", "", true}, + {" ", "", false}, {"bat", "batbaz", true}, {"nothing", "batbaz", true}, {"nothing", "", false},