Custom completions coded in Go (instead of Bash) #1035

marckhouzam · 2020-02-16T21:43:49Z

This PR allows programs using Cobra to code their custom completions in Go directly inside the program itself.

It is a port to Cobra of the custom completion solution introduced in Helm 3.1 (helm/helm#7323).

It is fully-backwards compatible.
It also allows to use both the new Go custom completions and the existing Bash custom completions at the same time.

The PR introduces a new hidden sub-command: __complete that is to be called by the auto-completion script. I chose the name with a double underscore prefix to make sure this command will not collide with any command of a program.

Along the lines of the existing ValidArgs field, support is added for the ValidArgsFunction field. The program can register a function that will provide custom completion through this newly added field.

For flags, the new function Command.RegisterFlagCompletionFunc() is added. The program can register a custom completion function for a flag using this new function.

The bash completion script, before calling the completion *_custom_func, will now verify if Command.ValidArgsFunction has been specified. If so, the completion script will call the __complete command passing it all command-line arguments; the __complete command will call the function specified by Command.ValidArgsFunction to obtain completions from the program itself.

To be as feature-rich as custom completions written in Bash, the PR also allows Go completions to control:

if a single completion should be followed by a space
if file completion should be performed when no completions are available

Some advantages of using Go instead of Bash for custom completions:

Cobra users are Go developers
Access to the full program code base to perform completions instead of being limited to relying on externally-exposed functionality.
Debugging and development much easier because developers can directly call e.g., <program> __complete <cmd> ""<ENTER> to test the corresponding custom completion code
The Go debugger can even be used to troubleshoot custom completion code
Go tests can be written for the custom completion code
Flag parsing done automatically by Cobra (instead of having to be coded in bash in the completion script)
No need to make sure the custom completion code works for both bash and zsh (for those programs that share completions between the two)
No more issues with the use of shell tools (e.g., echo, cat, tail) that work differently on different OSes

To illustrate a working program using this PR, I've ported Helm from its own solution to using this PR in Cobra: https://github.com/VilledeMontreal/helm/tree/feat/compInGoWithCobra
Here is an example of using the new ValidArgsFunction field:
https://github.com/VilledeMontreal/helm/blob/2c88891a96eb6cb27d124d5a6d750eee85abcc8c/cmd/helm/status.go#L55
and an example of using RegisterFlagCompletionFunc():
https://github.com/VilledeMontreal/helm/blob/2c88891a96eb6cb27d124d5a6d750eee85abcc8c/cmd/helm/root.go#L86

Go tests have been added in Cobra for this PR and the documentation for bash_completion has been updated.

AdamSLevy · 2020-02-21T02:08:35Z

You may be interested in this golang package as it is related to your intent here: https://github.com/posener/complete

I use it alongside cobra to achieve the same end as this PR.

jharshman · 2020-02-26T17:36:10Z

@marckhouzam this is great work!

Can this be extended to generate completions for other shells. We currently support ZSH but have had requests for others like FiSH etc..

There's a lot to take in on this PR so please keep in mind that the review process for this one might be more lengthy. That being said I have slated this for inclusion in our 1.0.0 release.

marckhouzam · 2020-02-26T22:25:12Z

@marckhouzam this is great work!

Thanks for having a look!

Can this be extended to generate completions for other shells. We currently support ZSH but have had requests for others like FiSH etc..

I didn't want to come on too strong, so I started with posting the functionality of this PR. However you are absolutely right! This can be extended to provide full completions to other shells. In fact, I'm about to post a PR to the helm project that will support full Fish completion (command names, flag names and custom completions). And the fish script is less than 100 lines.

There's a lot to take in on this PR so please keep in mind that the review process for this one might be more lengthy.

No problem at all. I'll do my best to be responsive to any comment you may have.

That being said I have slated this for inclusion in our 1.0.0 release.

🎉 ❤️

marckhouzam · 2020-02-27T11:44:49Z

Here's the Helm PR for full Fish completion:
helm/helm#7690

bash_completions.go

custom_completions.go

jharshman · 2020-03-27T22:43:04Z

@marckhouzam I'm trying to validate this on my end and can't seem to get the statically defined autocompletion functioning.

i.e:

var rootCmd = &cobra.Command{
	ValidArgs: []string{"pod", "rs", "ds"},
	Use:       "testprog1",
	Run: func(cmd *cobra.Command, args []string) {
		fmt.Println("rootCmd called")
	},
}

var childCmd = &cobra.Command{
	Use: "pod",
	Run: func(cmd *cobra.Command, args []string) {
		fmt.Println("child called")
	},
}

However, when i compile it and run ./testprog1 [tab][tab] I don't get any completion suggestions where I assume I should get "pod", "rs", or "ds".
Am I mistaken or missing something key here?

marckhouzam · 2020-03-28T01:32:33Z

However, when i compile it and run ./testprog1 [tab][tab] I don't get any completion suggestions where I assume I should get "pod", "rs", or "ds".
Am I mistaken or missing something key here?

Thanks @jharshman for trying things out!

There are a couple things going on here:

You will always need to source a completion script first, before completions can work
This PR only introduces Go Custom Completions (the ones coded by the program using cobra). It does not modify the way Cobra provides command, flag, or ValidArgs completions. So to give this code a spin, you need to try the new ValidArgsFunction field.

Here is a test program and how to test with it. That's how I ran it anyway.

main.go:

package main

import (
	"fmt"
	"os"

	"github.com/spf13/cobra"
)

var rootCmd = &cobra.Command{
	ValidArgs: []string{"pod", "rs", "ds"},
	Use:       "testprog1",
	Run: func(cmd *cobra.Command, args []string) {
		fmt.Println("rootCmd called")
	},
}

var childCmd = &cobra.Command{
	Use: "pod",
	ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.BashCompDirective) {
		return []string{"these", "are", "custom", "completions"}, cobra.BashCompDirectiveDefault
	},
	Run: func(cmd *cobra.Command, args []string) {
		fmt.Println("child called")
	},
}

