-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Improve zsh completion #970
Conversation
f97a486
to
d973468
Compare
d973468
to
c88114c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lovely! This all looks good to me. Thank you for improving auto complete support in zsh!
I'll let this bake for a bit as requested. Ping me if I forget about it. :-)
- Use groups with _arguments - Conditionally complete 'negation' options (--messages, --no-column, &c.) - Improve option exclusivity - Improve option descriptions - Improve completion of colour 'whens' - Improve completion of colour specs - Remove some unnecessary work-arounds - Use more idiomatic conventions
6f9bc39
to
d33e208
Compare
I found two or three trivial wording choices that i didn't like, but otherwise i guess this is as OK as it'll get for now. Rebased my subsequent fixes, ready to merge whenever you like. As i was messing with it, though, i noticed that the interaction amongst
Let me know how you feel and i'll do another PR for those. |
@okdana Thanks for doing this! As to your questions... They are good ones. My feeling is that the interaction between As for In the course of working on libripgrep, I'm going to try and revisit the interactions between flags because it's going to get worse with the rise of support for additional output formats (thinking Ackmate and JSON lines). So I guess I'd say sit tight for now and we can try to knock out all that weirdness in one swoop. |
I'd been meaning to update this, and i think it's in a state now where it's much better than it was.
I propose that we give this a few days to sit before merging, just to make sure i haven't messed anything up (it is a decent number of lines changed). It's been working for me so far in zsh 5.3 and 5.5, though. If any zsh users watching the repo care to test, that would be pretty cool.
Full details:
I've updated the function to use spec grouping with
_arguments
. This makes it so much easier (IMO) to read and maintain option exclusions. The down side is that grouping is only available in zsh 5.4+ (released in 2017); on older versions of zsh i simply strip the groups out. This makes option exclusivity pretty inaccurate on those older versions, but i think it's worth it — it's not like it breaks anything, it just continues to show options that might not be useful with options you've already suppliedI mentioned previously that i wasn't sure whether to complete rarely used 'negation' options like
--messages
and--no-fixed-strings
. I decided i would complete them, but only if it seems like the user is specifically requesting them, or if they've enabled a style requesting that they always be completed. So if you dorg --
Tab you won't get the million--no-whatever
options, but you will if you dorg --n
Tab. To be clear, this only affects the (mostly newer) options that were added solely to negate the effects of other options; stuff that's more generally useful like--no-heading
and--no-mmap
is still completed unconditionallyI've improved the accuracy of option exclusivity (for zsh 5.4+)
I've improved several option/optarg descriptions
I've improved completion of colour 'whens' (
always
,never
, &c.) — they now have descriptionsI've improved completion of colour specs, in particular the handling of the colons. This was the biggest irritation for me with previous iterations of the function tbh
I've removed some unnecessary work-arounds
I've generally replaced some uncommon/undesirable constructs by more idiomatic ones (
print
, quoting stuff, tag names,ret
to support users withprefix-needed
off, &c.)ETA: The CI script failed on some of the AppVeyor platforms because they had an older version of zsh than i expected. Hopefully fixed.