-
Notifications
You must be signed in to change notification settings - Fork 995
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
feat(zsh): Add default completion as fallback #2331
base: main
Are you sure you want to change the base?
Conversation
This is simple, but I am not sure it is correct. I am researching this to review the PR, and not an expert! I don't see in that documentation any mention of the result code of calling There are a huge number of utility functions! I expect there is support for considering multiple or fallback targets somewhere in there... (Styles? Tags? Other functions? So many possibilities!) For instance, I found howto documentation covering |
Thanks for taking the time @shadowspawn to look into it!
I am not an expert either, the missing file completion was just bugging me enough to give it a shot. 😄
I am sure to have seen it somewhere but I am not able to find it again and I also cannot find any documentation on it. It does work but it might be just a quirk. I went ahead and changed the code to make it more explicit. The docs are mentioning "completion after an unrecognised subcommand" as an error condition which I thought does fit here.
I agree that there might be better solutions but it becomes really complicated very quickly. For now I will stick with my simple approach which is sufficient for my use case. Thanks again for your time and pointing out this flaw, I hope the change clears your concern. |
I am happier checking What does If array, this looks a bit strange, and does not work when I try it locally: $ array=(one two three)
$ echo \${#array[@]}
zsh: no matches found: ${#array[@]} I think the array length check might look like: $ array=(one two three)
$ echo ${#array}
3 Using double brackets also gives safer test behaviour, so line might be: if [[ ${#reply} -gt 0 ]]; then The difference between |
Fallback to default completion if no match was found (see https://zsh.sourceforge.io/Doc/Release/Completion-System.html#Completion-Functions for details on `_default`).
bfd56cf
to
d394733
Compare
Thanks again for your time and input @shadowspawn!
As far as I see it, yargs/lib/completion-templates.ts Line 43 in e517318
Did you by any chance keep the backslash? This is only in the template because we are within an (JavaScript) template literal and therefore the dollar sign must be escaped. For me (without the backslash) it looks like this in the shell: % array=(one two three)
% echo ${#array[@]}
3
As far as I understand the zsh documentation both versions can be used interchangeably (and there is even a third version:
Agree, changed accordingly. Thanks for the references! |
I missed being inside (JavaScript) template literal! Apologies. I was just looking at the diff and saw all shell code... |
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.
I gave this a light test comparing zsh and bash completions, and LGTM.
Hi, please consider pulling this PR, thank you |
I just stumbled over this PR while searching for a way to make zsh completion work, when I have an option that expect a filepath value. I simply patched my
I then removed the leading Result is: then it works nicely! All that's needed (after a refresh; e.g. via So this PR certainly seems a nice addition to make zsh completion work regarding filepath completion. Additional infos: |
Fallback to default completion if no match was found (see https://zsh.sourceforge.io/Doc/Release/Completion-System.html#Completion-Functions for details on
_default
).Addresses #2113 and harmonizes the behavior of zsh/bash completion, see
-o default
in the bash completion template:yargs/lib/completion-templates.ts
Line 27 in e517318