Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Add support for persistent flags #1568

Merged
merged 2 commits into from
Nov 27, 2022

Conversation

dearchap
Copy link
Contributor

@dearchap dearchap commented Nov 6, 2022

What type of PR is this?

(REQUIRED)

  • feature

What this PR does / why we need it:

(REQUIRED)

Long asked for feature to have flags in flexible order

Which issue(s) this PR fixes:

(REQUIRED)

Fixes #427
Fixes #1113

Special notes for your reviewer:

(fill-in or delete this section)

Testing

(fill-in or delete this section)

go test -run=TestPersistentFlag

Release Notes

(REQUIRED)

Flags can be marked as persistent in which case they can appear in subcommands. Flags with the same name in subcommand take precedence over commands defined higher in the hierarchy. Since a flag can have multiple names/aliases it is upto the user to ensure that none of the aliases of persistent flags are defined in subcommands. Even if one of the names is matched in a subcommand none of the aliases are applied to subcommand flag parsing

@dearchap dearchap requested a review from a team as a code owner November 6, 2022 01:39
@abitrolly
Copy link
Contributor

And what happens if local flags conflict with global?

@dearchap
Copy link
Contributor Author

dearchap commented Nov 6, 2022

Local wins over global

@abitrolly
Copy link
Contributor

With no warning during compilation?

@dearchap
Copy link
Contributor Author

dearchap commented Nov 6, 2022

This cant be detected during compile time. At runtime the local flags are applied to flagset first and then we walk higher up the chain . Only those flags marked as persistent up the chain will be applied to current flagset only if none of their names/aliases conflict with local . It is up to the user to make sure that the conflicts dont happen.

Copy link
Member

@meatballhat meatballhat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very exciting! Thank you for doing this work 🎉

app_test.go Outdated Show resolved Hide resolved
cmd/urfave-cli-genflags/main.go Outdated Show resolved Hide resolved
command.go Outdated Show resolved Hide resolved
command.go Outdated Show resolved Hide resolved
@abitrolly
Copy link
Contributor

@dearchap just to clarify. If -v is defined globally as --verbose and in subsommand as --version, will this

go -v subcommand -v

be equal to this

go --verbose subcommand --version

?

@dearchap
Copy link
Contributor Author

dearchap commented Nov 6, 2022

@abitrolly Yes correct.

flag.go Show resolved Hide resolved
command.go Outdated Show resolved Hide resolved
command.go Outdated Show resolved Hide resolved
flag.go Show resolved Hide resolved
@meatballhat meatballhat added this to the Release 3.x milestone Nov 7, 2022
Copy link
Member

@meatballhat meatballhat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉🍕🎷🐻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants