click-contrib / click-repl

Subcommand REPL for click apps
MIT License
215 stars 41 forks source link

Improve completion by delegating to Click #25

Closed bobwhitelock closed 5 years ago

bobwhitelock commented 7 years ago

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 https://github.com/pallets/click/pull/681. Any additional improvements to Click's completion will also become available to us.

untitaker commented 7 years ago

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

bobwhitelock commented 7 years ago

@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 commented 7 years ago

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

bobwhitelock commented 7 years ago

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

untitaker commented 5 years ago

closing due to inactivity