gh-88753: Make BooleanOptionalAction's addition of default to help more similar to other actions #27808
Conversation
abadger
force-pushed
the
fix-booleanoptionalaction-help
branch
from
f8a9c88 to
6211917
Compare
Lib/test/test_argparse.py
Outdated
| @@ -3603,9 +3603,10 @@ class TestHelpUsage(HelpTestCase): | |||
| -h, --help show this help message and exit | |||
| -w W [W ...] w | |||
| -x [X ...] x | |||
| --foo, --no-foo Whether to foo | |||
| --foo, --no-foo Whether to foo (default: None) | |||
There was a problem hiding this comment.
IMO showing None here is just as confusing as SUPRESS. Because, I mean, from the tool user's perspective, what does it mean?
There was a problem hiding this comment.
Just like any other argument, I believe conveying the meaning to the user is the responsibility of the description:
--uid The user id to assign to the user. If None, then let the
system assign the next available value. (default: None)
--hide, --no-hide-user Whether to omit the user from the graphical system login.
If None, then use the system default (default: None)
There was a problem hiding this comment.
However, I think this is a good example of why I mentioned that BooleanOptionalArgument adding defaults should be revisited. The argparse architecture gives the programmer the ability to use a formatter_class. If you want to display default values for your arguments but not if they were None, then you can write your own HelpFormatter to do that. Since BooleanOptionalArguments modifies the help string on its own, not using a formatter class, the programmer cannot use argparse's standard features to choose different cases where the default should not be emitted.
Removing this formatting feature from BooleanOptionalArguments gives the programmer the flexibility to choose a formatter that does what they decide is appropriate. Leaving it in means that you'll always be stuck with someone having to work around the choices that BooleanOptionalArguments makes.
There was a problem hiding this comment.
abadger: it may be better then to avoid displaying default value because the author of the help string can integrate it into the help, if he feels it's needed. In other words, when help message is not provided, it will be less confusing; if the author feels it's important enough to explain the effect of special value, they will test the output and see that default value needs to be integrated into the help. In either case, confusion will be avoided.
|
This PR is stale because it has been open for 30 days with no activity. |
|
Suggest a change to docs is worth a mention. |
|
Working on first rebasing this and then changing according to @ambv's suggestion |
abadger
force-pushed
the
fix-booleanoptionalaction-help
branch
3 times, most recently
from
d5fd5a9 to
9d00eca
Compare
abadger
changed the title
abadger
force-pushed
the
fix-booleanoptionalaction-help
branch
from
9d00eca to
7dd047c
Compare
… to other actions. Help for other actions omit the default value if default is SUPPRESS or already contains the special format string '%(default)'. Add those special cases to BooleanOptionalAction's help formatting too. Other actions emit (default: None) when the default value is None. This allows documenting that the unset case is treated differently. Add this functionality to BooleanOptionalAction as well. Fixes https://bugs.python.org/issue44587 so that default=SUPPRESS is not emitted. Fixes https://bugs.python.org/issue38956 as this code will detect whether '%(default)s' has already been specified in the help string.
Signed-off-by: Micky Yun Chan (michiboo): <[email protected]>
abadger
force-pushed
the
fix-booleanoptionalaction-help
branch
from
792082f to
dc4babf
Compare
|
Okay, the original strategy which includes the default value in the help message unconditionally is in the first commit. The second commit changes this to do as @akulakov proposed and takes the default value out of the help message. People can use This is based on @michiboo 's earlier PR. |
Help for other actions omit the default value if default is SUPPRESS or
already contains the special format string '%(default)'. Add those
special cases to BooleanOptionalAction's help formatting too.
Other actions emit (default: None) when the default value is None. This
allows documenting that the unset case is treated differently. Add this
functionality to BooleanOptionalAction as well.
Fixes https://bugs.python.org/issue44587 so that default=SUPPRESS is not
emitted.
Fixes https://bugs.python.org/issue38956 as this code will detect
whether '%(default)s' has already been specified in the help string.
https://bugs.python.org/issue44587
Fixes: #88753