Skip to content

Use native zsh completion script generator#3137

Merged
dgageot merged 2 commits intoGoogleContainerTools:masterfrom
corneliusweig:w/zsh-completion
Nov 6, 2019
Merged

Use native zsh completion script generator#3137
dgageot merged 2 commits intoGoogleContainerTools:masterfrom
corneliusweig:w/zsh-completion

Conversation

@corneliusweig
Copy link
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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