-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[CI:DOCS] Fix fish completion issue if the command is prefixed with a space #9079
[CI:DOCS] Fix fish completion issue if the command is prefixed with a space #9079
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Luap99 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Update the completion script like spf13/cobra#1249. [NO TESTS NEEDED] Fixes containers#8829 Signed-off-by: Paul Holzinger <paul.holzinger@web.de>
216be13
to
8c1768e
Compare
Well, this sure brings up an interesting monkey wrench! As I interpret it, this PR is a manually-done commit, cherrypicking specific cobra fixes but not bringing in the new cobra itself. The obvious question then is, won't the automatically-generated completions override this change? Ha ha, joke's on me. There are no automatically-generated completions. We're shipping old completion files with no updates. Effective this week, this will change. I will be adding a CI test that will (1) implement a @Luap99 I'm afraid that my proposed change will undo your PR. Since mine is just a proposal, not yet implemented or even agreed-upon by the team, I'm OK with this PR merging; but we need to start the conversation right now re: bringing in new cobra. Thanks as always for raising great questions that improve our code base! |
Ed, I already had this done when we originally merged my completions PR #6442. The problem is that upstream cobra does not have the nice features and bug fixes that I want. There are a lot of open completion PRs. #6442 was merged with my cobra fork. This was not sustainable as pointed out in #8528. So I reverted this #8534. I concluded that it is the best user experience to keep the new (unmerged) scripts. I was really hoping that this issue would have been resolved by now, but none of the cobra PRs have been merged. I just got the reminder about #8829 and said whatever, I will fix this issue myself. I do not like to maintain this but I rather give a better user experience than waiting for upstream to merge the fixes. |
To give some more context. I am talking about the following cobra PRs:
Except spf13/cobra#1258 because this one needs changes in the go code all of the other PRs are already included in our completions files in the |
Point taken, thank you. I will hold off on my CI checks. I can't think of any way to track this problem, other than for a human to watch for a new upstream release, then vendor it in, then alert me to go ahead with the new checks. Can you think of a better way? |
That's what I am doing. I follow upstream closely. |
LGTM |
/lgtm I wonder if we should bring more attention to this upstream - would be nice to finally get those PRs merged |
Update the completion script like spf13/cobra#1249.
Fixes #8829