-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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 DefaultCommand field to App #1388
Conversation
Just had the thought that it might make sense to have the flag name check also check for default command's subcommands (should they exist), to make the whole thing real seamless. |
I dont see a real use case for this. Are there any real world clis which make use of this functionality. |
Also, if the feature is able to be implemented (which, well, it clearly is) then why not implement it? It adds functional value to the package, and doesn't seem to be -- at least imo -- any sort of feature creep. |
@jalavosus Hello and sorry for the delay 😭 ! Are you up for resolving conflicts and getting tests happy? I agree with you regarding examples in the wild and I'd like to move forward with this work 👍🏼 |
See issue #1307 for context.
subcommand names of a default command (should it be set)
057d8a5
to
1dfa982
Compare
@meatballhat Rebased onto main (hence the force push) and fixed the failing test (just needed to run |
@jalavosus Thank you! Are you able to appease codecov, too? 😅 |
I've been trying to, it does not like |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lovely! 🤩
See issue #1307 for context.
What type of PR is this?
What this PR does / why we need it:
Enables a CLI application to run a default
Command
(by name) if no command name is passed as a cli argument.Which issue(s) this PR fixes:
Fixes #1307
Special notes for your reviewer:
Genuinely unsure if I implemented this correctly, but the test cases look sane on my end and they pass.
Release Notes