-
-
Notifications
You must be signed in to change notification settings - Fork 242
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
Fix completions broken since click>=8 #473
base: master
Are you sure you want to change the base?
Conversation
Having some other issues with completion:
so I'll mark this WIP for now |
Thanks you for this @voidus 🙏 Keep it up 💪 |
Okay this was a bit of a ride, I ended up mostly rewriting the completion. I started implementing it while respecting the multi-word features (that is, being able to enter Finally, I ended up ignoring it, because it was not consistently supported anyways. But that's not the point here. It would be great if you could have a look and let me know what you think, I ended up changing more than I planned. |
squashed the two commits |
@jmaupetit could you have a look please? |
@voidus @jmaupetit any chance to get this in? |
I also added this patch to https://github.com/nixos/nixpkgs so I have it installed cleanly, so I'm not in a personal rush but I'm very much up to working in any requested changes. |
They changed the api and the were still calling the functions using the old api style. Sadly, the new api doesnt make it easy to test. I have tried for a while but couldnt get anywhere, so now I tested it manually.
43ad061
to
9040eeb
Compare
Rebased on main, no change was needed |
I've just downloaded this locally and it looks like it's fixing the problems described in #477. |
Works fine (and finally again) for me. Completions are hard, but very useful to me. Please include in upstream! |
@jmaupetit Any chance to review and merge this in? |
If jmaupetit isn't available, maybe @willdurand could you have a look? This seems to affect multiple people. |
I will add myself to the list of affected people who would like to see this merged. Happy New Year! |
@voidus Thank you very much! Used your version of watson.zsh-completion and the completion is working again! Please finish the PR. UPD: Works with errors. But works. Waiting for this PR |
FYI, I've stopped using watson. I'd still be happy to tweak this PR if it needs additional work but I won't push for it to be merged. |
I’ve been using this branch directly, since it isn’t merged into master yet. I’ve done this for months now and noticed no problems with it.
Off-topic, but I’m curious: did you switch to some other tool for time tracking? Watson does its job, but I’d rather use something that is actually maintained. |
Quoting click changelog for 8.0.0
I think this fix warrants a 2.1.1 release, since missing completions are kind of a big deal for a lot of people. (Shamelessly generalizing from my experience)