From 0c1ed6f2bc681cf2ac9fdc27eda5351f394be232 Mon Sep 17 00:00:00 2001 From: lihan3238 Date: Sat, 9 May 2026 03:18:14 +0800 Subject: [PATCH 1/6] fix: allow DefaultCommand to handle its own flags When DefaultCommand is set, flags defined on the default subcommand should work even without specifying the subcommand name. Previously, unknown flags were rejected during parent command parsing before the default command routing could occur. Pass unknown flags through as positional args when DefaultCommand is set, allowing the default command to handle them correctly. Fixes #2249 --- command_parse.go | 10 ++++++++++ command_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/command_parse.go b/command_parse.go index 2b9a481df2..518cbc64e3 100644 --- a/command_parse.go +++ b/command_parse.go @@ -199,6 +199,12 @@ func (cmd *Command) parseFlags(args Args) (Args, error) { posArgs = append(posArgs, rargs...) return &stringSliceArgs{posArgs}, nil } + // When DefaultCommand is set, pass unknown flags through as positional args + // so the default command can handle them (fixes #2249) + if cmd.DefaultCommand != "" { + posArgs = append(posArgs, rargs...) + return &stringSliceArgs{posArgs}, nil + } return &stringSliceArgs{posArgs}, fmt.Errorf("%s%s", providedButNotDefinedErrMsg, flagName) } @@ -206,6 +212,10 @@ func (cmd *Command) parseFlags(args Args) (Args, error) { for index, c := range flagName { tracef("processing flag (fName=%[1]q)", string(c)) if sf := cmd.lookupFlag(string(c)); sf == nil { + if cmd.DefaultCommand != "" { + posArgs = append(posArgs, rargs...) + return &stringSliceArgs{posArgs}, nil + } return &stringSliceArgs{posArgs}, fmt.Errorf("%s%s", providedButNotDefinedErrMsg, flagName) } else if fb, ok := sf.(boolFlag); ok && fb.IsBoolFlag() { fv := flagVal diff --git a/command_test.go b/command_test.go index 9d77c12f23..9719793c67 100644 --- a/command_test.go +++ b/command_test.go @@ -5772,6 +5772,7 @@ func TestFlagEqualsEmptyValue(t *testing.T) { }) } + // TestCommand_NoDefaultCmdArgMatchingFlag tests the argument set // of a command which has no default command, and has a flag with // the name of the next argument @@ -5801,3 +5802,46 @@ func TestCommand_NoDefaultCmdArgMatchingFlag(t *testing.T) { require.NoError(t, err) require.Equal(t, &expectedArgs, actualArgs) } + +func TestDefaultCommandWithSubcommandFlags(t *testing.T) { + // Regression test for https://github.com/urfave/cli/issues/2249 + // When DefaultCommand is set, flags defined on the default subcommand + // should work even when the subcommand name is omitted. + cmd := &Command{ + DefaultCommand: "run1", + Commands: []*Command{ + { + Name: "run1", + Usage: "run the main application", + Action: func(ctx context.Context, cmd *Command) error { + if cmd.String("foo") != "bar" { + return fmt.Errorf("expected foo=bar, got %s", cmd.String("foo")) + } + return nil + }, + Flags: []Flag{ + &StringFlag{ + Name: "foo", + Required: true, + }, + }, + }, + { + Name: "run2", + Action: func(ctx context.Context, cmd *Command) error { + return nil + }, + Flags: []Flag{ + &StringFlag{ + Name: "bar", + Required: true, + }, + }, + }, + }, + } + + // Using flag without subcommand name should route to default command + err := cmd.Run(buildTestContext(t), []string{"c", "--foo", "bar"}) + assert.NoError(t, err) +} From 9dc866d2596a68daf52646103d27cd9f78cdbaa9 Mon Sep 17 00:00:00 2001 From: lihan3238 Date: Tue, 12 May 2026 17:24:21 +0800 Subject: [PATCH 2/6] test: add short flag coverage for DefaultCommand flag passthrough Covers the shortOptionHandling path in parseFlags where unknown short flags are passed through when DefaultCommand is set, improving codecov patch coverage from 50% to 100%. --- command_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/command_test.go b/command_test.go index 9719793c67..b88b9f3247 100644 --- a/command_test.go +++ b/command_test.go @@ -5845,3 +5845,31 @@ func TestDefaultCommandWithSubcommandFlags(t *testing.T) { err := cmd.Run(buildTestContext(t), []string{"c", "--foo", "bar"}) assert.NoError(t, err) } + +func TestDefaultCommandWithShortFlag(t *testing.T) { + // Covers the short-flag splitting path in parseFlags when DefaultCommand is set + cmd := &Command{ + DefaultCommand: "run", + Commands: []*Command{ + { + Name: "run", + Usage: "run the app", + Action: func(ctx context.Context, cmd *Command) error { + if cmd.String("foo") != "baz" { + return fmt.Errorf("expected foo=baz, got %s", cmd.String("foo")) + } + return nil + }, + Flags: []Flag{ + &StringFlag{ + Name: "foo", + Aliases: []string{"f"}, + }, + }, + }, + }, + } + + err := cmd.Run(buildTestContext(t), []string{"c", "-f", "baz"}) + assert.NoError(t, err) +} From b0ab9d440ef622ff3f3f3a7c73d76f0b1f32b896 Mon Sep 17 00:00:00 2001 From: lihan3238 Date: Tue, 12 May 2026 18:42:30 +0800 Subject: [PATCH 3/6] test: add TestDefaultCommandWithShortFlagHandling for codecov --- command_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/command_test.go b/command_test.go index b88b9f3247..365cbfd0c6 100644 --- a/command_test.go +++ b/command_test.go @@ -5873,3 +5873,31 @@ func TestDefaultCommandWithShortFlag(t *testing.T) { err := cmd.Run(buildTestContext(t), []string{"c", "-f", "baz"}) assert.NoError(t, err) } + +func TestDefaultCommandWithShortFlagHandling(t *testing.T) { + // Covers the shortOptionHandling for-loop path when DefaultCommand is set + cmd := &Command{ + UseShortOptionHandling: true, + DefaultCommand: "run", + Commands: []*Command{ + { + Name: "run", + Usage: "run the app", + Action: func(ctx context.Context, cmd *Command) error { + if cmd.String("foo") != "baz" { + return fmt.Errorf("expected foo=baz, got %s", cmd.String("foo")) + } + return nil + }, + Flags: []Flag{ + &StringFlag{ + Name: "foo", + Aliases: []string{"f"}, + }, + }, + }, + }, + } + err := cmd.Run(buildTestContext(t), []string{"c", "-f", "baz"}) + assert.NoError(t, err) +} From 7cbecbe92963bf6bcf40d40a601e2ab7d70d5637 Mon Sep 17 00:00:00 2001 From: lihan3238 Date: Sat, 16 May 2026 17:48:31 +0800 Subject: [PATCH 4/6] fix: address review feedback on DefaultCommand flag handling - Only pass unknown flags to DefaultCommand when the entire flag is unknown (index == 0), not when hitting unknown chars mid-split - Move assertion outside action callback with actionExecuted guard - Rename confusing flag name 'bar' to 'value' to avoid ambiguity with the 'bar' value used for the 'foo' flag --- command_parse.go | 2 +- command_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/command_parse.go b/command_parse.go index 518cbc64e3..939ad90b85 100644 --- a/command_parse.go +++ b/command_parse.go @@ -212,7 +212,7 @@ func (cmd *Command) parseFlags(args Args) (Args, error) { for index, c := range flagName { tracef("processing flag (fName=%[1]q)", string(c)) if sf := cmd.lookupFlag(string(c)); sf == nil { - if cmd.DefaultCommand != "" { + if index == 0 && cmd.DefaultCommand != "" { posArgs = append(posArgs, rargs...) return &stringSliceArgs{posArgs}, nil } diff --git a/command_test.go b/command_test.go index 365cbfd0c6..44fdfd6f60 100644 --- a/command_test.go +++ b/command_test.go @@ -5807,6 +5807,7 @@ func TestDefaultCommandWithSubcommandFlags(t *testing.T) { // Regression test for https://github.com/urfave/cli/issues/2249 // When DefaultCommand is set, flags defined on the default subcommand // should work even when the subcommand name is omitted. + actionExecuted := false cmd := &Command{ DefaultCommand: "run1", Commands: []*Command{ @@ -5814,9 +5815,7 @@ func TestDefaultCommandWithSubcommandFlags(t *testing.T) { Name: "run1", Usage: "run the main application", Action: func(ctx context.Context, cmd *Command) error { - if cmd.String("foo") != "bar" { - return fmt.Errorf("expected foo=bar, got %s", cmd.String("foo")) - } + actionExecuted = true return nil }, Flags: []Flag{ @@ -5833,7 +5832,7 @@ func TestDefaultCommandWithSubcommandFlags(t *testing.T) { }, Flags: []Flag{ &StringFlag{ - Name: "bar", + Name: "value", Required: true, }, }, @@ -5844,6 +5843,7 @@ func TestDefaultCommandWithSubcommandFlags(t *testing.T) { // Using flag without subcommand name should route to default command err := cmd.Run(buildTestContext(t), []string{"c", "--foo", "bar"}) assert.NoError(t, err) + assert.True(t, actionExecuted, "expected run1 action to be executed") } func TestDefaultCommandWithShortFlag(t *testing.T) { From 9473f04ac7746ec0a3c0007a2e4428d1a5eeb942 Mon Sep 17 00:00:00 2001 From: lihan3238 Date: Sat, 16 May 2026 23:18:04 +0800 Subject: [PATCH 5/6] test: add actionRun guard to short flag tests per review feedback --- command_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/command_test.go b/command_test.go index 44fdfd6f60..47042065bc 100644 --- a/command_test.go +++ b/command_test.go @@ -5848,6 +5848,7 @@ func TestDefaultCommandWithSubcommandFlags(t *testing.T) { func TestDefaultCommandWithShortFlag(t *testing.T) { // Covers the short-flag splitting path in parseFlags when DefaultCommand is set + actionRun := false cmd := &Command{ DefaultCommand: "run", Commands: []*Command{ @@ -5855,6 +5856,7 @@ func TestDefaultCommandWithShortFlag(t *testing.T) { Name: "run", Usage: "run the app", Action: func(ctx context.Context, cmd *Command) error { + actionRun = true if cmd.String("foo") != "baz" { return fmt.Errorf("expected foo=baz, got %s", cmd.String("foo")) } @@ -5872,10 +5874,12 @@ func TestDefaultCommandWithShortFlag(t *testing.T) { err := cmd.Run(buildTestContext(t), []string{"c", "-f", "baz"}) assert.NoError(t, err) + assert.True(t, actionRun, "expected run action to be executed") } func TestDefaultCommandWithShortFlagHandling(t *testing.T) { // Covers the shortOptionHandling for-loop path when DefaultCommand is set + actionRun := false cmd := &Command{ UseShortOptionHandling: true, DefaultCommand: "run", @@ -5884,6 +5888,7 @@ func TestDefaultCommandWithShortFlagHandling(t *testing.T) { Name: "run", Usage: "run the app", Action: func(ctx context.Context, cmd *Command) error { + actionRun = true if cmd.String("foo") != "baz" { return fmt.Errorf("expected foo=baz, got %s", cmd.String("foo")) } @@ -5900,4 +5905,5 @@ func TestDefaultCommandWithShortFlagHandling(t *testing.T) { } err := cmd.Run(buildTestContext(t), []string{"c", "-f", "baz"}) assert.NoError(t, err) + assert.True(t, actionRun, "expected run action to be executed") } From 1d7246bc623b79ca588690db684851b59e6138bc Mon Sep 17 00:00:00 2001 From: lihan3238 Date: Mon, 18 May 2026 01:07:50 +0800 Subject: [PATCH 6/6] style: fix goimports and gofumpt formatting in command_test.go --- command_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/command_test.go b/command_test.go index 47042065bc..f186573a39 100644 --- a/command_test.go +++ b/command_test.go @@ -5772,7 +5772,6 @@ func TestFlagEqualsEmptyValue(t *testing.T) { }) } - // TestCommand_NoDefaultCmdArgMatchingFlag tests the argument set // of a command which has no default command, and has a flag with // the name of the next argument