-
-
Notifications
You must be signed in to change notification settings - Fork 672
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
Boolean typer.Option missing type and default value in --help #493
Comments
Further context, command line output from the reproduction steps:
|
I have identified the code in question that is causing the default value block to be missing. But this is a bit of a multi-faceted issue. In Click 8.x, there was a design decision made that flags should not display a default value, The code in question within Click is located here: https://github.com/pallets/click/blob/main/src/click/core.py#L2771 However from a UX perspective it's not ideal that there is no default value set on a flag. How is a user meant to know if toggling that flag sets the underlying variable to True or False? The name of the flag should help with this, but to make it as simple as possible it's much more preferable to show the actual default value. But... the code that actually controls this within Typer is located in core.py, here: https://github.com/tiangolo/typer/blob/master/typer/core.py#L515 This is almost a direct carbon copy of the code above in Click's core module. But click's module has this extra code underneath: if show_default_is_str or (show_default and (default_value is not None)):
if show_default_is_str:
default_string = f"({self.show_default})"
elif isinstance(default_value, (list, tuple)):
default_string = ", ".join(str(d) for d in default_value)
elif inspect.isfunction(default_value):
default_string = _("(dynamic)")
elif self.is_bool_flag and self.secondary_opts:
# For boolean flags that have distinct True/False opts,
# use the opt without prefix instead of the value.
default_string = split_opt(
(self.opts if self.default else self.secondary_opts)[0]
)[1]
elif self.is_bool_flag and not self.secondary_opts and not default_value:
default_string = ""
else:
default_string = str(default_value)
if default_string:
extra.append(_("default: {default}").format(default=default_string)) Whereas Typer has this: # Typer override:
# Extracted to _extract_default() to allow re-using it in rich_utils
default_value = self._extract_default_help_str(ctx=ctx)
# Typer override end
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)
):
# Typer override:
# Extracted to _get_default_string() to allow re-using it in rich_utils
default_string = self._get_default_string(
ctx=ctx,
show_default_is_str=show_default_is_str,
default_value=default_value,
)
# Typer override end
if default_string:
extra.append(_("default: {default}").format(default=default_string)) Typer is doing some shenanigans up in line 148: def _extract_default_help_str(
obj: Union["TyperArgument", "TyperOption"], *, ctx: click.Context
) -> Optional[Union[Any, Callable[[], Any]]]:
# Extracted from click.core.Option.get_help_record() to be reused by
# rich_utils avoiding RegEx hacks
# Temporarily enable resilient parsing to avoid type casting
# failing for the default. Might be possible to extend this to
# help formatting in general.
resilient = ctx.resilient_parsing
ctx.resilient_parsing = True
try:
if _get_click_major() > 7:
default_value = obj.get_default(ctx, call=False)
else:
if inspect.isfunction(obj.default):
default_value = "(dynamic)"
else:
default_value = obj.default
finally:
ctx.resilient_parsing = resilient
return default_value In theory we could do some special handling to override the Click design decision that's causing this. Or we can ask the Click devs if they'd be willing to allow some sort of configuration option in the click.core.Option class to specify that we always want a default value shown for variables defined as a flag (regardless of whether it defaults to True or False). I'm happy to raise an issue over in the Click repository and cross reference this one @tiangolo, I'll wait on your advice before doing so. If you'd like to handle this situation directly within Typer, then I won't raise that ticket with the Click guys. |
Further info that this change was an intended design decision on Click's part: |
Hello- I would like to show interest in fixing this issue. I can confirm on my app that bool options do not show the data type on the command line --help command. |
First Check
Commit to Help
Example Code
Description
TEXT
noted, and a listed default value in the main2 command.This is actually a contradiction of what's currently in the Typer documentation located here: https://typer.tiangolo.com/tutorial/parameter-types/bool/
The docs shows the following image:
It can clearly be seen that at the time of that documentation being created, it was intended for an optional parameter to be specified with a "flag" style, but also retain its default value. Testing has shown that this may be related to the upgrade from Click 7.1.2 -> Click 8.x, but I was unable to pin down whether or not this issue relates to Click itself, or if it's due to Typer's new compatibility code that uses Rich for --help menus rather than Click's help formatter.
Typer 0.4.1 and Click 7.1.2 did not show this issue when tested, but Typer 0.6.1 and Click 8.1.3 did, as does Typer 0.7 and Click 8.1.3.
Ideally the intended result would be to have the type be shown as well. So it would be
--some-param BOOL [Default: False]
or something like that, to keep things consistent with all other types.Operating System
Windows
Operating System Details
No response
Typer Version
0.7
Python Version
3.8.10
Additional Context
No response
The text was updated successfully, but these errors were encountered: