Fix Zsh completions with colons #2846
Conversation
|
Thanks for taking the time to submit this PR. We will review this. |
866b128 to
11a1428
Compare
|
Rebased onto and retargeted to |
|
Apologies for the delay. This looks pretty good, but it's a bit hard to review. We'd ideally need some way to create tests for autocomplete. Does anyone know how we could possibly trigger autocomplete? If so, we might be able to use scrut to create a test for it. I think it should be fine with just a note in the changes and if that's done, we can merge this afaik. |
src/click/shell_completion.py
Outdated
| """""" | ||
| # See issue #1812 and issue #2703 for further context. (TL;DR: | ||
| # colons sometimes need special handling.) | ||
| # | ||
| # Items *with* help are registered with zsh's `_describe`, which | ||
| # splits off the help/description after the first colon. So | ||
| # escape all colons in item.value so that they arrive intact. | ||
| # | ||
| # Items *without* help are registered with zsh's `compadd`, | ||
| # which does no colon handling. So do *not* escape colons in | ||
| # item.value, lest the completions themselves ultimately get | ||
| # mangled. | ||
| # | ||
| # Finally, there is no functional difference between using an | ||
| # empty help and using "_" as help. |
There was a problem hiding this comment.
Clean this up a tad perhaps? It'd be nice to have this explained in-line if possible.
There was a problem hiding this comment.
Commentary rewritten in the current iteration of this PR.
11a1428 to
6111c55
Compare
|
Rebased onto
Can't help you there, sorry. To test my submission, I did write a small < 50-line click application that completed filenames manually, sometimes adding help texts, sometimes even with colons, and then interactively tab-completed arguments in a |
791aad9 to
22718de
Compare
|
I've rebased this on stable. Then I've removed the ZSH template check, I'm not sure why we're checking the output of the ZSH completion here. We only really care about the output, don't we? Let me know in case I missed something. Otherwise this seems good to land if CI is good. |
Resolves pallets#2846. Co-authored-by: Andreas Backx <andreas@backx.org>
The Zsh completion script uses different primitives to load completion items into Zsh depending on whether a completion item has a description or not.
_describefunction is used, which expects item plus description pairs as values, separated by a colon. In turn, any colon within the item must be escaped.compaddbuiltin is used, which expects literal items, without escaping. Actually escaping colons in the item will corrupt the item.click's built-in Zsh completion class does not escape colons in the item. So it only handles the second case correctly, not the first; see e.g. #2703. This PR fixes this by choosing a suitable serialization, for each item separately, according to the abovementioned escaping requirements.Since at its core this is an issue of two systems not agreeing on the format for exchanging data, and since we principally control both systems, we could fix this on either side. However, because the Zsh completion script is typically already installed in the user's shell, which is both harder to detect within
clickand harder to change or influence, we opt to change theclickside instead, even if it the serialization needs to be more irregular.Resolves #2703
CHANGES.rstsummarizing the change and linking to the issue... versionchanged::entries in any relevant code docs.