-
Notifications
You must be signed in to change notification settings - Fork 454
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(zsh): Fix variable completion #697
Conversation
Awesome! I found a couple issues: This shows variables, but the "variables" header is at the top of the output, not above the variables:
This completes VARIABLE_NAME:
But it adds a This produces an error:
It looks like perhaps 'foo' isn't being passed to the underlying invocation? |
The tests were failing because a new version of clippy introduced a new lint, and because the generated completion scripts were behind the code in |
By convention, one argument can be followed by at most one flag asaik. It's out of my knowledge to pass two argument to one flag. I can remove the completion part of
Can I have a look at your zshrc?
Thanks :) |
Ah, okay. I think in that case it's probably better not to try to complete
Yup, it's here: https://github.com/casey/local/blob/master/etc/zshrc It refers to other files, which are also in that repo, and I think they're all in this folder: https://github.com/casey/local/tree/master/etc/zsh |
Thanks :)
This issue doesn't exist on my side. I'll have a look at your code once I have time. |
Hmm, interesting. I don't want to hold up this diff because of a problem that might only be on my end. I pushed a diff that makes the test pass, but when testing locally I ran into the following issue:
Completes to:
The |
I noticed that this PR introduces a |
I'm afraid not. Probably checking the first argument that matches the name of an existing recipe should work if you'd like to avoid the args checking hack. I'll try that once I have time. |
That would be great. My expectation is that I'll forget to update this script when I add new command line options. If the completion script has fewer features, like not completing certain things that it could, I think that worth it if it doesn't get out of date. |
Hi @casey, about:
Do you think |
This is definitely a good idea, but I don't think it can be implemented without undesirable side-effects with the argument parser, clap, that Just uses. When I set the "value delimiter" for (The reason it fails is because it interprets |
59a2278
to
39ab33a
Compare
I think this has an issue, if any flags or variables are given, recipe name tab completion doesn't work, and it instead shows the usage string for the first recipe:
|
It seems that I've accidentally removed one line from the source code. It should be fine now. |
It looks like there's some state that's persisting between calls to the completion script.
|
Thanks for the effort on debugging. Fixed now. |
Awesome, looks good! |
This PR contains the following fixes:
--set
flagCloses #695