CLI: do not provide command hints during tab-completion #5012
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `verdi` command uses a custom `Group` subclass that overrides the `get_command` method to provide some additional information in case the provided command name does not exist. These hints should help the user spot potential typos by giving a list of existing commands with similar names. However, this feature was also being triggered during tab-completion. For example, typing `verdi comput list` followed by activating tab-completion would result in the error being displayed since `comput` is not a valid command. In this case, one does not want to display the error message with hint at all. The tricky part is that there is no canonical way to distinguish between a normal command execution and a tab-completion event. The best bet is to use the `resilient_parsing` attribute on the `Context` which is set to `True` during tab-completion. Although this attribute was introduced into `click` directly to support auto-completion functionality, the problem is that this is not the only use case for which this flag can be set. It is therefore possible that there is some code path where this flag is set to `True` but it does not actually correspond to a tab-completion event. For now there doesn't seem to be a better solution though and in most cases this approach should work.
Codecov Report
@@ Coverage Diff @@
## develop #5012 +/- ##
===========================================
+ Coverage 80.12% 80.12% +0.01%
===========================================
Files 515 515
Lines 36692 36694 +2
===========================================
+ Hits 29397 29399 +2
Misses 7295 7295
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
giovannipizzi
approved these changes
Jul 6, 2021
There was a problem hiding this comment.
Thanks! I think that's OK - as a note (for myself mostly), I tried and just returning will trigger the 'default' bash completion (files in the current folder). I think that's OK and in line with other codes.
Also, I think using resilient_parsing is OK - maybe there are other places where this will be needed, but I think we're being more resilient than before with this change, so this should be good.
sphuber
added a commit
that referenced
this pull request
Aug 9, 2021
The `verdi` command uses a custom `Group` subclass that overrides the `get_command` method to provide some additional information in case the provided command name does not exist. These hints should help the user spot potential typos by giving a list of existing commands with similar names. However, this feature was also being triggered during tab-completion. For example, typing `verdi comput list` followed by activating tab-completion would result in the error being displayed since `comput` is not a valid command. In this case, one does not want to display the error message with hint at all. The tricky part is that there is no canonical way to distinguish between a normal command execution and a tab-completion event. The best bet is to use the `resilient_parsing` attribute on the `Context` which is set to `True` during tab-completion. Although this attribute was introduced into `click` directly to support auto-completion functionality, the problem is that this is not the only use case for which this flag can be set. It is therefore possible that there is some code path where this flag is set to `True` but it does not actually correspond to a tab-completion event. For now there doesn't seem to be a better solution though and in most cases this approach should work. Cherry-pick: 8e763bb
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #3815
The
verdicommand uses a customGroupsubclass that overrides theget_commandmethod to provide some additional information in case theprovided command name does not exist. These hints should help the user
spot potential typos by giving a list of existing commands with similar
names.
However, this feature was also being triggered during tab-completion.
For example, typing
verdi comput listfollowed by activatingtab-completion would result in the error being displayed since
computis not a valid command. In this case, one does not want to display the
error message with hint at all.
The tricky part is that there is no canonical way to distinguish between
a normal command execution and a tab-completion event. The best bet is
to use the
resilient_parsingattribute on theContextwhich is setto
Trueduring tab-completion. Although this attribute was introducedinto
clickdirectly to support auto-completion functionality, theproblem is that this is not the only use case for which this flag can be
set. It is therefore possible that there is some code path where this
flag is set to
Truebut it does not actually correspond to atab-completion event. For now there doesn't seem to be a better solution
though and in most cases this approach should work.