Add zsh completion to svcat #2023

kikisdeliveryservice · 2018-05-08T18:54:39Z

closes: #1912

This is my first PR for svcat.

Issue 1912 requires two steps to close:

1. bump spf13/cobra to 0.0.1 (or above)
2. add zsh completion - zsh completion for svcat is added

carolynvs

I think we can get cobra to do even more of the work for us. Give it a try and let me know if that works.

Then let's add a test for zsh to this test function:

https://github.com/kubernetes-incubator/service-catalog/blob/6c3b534ef42f177f79aff53e943c1378cccae255/cmd/svcat/svcat_test.go#L151

touch cmd/svcat/testdata/output/completion-zsh.txt
Add a test case for zsh to that function, and have it write the output to the file you created in step 1.
Run the tests and tell it to update the output file from step 1 with the output. go test ./cmd/svcat -update.

This follows the "golden file" test pattern that you sometimes see in Go. The test's output is compared against the contents of that golden file in the output directory, and the test fails if they don't match. To make it easier to manage the golden files, we have a -update flag for our tests that will update the golden file to match the output from the test run, instead of failing the test.

carolynvs · 2018-05-15T15:44:30Z

cmd/svcat/completion/completion_cmd.go

@@ -117,3 +119,154 @@ func (c *completionCmd) Run() error {
 func runCompletionBash(w io.Writer, cmd *cobra.Command) error {
 	return cmd.Root().GenBashCompletion(w)
 }
+
+func runCompletionZsh(out io.Writer, cmd *cobra.Command) error {


Can we replace this function's body with this instead?

return cmd.Root().GenZshCompletion(w)

Turns out that the workaround you have is the best way at the moment, cobra is still working on getting the zsh functionality 100%. 👍

I would recommend moving the entire function to it's own file, like completion_cmd_zsh.go

well, meh... the file only doubles and it is ~250 likes. I take it back.

kikisdeliveryservice · 2018-05-15T21:26:16Z

Hi @carolynvs background: I wasn't able to get the GenZshCompletion() working a few iterations back so I ended up implementing the kubectl / helm way (which does indeed work).

As requested I have re-implemented using cobra's GenZshCompletion. It doesn't seem to be working for me still but you can pull and try it. We can always revert back to the previous change if you confirm using GenZshCompletion() is no good.

carolynvs

LGTM once you confirm that you've tested this out with a zsh shell and the following works?

svcat v<TAB> completes to svcat version
svcat get b<TAB> completes to svcat get bindings.

kikisdeliveryservice · 2018-05-16T18:56:07Z

@carolynvs

yes!
svcat get b actually has two options: bindings and brokers. so can confirm:
svcat get b --> shows on next line: bindings brokers
svcat get bi --> svcat get bindings
svcat get br --> svcat get brokers

carolynvs

LGTM

Thanks for your patience in getting this figured out and implemented! I had no idea when I created the issue that it was going to require all that extra workaround logic. Great job! 👍

This reverts commit 372d60a.

teaguecole · 2018-05-17T20:30:14Z

I just cloned her branch and it is glorious. The only thing, and I dont think it's a big deal is that you can't complete svcat help... but the svcat command with no arguments is the exact same thing so I give this my LGTM (even though I don't have those powers). TL;DR tested all the options and they work (minus help)

kikisdeliveryservice · 2018-05-17T21:21:29Z

Thanks for testing @teaguecole

As a note, I don't believe that autocomplete for help is available for any of the shells for any of the clis that svcat zsh tracks (including svcat bash & kubectl bash/zsh), which makes sense to me given the purpose of autocomplete.

arschles · 2018-05-17T22:38:46Z

LGTM, but I'll let folks from another org give the final LGTM label

n3wscott · 2018-05-18T15:37:37Z

cmd/svcat/completion/completion_cmd.go

@@ -117,3 +119,154 @@ func (c *completionCmd) Run() error {
 func runCompletionBash(w io.Writer, cmd *cobra.Command) error {
 	return cmd.Root().GenBashCompletion(w)
 }
+
+func runCompletionZsh(out io.Writer, cmd *cobra.Command) error {


I would recommend moving the entire function to it's own file, like completion_cmd_zsh.go

n3wscott · 2018-05-18T15:38:20Z

cmd/svcat/completion/completion_cmd.go

@@ -117,3 +119,154 @@ func (c *completionCmd) Run() error {
 func runCompletionBash(w io.Writer, cmd *cobra.Command) error {
 	return cmd.Root().GenBashCompletion(w)
 }
+
+func runCompletionZsh(out io.Writer, cmd *cobra.Command) error {


well, meh... the file only doubles and it is ~250 likes. I take it back.

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 8, 2018

MHBauer added the in-progress label May 8, 2018

carolynvs self-requested a review May 8, 2018 18:58

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 8, 2018

carolynvs suggested changes May 15, 2018

View reviewed changes

kikisdeliveryservice force-pushed the bug1912 branch from a548181 to e88619e Compare May 16, 2018 02:46

carolynvs approved these changes May 16, 2018

View reviewed changes

carolynvs changed the title ~~WIP : Add zsh completion to svcat~~ Add zsh completion to svcat May 16, 2018

carolynvs removed the in-progress label May 16, 2018

carolynvs approved these changes May 16, 2018

View reviewed changes

carolynvs added the LGTM1 label May 16, 2018

kikisdeliveryservice added 15 commits May 17, 2018 12:44

initial step to bump spf13/cobra to 0.0.1

a169ef5

undo gopkg.lock change

e338016

bump cobra version to 0.0.1

e2d7df4

bump cobra version to 0.0.2

68326a2

update vendor files

7d66f9c

update vendor/ spf13/pflag & update Gopkg.lock with new version

e3888bb

update spf13/pflag vendor files

d60a5f1

update completion-bash test data to reconcile with spf13/cobra upgrade

4dc9b92

add zsh completion skeleton

6f965b4

add zsh completion for svcat

3cc97f4

fix formatting of completion_cmd.go

d6c9191

changes to conform with linter

0811d13

update existing test files to pass with zsh completion

3f2b8a8

implement using cobra's genzshcompletion

ccabc60

Revert "implement using cobra's genzshcompletion"

fdfc685

This reverts commit 372d60a.

kikisdeliveryservice added 5 commits May 17, 2018 12:52

add test for zsh completion

65e8422

update output with zsh

ba0b286

formatting

1cbb6cf

update golden files

7f129b0

rebase to resolve gopkg.lock: dep inputs-digest hash conflict

e06ea1b

kikisdeliveryservice force-pushed the bug1912 branch from e88619e to e06ea1b Compare May 17, 2018 19:56

n3wscott approved these changes May 18, 2018

View reviewed changes

n3wscott added the LGTM2 label May 18, 2018

carolynvs merged commit 2fd20f3 into kubernetes-retired:master May 18, 2018

kikisdeliveryservice deleted the bug1912 branch May 23, 2018 06:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add zsh completion to svcat #2023

Add zsh completion to svcat #2023

kikisdeliveryservice commented May 8, 2018 •

edited

Loading

carolynvs left a comment

carolynvs May 15, 2018

carolynvs May 16, 2018

n3wscott May 18, 2018

n3wscott May 18, 2018

kikisdeliveryservice commented May 15, 2018 •

edited

Loading

carolynvs left a comment

kikisdeliveryservice commented May 16, 2018

carolynvs left a comment

teaguecole commented May 17, 2018

kikisdeliveryservice commented May 17, 2018

arschles commented May 17, 2018

n3wscott May 18, 2018

n3wscott May 18, 2018

Add zsh completion to svcat #2023

Add zsh completion to svcat #2023

Conversation

kikisdeliveryservice commented May 8, 2018 • edited Loading

carolynvs left a comment

Choose a reason for hiding this comment

carolynvs May 15, 2018

Choose a reason for hiding this comment

carolynvs May 16, 2018

Choose a reason for hiding this comment

n3wscott May 18, 2018

Choose a reason for hiding this comment

n3wscott May 18, 2018

Choose a reason for hiding this comment

kikisdeliveryservice commented May 15, 2018 • edited Loading

carolynvs left a comment

Choose a reason for hiding this comment

kikisdeliveryservice commented May 16, 2018

carolynvs left a comment

Choose a reason for hiding this comment

teaguecole commented May 17, 2018

kikisdeliveryservice commented May 17, 2018

arschles commented May 17, 2018

n3wscott May 18, 2018

Choose a reason for hiding this comment

n3wscott May 18, 2018

Choose a reason for hiding this comment

kikisdeliveryservice commented May 8, 2018 •

edited

Loading

kikisdeliveryservice commented May 15, 2018 •

edited

Loading