Repository: spf13/cobra
Pull request #1146 (2020-06-26 21:10:14)

Bash completion V2 with completion descriptions
With Cobra supporting shell completion for Bash, Zsh, Fish and Powershell, having a uniform experience across all shells is a desirable feature.  With Fish (#1048) , Zsh (#1070) and PowerShell (#1208) sharing the same Go completion logic, their behaviour is strongly aligned.

This PR uses the same Go completion code to power Bash completion.

However, it is important to keep support for legacy bash completion for backwards-compatibility of existing bash custom completions (`cmd.BashCompletionFunction` and `BashCompCustom`) which are being used by many programs out there. ~Therefore this PR also adds support for legacy bash custom completion through Go completions (see more below).~ Therefore this PR introduces a V2 version of bash completion while retaining the current V1 support.

**Completion descriptions:**
The simplicity of the proposed bash completion script allowed to also add descriptions to bash completions.
 ```
bash-5.0$ helm show [tab][tab]
all     (show all information of the chart)  readme  (show the chart's README)
chart   (show the chart's definition)        values  (show the chart's values)
```
Descriptions are not actually supported natively in Bash, but I fell upon a [Stack Overflow answer](https://stackoverflow.com/questions/7267185/bash-autocompletion-add-description-for-possible-completions/10130007#10130007) that suggested how it could be implemented by the script.
With Fish, Zsh and PowerShell all supporting completion descriptions, I felt it would be a nice addition for Bash (can be disabled).

~**Maintenance**:
Having a v2 version for bash completion while keeping v1 would impose that Cobra maintainers maintain both v1 and v2 versions.
To reduce this maintenance burden, the PR completely replaces bash completion v1 with this new approach.~
 
The current bash completion logic has been extremely useful to ***many*** projects over the last 5 years and has been an amazing initiative by @eparis and all maintainers and contributors that have worked on it.  It has been the inspiration behind this proposed evolution.

**Backwards compatibility**
The PR retains backwards compatibility not only in compilation but in also honouring legacy bash custom completion.  

~To do so, it introduces two new (internal) `ShellCompDirective`:`shellCompDirectiveLegacyCustomComp` and `shellCompDirectiveLegacyCustomArgsComp` which are used to tell the new bash completion script to perform legacy bash completion as was done before, when appropriate.~

~One complexity is that bash code inserted by the program using Cobra could technically reference any variable or function that was part of the previous bash completion script (e.g., `two_word_flags`, `must_have_one_flag`, etc).  However, Iooking at `kubectl`, it seems that the `$last_command` and `$nouns` variables should be sufficient. Therefore, the new solution sets those two variables to allow the injected legacy custom bash code to use them.~

~I have tested this solution with `kubectl` and have confirmed that its legacy completion continues to work.  However, there is still a risk of breaking some completions for some programs out there, if they use other variables or functions of the current bash script.~

Here is what I feel are our options for backwards compatibility:

1. Ignore this PR and keep bash completion as is
1. Keep the current bash completion and introduce a V2 using custom Go completions
1. Keep but deprecate the current bash completion and introduce a V2 using custom Go completions which also support legacy custom completions.  This would give an easy migration path to programs to move to the V2 solution, while not even having to get rid of their legacy custom completion code.  The current bash completion could then be removed in a 2.0 version of Cobra. (commits 1 and 2 of this PR)
1. Replace the current bash completion with the new custom Go completions which also support legacy custom completions almost completely (commits 1, 2 and 3 of this PR)

**This PR implements option 2.**


**Advantages of this new approach**:
1. Aligned behaviour across bash, zsh and fish
1. ~Reduced maintenance~
1. Fixes and improvements to completion support would automatically propagate to all four shells
1. **New support for completion descriptions for bash**
1. Fixed-size completion script of less than 300 lines (in comparaison the v1 script for `kubectl` is over 12,000 lines)


Luap99: Hi @marckhouzam, thank you for your efforts.

> The simplicity of the proposed bash completion script allowed to also add descriptions to bash completions.

I like that feature.

I tested this branch and it works very good. 
One think i noticed is that there is a problem with the bash completion if they contain a colon or a equal sign.
I defined the completion like this:
```
// If there is only a recommended value before we reach the equal sign or the colon, then the completion works as expected.
// If the values differ afterwards, completion fails in bash, but works in fish and zsh
rootCmd.Flags().String("equalSignWorks", "", "test custom comp with equal sign")
rootCmd.RegisterFlagCompletionFunc("equalSignWorks", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
	return []string{"first=Comp\tthe first value", "second=Comp\tthe second value", "forth=Comp\tthe forth value"}, cobra.ShellCompDirectiveNoFileComp
})

rootCmd.Flags().String("equalSignWorksNot", "", "test custom comp with equal sign")
rootCmd.RegisterFlagCompletionFunc("equalSignWorksNot", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
	return []string{"first=Comp1\tthe first value", "first=Comp2\tthe second value", "first=Comp3\tthe forth value"}, cobra.ShellCompDirectiveNoFileComp
})

rootCmd.Flags().String("colonWorks", "", "test custom comp with colon one value works")
rootCmd.RegisterFlagCompletionFunc("colonWorks", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
	return []string{"first:ONE\tthe first value", "second\tthe second value", "forth\tthe forth value"}, cobra.ShellCompDirectiveNoFileComp
})

rootCmd.Flags().String("colonWorksNot", "", "test custom comp with colon doesnt work")
rootCmd.RegisterFlagCompletionFunc("colonWorksNot", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
	return []string{"first:ONE\tthe first value", "first:SECOND\tthe second value", "second:Comp1\tthe forth value"}, cobra.ShellCompDirectiveNoFileComp
})
```
I can confirm that this already exists in the current version.

marckhouzam: > One thing I noticed is that there is a problem with the bash completions if they contain a colon or an equal sign.

Thank you @Luap99 for trying this feature and reporting on it.
I tried your example code and reproduced the problem.

The reason `:` and `=` are causing problems is that they are part of the characters that bash considers as word separators.  By default both are part of `$COMP_WORDBREAKS`:
```
bash-5.0$ echo $COMP_WORDBREAKS
"'><=;|&(:
```
You can remove them from `$COMP_WORDBREAKS` if you wish, **but it could affect other completion scripts you use**.

For example:
- to remove the colon: `COMP_WORDBREAKS=${COMP_WORDBREAKS//:}`
- to remove the equal: `COMP_WORDBREAKS=${COMP_WORDBREAKS//=}`
- to remove both: `COMP_WORDBREAKS=${COMP_WORDBREAKS//[:=]}`

Removing the `:` may or may not be a good idea depending on if the program uses `:` as a separator.  For example `git` completion needs `COMP_WORDBREAKS` to contain the `:`, although `git`'s completion script will add it back if you remove it (`git` does not use Cobra).  See https://github.com/git/git/blob/a08a83db2bf27f015bec9a435f6d73e223c21c5e/contrib/completion/git-completion.bash#L45

Removing the `=` has the same risk, but will also prevent the `--flag=` completion form from working properly.  If you don't use the `--flag=` form you may not care about this.

I believe it would be pretty complex to work around this transparently in Cobra; and causing more complexity is the fact that we don't know if the `:` should or should not be a delimiter for the program using Cobra.

Do you have an example of this causing your program issues, or were you just doing detailed testing?

We'll probably have to live with it, just like the current Cobra bash completion lives with it.

P.S. You are right that the `:` seems to work for zsh (I didn't try fish), but the `=` does cause problems with the `--flag=` form of completion even for zsh.

Luap99: Thank you for your information. I didn't knew `$COMP_WORDBREAKS` exists. I agree that we should properly not mess with it.

> Do you have an example of this causing your program issues, or were you just doing detailed testing?

Unfortunately this a real problem. For example we have the following format for images `repo/path/imagename:tag`. You can have the same images with different tags. 
Since this works currently in the handwritten script i looked it up and found that the shell builtin function `__ltrim_colon_completions "$cur"` is used there and there is no modification of `$COMP_WORDBREAKS`. 
```
$ type __ltrim_colon_completions 
__ltrim_colon_completions ist eine Funktion.
__ltrim_colon_completions () 
{ 
    if [[ "$1" == *:* && "$COMP_WORDBREAKS" == *:* ]]; then
        local colon_word=${1%"${1##*:}"};
        local i=${#COMPREPLY[*]};
        while [[ $((--i)) -ge 0 ]]; do
            COMPREPLY[$i]=${COMPREPLY[$i]#"$colon_word"};
        done;
    fi
}
```


The same goes for the equal form. we have many flags which accept something like this: 

- `filter=all`
- `filter=stopped`
- `filter=running`
or
- `label=role:`
- `label=type:`
- `label=user:`

I found out that this is currently handeld by this function: https://github.com/containers/libpod/blob/master/completions/bash/podman#L301
```
# __podman_map_key_of_current_option returns `key` if we are currently completing the
# value of a map option (`key=value`) which matches the extglob given as an argument.
# This function is needed for key-specific completions.
__podman_map_key_of_current_option() {
	local glob="$1"

	local key glob_pos
	if [ "$cur" = "=" ] ; then        # key= case
		key="$prev"
		glob_pos=$((cword - 2))
	elif [[ $cur == *=* ]] ; then     # key=value case (OSX)
		key=${cur%=*}
		glob_pos=$((cword - 1))
	elif [ "$prev" = "=" ] ; then
		key=${words[$cword - 2]}  # key=value case
		glob_pos=$((cword - 3))
	else
		return
	fi

	[ "${words[$glob_pos]}" = "=" ] && ((glob_pos--))  # --option=key=value syntax

	[[ ${words[$glob_pos]} == @($glob) ]] && echo "$key"
}
```
It then gets called here: https://github.com/containers/libpod/blob/master/completions/bash/podman#L2183

> I believe it would be pretty complex to work around this transparently in Cobra; and causing more complexity is the fact that we don't know if the : should or should not be a delimiter for the program using Cobra.

Yes you are right we don't know that so my only thought would be to add this as `cobra.ShellCompDirective` something like:

- `ShellCompDirectiveNoEqualBreak`
- `ShellCompDirectiveNoColonBreak`

I know that this would make your script a lot more complicated but i would really like if we could handle this.
marckhouzam: Thank you @Luap99, this information is very useful.  Your idea of new `ShellCompDirectives` is interesting as it would give control to the program using Cobra. I will give it some thought and try out different things.  If you would like to try out things on your end, don't hesitate.
marckhouzam: @Luap99 I pushed a new commit which I believe fixes the problem with the `:` and the `=` when they are part of a completion.  It was not as difficult as I expected, thanks to your hints.  Also, I believe this approach is correct for any program, as Cobra does not do special completion for `:` or `=`.

Could you try this new version and let me know if it fixes your problem?

P.S. I haven't yet looked at the escaping problem that you have mentioned.  I will soon.
Luap99: > Could you try this new version and let me know if it fixes your problem?

Yes that works perfectly. Thanks.
Luap99: @marckhouzam Thanks. This works perfectly now. No more issues from my side.
marckhouzam: This latest version completely replaces the current bash completion solution.
It should be (almost completely) backwards compatible as it support legacy custom completion.

To do so, it introduces two new (internal) `ShellCompDirective`:
1. `shellCompDirectiveLegacyCustomComp`
1. `shellCompDirectiveLegacyCustomArgsComp`

which are used to tell the new bash completion script to perform legacy bash completion as was done before, when appropriate.

One complexity is that bash code inserted by the program using Cobra could technically reference any variable or function that was part of the previous bash completion script (e.g., `two_word_flags`, `must_have_one_flag`, etc).  However, Iooking at `kubectl`, it seems that the `$last_command` and `$nouns` variables should be sufficient. Therefore, the new solution sets those two variables to allow the injected legacy custom bash code to use them.

I have tested this solution with `kubectl` and have confirmed that its legacy completion continues to work.  However, there is still a risk of breaking some completions for some programs out there, if they use other variables or functions of the current bash script.  Here is what I feel are our options for backwards compatibility:

1. Ignore this PR and keep bash completion as is
1. Keep the current bash completion and introduce a V2 using custom Go completions (commit 1 of this PR)
1. Keep but deprecate the current bash completion and introduce a V2 using custom Go completions which also support legacy custom completions.  This would give an easy migration path to programs to move to the V2 solution, while not even having to get rid of their legacy custom completion code.  The current bash completion could then be removed in a 2.0 version of Cobra. (commits 1 and 2 of this PR)
1. Replace the current bash completion with the new custom Go completions which also support legacy custom completions almost completely (commits 1, 2 and 3 of this PR)
marckhouzam: @Luap99 if you are able to test this last version, it would help confirm it is still good.

I believe this is as far as I can go.  I will wait for the maintainers to decide if they are interested in including this PR and if so, how they want to go about it. /cc @jharshman @jpmcb 
Luap99: @marckhouzam I can confirm this still works for me with and without descriptions. But i do not use any legacy bash completion functions/annotations.

I have one concern though. The API for generating the shell completions scripts would differ even more:
|            | no desc                    | with desc                 |
|------------|----------------------------|---------------------------|
| bash       | GenBashCompletion          | GenBashCompletionWithDesc |
| zsh        | GenZshCompletionNoDesc     | GenZshCompletion          |
| fish       | GenFishCompletion(...,false) | GenFishCompletion(...,true) |
| powershell | GenPowershellCompletion    | ---                       |

I know this is done to keep it backwards compatible, but this is very unintuitive.
I would prefer the fish completion approach.
marckhouzam: > I can confirm this still works for me with and without descriptions. But i do not use any legacy bash completion functions/annotations.

Thanks for check @Luap99.  I just wanted to make sure it still behaved as it did for you before.

> I have one concern though. The API for generating the shell completions scripts would differ even more:
> I know this is done to keep it backwards compatible, but this is very unintuitive.
> I would prefer the fish completion approach.

Yes, this is pretty bad.  Three shells, three different API for the program to use.  We couldn't have done much worse if we tried 😢 .

The functions `GenBashCompletion()` and `GenZshCompletion()` already exist in a Cobra release, so we need to keep them or else we would break compilation for programs.  And in Go, as you probably know, we cannot use the same name for another function, even if it has more parameters.  So the only improvement on this that I can see is to align bash v2 with zsh and have `GenBashCompletion()` and `GenBashCompletionNoDesc()`.

However, I felt that automatically enabling descriptions for bash completions for existing programs, would be a little too harsh, so I choose to leave descriptions off like before and require the program to opt-in by explicitly calling `GenBashCompletionWithDesc()`.  This decision may not be right.  If this PR gets considered for inclusion, we can see what the maintainers recommend.
marckhouzam: Actually, another option is to create a set of new functions that duplicate some existing ones:
```
GenBashCompletionWithDesc(..., bool)
GenZshCompletionWithDesc(..., bool)
GenFshCompletionWithDesc(..., bool)
```
or some similar type of name.  This would lead to this:

||no desc | with desc
--|-- | --
bash | GenBashCompletion() **and** GenBashCompletionWithDesc(...,false) | GenBashCompletionWithDesc(...,true)
zsh | GenZshCompletionNoDesc() **and** GenZshCompletionWithDesc(...,false) | GenZshCompletion() **and** GenZshCompletionWithDesc(...,true)
fish | GenFishCompletion(...,false) **and** GenFishCompletionWithDesc(...,false)| GenFishCompletion(...,true) **and** GenFishCompletionWithDesc(...,true)

Not sure if that is better or not.
Luap99: What about adding a new function: `GenShellCompletion(shellname,ioWriter,desc)` and using an switch inside to generate the shell completions script for the passed shell name.
The advantage would be that someone could just call GenShellCompletion with args[0] in their `Run` function and doesn't have to use an switch like in your example in the [shell_completions](https://github.com/spf13/cobra/blob/master/shell_completions.md) file.
marckhouzam: > What about adding a new function: `GenShellCompletion(shellname,ioWriter,desc)` and using an switch inside to generate the shell completions script for the passed shell name.
> The advantage would be that someone could just call GenShellCompletion with args[0] in their `Run` function and doesn't have to use an switch like in your example in the [shell_completions](https://github.com/spf13/cobra/blob/master/shell_completions.md) file.

@Luap99 I like the idea, I'll try it out.
And you're making me wonder, since the `completion` command is kind of standard and becoming larger (with 4 shells supported), maybe it would be interesting for Cobra to provide the `completion` command automatically (if it is not already provided by the program)?  It would lower the barrier to entry for completion, as any program using Cobra would immediately have the `completion` command available.  Kind of like the `help` command.  This would be for another PR though.
marckhouzam: > maybe it would be interesting for Cobra to provide the `completion` command automatically (if it is not already provided by the program)?

I've posted a PR for this: #1192 
Luap99: @marckhouzam needs rebase

Also can you point me to a legacy bash completion implementation? Then I would test if everything still works as expected.
marckhouzam: > @marckhouzam needs rebase

@Luap99 Thanks for the poke. Done.

> Also can you point me to a legacy bash completion implementation? Then I would test if everything still works as expected.

Helm no longer uses it but `kubectl` still uses the legacy bash completion (although they have started migrating, see https://github.com/kubernetes/kubernetes/pull/93714).
But there must be many others that I don't know.  Doing a search for `BashCompletionFunction:` on GitHub may help.

Thanks for you help with this!
Luap99: @marckhouzam Special chars are not you friend. :stuck_out_tongue_winking_eye:

Consider something like this: `ValidArgs: []string{"dangerous;echo rm -rf /", "s p a c e 1\t1", "s p a c e 2\t2"},`. I know such things are very unlikely but I would like to handle all cases.
Currently this would get completed as `s p a c e 1` but it has to be `s\ p\ a\ c\ e\ 1` so it can be used as correct input.

I came up with the following after some trail and error:
```diff
diff --git a/bash_completions.go b/bash_completions.go
index 2ceebc5..68687c5 100644
--- a/bash_completions.go
+++ b/bash_completions.go
@@ -234,26 +234,36 @@ __%[1]s_perform_completion()
         done < <(printf "%%s\n" "${out[@]}")
 
         while IFS='' read -r comp; do
+
+            # Split value and description here
+            # The value must be escaped so that it can be used as input again
+            # "value with space" must be completed as "value\ with\ space"
+            if [[ "$comp" == *\ \ * ]]; then
+                desc="  ${comp#*\ \ }"
+                comp=$(printf "%%q" "${comp%%%%\ \ *}")
+            else
+                comp=$(printf "%%q" "$comp")
+                desc=""
+            fi
+
+            # If there is a single completion left, remove the description text
+            if [ ${#completions[*]} -eq 1 ]; then
+                __%[1]s_debug "Removed description from single completion, which is now: ${comp}"
+                desc=""
+            fi
+
             # Although this script should only be used for bash
             # there may be programs that still convert the bash
             # script into a zsh one.  To continue supporting those
             # programs, we do this single adaptation for zsh
             if [ -n "${ZSH_VERSION}" ]; then
                 # zsh completion needs --flag= prefix
-                COMPREPLY+=("$flagPrefix$comp")
+                COMPREPLY+=("$flagPrefix$comp$desc")
             else
-                COMPREPLY+=("$comp")
+                COMPREPLY+=("$comp$desc")
             fi
         done < <(compgen -W "${completions[*]}" -- "$cur")
 
-        # If there is a single completion left, remove the description text
-        if [ ${#COMPREPLY[*]} -eq 1 ]; then
-            __%[1]s_debug "COMPREPLY[0]: ${COMPREPLY[0]}"
-            comp="${COMPREPLY[0]%%%% *}"
-            __%[1]s_debug "Removed description from single completion, which is now: ${comp}"
-            COMPREPLY=()
-            COMPREPLY+=("$comp")
-        fi
     fi
 
     __%[1]s_handle_special_char "$cur" :
```
Note: It is important to split the value and descriptions after 2 spaces.
I'm not sure if this is a good implementation but it solves the problem.

I didn't tested legacy completions yet.
github-actions[bot]: This PR is being marked as stale due to a long period of inactivity
vrothberg: @eparis could you have a look at this PR? Podman would love to use it.
jpmcb: Hi @vrothberg @marckhouzam was AFK for holidays etc. What's the state of this PR? Do we still need to handle the special strings case @Luap99 mentioned?
Luap99: Here are my reasons why I think this is superior to the current bash completion:

1. It only recommends `--flag` instead of both `--flag=` and `--flag` which is a poor ux.
2. The completion script has a fixed and small size, less than 300 lines. Trying the current bash script generation with podman it will generate a script with almost 18k lines.
3. Aligned behaviour with zsh and fish => easier maintenance. For example this would fix #1277
4. With this approach you can hide persistent flags on subcommands via a `PersistentPreRunE` function. This is something we do not want to be shown in podman because of the huge amount of flags. (#1132)
https://github.com/containers/podman/blob/0c2a43b99db8a4fd75412a277da6de2731017d3e/cmd/podman/root.go#L147-L153
5. Support for descriptions.
6. Support for completions which includes a `=` or `:`. This is a big one since most of our completions contain these characters.

That said, I agree with @marckhouzam. The bug fixes PRs should be merged first. To be precise I think of #1284, #1258, #1249, and #1213.

#1258 is especially important, because this is something I cannot work around. With the PRs which just adjust the shell script, like this one, I can still copy out the script template and use it without having to call `cmd.GenBashCompletion()`

Unfortunately, I cannot say much about the backwards compatibility.
marckhouzam: I've been thinking about this bash completion v2 PR and here is what I suggest.

Let's provide explicit functions to choose the v2 version (`GenBashCompletionV2()`), while keeping the current functions pointing to the v1 version (`GenBashCompletion()`).
That way the PR could be merged with no risk of breaking existing programs but it will give the opportunity for those that would like to use the V2 version to do so.
In the future, if the maintainers ever want to deprecate the v1 version, we could simply make `GenBashCompletion()` call `GenBashCompletionV2()`.

@jpmcb would that be a good way to get this PR forward?  I'll update the PR with this change soon.
Luap99: @marckhouzam Thanks for continuing this PR.
We are using this since November in Podman and it works really great.

I found a couple of issues:
1. Running `tesprog mycommand > somefile[TAB]` does not work correct. It will write the completion output of `testprog __complete mycommand` to the current file name and thus maybe overwrite an important file.

2. Moving the cursor backwards and trying to complete a flag with an argument does not work(no completion). e.g `testprog subcmd --flagWithArg [TAB] someCommandArg`. However `testprog subcmd --[TAB] someCommandArg` works.

3. The logic for handling the colon and equal sign is great but it does not work if my completions contain both characters.
```golang
func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
	return []string{"someval:arg=1", "someval:arg=2"}, cobra.ShellCompDirectiveNoFileComp
}
```
`testprog --flag someval:arg=[TAB]` will complete to `someval:arg=arg=` However if you change the colon and equal sign it will work. e.g. `someval=arg:1` will work. Obviously I cannot change our arguments so it would be cool if this would work. 


The first issue should definitely addressed before merging this. The others are minor issues which are not that important IMO.
I am really want this getting merged. It is a huge improvement as I stated here: https://github.com/spf13/cobra/pull/1146#issuecomment-739256391.
marckhouzam: > 1. Running `tesprog mycommand > somefile[TAB]` does not work correct. It will write the completion
> output of `testprog __complete mycommand` to the current file name and thus maybe overwrite an important file.

That is quite the bug you found @Luap99 !

Unlike `zsh`, `fish` or `powershell`, `bash` does not seem to split the completion command-line on the `>` but instead does completion on the whole line.  For example `kubectl completion bash > out<TAB>` triggers the `kubectl` completion script with arguments being the whole line: `kubectl completion bash > out`.  Normally, Cobra bash completion will not find any completion following `> out` and will fallback to file completion, which is probably why this was not a problem before.

However, if a `ValidArgsFunction` is defined on the final cobra command, the completion script, even the v1 version, will call the `__complete` command, with all arguments, including the `> out` redirect.  So this bug you found also happens with current Go completions.  For example `helm completion bash` defines a `ValidArgsFunction` to turn off file completion.  So, when doing `helm completion bash >out<TAB>` (in bash completion v1), the go completion function will be triggered in the bash script and it will execute `helm __completeNoDesc completion bash > out`.  This is where the `out` file gets overwritten.  This of course happens all the time for bash v2 since it always uses Go completions.

I'll look into the best way to fix this.

But we should open an issue for the current bash completion.  Since you found the problem I'll let you open it; if you don't have the time, I'll do it.  Just let me know.

Thanks for your always extremely thorough testing!
Luap99: I opened #1334 and also proposed a possible solution there.
marckhouzam: I pushed an updated version which:
- fixes #1334 for bash >= 4.2 (@Luap99, please try this version to make sure it works as you expect)
- add documentation describing the option of using bash completion v2
- removes the support I had added for legacy custom completion from this v2 version (as long as we keep the v1, might as well not burden ourselves in trying to support the legacy solution in v2 as well)

I'll check later how to fix #1334 with bash < 4.2
marckhouzam: The last forced-push was simply to make the bash v2 script a little more modular by splitting it into more functions.  I'm hoping it will allow to more easily evolve the scripts to add more features.
Luap99: @marckhouzam CI is red. Looks like the linter isn't happy. See the changes in #1044.
jpmcb: Hi all - as far as direction for this PR, I think Marc's suggestions make sense.

> Let's provide explicit functions to choose the v2 version (GenBashCompletionV2()), while keeping the current functions pointing to the v1 version (GenBashCompletion()). That way the PR could be merged with no risk of breaking existing programs but it will give the opportunity for those that would like to use the V2 version to do so.

I think this is the way to go. One of my biggest concerns with Cobra in general is shipping accidental breaking changes to existing projects like Kubenetes. We've done this accidentally (and without us knowing) in the past and I'd hate to de-stabilize this library.

> In the future, if the maintainers ever want to deprecate the v1 version, we could simply make GenBashCompletion() call GenBashCompletionV2().

Great call out. This would be the way to go. More work for "future" us, but at least we would be able to call out these breaking changes for the community when we decided to ship them.
marckhouzam: I rebased and I fixed a small issue where the formatting of descriptions was mistakenly considering even completions that didn't match.
marckhouzam: Since #1192 has been merged and now provides a default `completion` command, I have made this PR point the default `completion` command to the new bash completion V2.

I feel this is the right thing to do because, as the default `completion` command has not been released yet, anyone that would start using it would not have their own completion already implemented, so they won't have any legacy custom completion (which are not supported in V2).  Pointing these new clients to V2 will provide them with the benefits of the v2 implementation, but would also give us more exposure without affecting existing programs like `kubectl`.

This approach requires that the current PR be merged **before** the next release so that it is available at the same time as the default `completion` command.  If not, then we *could* have backwards-compatibility issues as some users of the default `completion` command *could* have started using the legacy custom completion (although unlikely).

@jpmcb Are you ok with this approach?  Would you comfortable merging this before the next release?
marckhouzam: I've added support for testing this bash v2 completion script in https://github.com/marckhouzam/cobra-completion-testing

Since that change won't work with Cobra's master branch which does not contain bash v2, one can use the tests by checking out the branch `feat/bashV2` from https://github.com/marckhouzam/cobra-completion-testing.

To test bash v2 just do `make bash` which will test both bash v1 and bash v2, including descriptions for bash v2.
marckhouzam: I just rebased the PR so that the test would pass.
marckhouzam: I did a rebase on this and added a very small commit that updates some of the code comments so that they mention "shell completion" instead of "bash completion": https://github.com/spf13/cobra/pull/1146/commits/2cc38d483e988b41b78d34f161b18c84d925b052
sudo-bmitch: LGTM

I ran some local testing on this after facing a similar problem (args have colons in the string) and it solved my issues.
marckhouzam: > LGTM
> 
> Some minor issues described in earlier comments should not block this.

The only issue we were worried about was #1334, but this problem only affects bash < 4.2 or when using an old version of the bash-completions package.  

We should not delay progress for such old versions.  In fact `kubectl` bash completion documentation mentions that bash 4.1+ and bash-completions@2 are required: https://kubernetes.io/docs/tasks/tools/included/optional-kubectl-configs-bash-mac/

So, I agree this is ready to be merged.

Patch:
diff --git a/bash_completions.md b/bash_completions.md
index 130f99b..52919b2 100644
--- a/bash_completions.md
+++ b/bash_completions.md
@@ -6,6 +6,8 @@ Please refer to [Shell Completions](shell_completions.md) for details.
 
 For backward compatibility, Cobra still supports its legacy dynamic completion solution (described below).  Unlike the `ValidArgsFunction` solution, the legacy solution will only work for Bash shell-completion and not for other shells. This legacy solution can be used along-side `ValidArgsFunction` and `RegisterFlagCompletionFunc()`, as long as both solutions are not used for the same command.  This provides a path to gradually migrate from the legacy solution to the new solution.
 
+**Note**: Cobra's default `completion` command uses bash completion V2.  If you are currently using Cobra's legacy dynamic completion solution, you should not use the default `completion` command but continue using your own.
+
 The legacy solution allows you to inject bash functions into the bash completion script.  Those bash functions are responsible for providing the completion choices for your own completions.
 
 Some code that works in kubernetes:
diff --git a/bash_completionsV2.go b/bash_completionsV2.go
new file mode 100644
index 0000000..8859b57
--- /dev/null
+++ b/bash_completionsV2.go
@@ -0,0 +1,302 @@
+package cobra
+
+import (
+	"bytes"
+	"fmt"
+	"io"
+	"os"
+)
+
+func (c *Command) genBashCompletion(w io.Writer, includeDesc bool) error {
+	buf := new(bytes.Buffer)
+	genBashComp(buf, c.Name(), includeDesc)
+	_, err := buf.WriteTo(w)
+	return err
+}
+
+func genBashComp(buf io.StringWriter, name string, includeDesc bool) {
+	compCmd := ShellCompRequestCmd
+	if !includeDesc {
+		compCmd = ShellCompNoDescRequestCmd
+	}
+
+	WriteStringAndCheck(buf, fmt.Sprintf(`# bash completion V2 for %-36[1]s -*- shell-script -*-
+
+__%[1]s_debug()
+{
+    if [[ -n ${BASH_COMP_DEBUG_FILE:-} ]]; then
+        echo "$*" >> "${BASH_COMP_DEBUG_FILE}"
+    fi
+}
+
+# Macs have bash3 for which the bash-completion package doesn't include
+# _init_completion. This is a minimal version of that function.
+__%[1]s_init_completion()
+{
+    COMPREPLY=()
+    _get_comp_words_by_ref "$@" cur prev words cword
+}
+
+# This function calls the %[1]s program to obtain the completion
+# results and the directive.  It fills the 'out' and 'directive' vars.
+__%[1]s_get_completion_results() {
+    local requestComp lastParam lastChar args
+
+    # Prepare the command to request completions for the program.
+    # Calling ${words[0]} instead of directly %[1]s allows to handle aliases
+    args=("${words[@]:1}")
+    requestComp="${words[0]} %[2]s ${args[*]}"
+
+    lastParam=${words[$((${#words[@]}-1))]}
+    lastChar=${lastParam:$((${#lastParam}-1)):1}
+    __%[1]s_debug "lastParam ${lastParam}, lastChar ${lastChar}"
+
+    if [ -z "${cur}" ] && [ "${lastChar}" != "=" ]; then
+        # If the last parameter is complete (there is a space following it)
+        # We add an extra empty parameter so we can indicate this to the go method.
+        __%[1]s_debug "Adding extra empty parameter"
+        requestComp="${requestComp} ''"
+    fi
+
+    # When completing a flag with an = (e.g., %[1]s -n=<TAB>)
+    # bash focuses on the part after the =, so we need to remove
+    # the flag part from $cur
+    if [[ "${cur}" == -*=* ]]; then
+        cur="${cur#*=}"
+    fi
+
+    __%[1]s_debug "Calling ${requestComp}"
+    # Use eval to handle any environment variables and such
+    out=$(eval "${requestComp}" 2>/dev/null)
+
+    # Extract the directive integer at the very end of the output following a colon (:)
+    directive=${out##*:}
+    # Remove the directive
+    out=${out%%:*}
+    if [ "${directive}" = "${out}" ]; then
+        # There is not directive specified
+        directive=0
+    fi
+    __%[1]s_debug "The completion directive is: ${directive}"
+    __%[1]s_debug "The completions are: ${out[*]}"
+}
+
+__%[1]s_process_completion_results() {
+    local shellCompDirectiveError=%[3]d
+    local shellCompDirectiveNoSpace=%[4]d
+    local shellCompDirectiveNoFileComp=%[5]d
+    local shellCompDirectiveFilterFileExt=%[6]d
+    local shellCompDirectiveFilterDirs=%[7]d
+
+    if [ $((directive & shellCompDirectiveError)) -ne 0 ]; then
+        # Error code.  No completion.
+        __%[1]s_debug "Received error from custom completion go code"
+        return
+    else
+        if [ $((directive & shellCompDirectiveNoSpace)) -ne 0 ]; then
+            if [[ $(type -t compopt) = "builtin" ]]; then
+                __%[1]s_debug "Activating no space"
+                compopt -o nospace
+            else
+                __%[1]s_debug "No space directive not supported in this version of bash"
+            fi
+        fi
+        if [ $((directive & shellCompDirectiveNoFileComp)) -ne 0 ]; then
+            if [[ $(type -t compopt) = "builtin" ]]; then
+                __%[1]s_debug "Activating no file completion"
+                compopt +o default
+            else
+                __%[1]s_debug "No file completion directive not supported in this version of bash"
+            fi
+        fi
+    fi
+
+    if [ $((directive & shellCompDirectiveFilterFileExt)) -ne 0 ]; then
+        # File extension filtering
+        local fullFilter filter filteringCmd
+
+        # Do not use quotes around the $out variable or else newline
+        # characters will be kept.
+        for filter in ${out[*]}; do
+            fullFilter+="$filter|"
+        done
+
+        filteringCmd="_filedir $fullFilter"
+        __%[1]s_debug "File filtering command: $filteringCmd"
+        $filteringCmd
+    elif [ $((directive & shellCompDirectiveFilterDirs)) -ne 0 ]; then
+        # File completion for directories only
+
+        # Use printf to strip any trailing newline
+        local subdir
+        subdir=$(printf "%%s" "${out[0]}")
+        if [ -n "$subdir" ]; then
+            __%[1]s_debug "Listing directories in $subdir"
+            pushd "$subdir" >/dev/null 2>&1 && _filedir -d && popd >/dev/null 2>&1 || return
+        else
+            __%[1]s_debug "Listing directories in ."
+            _filedir -d
+        fi
+    else
+        __%[1]s_handle_standard_completion_case
+    fi
+
+    __%[1]s_handle_special_char "$cur" :
+    __%[1]s_handle_special_char "$cur" =
+}
+
+__%[1]s_handle_standard_completion_case() {
+    local tab comp
+    tab=$(printf '\t')
+
+    local longest=0
+    # Look for the longest completion so that we can format things nicely
+    while IFS='' read -r comp; do
+        # Strip any description before checking the length
+        comp=${comp%%%%$tab*}
+        # Only consider the completions that match
+        comp=$(compgen -W "$comp" -- "$cur")
+        if ((${#comp}>longest)); then
+            longest=${#comp}
+        fi
+    done < <(printf "%%s\n" "${out[@]}")
+
+    local completions=()
+    while IFS='' read -r comp; do
+        if [ -z "$comp" ]; then
+            continue
+        fi
+
+        __%[1]s_debug "Original comp: $comp"
+        comp="$(__%[1]s_format_comp_descriptions "$comp" "$longest")"
+        __%[1]s_debug "Final comp: $comp"
+        completions+=("$comp")
+    done < <(printf "%%s\n" "${out[@]}")
+
+    while IFS='' read -r comp; do
+        COMPREPLY+=("$comp")
+    done < <(compgen -W "${completions[*]}" -- "$cur")
+
+    # If there is a single completion left, remove the description text
+    if [ ${#COMPREPLY[*]} -eq 1 ]; then
+        __%[1]s_debug "COMPREPLY[0]: ${COMPREPLY[0]}"
+        comp="${COMPREPLY[0]%%%% *}"
+        __%[1]s_debug "Removed description from single completion, which is now: ${comp}"
+        COMPREPLY=()
+        COMPREPLY+=("$comp")
+    fi
+}
+
+__%[1]s_handle_special_char()
+{
+    local comp="$1"
+    local char=$2
+    if [[ "$comp" == *${char}* && "$COMP_WORDBREAKS" == *${char}* ]]; then
+        local word=${comp%%"${comp##*${char}}"}
+        local idx=${#COMPREPLY[*]}
+        while [[ $((--idx)) -ge 0 ]]; do
+            COMPREPLY[$idx]=${COMPREPLY[$idx]#"$word"}
+        done
+    fi
+}
+
+__%[1]s_format_comp_descriptions()
+{
+    local tab
+    tab=$(printf '\t')
+    local comp="$1"
+    local longest=$2
+
+    # Properly format the description string which follows a tab character if there is one
+    if [[ "$comp" == *$tab* ]]; then
+        desc=${comp#*$tab}
+        comp=${comp%%%%$tab*}
+
+        # $COLUMNS stores the current shell width.
+        # Remove an extra 4 because we add 2 spaces and 2 parentheses.
+        maxdesclength=$(( COLUMNS - longest - 4 ))
+
+        # Make sure we can fit a description of at least 8 characters
+        # if we are to align the descriptions.
+        if [[ $maxdesclength -gt 8 ]]; then
+            # Add the proper number of spaces to align the descriptions
+            for ((i = ${#comp} ; i < longest ; i++)); do
+                comp+=" "
+            done
+        else
+            # Don't pad the descriptions so we can fit more text after the completion
+            maxdesclength=$(( COLUMNS - ${#comp} - 4 ))
+        fi
+
+        # If there is enough space for any description text,
+        # truncate the descriptions that are too long for the shell width
+        if [ $maxdesclength -gt 0 ]; then
+            if [ ${#desc} -gt $maxdesclength ]; then
+                desc=${desc:0:$(( maxdesclength - 1 ))}
+                desc+="…"
+            fi
+            comp+="  ($desc)"
+        fi
+    fi
+
+    # Must use printf to escape all special characters
+    printf "%%q" "${comp}"
+}
+
+__start_%[1]s()
+{
+    local cur prev words cword split
+
+    COMPREPLY=()
+
+    # Call _init_completion from the bash-completion package
+    # to prepare the arguments properly
+    if declare -F _init_completion >/dev/null 2>&1; then
+        _init_completion -n "=:" || return
+    else
+        __%[1]s_init_completion -n "=:" || return
+    fi
+
+    __%[1]s_debug
+    __%[1]s_debug "========= starting completion logic =========="
+    __%[1]s_debug "cur is ${cur}, words[*] is ${words[*]}, #words[@] is ${#words[@]}, cword is $cword"
+
+    # The user could have moved the cursor backwards on the command-line.
+    # We need to trigger completion from the $cword location, so we need
+    # to truncate the command-line ($words) up to the $cword location.
+    words=("${words[@]:0:$cword+1}")
+    __%[1]s_debug "Truncated words[*]: ${words[*]},"
+
+    local out directive
+    __%[1]s_get_completion_results
+    __%[1]s_process_completion_results
+}
+
+if [[ $(type -t compopt) = "builtin" ]]; then
+    complete -o default -F __start_%[1]s %[1]s
+else
+    complete -o default -o nospace -F __start_%[1]s %[1]s
+fi
+
+# ex: ts=4 sw=4 et filetype=sh
+`, name, compCmd,
+		ShellCompDirectiveError, ShellCompDirectiveNoSpace, ShellCompDirectiveNoFileComp,
+		ShellCompDirectiveFilterFileExt, ShellCompDirectiveFilterDirs))
+}
+
+// GenBashCompletionFileV2 generates Bash completion version 2.
+func (c *Command) GenBashCompletionFileV2(filename string, includeDesc bool) error {
+	outFile, err := os.Create(filename)
+	if err != nil {
+		return err
+	}
+	defer outFile.Close()
+
+	return c.GenBashCompletionV2(outFile, includeDesc)
+}
+
+// GenBashCompletionV2 generates Bash completion file version 2
+// and writes it to the passed writer.
+func (c *Command) GenBashCompletionV2(w io.Writer, includeDesc bool) error {
+	return c.genBashCompletion(w, includeDesc)
+}
diff --git a/command.go b/command.go
index 5c85c89..2cc1889 100644
--- a/command.go
+++ b/command.go
@@ -63,9 +63,9 @@ type Command struct {
 	// Example is examples of how to use the command.
 	Example string
 
-	// ValidArgs is list of all valid non-flag arguments that are accepted in bash completions
+	// ValidArgs is list of all valid non-flag arguments that are accepted in shell completions
 	ValidArgs []string
-	// ValidArgsFunction is an optional function that provides valid non-flag arguments for bash completion.
+	// ValidArgsFunction is an optional function that provides valid non-flag arguments for shell completion.
 	// It is a dynamic version of using ValidArgs.
 	// Only one of ValidArgs and ValidArgsFunction can be used for a command.
 	ValidArgsFunction func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective)
@@ -74,11 +74,12 @@ type Command struct {
 	Args PositionalArgs
 
 	// ArgAliases is List of aliases for ValidArgs.
-	// These are not suggested to the user in the bash completion,
+	// These are not suggested to the user in the shell completion,
 	// but accepted if entered manually.
 	ArgAliases []string
 
-	// BashCompletionFunction is custom functions used by the bash autocompletion generator.
+	// BashCompletionFunction is custom bash functions used by the legacy bash autocompletion generator.
+	// For portability with other shells, it is recommended to instead use ValidArgsFunction
 	BashCompletionFunction string
 
 	// Deprecated defines, if this command is deprecated and should print this string when used.
@@ -938,7 +939,7 @@ func (c *Command) ExecuteC() (cmd *Command, err error) {
 		args = os.Args[1:]
 	}
 
-	// initialize the hidden command to be used for bash completion
+	// initialize the hidden command to be used for shell completion
 	c.initCompleteCmd(args)
 
 	var flags []string
diff --git a/completions.go b/completions.go
index 04dc71e..5dfea61 100644
--- a/completions.go
+++ b/completions.go
@@ -34,7 +34,6 @@ const (
 
 	// ShellCompDirectiveNoFileComp indicates that the shell should not provide
 	// file completion even when no completion is provided.
-	// This currently does not work for zsh or bash < 4
 	ShellCompDirectiveNoFileComp
 
 	// ShellCompDirectiveFilterFileExt indicates that the provided completions
@@ -592,9 +591,12 @@ You will need to start a new shell for this setup to take effect.
 		DisableFlagsInUseLine: true,
 		ValidArgsFunction:     NoFileCompletions,
 		RunE: func(cmd *Command, args []string) error {
-			return cmd.Root().GenBashCompletion(out)
+			return cmd.Root().GenBashCompletionV2(out, !noDesc)
 		},
 	}
+	if haveNoDescFlag {
+		bash.Flags().BoolVar(&noDesc, compCmdNoDescFlagName, compCmdNoDescFlagDefault, compCmdNoDescFlagDesc)
+	}
 
 	zsh := &Command{
 		Use:   "zsh",
diff --git a/shell_completions.md b/shell_completions.md
index 0b51cff..b2b261d 100644
--- a/shell_completions.md
+++ b/shell_completions.md
@@ -400,6 +400,36 @@ completion     firstcommand   secondcommand
 For backward compatibility, Cobra still supports its bash legacy dynamic completion solution.
 Please refer to [Bash Completions](bash_completions.md) for details.
 
+### Bash completion V2
+
+Cobra provides two versions for bash completion.  The original bash completion (which started it all!) can be used by calling
+`GenBashCompletion()` or `GenBashCompletionFile()`.
+
+A new V2 bash completion version is also available.  This version can be used by calling `GenBashCompletionV2()` or
+`GenBashCompletionFileV2()`.  The V2 version does **not** support the legacy dynamic completion (see [Bash Completions]
+(bash_completions.md)) but instead works only with the Go dynamic completion solution described in this Readme.
+Unless your program already uses the legacy dynamic completion solution, it is recommended that you use the bash
+completion V2 solution which provides the following extra features:
+- Supports completion descriptions (like the other shells)
+- Small completion script of less than 300 lines (v1 generates scripts of thousands of lines; `kubectl` for example has a bash v1 completion script of over 13K lines)
+- Streamlined user experience thanks to a completion behavior aligned with the other shells 
+
+`Bash` completion V2 supports descriptions for completions. When calling `GenBashCompletionV2()` or `GenBashCompletionFileV2()`
+you must provide these functions with a parameter indicating if the completions should be annotated with a description; Cobra
+will provide the description automatically based on usage information.  You can choose to make this option configurable by
+your users.
+
+```
+# With descriptions
+$ helm s[tab][tab]
+search  (search for a keyword in charts)           status  (display the status of the named release)
+show    (show information of a chart)
+
+# Without descriptions
+$ helm s[tab][tab]
+search  show  status
+```
+**Note**: Cobra's default `completion` command uses bash completion V2.  If for some reason you need to use bash completion V1, you will need to implement your own `completion` command. 
 ## Zsh completions
 
 Cobra supports native zsh completion generated from the root `cobra.Command`.


Test patch:
diff --git a/completions_test.go b/completions_test.go
index 9bd6a41..db363ae 100644
--- a/completions_test.go
+++ b/completions_test.go
@@ -2097,9 +2097,9 @@ func TestDefaultCompletionCmd(t *testing.T) {
 	removeCompCmd(rootCmd)
 
 	var compCmd *Command
-	// Test that the --no-descriptions flag is present for the relevant shells only
+	// Test that the --no-descriptions flag is present on all shells
 	assertNoErr(t, rootCmd.Execute())
-	for _, shell := range []string{"fish", "powershell", "zsh"} {
+	for _, shell := range []string{"bash", "fish", "powershell", "zsh"} {
 		if compCmd, _, err = rootCmd.Find([]string{compCmdName, shell}); err != nil {
 			t.Errorf("Unexpected error: %v", err)
 		}
@@ -2107,14 +2107,6 @@ func TestDefaultCompletionCmd(t *testing.T) {
 			t.Errorf("Missing --%s flag for %s shell", compCmdNoDescFlagName, shell)
 		}
 	}
-	for _, shell := range []string{"bash"} {
-		if compCmd, _, err = rootCmd.Find([]string{compCmdName, shell}); err != nil {
-			t.Errorf("Unexpected error: %v", err)
-		}
-		if flag := compCmd.Flags().Lookup(compCmdNoDescFlagName); flag != nil {
-			t.Errorf("Unexpected --%s flag for %s shell", compCmdNoDescFlagName, shell)
-		}
-	}
 	// Remove completion command for the next test
 	removeCompCmd(rootCmd)