cobra
cobra copied to clipboard
[RFC] Redirect bash completion v1 to v2 when possible
We are no longer actively maintaining bash completion v1 in favor of its more rich v2 version. Previously, using bash completion v2 required projects to be aware of its existence and to explicitly call GenBashCompletionV2()
.
With this commit, any projects calling GenBashCompletion()
will automatically be redirected to using the v2 version.
One exception is if the project uses the legacy custom completion logic which is not supported in v2. We can detect that by looking for the use of the field BashCompletionFunction
on the root command.
If we ignore the tests and doc, this is a 2 line change in bash_completions.go
Note that descriptions are kept off when calling GenBashCompletion()
. This means that to enable completion descriptions for bash, a project must still explicitly call GenBashCompletionV2()
. I chose this approach because I was hesitant to enable descriptions without getting projects to make the decision themselves.
cc @jpmcb @scop @Luap99
I am ok with the change but I wonder what the motivation is? Assuming that you will not remove the old script until a major version bump I don't see the benefit.
I should have explained in the PR description, sorry. Let me start by saying that I agree this is not a clearcut decision, and I welcome any input for or against it.
Basically I'm trying to improve the quality of bash completion for projects using Cobra. I don't think it is easy for projects to know that there is such a thing as a V2 version and therefore I feel projects are missing out on bug fixes and features. I opted to propose this change after opening this bug https://github.com/vmware-tanzu/tanzu-framework/issues/3963; the bug made me realize that bash completion can be affected by subtle bugs that may go unreported/unfixed although they have already been addressed by Cobra's bash completion V2 solution.
But maybe this is overreaching. I'd like people's honest opinion on this and if there is too much hesitation, we'll just close it.
I think it is a good idea overall. IMO there shouldn't be a reason to use the V1 version but then again I don't really know how and where is is used. One thing that we can do without changing behaviour is to add a // Deprecated:
comment which will be detected by some linters, i.e. https://staticcheck.io/docs/checks/#SA1019, and make authors aware that they should switch.
Seems fine to me too. Deprecated:
on the original functions seconded, more info at https://github.com/golang/go/wiki/Deprecated
The Cobra project currently lacks enough contributors to adequately respond to all PRs. This bot triages issues and PRs according to the following rules:
- After 60d of inactivity, lifecycle/stale is applied. - After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied and the PR is closed. You can:
- Make a comment to remove the stale label and show your support. The 60 days reset. - If a PR has lifecycle/rotten and is closed, comment and ask maintainers if they'd be interested in reopening.