Improve zsh completion function

[okdana]

This change is related to #532, though the actual problem there had already been fixed by the earlier addition of the hand-written completion function. In going over that function i found several issues/limitations which i've corrected:

• Added missing options (most notably -o)

• Fixed the confusion between --count and --max-count

• Improved the consistency of the wording — The previous descriptions had mixed usage of capitalisation, punctuation, contractions, &c., some of them were copied from the --help output verbatim, and so on

• Added completion for encodings — I didn't want to include a gigantic list of every single individual encoding, so i compressed them all into a few lines by making use of brace expansion. As i noted in the code, it's super hard to read this way, but i doubt that list will be changing any time soon, so i think the brevity is worth it (and you could expand them all by just passing them to printf '%s\n', anyway)

• Added completion for colour specs — Might need revisiting if/when Add extended color support #452 lands

• Added partial completion for type specs — I wanted to make it so that it would complete a list of types if you gave it an include: directive, but i ran up against the limits of my knowledge of zsh's completion functions trying to figure out how to do that whilst making it clear that you can provide a glob or an include directive

I think the structure of this file is kind of weird, honestly — how are future maintainers supposed to intuit which options are 'common'? And is the break-down here even accurate? Is --encoding really more commonly used than -m or -C? I highly doubt it.

If i had written it i probably would have just put all of the options in alphabetical order. I was afraid it'd be bad etiquette for me to do that here, though, so i didn't.

[BurntSushi]

This is awesome work, thank you! If the bash completion were this good, I might actually use it, so color me green with envy. :-)

If i had written it i probably would have just put all of the options in alphabetical order. I was afraid it'd be bad etiquette for me to do that here, though, so i didn't.

That would be fine. I'm not sure about why it's split like that either. The man page is split that way, but --encoding is certainly not considered common, as you say. :-)

[lilianmoraru]

These completion files are generated by clap-rs.
Is it okay to modify these by hand?
I see 2 problems:

• either next time clap-rs generates the completion file, it overwrites everything
• or this completion file is maintained by hand and then it could easily diverge from the flags that are set with clap-rs.

If there are problems with the completion, it should probably be reported to clap-rs, the maintainer is very responsive.

[lilianmoraru]

Ow, sorry, it was written by hand already.
Anyway should probably be reported to clap-rs what exactly the library should do better.

[okdana]

There are several out-standing issues related to zsh completion in the clap repo, many of which are relevant to ripgrep.

The two main issues with the auto-generated function were that it didn't complete operands (pattern and path) correctly, and it didn't account for options that take arguments (except for --color). I think both of those have been reported.

Another (less critical) issue is that it is currently no-where near advanced enough to generate code for complex option-argument completion like what i've added for --colors and --type-add. It looks like they're discussing how they might eventually do that though.

In the mean time, it is bothersome that the completion function can get out-of-synch with the arguments that are actually in the app. Maybe it could be part of the test suite somehow?

I didn't actually try this, but i think running a script like this would be an effective way of ensuring that the options in the --help output match the ones in the completion function:

while read -r option; do
    ./target/release/rg -q -- "${option}" ./complete/_rg || {
        echo "Completion function missing option: ${option}" >&2
        exit 1
    }
done < <(
    ./target/release/rg --help |
    ./target/release/rg -io -- ' (-[a-z0-9]|--[a-z0-9-]+)\b' |
    tr -d ' ' |
    sort -u
)

Not sure how worried anyone is about it, though, since it's not critical functionality. 🤷‍♀️

[BurntSushi]

I'm in favor of adding things to CI to help keep this stuff in sync. However, it must not ever have false negatives (or more precisely, any false negatives must be intermittent). False positives are not ideal obviously, but I could live with them (because it's no worse than what we have now, which is nothing).