Skip to content

fix: reject a parent's Local flag inside a short option group - #2445

Open
fadiroot wants to merge 1 commit into
urfave:mainfrom
fadiroot:fix/short-option-group-local-flags
Open

fadiroot wants to merge 1 commit into
urfave:mainfrom
fadiroot:fix/short-option-group-local-flags

Conversation

@fadiroot

Copy link
Copy Markdown

What type of PR is this?

  • bug

What this PR does / why we need it:

With UseShortOptionHandling, a flag that is not found as a whole is split into single-rune flags. Each of those was looked up with lookupFlag, which walks the whole Lineage() and ignores Local. The normal parse path uses lookupAppliedFlag, which only sees the command's own flags plus the persistent ones inherited from its ancestors.

So with

cmd := &Command{Name: "app", UseShortOptionHandling: true,
    Flags: []Flag{&BoolFlag{Name: "debug", Aliases: []string{"d"}, Local: true}},
    Commands: []*Command{{Name: "sub", Flags: []Flag{&BoolFlag{Name: "x"}}}}}

app sub -xd and app sub -d are accepted and set the parent's debug flag (its Action never runs either, since it is not among the child's applied flags), while app sub --debug is rejected with flag provided but not defined, as TestLocalFlagError asserts. FlagBase.Local documents that a Local flag is not applied to subcommands.

  • command_parse.go: use lookupAppliedFlag for the split single-rune flags, same as the whole-flag path.
  • command_test.go: TestCommand_UseShortOptionHandling_LocalFlagNotInherited covers -xd, -dx, -d (all rejected) and checks the subcommand's own short flags still work in a group. It fails on main and passes with this change; the full suite passes.

Note: #2443 also touches this loop (pre-checking every flag of a group). The two changes are independent; if that lands first this is a one-identifier rebase.

Release Notes

A parent's `Local` flag is no longer accepted by subcommands when given inside a short option group.

With UseShortOptionHandling, a flag that is not found as a whole is split
into single-rune flags, and each of those was looked up with lookupFlag,
which walks the whole command lineage and ignores Local. The normal path
uses lookupAppliedFlag, which only sees the command's own flags plus the
persistent ones inherited from its ancestors. So `app sub -xd` (and `app
sub -d`) silently set a parent's `-d` flag marked Local: true, while
`app sub --debug` is rejected with "flag provided but not defined".

Use lookupAppliedFlag for the split flags as well.
@fadiroot
fadiroot requested a review from a team as a code owner September 24, 2026 12:53

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants