Skip to content
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

Use native zsh completion script generator #3137

Merged
merged 2 commits into from
Nov 6, 2019

Conversation

corneliusweig
Copy link
Contributor

Description

So far, Skaffold used a wrapper code borrowed from kubectl to use the bash completion script for zsh. Cobra v0.0.5 introduced a native completion script generator for zsh. To support the source <(...) use-case, a minor tweak is necessary (also see spf13/cobra#887).
This PR does the required changes to make use of this native zsh completion scrip generator. It offers inline help text when making completion suggestions.

User facing changes

Users of the zsh completion script (generated with source <( skaffold completion zsh)) will get improved completion suggestions:

  • When suggesting flags for completion, also a small help text is displayed
  • After flags expecting an argument, no flags are suggested

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

  • [ n/a] Includes unit tests
  • Mentions any output changes.
  • Adds documentation as needed: user docs, YAML reference, CLI reference.
  • Adds integration tests if needed.

Reviewer Notes

  • The code flow looks good.
  • Unit test added.
  • User facing changes look good.

Release Notes

Zsh completion now shows inline help text and understands the positional context of flags.

So far, Skaffold used a wrapper code borrowed from kubectl to use the bash completion script for zsh. Cobra v0.0.5 introduced a native completion script generator for zsh. To support the `source <(...)` use-case, a minor tweak is necessary (also see spf13/cobra#887)

Signed-off-by: Cornelius Weig <22861411+corneliusweig@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Oct 29, 2019

Codecov Report

Merging #3137 into master will increase coverage by 0.15%.
The diff coverage is 0%.

Impacted Files Coverage Δ
cmd/skaffold/app/cmd/completion.go 14.28% <0%> (+1.78%) ⬆️
pkg/skaffold/build/local/local.go 31.57% <0%> (-1.76%) ⬇️
pkg/skaffold/runner/diagnose.go 12.5% <0%> (-0.36%) ⬇️
pkg/skaffold/kubernetes/context/context.go 100% <0%> (ø) ⬆️
pkg/skaffold/schema/versions.go 70.83% <0%> (ø) ⬆️
pkg/skaffold/config/options.go 100% <0%> (ø) ⬆️
pkg/skaffold/schema/v1beta17/upgrade.go 77.77% <0%> (ø)
pkg/skaffold/build/local/buildpacks.go 0% <0%> (ø)
pkg/skaffold/build/buildpacks/lifecycle.go 81.81% <0%> (ø)
pkg/skaffold/build/buildpacks/metadata.go 27.27% <0%> (ø)
... and 8 more

@balopat balopat added the kokoro:run runs the kokoro jobs on a PR label Oct 30, 2019
Copy link
Contributor

@balopat balopat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for this, awesome! :)

@corneliusweig
Copy link
Contributor Author

The credit goes to @babysnakes

out.Write(buf.Bytes())
io.WriteString(out, zshTail)
_ = rootCmd(cmd).GenZshCompletion(out)
_, _ = io.WriteString(out, zshCompdef)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could it be?

rootCmd(cmd).GenZshCompletion(out)
io.WriteString(out, zshCompdef)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Only my IDE complains about it, so I explicitly ignored the errors.

@dgageot dgageot added kokoro:run runs the kokoro jobs on a PR and removed kokoro:run runs the kokoro jobs on a PR labels Oct 31, 2019
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Oct 31, 2019
@balopat balopat added the kokoro:run runs the kokoro jobs on a PR label Nov 1, 2019
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Nov 1, 2019
@dgageot
Copy link
Contributor

dgageot commented Nov 6, 2019

Let's ignore kokoro

@dgageot dgageot merged commit 7338942 into GoogleContainerTools:master Nov 6, 2019
@corneliusweig corneliusweig deleted the w/zsh-completion branch November 6, 2019 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants