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

Add autocompletion callback for Options #27

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

chrisjsewell
Copy link

As explained here in the Click documentation, and implemented here: https://github.com/pallets/click/blob/93b1699cde5fbafe8a237f8f0d21c8f687b78f2f/click/_bashcomplete.py#L185, click Parameters have an autocompletion argument that is currently not supported in click-completion.
This small addition rectifies that :)

Note: `Choices` take precedant over autocompletions
click_completion/core.py Outdated Show resolved Hide resolved
@@ -110,7 +110,10 @@ def get_choices(cli, prog_name, args, incomplete):
break
choices = []
if optctx:
choices += [c if isinstance(c, tuple) else (c, None) for c in optctx.type.complete(ctx, incomplete)]
choices += [c if isinstance(c, tuple) else(c, None) for c in optctx.type.complete(ctx, incomplete)]
if not choices and hasattr(optctx, 'autocompletion') and optctx.autocompletion is not None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

so it means that the choices provided by the type have the precedence over the autocompletion provided by the user.
Shouldn't it be the reverse? Or maybe merge both?

Copy link
Author

Choose a reason for hiding this comment

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

That's the order in the click code: https://github.com/pallets/click/blob/93b1699cde5fbafe8a237f8f0d21c8f687b78f2f/click/_bashcomplete.py#L172

Realistically I guess you shouldn't be using both. But certainly for click.Choice, I think that should take precedence over the auto-completion, because the input is also validated against the contents of click.Choice, so it would be erroneous to be offering any other completions.

Co-Authored-By: Gaëtan Lehmann <[email protected]>
@Konubinix
Copy link
Collaborator

To summarize what exists for now:

  • in click, in the param is of type choice, complete against the choice, else use the context autocomplete if it exists.
  • in click-completion: use the result of the method complete of the type instance

By chance, the Choice class has a complete method that provided the completion against the choice.

What is counter intuitive in the code of click-completion is that we call choice the result of the method complete. That explains why you confounded with the Choice class.

In my mind, the more sensible is to first call get_user_autocompletions (instezd of rewriting part of it in click-completion) , then add the results of the complete method of the type.

Also, I find getattr(optctx, "autocompletion", None) is not None shorter and more readable than hasattr(optctx, 'autocompletion') and optctx.autocompletion is not None.

@Konubinix
Copy link
Collaborator

I did not follow what was done in the other PR and in click. Is this still relevant?

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

Successfully merging this pull request may close these issues.

3 participants