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

Redesign completion system #1484

Closed
davidism opened this issue Mar 2, 2020 · 7 comments · Fixed by #1622
Closed

Redesign completion system #1484

davidism opened this issue Mar 2, 2020 · 7 comments · Fixed by #1622
Labels
f:completion feature: shell completion
Milestone

Comments

@davidism
Copy link
Member

davidism commented Mar 2, 2020

I've been trying to review #1403 (comment), which adds type-based completions, and it made me realize just how messy completion is, both within Click and with how the shells implement and document their systems.

Right now, a project would provide a much better user experience right now by providing their own handwritten completion script rather than using Click's. Due to its dynamic nature, Click probably won't be able to generate an equivalent script, but I think we can at least get closer.

Because Click only returns completion values, the shell has no metadata about what's actually being completed. This means we keep having to re-implement things that the shell should be doing, such as escaping special characters, adding spaces (except for directories), sorting, etc. And if the user wants to provide their own completions, they have to remember to do all that too.

There's no reason we just have to return completions. We already support returning descriptions, presumably we could extend that more. Wouldn't it be cool if Click could indicate to the completion script that it should use other functions that Bash or ZSH provide?

@davidism davidism added the f:completion feature: shell completion label Mar 2, 2020
@tiangolo
Copy link
Contributor

tiangolo commented Mar 12, 2020

I'm interested in this refactor and I could probably help. 🤓

I recently started a PR for click-completion: click-contrib/click-completion#34, to fix user-provided completions. And I got to see how it ends up re-implementing a lot of the code from Click itself, it's not exactly the same, but it's quite similar, and newer features/additions to Click are not there (yet). I imagine at some point the two codebases started to diverge and it became every time more difficult to re-sync click-completion. It would be great to have the basic stuff in Click directly.

For example, adding basic PowerShell completion support for Click would probably be quite straightforward given there are scripts for Bash, Zsh, and Fish.

I have been playing with it and it's fairly straightforward for PowerShell Core (6 and 7), and it can be installed/tested in Linux and others. Unfortunately, Windows comes with PowerShell 5 by default (although a newer version can be easily installed). For PowerShell 5 it seems the only way is with the (a bit more complex) approach done in click-completion.


One thing I've seen is that the current completion is relatively coupled with how Bash completion works, with the COMP_WORDS and COMP_CWORD variables, etc, and works around it for the other shells.

I think this might make the shell scripts/templates a bit more complex, and so, probably a bit more difficult to maintain.

I think it could be easier if the shell scripts (in each shell language) to handle completion for each shell are as simple as possible, and most of the glue code is done in Python instead of in each shell language (inside of a Python string).

The invocation with _FOO_COMPLETE= would be the same, but the generated shell script could be different, possibly not using COMP_WORDS and COMP_CWORD for all shells. And each shell would have its own Python function, as with do_complete_fish().

For example, if that's acceptable, adding basic PowerShell completion would be quite straightforward. But wrapping it around COMP_WORDS and COMP_CWORD would require quite some more PowerShell-fu (and I don't even know how to do it).

Just my 2 cents.


Another independent idea, currently click-completion monkeypatches and re-writes some parts of Click. Maybe some sections could be made injectable. Or some parts could be extracted in functions so that it's easier to monkeypatch reducing the amount of re-implementation code for custom solutions that don't want to use the defaults in Click.

For example, click-completion is re-implementing the full get_choices just to replace a small section that checks for options. If that check was in a function, click-completion could monkeypatch just that function.

Another example, I'm currently monkeypatching click-completion's get_choices with a custom function that then re-uses Click's get_choices instead of click-completion's, so that when click-completion monkeypatches Click's functions it uses my own get_choices. 😵 🤷‍♂️

That's in Typer CLI: https://github.com/tiangolo/typer-cli/blob/master/typer_cli/main.py#L149

@davidism
Copy link
Member Author

I think it could be easier if the shell scripts (in each shell language) to handle completion for each shell are as simple as possible, and most of the glue code is done in Python instead of in each shell language (inside of a Python string).

This is basically the current situation which I think we should move away from. Right now we provide fairly simple shell scripts that call out to Click and expect a newline delimited list of completions back. For the way Bash completion works, this is sort of fine. But for ZSH, this completely breaks path handling, for example, because selecting a menu completion of partial paths always adds a space, even if you type / to continue the path, unless you're specifically using the _files helper.

Instead I'd like to be able to take advantages of the features of any given shell. For example, instead of returning a list of paths, we should be telling the ZSH script to use the _files helper, and telling the Bash script to use -A files.


Here's a design I've been thinking about that could accomplish this and would hopefully be reasonable to maintain in the future.

Right now, parameters take an autocompletions function to provide custom completions. #1403 proposed a completions method on types, so you can either provide parameter-specific completions or get a default implementation based on the parameter's type; I still want to add this.

As you suggested, we should completely split the handlers for each shell, so there is a dedicated handler for each that can take shell-specific environment variables and produce shell-specific results.

Instead of only returning a list of completions, the parameter completion methods should now be able to return something that indicates a different shell completion behavior. For example, the File type could return a special string "files" instead of a list, and whatever completion system is handling it can decide what to do.

Here's a very rough outline of an API for this. What would be nice about splitting these up into classes is that we could provide a way for an extension to plug in support for a new shell, such as PowerShell.

class ShellComplete:
    def __init__(self, complete_var):
        # get shell-specific environment variables
        # set instance variables

    def source(self):
        # return bash completion script

    def complete(self):
        # get completion response from parameter/type
        # handle special commands like "files" in shell-specific way
        # bash doesn't quote special characters for custom responses, so do quoting here, etc.
        # output command type (plain, files, etc.)
        # output any other information for the command type, such as custom completions

class BashComplete(ShellComplete):
    ...

The final piece of this is that the shell scripts would need to get more complicated, as they would now use the first line of the response to decide what to do. For example, ZSH decides to call _files.


We might also consider sending the "version" of the script back to Click, so that it can show a warning if we make more incompatible changes to the completion system in the future. "Completion definition outdated, consult {prog_name} docs to generate an updated completion script."

@tiangolo
Copy link
Contributor

Get it, yep, that looks great. 👍

@davidism
Copy link
Member Author

davidism commented Jul 3, 2020

Here's an outline of the current and proposed completion workflow.

Setup (won't change):

  1. example.py defines the Click app.
  2. setup.py defines a console script entry point.
  3. Install the app and entry point: pip install -e ..

Registering the completion script (current):

  1. https://click.palletsprojects.com/en/7.x/bashcomplete/?highlight=completion#activation calls the app with a special env var requesting the script
  2. Command.main() sees the request for the completion script, goes into completion script mode.
  3. click._bashcomplete outputs a script for the given shell.

New implementation:

  1. Call app with a special env var.
  2. Go into completion script mode.
  3. click.shell_completion (new module) picks a completion class to use for the given shell.
  4. The completion object returns a string with a script.
  5. The script is echoed.

Triggering completion (won't change):

  1. example <tab> tells the shell to look for the completion script registered for example.
  2. The shell calls the completion script with some information about the command line.
  3. The script calls the application with various env vars set.

Calculating completions (current):

  1. Command.main() sees the env vars and goes into completion mode
  2. click._bashcomplete starts walking through the click objects, setting up contexts until it gets to the partial value.
  3. Once it sets up the last complete value, it collects command names/option names/choice options/files (sort of) based on the partial next value
  4. If it's zsh, it returns help text, otherwise it strips that off.
  5. The list of completions is echoed.
  6. The completion script sees the output and splits it into values.
  7. A shell command is used to add the values as completions to the shell.

New implementation:

  1. Command.main() sees the env var and goes into completion mode
  2. click.shell_completion (new module) picks a completion class to use for the given shell.
  3. The completion object starts walking through the click options, setting up contexts until it gets to the partial value.
  4. Every click object (command, group, option, param type, etc.) has a shell_complete method.
  5. Once it gets to the last complete value, it calls shell_complete on it with the partial value.
  6. shell_complete will return some sort of metadata with whatever we determine is useful.
  7. The completion object decides how to echo this metadata in a format that the completion script will parse.
  8. For example, might say that metadata is echoed as key value, one per line, then a blank line, then values one per line.
  9. the completion script gets the output, reads the metadata and values, decides what shell commands to issue.

Bash is pretty simple, but zsh and fish can do a lot more. Things that we might return:

  • value type (file/directory, mostly)
  • option help (Click already does this for zsh)
  • command help (not sure if Click already does this)
  • groups (options, arguments, subcommands)

We don't have to do all that now, we mostly want to ensure we have an extensible system that could be extended to send that. Other bonus things we want to do later:

  • Autodetect the shell.
  • Add a version to the script for Click to detect if it needs to be upgraded.

Unfortunately, bash and zsh documentation is detailed while also being impenetrable. That said, here are a bunch of docs links:

@amy-lei
Copy link
Contributor

amy-lei commented Jul 6, 2020

@kx-chen and I will be working on this! 😃

@DEvil0000
Copy link

As a addition to the known issue with file/path completion I have one more areas to improve completion.
When there is nothing typed it should show all possible not yet used options. At the moment for getting option completion you must type the first character of the option to show/complete them.

@davidism
Copy link
Member Author

davidism commented Oct 3, 2020

The new system has been merged. The documentation can be found here for now: https://click.palletsprojects.com/en/master/shell-completion/. Pretty much everything @tiangolo and I discussed is implemented in some way.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
f:completion feature: shell completion
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants