Add autocomplete for bash and zsh in lokoctl #880

knrt10 · 2020-08-28T12:20:43Z

Fixes #18

Signed-off-by: knrt10 kautilya@kinvolk.io

cli/cmd/completion.go

docs/cli/lokoctl_completion_zsh.md

cli/cmd/completion.go

knrt10 · 2020-08-29T08:14:59Z

@invidian, so I think, we should currently go for this until cobra releases it's new updates. Do you agree?

knrt10 · 2020-08-29T08:16:24Z

For 0.4 we can release it with this, if by 0.5 cobra releases their new updates, we can change it to GenZshCompletion

invidian · 2020-08-31T06:23:25Z

For 0.4 we can release it with this, if by 0.5 cobra releases their new updates, we can change it to GenZshCompletion

Or we could do bash completion for 0.4 and zsh completion for the next release? What do you think @johananl? Or we could postpone this completely.

knrt10 · 2020-08-31T06:56:07Z

As mentioned in #880 (comment) we don't know when the next cobra release will be, but if we do want to use GenZshCompletion one alternative is to use, ValidArgs instead of ValidArgsFunction and provide arguments explicitly for both cluster apply and delete.

invidian

Some nits, otherwise LGTM 👍

cli/cmd/completion.go

docs/cli/lokoctl.md

cli/cmd/completion.go

knrt10 · 2020-08-31T11:52:31Z

@johananl could you please review again?

requested again

johananl

Thanks @knrt10.

I'm still seeing small inconsistencies. I've highlighted some of them inline. We also seem to be using multiple variations of "completion/autocompletion" and "script/code". Let's settle on one phrasing 😉

cli/cmd/completion.go

johananl · 2020-08-31T14:13:23Z

cli/cmd/completion.go

+  # Set the lokoctl completion code for zsh to autoload on startup.
+  lokoctl completion zsh > "${fpath[1]}/_lokoctl" && exec $SHELL`
+
+const bashCompDesc = `  ### If running Bash 3.2 that is included with macOS, install bash completion using homebrew.


Why use ###? Is this some sort of convention?

For docs, otherwise, they look wierd, because single # is considered as Header 1

cli/cmd/completion.go

knrt10 · 2020-08-31T14:40:24Z

Updated

invidian

Just one nit, otherwise looks fine to me.

cli/cmd/completion.go

iaguis

Some small comments.

cli/cmd/completion.go

Fixes #18 Signed-off-by: knrt10 <kautilya@kinvolk.io>

iaguis

lgtm

invidian

LGTM

knrt10 · 2020-09-01T10:24:54Z

tested for both zsh and bash. Merging now. Thanks, everyone for the review

knrt10 requested review from iaguis, surajssd, ipochi, johananl and invidian August 28, 2020 12:20

knrt10 force-pushed the knrt10/add-autocompletion branch 6 times, most recently from b51fea4 to b661405 Compare August 28, 2020 13:41

invidian reviewed Aug 28, 2020

View reviewed changes

cli/cmd/completion.go Outdated Show resolved Hide resolved

BrainBlasted reviewed Aug 28, 2020

View reviewed changes

docs/cli/lokoctl_completion_zsh.md Outdated Show resolved Hide resolved

cli/cmd/completion.go Outdated Show resolved Hide resolved

knrt10 force-pushed the knrt10/add-autocompletion branch from b661405 to b22d04a Compare August 29, 2020 05:54

knrt10 requested a review from BrainBlasted August 29, 2020 05:54

invidian suggested changes Aug 31, 2020

View reviewed changes

knrt10 force-pushed the knrt10/add-autocompletion branch from b22d04a to af14a88 Compare August 31, 2020 10:20

knrt10 requested a review from invidian August 31, 2020 10:20

knrt10 force-pushed the knrt10/add-autocompletion branch 2 times, most recently from b4c5ebf to b016146 Compare August 31, 2020 10:22

invidian suggested changes Aug 31, 2020

View reviewed changes

docs/cli/lokoctl.md Outdated Show resolved Hide resolved

cli/cmd/completion.go Outdated Show resolved Hide resolved

cli/cmd/completion.go Outdated Show resolved Hide resolved

knrt10 force-pushed the knrt10/add-autocompletion branch from b016146 to cdabe7f Compare August 31, 2020 10:42

knrt10 requested a review from invidian August 31, 2020 10:42

invidian previously requested changes Aug 31, 2020

View reviewed changes

cli/cmd/completion.go Outdated Show resolved Hide resolved

knrt10 force-pushed the knrt10/add-autocompletion branch from cdabe7f to 638a75f Compare August 31, 2020 11:52

knrt10 requested a review from invidian August 31, 2020 11:52

knrt10 force-pushed the knrt10/add-autocompletion branch 2 times, most recently from 1b81139 to dba8989 Compare August 31, 2020 11:59

BrainBlasted previously approved these changes Aug 31, 2020

View reviewed changes

johananl reviewed Aug 31, 2020

View reviewed changes

knrt10 dismissed BrainBlasted’s stale review via bb17567 August 31, 2020 14:40

knrt10 force-pushed the knrt10/add-autocompletion branch from dba8989 to bb17567 Compare August 31, 2020 14:40

knrt10 requested a review from johananl August 31, 2020 14:40

invidian suggested changes Aug 31, 2020

View reviewed changes

cli/cmd/completion.go Outdated Show resolved Hide resolved

knrt10 force-pushed the knrt10/add-autocompletion branch from bb17567 to d6c0a8c Compare August 31, 2020 14:52

knrt10 requested a review from invidian August 31, 2020 14:52

invidian previously approved these changes Aug 31, 2020

View reviewed changes

iaguis suggested changes Sep 1, 2020

View reviewed changes

cli/cmd/completion.go Show resolved Hide resolved

cli/cmd/completion.go Show resolved Hide resolved

cli/cmd/completion.go Show resolved Hide resolved

Add autocomplete for bash and zsh in lokoctl

9b69228

Fixes #18 Signed-off-by: knrt10 <kautilya@kinvolk.io>

knrt10 dismissed invidian’s stale review via 9b69228 September 1, 2020 09:03

knrt10 force-pushed the knrt10/add-autocompletion branch from d6c0a8c to 9b69228 Compare September 1, 2020 09:03

knrt10 requested review from iaguis and invidian September 1, 2020 09:03

iaguis approved these changes Sep 1, 2020

View reviewed changes

invidian approved these changes Sep 1, 2020

View reviewed changes

knrt10 merged commit 375f2c4 into master Sep 1, 2020

knrt10 deleted the knrt10/add-autocompletion branch September 1, 2020 10:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add autocomplete for bash and zsh in lokoctl #880

Add autocomplete for bash and zsh in lokoctl #880

knrt10 commented Aug 28, 2020

knrt10 commented Aug 29, 2020 •

edited

Loading

knrt10 commented Aug 29, 2020

invidian commented Aug 31, 2020

knrt10 commented Aug 31, 2020

invidian left a comment

knrt10 commented Aug 31, 2020 •

edited

Loading

johananl left a comment

johananl Aug 31, 2020

knrt10 Aug 31, 2020 •

edited

Loading

knrt10 commented Aug 31, 2020

invidian left a comment

iaguis left a comment

iaguis left a comment

invidian left a comment

knrt10 commented Sep 1, 2020 •

edited

Loading

Add autocomplete for bash and zsh in lokoctl #880

Add autocomplete for bash and zsh in lokoctl #880

Conversation

knrt10 commented Aug 28, 2020

knrt10 commented Aug 29, 2020 • edited Loading

knrt10 commented Aug 29, 2020

invidian commented Aug 31, 2020

knrt10 commented Aug 31, 2020

invidian left a comment

Choose a reason for hiding this comment

knrt10 commented Aug 31, 2020 • edited Loading

johananl left a comment

Choose a reason for hiding this comment

johananl Aug 31, 2020

Choose a reason for hiding this comment

knrt10 Aug 31, 2020 • edited Loading

Choose a reason for hiding this comment

knrt10 commented Aug 31, 2020

invidian left a comment

Choose a reason for hiding this comment

iaguis left a comment

Choose a reason for hiding this comment

iaguis left a comment

Choose a reason for hiding this comment

invidian left a comment

Choose a reason for hiding this comment

knrt10 commented Sep 1, 2020 • edited Loading

knrt10 commented Aug 29, 2020 •

edited

Loading

knrt10 commented Aug 31, 2020 •

edited

Loading

knrt10 Aug 31, 2020 •

edited

Loading

knrt10 commented Sep 1, 2020 •

edited

Loading