cobra icon indicating copy to clipboard operation
cobra copied to clipboard

Add default completion command even if there are no other sub-commands

Open marckhouzam opened this issue 3 years ago • 10 comments

This supersedes #1450 and #1392

When a program has no sub-commands, its root command can accept arguments. If we add the default "completion" command to such programs they will now have a sub-command and will no longer accept arguments.

What we do instead for this special case, is only add the "completion" command if it is being called.

We want to have the "completion" command for such programs because it will allow the completion of flags and of arguments (if provided by the program).

@scop can you confirm this is what you were hoping for?

Testing done

# Here is a program with no sub-cmds
$ cat tests/minimal.go
package main

import (
	"fmt"

	"github.com/spf13/cobra"
)

var rootCmd = &cobra.Command{
	Use: "prog",
	Run: func(cmd *cobra.Command, args []string) {
		fmt.Println("value:", value)
		fmt.Println("args:", args)
	},
}

var value string

func main() {
	rootCmd.Flags().StringVarP(&value, "value", "e", "", "value")
	rootCmd.Execute()
}

$ go build -o minimal ./tests/minimal.go

$ ./minimal
value:
args: []
$ ./minimal arg1
value:
args: [arg1]
$ ./minimal arg1 arg2
value:
args: [arg1 arg2]
$ ./minimal arg1 arg2 --value 1
value: 1
args: [arg1 arg2]

# Notice no 'completion' command visible
$ ./minimal -h
Usage:
  prog [flags]

Flags:
  -h, --help           help for prog
  -e, --value string   value

# But the completion command does exist
$ ./minimal completion -h
Generate the autocompletion script for prog for the specified shell.
See each sub-command's help for details on how to use the generated script.

Usage:
  prog completion [command]

Available Commands:
  bash        Generate the autocompletion script for bash
  fish        Generate the autocompletion script for fish
  powershell  Generate the autocompletion script for powershell
  zsh         Generate the autocompletion script for zsh

Flags:
  -h, --help   help for completion

Use "prog completion [command] --help" for more information about a command.

# Shell completion for the 'completion' command itself is not triggered,
# but it is triggered for sub-commands of 'completion'
$ ./minimal __complete compl
:0
Completion ended with directive: ShellCompDirectiveDefault
$ ./minimal __complete completion ''
bash	Generate the autocompletion script for bash
fish	Generate the autocompletion script for fish
powershell	Generate the autocompletion script for powershell
zsh	Generate the autocompletion script for zsh
:4
Completion ended with directive: ShellCompDirectiveNoFileComp

marckhouzam avatar Dec 11 '21 19:12 marckhouzam

Yep, seems to work nicely, thanks!

An observation is that as the completion command is hidden, no completions are generated for its arguments (i.e. "foo completion Tab"), but that's such a minor detail and not a result of this change I believe, that let's not get stuck with that :)

scop avatar Dec 12 '21 20:12 scop

An observation is that as the completion command is hidden, no completions are generated for its arguments (i.e. "foo completion Tab")

Nice attention to detail! The completions should work for hidden commands. In this case however, when we call the "__complete" command to get the completions, we don't create the "completion" command so its subcommands don't get completed.

Let me try to see if I can fix it.

marckhouzam avatar Dec 12 '21 21:12 marckhouzam

While working on this I ran into another completion bug for programs that don't have sub-commands. I've opened issue #1562 and posted a fix in #1563.

I'm still looking at fixing the issue @scop reported above.

marckhouzam avatar Dec 14 '21 14:12 marckhouzam

This PR is being marked as stale due to a long period of inactivity

github-actions[bot] avatar Feb 13 '22 00:02 github-actions[bot]

This looks good, was just about to dig into this myself.

Thanks!

caarlos0 avatar Jun 29 '22 19:06 caarlos0

I've let this rot for too long. Let's plan it for the next release. Come August iI will run some tests on it and make sure it is good.

marckhouzam avatar Jun 29 '22 22:06 marckhouzam

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Jul 21 '22 02:07 CLAassistant

anything I can do to help getting this merged?

caarlos0 avatar Jun 13 '23 18:06 caarlos0

I've finally re-worked this and I believe it is ready. Please refer to the description of the PR to see the testing that I did, which shows the new behaviour.

marckhouzam avatar Jun 19 '23 19:06 marckhouzam

@jpmcb When you have moment, this is ready for review. Don't hesitate to ask for clarifications.

marckhouzam avatar Jul 12 '23 19:07 marckhouzam