Skip to content
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

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

marckhouzam
Copy link
Collaborator

@marckhouzam marckhouzam commented Dec 11, 2021

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

@scop
Copy link
Contributor

scop commented Dec 12, 2021

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 :)

@marckhouzam
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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.

@github-actions
Copy link

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

@marckhouzam marckhouzam added area/shell-completion All shell completions and removed kind/stale labels Feb 24, 2022
@caarlos0
Copy link
Contributor

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

Thanks!

@marckhouzam
Copy link
Collaborator Author

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 marckhouzam added this to the 1.6.0 milestone Jun 29, 2022
@CLAassistant
Copy link

CLAassistant commented Jul 21, 2022

CLA assistant check
All committers have signed the CLA.

@caarlos0
Copy link
Contributor

anything I can do to help getting this merged?

@marckhouzam
Copy link
Collaborator Author

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.

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, or if it is being completed itself.

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).

Signed-off-by: Marc Khouzam <marc.khouzam@gmail.com>
@marckhouzam
Copy link
Collaborator Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants