Skip to content
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

Determine whether to add a subparser to completion by its presence in _get_subactions() #47

Merged
merged 3 commits into from
Aug 24, 2021

Conversation

jimporter
Copy link
Contributor

@jimporter jimporter commented Aug 23, 2021

This is just a small change to the logic for when to generate completion for a subparser. The new logic is the same that argparse uses for showing a subparser in the list of options (i.e. if you don't pass help to add_parser, that subparser is omitted from your program's overall help message).

This is particularly important for subparsers which pass add_help=False in order to provide their own help implementation. This is something I do in one of my projects. See here and here for an example.

Note: this is a breaking change, since now subparsers with no help kwarg will no longer have shell-completion generated for them. I think this is the correct behavior though, and fixing this is easy enough for users of shtab: just set help to something like None or '' (or even better, add an actual help message!).

@jimporter jimporter changed the title Determine whether to add a subparser to completion by its presences in _get_subactions() Determine whether to add a subparser to completion by its presence in _get_subactions() Aug 23, 2021
@casperdcl
Copy link
Collaborator

what about using help=argparse.SUPPRESS instead?

@jimporter
Copy link
Contributor Author

what about using help=argparse.SUPPRESS instead?

Unfortunately, that results in myprogram --help printing "==SUPPRESS==" for the subcommand's help string. It seems the only way to hide the subcommand from the help listing is to omit the help argument when calling add_parser: https://github.com/python/cpython/blob/ac87b07a10e0ba2834e8de9cf0ea29a40fd882b1/Lib/argparse.py#L1166-L1170

As a further enhancement here, I notice that shtab uses subparser.description for the shell-completion help string. Since we can use the value of _get_subactions() to get the value of help, maybe it would be better to use that instead of the description (which might be too long to show anything useful in the shell).

Another thing I just noticed that I didn't account for is subcommand aliases. I'll write a test for this and fix the patch as needed...

… _get_subactions()

This is the logic argparse uses for showing a subparser in the list of
options, so shtab now matches that behavior. This is particularly important
for subparsers which pass `add_help=False` in order to provide their *own*
help implementation.
Copy link
Collaborator

@casperdcl casperdcl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slight tweaks

shtab/__init__.py Outdated Show resolved Hide resolved
shtab/__init__.py Show resolved Hide resolved
shtab/__init__.py Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Aug 24, 2021

Codecov Report

Merging #47 (b1bf591) into master (76c5ead) will increase coverage by 0.22%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #47      +/-   ##
==========================================
+ Coverage   88.57%   88.80%   +0.22%     
==========================================
  Files           3        3              
  Lines         245      250       +5     
==========================================
+ Hits          217      222       +5     
  Misses         28       28              
Impacted Files Coverage Δ
shtab/__init__.py 92.19% <100.00%> (+0.19%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 76c5ead...b1bf591. Read the comment docs.

@casperdcl casperdcl added enhancement New feature or request and removed help-wanted We need help labels Aug 24, 2021
@casperdcl casperdcl merged commit 1e1d6ef into iterative:master Aug 24, 2021
@casperdcl
Copy link
Collaborator

/tag v1.4.0 1e1d6ef

@jimporter
Copy link
Contributor Author

Thanks for the quick review/merge!

@casperdcl
Copy link
Collaborator

Thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants