From 88479abd03115a163d841387aba3b2d21c309f41 Mon Sep 17 00:00:00 2001 From: lihan3238 Date: Sat, 9 May 2026 03:18:14 +0800 Subject: [PATCH 1/4] 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 | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 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 72fa6a6628..c9523704b3 100644 --- a/command_test.go +++ b/command_test.go @@ -5724,3 +5724,46 @@ func TestFlagEqualsEmptyValue(t *testing.T) { assert.Equal(t, []string{"positional"}, args) }) } + +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 e23258af8aad1512e190607a187278b82767b1c6 Mon Sep 17 00:00:00 2001 From: lihan3238 Date: Tue, 12 May 2026 17:24:21 +0800 Subject: [PATCH 2/4] 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 c9523704b3..19e7149ebd 100644 --- a/command_test.go +++ b/command_test.go @@ -5767,3 +5767,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 ddafb58402429d0d15cb4af2e68b1bf194289f2a Mon Sep 17 00:00:00 2001 From: lihan3238 Date: Tue, 12 May 2026 18:42:30 +0800 Subject: [PATCH 3/4] 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 19e7149ebd..7337a356fe 100644 --- a/command_test.go +++ b/command_test.go @@ -5795,3 +5795,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 f05c7f1dcb02f74102aeb34edec719e07e05a1bf Mon Sep 17 00:00:00 2001 From: lihan3238 Date: Sat, 16 May 2026 17:48:31 +0800 Subject: [PATCH 4/4] 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 7337a356fe..4eb30c0b1b 100644 --- a/command_test.go +++ b/command_test.go @@ -5729,6 +5729,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{ @@ -5736,9 +5737,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{ @@ -5755,7 +5754,7 @@ func TestDefaultCommandWithSubcommandFlags(t *testing.T) { }, Flags: []Flag{ &StringFlag{ - Name: "bar", + Name: "value", Required: true, }, }, @@ -5766,6 +5765,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) {