-
Notifications
You must be signed in to change notification settings - Fork 192
Update cobra to support plugin flag completion #1251
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm currently working on cutting a cobra release for v1.2.2
I propose we wait for a release instead of pinning to an arbitrary sha.
@jpmcb I can update after cobra releases or if someone else (Dependabot) beats me to it we can close this PR. Either way, I verified that the Tanzu CLI completion issue is fixed by updating cobra. |
Awesome!! Very glad this fix worked for you :D I'll see how pushing through a release goes (hopefully by this week). If we need this feature to be in a |
v1.3.0 has been cut! https://github.com/spf13/cobra/releases/tag/v1.3.0 Let me know if y'all need any support on this one!! |
Rebased on main and updated cobra to 1.3.0 |
Rebased again |
This version includes spf13/cobra#1161 which fixes an issue where completion options are not offered for plugin flags. Signed-off-by: Scott Andrews <andrewssc@vmware.com>
@giri-varma PTAL, thanks! |
@jpmcb I see that the change has been incorporated. Do you see any issues merging this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the cobra perspective, yes this is fine!
What this PR does / why we need it
Updates cobra to a version that includes spf13/cobra#1161 which
fixes an issue where completion options are not offered for plugin
flags.
Without this fix, the completion handler inside cobra is interpreted as returning flags for the command hosting the plugin. With this fix, when flag parsing is disabled for a commend, the custom completion hook is invoked, which the Tanzu CLI uses to collect completion info from plugins.
Describe testing done for PR
I used the special hook provided by cobra that is used by shell completions to compare the suggestions with and without this PR.
Without this PR (plugin flag completions are not available):
With this PR (plugin flag completions are available):
Release note
PR Checklist
Additional information
Special notes for your reviewer
There is no tagged version of cobra that contains this patch, this commit is the HEAD of the master branch. We can switch back to tagged releases once there is a new release.
cc @jpmcb (who helped land the upstream PR)