-
-
Notifications
You must be signed in to change notification settings - Fork 768
🐛 Avoid printing default: None
in the help section when using Rich
#1120
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
🐛 Avoid printing default: None
in the help section when using Rich
#1120
Conversation
…s installed Fixes fastapi#465 Improves consitency with typer/core.py TyperArgument.get_help_record, which does not print the default value if it is None.
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 for your PR @mattmess1221! I'll put this on my queue to review in detail and will get back to you 🙏
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 for your work on this, @mattmess1221!
It's true that in Rich mode, the help text prints [default: None]
which is unhelpful, because the argument is still required and the default effectively useless.
When printing the help without Rich, these None
defaults are suppressed with the code in core.py
as you referenced:
Lines 351 to 355 in 74e0923
show_default_is_str = isinstance(self.show_default, str) | |
if show_default_is_str or ( | |
default_value is not None and (self.show_default or ctx.show_default) | |
): |
It makes sense to ensure the same behaviour whether or not we're printing with Rich, and for clarity I agree to copy the structure from core.py
into rich_utils.py
. This also means adding the check for ctx.show_default
which seems to have been missing previously.
I'll leave this to Tiangolo for a final review.
Thanks again!
default: None
in the help section when using Rich
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.
Nice, thank you! 🚀
And thanks for the review @svlandeg 🙌
This will be available in Typer 0.17.2 in the next minutes. 🤓
Related to #465
Improves consistency with
Typer(rich_markup_mode=None)
(TyperArgument.get_help_record), which does not print the default value if it is None.