var completionCmd = &cobra.Command{
	Use: "completion",
	Run: func(cmd *cobra.Command, args []string) {
		cmd.Root().GenBashCompletion(os.Stdout)
	},
}

func main() {
	rootCmd.AddCommand(childCmd)
	rootCmd.AddCommand(completionCmd)
	rootCmd.Execute()
}

go.mod (you'll need to modify the path to your cobra git repo)

go 1.13
replace github.com/spf13/cobra => /Users/marckhouzam/git/cobra

To build and test:

bash

GO111MODULE=on go build -o testprog1 main.go
source <(./testprog1 completion)

./testprog1 <TAB><TAB>
# Should show: ds   pod  rs
# The completions above are done by cobra's existing script

./testprog1 pod <TAB><TAB>
# Should show: are          completions  custom       these
# The completions above are done by this PR through the ValidArgsFunction field called by the cobra completion script

# To debug the ValidArgsFunction code
./testprog1 __complete pod ""
# After pressing <enter> should show:
these
are
custom
completions
:0
Completion ended with directive: BashCompDirectiveDefault

Let me know if I can help in any other way, and thanks again.

jharshman · 2020-03-28T02:04:09Z

Ah - thank you for clarifying that. Silly mistake on my part. I'll continue my review of this change.

marckhouzam · 2020-03-28T03:01:15Z

BTW, allowing the __complete command to also complete flags, commands, and ValidArgs is added as part of #1048; specifically in commit 828c60b
However, the bash completion script of Cobra won't make use of that (not needed), but the Fish one will.

But you could test it manually (using #1048) like this:

./testprog1 __complete ""
# will show sub commands and validArgs

jharshman · 2020-03-28T03:58:50Z

@marckhouzam so for the ValidArgsFunction, it looks like static ValidArgs takes precedence over ValidArgsFunction if both are defined on a command. Is this right?

custom_completions_test.go

marckhouzam · 2020-03-28T15:57:28Z

@marckhouzam so for the ValidArgsFunction, it looks like static ValidArgs takes precedence over ValidArgsFunction if both are defined on a command. Is this right?

@jharshman Good observation. With this PR, we are only focusing on Custom Completions so I chose to follow the way the existing Bash completion script triggers custom completions. That meant that if there are ValidArgs then ValidArgsFunction does not get called. To be honest, this was the simplest choice, which avoided messing too much with the Bash script code. But from a user's perspective, this may not be the right choice.

In fact, in #1048 for Fish completion, where ValidArgs are handled in Go through the __completion command, I actually chose to allow both ValidArgs and ValidArgsFunction to be used together. You made me realize now this would cause incompatibilities between bash completions and fish completions.

So, what do we want to offer the user? For consistency, I think we have two options:

allow to use both ValidArgs and ValidArgsFunction together
disallow using both together and cause some configuration error if they are used together.

Allowing both gives more flexibility to the user, but could cause confusion if they don't realize they have used both... What do you think we should support?

jharshman · 2020-03-28T18:39:14Z

@marckhouzam my opinion on this is that a command can either use one or the other but not both. While being able to set both on a command might add more flexibility, I think it would be at the expense of simplicity.

So let's make these fields mutually exclusive and going with your second option please also add some documentation for the function and in the readme.

marckhouzam · 2020-03-28T19:59:02Z

@marckhouzam my opinion on this is that a command can either use one or the other but not both.

@jharshman Agreed. I will post an update and make matching changes to #1048.

This brings up a similar situation. Cobra allows a command to have sub-commands and arguments at the same time (your test program even does it, although it would need the Args: field on rootCmd to actually accept the specified ValidArgs). So we can have a situation where ValidArgs is specified as well as sub-commands. When doing completion, it seems that currently ValidArgs has precedence over sub-commands. To explain:

In testprog1 the rootCmd specifies ValidArgs of pod, rs, ds, but also a sub-command pod. In testing I saw that the completion script only proposes the values of ValidArgs. If ValidArgs did not include pod, the completions would not propose the sub-command pod at all. Cobra should probably do better to help the user by automatically including non-hidden sub-commands in the completions. But that is something else (does this deserve an issue?). For this PR:

What shall we do if ValidArgsFunction is specified at the same time as sub-commands? I assume we should propose both the sub-commands and the results of calling ValidArgsFunction? What do you say?

bash_completions.go

marckhouzam · 2020-03-29T02:28:16Z

So let's make these fields mutually exclusive and going with your second option please also add some documentation for the function and in the readme.

I pushed a commit with a comment added next to the ValidArgsFunction field, and mentioned it in the completion readme.

The code behaves as we want, only using ValidArgs if both are present, but I couldn't find a good way to warn the user at runtime.

jharshman · 2020-03-30T16:17:10Z

What shall we do if ValidArgsFunction is specified at the same time as sub-commands? I assume we should propose both the sub-commands and the results of calling ValidArgsFunction? What do you say?

Hm... this is a good point. The intention of ValidArgsFunction is to allow dynamic population of completions for arguments. I would say here we want to match the behavior of ValidArgs.

marckhouzam · 2020-04-02T03:45:28Z

The intention of ValidArgsFunction is to allow dynamic population of completions for arguments. I would say here we want to match the behavior of ValidArgs.

I have pushed a commit that makes ValidArgsFunction be treated like ValidArgs when there are also sub-commands, which is to show completions matching ValidArgsFunction but not sub-commands. I have however also opened #1076 to improve this behaviour.

@jharshman I believe I have addressed all comments for the moment.

marckhouzam · 2020-04-02T03:49:06Z

The force-push was rebase to master, as there were conflicts.

marckhouzam · 2020-04-02T11:56:24Z

The tests fail because of something to do with stderr output. Maybe because of the rebase to master. I will investigate.

jharshman · 2020-04-02T22:52:51Z

The tests fail because of something to do with stderr output

@marckhouzam yes I merged in a small change to start using StdErr and StdOut functions in the test code. There is another follow up issue to address the rest of the references in the code base.

Ref: #1053 & #1071

marckhouzam · 2020-04-03T02:58:01Z

@marckhouzam yes I merged in a small change to start using StdErr and StdOut functions in the test code.

@jharshman Thanks for the pointer. It is now fixed. I had made a mistake rebasing.

jharshman · 2020-04-03T16:28:08Z

@marckhouzam awesome - gonna give this one more pass and then I think we can call it good and start looking at #1048

jharshman · 2020-04-03T16:56:23Z

Okay one observation that I have made is that the ValidArgs behavior actually does return both the contents of ValidArgs and the subcommand. So If it makes sense, I think the ValidArgFunction should match this behavior.
What do you think @marckhouzam. I think we were mistaken in our interpretation of the behavior of ValidArgs. Can you also validate the behavior I'm seeing?

marckhouzam · 2020-04-03T17:16:13Z

Okay one observation that I have made is that the ValidArgs behavior actually does return both the contents of ValidArgs and the subcommand.

When I tested it, ValidArgs prevented subcommands from being completed. In the code it looks like that too. If you look at

cobra/bash_completions.go

Lines 121 to 124 in 138b98f

    
               completions=("${commands[@]}") 
        
               if [[ ${#must_have_one_noun[@]} -ne 0 ]]; then 
        
                   completions=("${must_have_one_noun[@]}") 
        
               fi

Notice that if there are ValidArgs (must_have_one_noun), then the completions array is overwritten completely, which removes the subcommands. It is just a question of changing the = to += to fix that (which makes me wonder if this was an oversight from the very start), but I think it should be done in a separate PR, for traceability, which is why I opened #1076.

How did you test on your side? Is it with the testprog1 we mentionned above? If so, could it be that you see the subcommand name (pod) being completed because it is also listed in the list of ValidArgs?

@marckhouzam awesome - gonna give this one more pass and then I think we can call it good and start looking at #1048

Awesome! I'll have to rebase #1048 on the new master once this PR is merged as #1048 is not currently based on the very latest version of this PR.

jharshman · 2020-04-03T17:26:26Z

could it be that you see the subcommand name (pod) being completed because it is also listed in the list of ValidArgs?

Yep, that was the reason, thanks for double checking!

Alright - I think this PR is ready to rock then!

jharshman · 2020-04-03T17:27:50Z

@marckhouzam actually can you squash your commits first. This has quite a large commit message.

This commit allows programs using Cobra to code their custom completions in Go instead of Bash. The new ValidArgsFunction field is added for commands, similarly to ValidArgs. For flags, the new function Command.RegisterFlagCompletionFunc() is added. When either of the above functions is used, the bash completion script will call the new hidden command '__complete', passing it all command-line arguments. The '__complete' command will call the function specified by Command.ValidArgsFunction or by Command.RegisterFlagCompletionFunc to obtain completions from the program itself. Signed-off-by: Marc Khouzam <marc.khouzam@montreal.ca>

marckhouzam · 2020-04-03T18:16:56Z

@marckhouzam actually can you squash your commits first. This has quite a large commit message.

@jharshman All cleaned up. Thanks again!

marckhouzam force-pushed the feat/customCompletionInGo branch from f919377 to b9c4570 Compare February 16, 2020 22:47

marckhouzam mentioned this pull request Feb 16, 2020

Add flag that gives the ability to disable bash default completion #757

Closed

jharshman added area/shell-completion All shell completions kind/feature A feature request for cobra; new or enhanced behavior labels Feb 26, 2020

jharshman added this to the 1.0.0 milestone Feb 26, 2020

marckhouzam force-pushed the feat/customCompletionInGo branch 2 times, most recently from f43e3ee to 96f8ab9 Compare February 28, 2020 18:45

marckhouzam mentioned this pull request Feb 29, 2020

Fish completion (including support for Go custom completion) #1048

Merged

jharshman reviewed Mar 18, 2020

View reviewed changes

bash_completions.go Outdated Show resolved Hide resolved

marckhouzam commented Mar 19, 2020

View reviewed changes

custom_completions.go Outdated Show resolved Hide resolved

jharshman reviewed Mar 28, 2020

View reviewed changes

custom_completions_test.go Outdated Show resolved Hide resolved

marckhouzam commented Mar 29, 2020

View reviewed changes

bash_completions.go Show resolved Hide resolved

marckhouzam mentioned this pull request Mar 30, 2020

Zsh completion V2 #1070

Merged

This was referenced Apr 1, 2020

Don't exclude 'help' from bash completions #1065

Closed

Completions do not suggest sub-commands if there are ValidArgs #1076

Closed

marckhouzam force-pushed the feat/customCompletionInGo branch from 9163479 to a2c2c3d Compare April 2, 2020 03:48

marckhouzam force-pushed the feat/customCompletionInGo branch from a2c2c3d to ed9b65c Compare April 3, 2020 02:21

marckhouzam force-pushed the feat/customCompletionInGo branch from 87b4ea2 to 9d736a0 Compare April 3, 2020 18:15

jharshman merged commit b80aeb1 into spf13:master Apr 3, 2020

marckhouzam deleted the feat/customCompletionInGo branch April 3, 2020 21:26

This was referenced Apr 5, 2020

Full completion support for Fish shell helm/helm#7690

Closed

Make bash completion provide sub-command names even if ValidArgs present #1088

Merged

majst01 mentioned this pull request Apr 12, 2020

cobra 1.0 allows to write bash_completions in go metal-stack/metalctl#7

Closed

jpmcb mentioned this pull request Apr 15, 2020

add example code of generating Zsh completion #1005

Closed

marckhouzam mentioned this pull request May 14, 2020

fix(comp): Prepare plugin completion for Cobra 1.0 helm/helm#7906

Merged

1 task

darthShadow mentioned this pull request Jul 17, 2020

Custom Completions in Go rclone/rclone#4443

Open

davidovich mentioned this pull request Dec 14, 2020

OnlyValidArgs does not take into account the new ValidArgsFunction #1298

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom completions coded in Go (instead of Bash) #1035

Custom completions coded in Go (instead of Bash) #1035

marckhouzam commented Feb 16, 2020 •

edited

Loading

AdamSLevy commented Feb 21, 2020

jharshman commented Feb 26, 2020

marckhouzam commented Feb 26, 2020 •

edited

Loading

marckhouzam commented Feb 27, 2020 •

edited

Loading

jharshman commented Mar 27, 2020

marckhouzam commented Mar 28, 2020 •

edited

Loading

jharshman commented Mar 28, 2020 •

edited

Loading

marckhouzam commented Mar 28, 2020

jharshman commented Mar 28, 2020

marckhouzam commented Mar 28, 2020 •

edited

Loading

jharshman commented Mar 28, 2020 •

edited

Loading

marckhouzam commented Mar 28, 2020

marckhouzam commented Mar 29, 2020

jharshman commented Mar 30, 2020

marckhouzam commented Apr 2, 2020

marckhouzam commented Apr 2, 2020

marckhouzam commented Apr 2, 2020

jharshman commented Apr 2, 2020 •

edited

Loading

marckhouzam commented Apr 3, 2020 •

edited

Loading

jharshman commented Apr 3, 2020

jharshman commented Apr 3, 2020

marckhouzam commented Apr 3, 2020

jharshman commented Apr 3, 2020

jharshman commented Apr 3, 2020

marckhouzam commented Apr 3, 2020

Custom completions coded in Go (instead of Bash) #1035

Custom completions coded in Go (instead of Bash) #1035

Conversation

marckhouzam commented Feb 16, 2020 • edited Loading

AdamSLevy commented Feb 21, 2020

jharshman commented Feb 26, 2020

marckhouzam commented Feb 26, 2020 • edited Loading

marckhouzam commented Feb 27, 2020 • edited Loading

jharshman commented Mar 27, 2020

marckhouzam commented Mar 28, 2020 • edited Loading

jharshman commented Mar 28, 2020 • edited Loading

marckhouzam commented Mar 28, 2020

jharshman commented Mar 28, 2020

marckhouzam commented Mar 28, 2020 • edited Loading

jharshman commented Mar 28, 2020 • edited Loading

marckhouzam commented Mar 28, 2020

marckhouzam commented Mar 29, 2020

jharshman commented Mar 30, 2020

marckhouzam commented Apr 2, 2020

marckhouzam commented Apr 2, 2020

marckhouzam commented Apr 2, 2020

jharshman commented Apr 2, 2020 • edited Loading

marckhouzam commented Apr 3, 2020 • edited Loading

jharshman commented Apr 3, 2020

jharshman commented Apr 3, 2020

marckhouzam commented Apr 3, 2020

jharshman commented Apr 3, 2020

jharshman commented Apr 3, 2020

marckhouzam commented Apr 3, 2020

marckhouzam commented Feb 16, 2020 •

edited

Loading

marckhouzam commented Feb 26, 2020 •

edited

Loading

marckhouzam commented Feb 27, 2020 •

edited

Loading

marckhouzam commented Mar 28, 2020 •

edited

Loading

jharshman commented Mar 28, 2020 •

edited

Loading

marckhouzam commented Mar 28, 2020 •

edited

Loading

jharshman commented Mar 28, 2020 •

edited

Loading

jharshman commented Apr 2, 2020 •

edited

Loading

marckhouzam commented Apr 3, 2020 •

edited

Loading