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

Improve completion by delegating to Click #25

Closed

Conversation

bobwhitelock
Copy link
Contributor

Previously we generated our own completions for available options and commands, in a similar way to how this was is done in the click._bashcomplete module.

However, this module has had various improvements made to it since this was originally done. Rather than duplicate similar code here (and have to do this again in future), this commit changes our completion to
delegate generating the available completions to Click. There is also additional handling around this to still display relevant help alongside the completions, where possible.

The end result is that this commit should make click-repl's completion function as before, except with some additional completions shown where these are relevant, such as the completion of choices added in
pallets/click#681. Any additional improvements to Click's completion will also become available to us.

Previously we generated our own completions for available options and
commands, in a similar way to how this was is done in the
`click._bashcomplete` module.

However, this module has had various improvements made to it since this
was originally done. Rather than duplicate similar code here (and have
to do this again in future), this commit changes our completion to
delegate generating the available completions to Click. There is also
additional handling around this to still display relevant help alongside
the completions, where possible.

The end result is that this commit should make click-repl's completion
function as before, except with some additional completions shown where
these are relevant, such as the completion of choices added in
pallets/click#681. Any additional improvements
to Click's completion will also become available to us.
@untitaker
Copy link
Collaborator

That makes sense, however I wonder why this requires more LoC than the previous version.

@bobwhitelock
Copy link
Contributor Author

@untitaker: It requires more code mainly because we still need to do a lot of what we were doing already, to find the options and commands valid at the current context, in order to show their help alongside the completions. It probably wouldn't be much more if we could just delegate everything to get_choices in click._bashcomplete, but that only produces text for the completions and we need access to the underlying object the completion is for in these cases to get the help text.

Another reason for the increase is that there's longer comments now.

@untitaker
Copy link
Collaborator

Ok, please try to split this up into a few more functions, right now I'm having a hard time comprehending.

@bobwhitelock
Copy link
Contributor Author

Sure, good idea. I'll try to do that soon.

@untitaker
Copy link
Collaborator

closing due to inactivity

@untitaker untitaker closed this Jul 29, 2018
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.

None yet

2 participants