add SuppressCompleter to skip completion for specific arguments while allowing help text

[dirk-thomas]

Fixes #223.

The first commit adds the SuppressCompleter as described in the feature request.

The second commit adds a method to it which makes the actual decision if the completion should be suppressed. The default implementation is to suppress the completion. But it allows to override the decision making in a subclass and use whatever reasoning to decide to show or suppress the completion.

[dirk-thomas]

I am not sure why I had to remove the trailing space for the existing option --bar?

[codecov-io]

Codecov Report

Merging #224 into master will increase coverage by 0.16%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #224      +/-   ##
==========================================
+ Coverage   87.97%   88.14%   +0.16%     
==========================================
  Files           5        5              
  Lines         632      641       +9     
==========================================
+ Hits          556      565       +9     
  Misses         76       76

Impacted Files	Coverage Δ
argcomplete/__init__.py	91.78% <100%> (+0.09%)	⬆️
argcomplete/completers.py	88.05% <100%> (+0.96%)	⬆️

---

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 e72c5fb...89c3a0e. Read the comment docs.

[kislyuk]

Please create a separate test instead of modifying an existing test.

Please add a docstring to SuppressCompleter. I'll make sure it makes its way into the autodoc (I think that file might not currently be indexed by autodoc).

[kislyuk]

The lack of a trailing space is suspicious. By default we add a trailing space if there is only one completion, but it seems like that shouldn't change in your test. Once you refactor the test case to be its own test case, I'll take a closer look into what's going on there.

[dirk-thomas]

Please create a separate test instead of modifying an existing test.

I thought it would make sense to put the additional check into the similar existing test. I have separated it in 89c3a0e.

Please add a docstring to SuppressCompleter.

Done in 1b3629e

The lack of a trailing space is suspicious.

With the separation the tests I didn't have to change the existing test and also the new test uses the trailing space.

---

I could squash the third commit with the first one as well as the fourth with the second if you would like to keep a clean commit history?

[kislyuk]

Thanks. That's all right, I can squash merge when I'm done reviewing, unless you really want me to rebase merge.

[evanunderscore]

The tests don't show what happens when the user types prog --bar <tab>. What should happen in this case?

[dirk-thomas]

The tests don't show what happens when the user types prog --bar <tab>. What should happen in this case?

For the specific test where one argument is expected after --bar I wouldn't expect any completion (or the default completion of files?). If --bar would be changed to action='action_true' I would expect "normal" completion of the remaining options to be shown. Basically the same as for the argparse.SUPPRESS case.

[dirk-thomas]

@kislyuk @evanunderscore Please let me know if there is anything else I can do for this patch. Thanks.

[kislyuk]

Looks good, I verified that there was no problem with the trailing space. Thanks for your patience @dirk-thomas.

[kislyuk]

Released in v1.9.2.