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

Fix regression when calling *_custom_func #1001

Merged
merged 1 commit into from Dec 26, 2019

Conversation

marckhouzam
Copy link
Collaborator

PR #889 introduced a small regression where the global variable $c is no longer set when *custom_func is called. This is because $c is re-used by mistake in the new read loop.

This PR simply changes the name of the variable used in the loop.

I've proposed a PR to the helm/helm projet which uses the $c variable in __helm_custom_func and when testing with the master branch of Cobra, things started failing. With this PR applied, the helm/helm PR works again.

See https://github.com/helm/helm/pull/7078/files#diff-84d051076d41e254e011a57e40b82a34R279 for the line using $c in the PR.

/cc @rsteube @jharshman

PR spf13#889 introduced a regression where the global variable $c is no
longer set when *custom_func is called.  This is because $c is re-used
by mistake in the read loop.

This PR simply changes the name of the variable used in the loop.

Signed-off-by: Marc Khouzam <marc.khouzam@gmail.com>
@claassistantio
Copy link

claassistantio commented Dec 12, 2019

CLA assistant check
All committers have signed the CLA.

@marckhouzam
Copy link
Collaborator Author

marckhouzam commented Dec 12, 2019

BTW, the helm project has a set of automated regression tests for shell auto-completion, which can help test such changes.
https://github.com/helm/acceptance-testing#shell-completion
https://github.com/helm/acceptance-testing/blob/master/scripts/completion-tests/completionTests.sh

@rsteube
Copy link
Contributor

rsteube commented Dec 12, 2019

LGTM

@jharshman jharshman self-requested a review December 26, 2019 17:55
@jharshman jharshman added area/shell-completion All shell completions kind/bug A bug in cobra; unintended behavior labels Dec 26, 2019
@jharshman jharshman merged commit bf26895 into spf13:master Dec 26, 2019
@marckhouzam marckhouzam deleted the fix/clobberedCVar branch March 10, 2021 00:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/shell-completion All shell completions kind/bug A bug in cobra; unintended behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants