Redesign Completion System

[amy-lei]

Resolves#1484
Resolves#780 (Files and paths)

Redesign the Completion to be more extensible:

• Created completion classes that extend ShellComplete
  • Users can write their own class that extends ShellComplete via add_completion_class
• Added a shell_complete function to every Click object (types, params, etc.) that takes in ctx, all_args, and incomplete to determine the metadata in the form (type, value, help)
• Shell scripts use metadata to determine the list of completions
  • dir indicates to shells to default to their own completion system for directories
  • file indicates to shells to default to their own completion system for files
  • plain indicates to the shell to use the value of metadata

See #1622 (comment) for more changes.

[jkowalleck]

Hello @amy-lei & @kx-chen

could you review MLH-Fellowship#28 and merge it into your branch, so #1622 gets updated according to #1529 ?

thanks in advance.

[davidism]

Pushed what I've been working on so far. It looks like it rewrites the entire rewrite, but most of the changes are cosmetic or reorganizing, not rewriting the logic for the most part. I'll rebase this into the original commit, to keep the rewrite history useful for later. Haven't looked at the docs or changelog yet, and the failing tests are Bash not being available in those envs.

• Removed top-level exports.
• Command subclasses call super().shell_complete() to extend the completion results.
• If an option has an autocompletion function, it is preferred over the type completion, rather than the opposite. This allows custom completion to override type completion, which makes more sense.
• Expanded ShellComplete class methods to make it easier to override/introspect specific parts of completion.
• Don't use abc for now, didn't seem to be much use for it. Don't check type in add_completion_class.
• Don't need deepcopy, args.copy() is sufficient.
• Made param names and docstrings consistent.
• Rewrote tests to use objects directly instead of decorators, which makes for more concise code. Added tests for the full source and complete env var invocations.

Things I noticed that I want to address:

• Use a class instead of a tuple for completion items.
• Should an autocompletion function be passed the param? I think we might have discussed this when we introduced it.
• Should autocompletion be renamed to shell_complete? (And stored as self.custom_shell_complete.)
• Extract completion code from BaseCommand.main. Then an extension could potentially replace the entire completion system, including how it's activated.
• Exit if complete_var is present in the env at all, not just if the completion instruction was valid. Doesn't seem to be any reason for the current need to return True or False from completion.
• Completion passes all the args to every function, not just the parsed args for the relevant object. This seems to be so Command.shell_complete can exclude options that have already been given. But it's not particularly accurate right now because it lets other names for the same option through, and can be tripped up if a group and its command both have an option with the same name. ctx.params can be used in most cases, but misses names that aren't exposed, but ctx.args is empty after parsing. Perhaps add a ctc._all_params to track all parsed options?

[kortina]

FYI, on @davidism hunch, I (in the pip installed click, version on my machine, 7.1.2) edited the file at:

$HOME/.pyenv/versions/3.7.0/lib/python3.7/site-packages/click/_bashcomplete.py

And removed the line in question:
https://github.com/pallets/click/blob/master/src/click/_bashcomplete.py#L69

Now it respects my zsh config!

Separately from this PR, do you think I should submit a PR that just removes that line?

[davidism]

I already got it. It was originally introduced as part of #1059, but didn't seem relevant to that, so I removed it.

[kortina]

ahhh, this is so awesome! 🙏🏿🙏🏿

[jkowalleck]

@davidism the latest changes from 2dc3348 to 0595b52 removed some tests.
i am not sure if they also removed some features, but you should have a closer look at it.

[davidism]

The file was renamed, and since it was also rewritten significantly git shows it as deleted (and a new file created). All the behaviors are still tested.

[jkowalleck]

The file was renamed, and since it was also rewritten significantly git shows it as deleted (and a new file created). All the behaviors are still tested.

@davidism
could you point out where the features of test_command_names()
are tested as it was in https://github.com/pallets/click/blob/0daa2e046d4fae1f81f8f4bee194189a81878f1e/tests/test_shellcomplete.py

[davidism]

https://github.com/pallets/click/pull/1622/files#diff-ccad5580eac2cb11fe983477ec9559daR204-R208

The longer test wasn't necessary, as they were all different ways to write the same underlying Click behavior, which is now what is represented in the test.

[jkowalleck]

@davidism thanks for making this clear.
appreciate it.

The longer test wasn't necessary, as they were all different ways to write the same underlying Click behavior, which is now what is represented in the test.
you are right. previously tests were functional. now they are more like unit-tests.