Closed callahad closed 3 years ago
This is missing unit tests. I'm not entirely certain it's possible to sanely test tab completion in Zsh with anything less than ttyrec. :( Hopefully that's not a blocker.
@callahad could we test the output/side-effects of the functions?
I genuinely don't know how to test the Zsh functions -- the builtins have guards that prevent them from executing outside of a completion context
$ _chruby
_arguments:comparguments:312: can only be called from completion function
On the Bash side... it's really just a single function call to a builtin, compgen
. I guess we could test that compgen
correctly filters values and places them into $COMPREPLY
, but that feels more like testing Bash itself, rather than something specific to this pull request.
Still, your call: if you want tests, I can probably whip up a couple on the Bash side. Just not sure how much value they'd add in this particular case.
Maybe we could use a coproc
and explicitly send in the tabs? http://wiki.bash-hackers.org/syntax/keywords/coproc
_chrubycomplete
?Is this Pull Request blocked by lack of test?
Is there a convention for these to be named like
_chrubycomplete
?
I think it should be _chruby
. Bash by default looks up completion by command name (warning: large file, syntax-highlight not working).
In addition I think it's better to put Bash and Zsh completions in different files. This way package maintainers can link Bash completion to /etc/bash_completion.d/
, and link Zsh completion to /usr/share/zsh/vendor-completions/
, avoiding the problem of compdef
at all.
Hi @callahad are you still interested in this PR? If not I can take it over.
It's all yours :) thank you for picking it up!
On Thu, Feb 6, 2020, 08:22 Franklin Yu notifications@github.com wrote:
Hi @callahad https://github.com/callahad are you still interested in this PR? If not I can take it over.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/postmodern/chruby/pull/297?email_source=notifications&email_token=AAAF5AMFFG2WP7TA45GYJJTRBPCD5A5CNFSM4AXTRL5KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEK6KO6Q#issuecomment-582788986, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAF5AJ7YW32WHY7LRHVI4TRBPCD5ANCNFSM4AXTRL5A .
Fixes #27