-
-
Notifications
You must be signed in to change notification settings - Fork 671
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
🐛 Ensure that options_metavar
is passed through correctly
#816
base: master
Are you sure you want to change the base?
Conversation
Typer was ignoring this setting when provided to commands. Refer to added test for details.
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.
Thanks!
I have yet to check the test case, but a few quick first comments:
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.
I double checked the unit test and found that the second option (c2
) is already working correctly on master
, while the first one (c1
) fails. Nevertheless, it's good to have both options checked here.
One thing I wonder about though. If you change "OPTIONS"
to "OPTS"
or something different entirely like "STUFF"
, the box with options in the help menu would still be named "Options"
.
$ python script.py --help
Usage: script.py [STUFF]
┌─ Options ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
│ --install-completion Install completion for the current shell. │
│ --show-completion Show completion for the current shell, to copy it or customize the installation. │
│ --help Show this message and exit. │
└─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘
Is this something that should be addressed as well?
I think that's a good idea! But the goal of this PR is to fix a bug that currently prevents I would classify your point here as an enhancement to the API to support configuration of the panel label/title. This could be captured as a new feature request, while merging this PR. |
options_metavar
is passed through correctly
Agreed that we can address this as future work! |
Typer was ignoring this setting when provided to commands. Refer to added test for details.