-
-
Notifications
You must be signed in to change notification settings - Fork 824
🐛 Ensure an optional argument is shown in brackets, even when metavar is set
#1409
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
Conversation
📝 Docs previewLast commit b482a50 at: https://a6d30ef0.typertiangolo.pages.dev Modified Pages |
| assert "[OPTIONS] ✨username✨" in result.output | ||
| assert "Usage: main [OPTIONS] [✨username✨]" in result.output | ||
| assert "Arguments" in result.output | ||
| assert "✨username✨" in result.output |
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.
Note that this second assert was always redundant, as L18 would already fail if this wasn't the case.
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.
Thank you! 🚀
This will be available in Typer 0.20.1, released in the next few hours. 🎉
As stated in the docs, currently when an argument has a default and is thus not required (e.g. tutorial003.py), the help string shows the argument name between brackets when printing the help string:
However, when we use
metavarto set a synonym for this argument, then suddenly it doesn't appear between brackets anymore, e.g. tutorial006.pyThis PR makes that consistent:
Follow-up work
I'd love feedback on this, because I'm looking into a few more cases where
metavarprocessing seems to be inconsistent, especially when combined with Rich formatting.I also have questions around adding the
type_varstuff at L394-403 incore.py, and whether that's actually used/documented anywhere in Typer. Those are probably follow-up discussions though, and this PR tries to single out a first fix before moving on.[Update]: now also created #1410, which I would review after this one.