subcommand handling is inconsistent

[didrocks]

This is with cobra v0.0.5.

This is some kind of meta-bug (we can separate this if needed, but I think a general view can help getting a global schema) to gather some of where subcommand support in cobra is falling short. It has a little overlap with bug #870, pr #842, and as I saw the issue discussion that once 1.0 is out, there is some willingness to revisit the maintainance governance, I think this is the right moment to discuss this.

Here is a small example:
foo has 3 subcommands, one having sub subcommand and another one being hidden:
foo:

• sub1 (alias "alias1")
  • subA
  • subB
• sub2
• sub3 (hidden).
  Flag --bar

The base code is:

package main

import (
	"fmt"
	"os"

	"github.com/spf13/cobra"
)

var (
	rootCmd = &cobra.Command{
		Use:           "foo",
		SilenceErrors: true,
	}

	subCmd1 = &cobra.Command{
		Use:     "sub1",
		Aliases: []string{"alias1"},
	}

	subCmdA = &cobra.Command{
		Use: "subsubA",
		Run: func(cmd *cobra.Command, args []string) {},
	}

	subCmdB = &cobra.Command{
		Use: "subsubB",
		Run: func(cmd *cobra.Command, args []string) {},
	}

	subCmd2 = &cobra.Command{
		Use: "sub2",
		Run: func(cmd *cobra.Command, args []string) {},
	}

	subCmd3 = &cobra.Command{
		Use:    "sub3",
		Hidden: true,
		Run:    func(cmd *cobra.Command, args []string) {},
	}
)

func init() {
	rootCmd.AddCommand(subCmd1, subCmd2, subCmd3)
	subCmd1.AddCommand(subCmdA, subCmdB)
	subCmd3.Flags().Bool("bar", false, "")
}

func main() {
	if err := rootCmd.Execute(); err != nil {
		fmt.Println(err)
		os.Exit(1)
	}
}

Requiring subcommands

We don't want to be able to run foo or foo sub1 without any arguments, but force providing a subcommand. We could specify ValidArgs at this stage, but the error message is then confusing and doesn't mention subcommands. This is easily doable by a dedicated runE:

...
rootCmd = &cobra.Command{
		Use:           "foo",
		SilenceErrors: true,
		RunE:          requireSubcommand,
	}

	subCmd1 = &cobra.Command{
		Use:     "sub1",
		Aliases: []string{"alias1"},
		RunE:    requireSubcommand,
	}
...
func requireSubcommand(cmd *cobra.Command, args []string) error {
	return fmt.Errorf("%s requires a subcommand", cmd.Name())
}
...

With this:

$ ./foo
Usage:
  foo [flags]
  foo [command]

Available Commands:
  help        Help about any command
  sub1        
  sub2        

Flags:
  -h, --help   help for foo

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

foo requires a subcommand
$ ./foo sub1
Usage:
  foo sub1 [flags]
  foo sub1 [command]

Aliases:
  sub1, alias1

Available Commands:
  subsubA     
  subsubB     

Flags:
  -h, --help   help for sub1

Use "foo sub1 [command] --help" for more information about a command.

sub1 requires a subcommand

Only sub1 and sub2 are shown, as expected, in addition to help.

Changing usage

Note though that the usage isn't correct:

  foo [flags]
  foo [command]

So, we change this to:

Use:           "foo COMMAND",
...
Use:     "sub1 COMMAND",
...

Now, the usage is a little bit better, but still not accurate:

Usage:
  foo sub1 COMMAND [flags]
  foo sub1 [command]

I couldn't find in cobra's code any exported function which enables removing the invalid "foo sub1 [command]" line.

I think in general a revisited way to tell "this command requires subcommands only" would help fixing the additional code and other issues.

Suggestions for command

Suggestions for subcommands works, just after the root command:

$ ./foo s
unknown command "s" for "foo"

Did you mean this?
        sub1
        sub2

cobra only shows non hidden matching commands and not the alias, great.

Help (which is a valid subcommand of the rootCmd isn't suggested)

$ ./foo h
unknown command "h" for "foo"

Alias are not suggested either

Similar, aliases are not suggested even if it should match:

$ ./foo a
unknown command "a" for "foo"

Sub-subcommand's suggestion doesn't work

Subcommands of sub1 doesn't show up any suggestions:

$ ./foo sub1 s
Usage:
  foo sub1 COMMAND [flags]
  foo sub1 [command]

Aliases:
  sub1, alias1

Available Commands:
  subsubA     
  subsubB     

Flags:
  -h, --help   help for sub1

Use "foo sub1 [command] --help" for more information about a command.

sub1 requires a subcommand

We can see that:

• runE was called (hence the error "sub1 requires a subcommand")
• no suggestion was provided. This is due to legacyArgs()`` not being called as commandFound.Args != nilfor subcommand (inFind()). This is the only place where the internal cmd.findSuggestions()is called (with valid args checking, more on that later). Even iflegacyArgs()would have been called, the check!cmd.HasParent()` would prevent it to give any suggestions.

I would prefer to not have to duplicate the strings and checks for the internal findSuggestions() (see the comment on the first bug report as well, where we have duplicated string).I found a way to mitigate this and the previous issues is to use Args: cobra.OnlyValidArgs, and expand ValidArgs dynamically.

...
	subCmd1 = &cobra.Command{
		Use:     "sub1 COMMAND",
		Aliases: []string{"alias1"},
		RunE:    requireSubcommand,
		Args: cobra.OnlyValidArgs,
	}
...
func init() {
        ...
	addSubcommandsToValidArgs(subCmd1)
}

func addSubcommandsToValidArgs(cmd *cobra.Command) {
	validArgs := cmd.ValidArgs
	for _, c := range cmd.Commands() {
		validArgs = append(validArgs, c.Name())
	}
	cmd.ValidArgs = validArgs
}
...

We now have subcommands suggestions:

$ ./foo sub1 s
Usage:
  foo sub1 COMMAND [flags]
  foo sub1 [command]

Aliases:
  sub1, alias1

Available Commands:
  subsubA     
  subsubB     

Flags:
  -h, --help   help for sub1

Use "foo sub1 [command] --help" for more information about a command.

invalid argument "s" for "foo sub1"

Did you mean this?
        subsubA
        subsubB

However, the experience vastly different from the one printing on the root cmd:

$ ./foo s
unknown command "s" for "foo"

Did you mean this?
        sub1
        sub2

-> this is way shorter (no usage printed), and we have "unkown command…" instead of "invalid argument".

If we add similarly the same kind of argument check on the root command, of course, the inconsistency is "fixed", but this is by using the less accurate version, the desired strings being in legacyArgs().

Note that this doesn't work though for aliases or "help" command, even if they are added and attached to the root command:

...
func addSubcommandsToValidArgs(cmd *cobra.Command) {
	validArgs := cmd.ValidArgs

	// Add subcommands and aliases
	for _, c := range cmd.Commands() {
		validArgs = append(validArgs, c.Name())
		validArgs = append(validArgs, c.Aliases...)
	}

	// Add help for root command
	if !cmd.HasParent() {
		fmt.Println("ooo")
		validArgs = append(validArgs, "help")
	}

	cmd.ValidArgs = validArgs
}
...
+ for rootCmd:
Args:          cobra.OnlyValidArgs,
and call to addSubcommandsToValidArgs(rootCmd)

This returns:

$ ./foo h
Usage:
  foo COMMAND [flags]
  foo [command]

Available Commands:
  help        Help about any command
  sub1        
  sub2        

Flags:
  -h, --help   help for foo

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

invalid argument "h" for "foo"

And aliases don't work either:

$ ./foo a
Usage:
  foo COMMAND [flags]
  foo [command]

Available Commands:
  help        Help about any command
  sub1        
  sub2        

Flags:
  -h, --help   help for foo

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

invalid argument "a" for "foo"

This is due to SuggestionsFor() only checking subcommands (so discaring "help") and their names (discaring thus Aliases).
Note that we could potentially override it from the program and attach a new function to each command, as it's exported, it will need to replace c.commands by c.Commands().

No suggestion for flags:

There is no suggestion for flags:

$ ./foo sub3 --b
ooo
Usage:
  foo sub3 [flags]

Flags:
      --bar    
  -h, --help   help for sub3

unknown flag: --b

"--bar" could be suggested. This is what the initial bug reported I linked at was about.

completions once entered in a "hidden" command

Completions for subcommands are also generally puzzled by this.
Note that to not puzzle the result with hidden commands, I needed to remove

addSubcommandsToValidArgs(rootCmd)

As other "help", "aliases" and hidden commands would show up (which is one of the limitation of the previous approach).

Via a call to rootCmd.GenBashCompletionFile and sourcing the resulting file, here is what we get:

$ foo sub[tab][tab]
sub1  sub2  

-> sub3 is hidden, as expected.

Aliases and help

However, Aliases, nor Help aren't completed $ foo [tab] completes to "foo sub".

Hidden command completion

Worse, hidden commands, once typed, doesn't complete and provide invalid completions:

$ foo sub3 [tab]
-> completes to $ foo sub3 sub[tab][tab]
sub1  sub2 

From a user perspective, I would expect not being completed by other commands, but by available flags on sub3 (as I typed the "hidden command" already).

Sorry for all this to be a little bit long to digest, but I wanted to put everything where I think cobra is falling short in term of subcommand handling vs suggestions and completions. I dont' like to open meta-bug, but I guess putting all those issues in perspective in a single place help if there is a desire to rework that part. Am I wrong on my approach and things can be mitigate in a more consistant way?

For reference, complete code

This is the one with some workarounds as previously stated (but doesn't fix all the issues as mentionned above)

package main

import (
	"fmt"
	"os"

	"github.com/spf13/cobra"
)

var (
	rootCmd = &cobra.Command{
		Use:           "foo COMMAND",
		SilenceErrors: true,
		RunE:          requireSubcommand,
		Args:          cobra.OnlyValidArgs,
	}

	subCmd1 = &cobra.Command{
		Use:     "sub1 COMMAND",
		Aliases: []string{"alias1"},
		RunE:    requireSubcommand,
		Args:    cobra.OnlyValidArgs,
	}

	subCmdA = &cobra.Command{
		Use: "subsubA",
		Run: func(cmd *cobra.Command, args []string) {},
	}

	subCmdB = &cobra.Command{
		Use: "subsubB",
		Run: func(cmd *cobra.Command, args []string) {},
	}

	subCmd2 = &cobra.Command{
		Use: "sub2",
		Run: func(cmd *cobra.Command, args []string) {},
	}

	subCmd3 = &cobra.Command{
		Use:    "sub3",
		Hidden: true,
		Run:    func(cmd *cobra.Command, args []string) {},
	}
)

func init() {
	rootCmd.AddCommand(subCmd1, subCmd2, subCmd3)
	subCmd1.AddCommand(subCmdA, subCmdB)
	subCmd3.Flags().Bool("bar", false, "")
        // disabled for shell completion examples
	//addSubcommandsToValidArgs(rootCmd)
	addSubcommandsToValidArgs(subCmd1)
}

func requireSubcommand(cmd *cobra.Command, args []string) error {
	return fmt.Errorf("%s requires a subcommand", cmd.Name())
}

func addSubcommandsToValidArgs(cmd *cobra.Command) {
	validArgs := cmd.ValidArgs

	// Add subcommands and aliases
	for _, c := range cmd.Commands() {
		validArgs = append(validArgs, c.Name())
		validArgs = append(validArgs, c.Aliases...)
	}

	// Add help for root command
	if !cmd.HasParent() {
		validArgs = append(validArgs, "help")
	}

	cmd.ValidArgs = validArgs
}

func main() {
	if err := rootCmd.Execute(); err != nil {
		fmt.Println(err)
		os.Exit(1)
	}
}

In summary, the remaining issues are:

• no "help" command suggestion
• no alias suggestion
• no flag suggestion
• incorrect completion when starting completing hidden command "sub3"

[didrocks]

In addition to the MP fixnig some of the subcommands, I think we can get a new help fixing most of the issues above (suggesting and requiring subcommands):

// subcommandsRequiredWithSuggestions will ensure we have a subcommand provided by the user and augments it with
// suggestion for commands, alias and help on root command.
func subcommandsRequiredWithSuggestions(cmd *cobra.Command, args []string) error {
	requireMsg := "%s requires a valid subcommand"
	// This will be triggered if cobra didn't find any subcommands.
	// Find some suggestions.
	var suggestions []string

	if len(args) != 0 && !cmd.DisableSuggestions {
		typedName := args[0]
		if cmd.SuggestionsMinimumDistance <= 0 {
			cmd.SuggestionsMinimumDistance = 2
		}
		// subcommand suggestions
		suggestions = append(cmd.SuggestionsFor(args[0]))

		// subcommand alias suggestions (with distance, not exact)
		for _, c := range cmd.Commands() {
			if c.IsAvailableCommand() {
				for _, alias := range c.Aliases {
					levenshteinDistance := ld(typedName, alias, true)
					suggestByLevenshtein := levenshteinDistance <= cmd.SuggestionsMinimumDistance
					suggestByPrefix := strings.HasPrefix(strings.ToLower(alias), strings.ToLower(typedName))
					if suggestByLevenshtein || suggestByPrefix {
						suggestions = append(suggestions, alias)
					}
				}
			}
		}

		// help for root command
		if !cmd.HasParent() {
			help := "help"
			levenshteinDistance := ld(typedName, help, true)
			suggestByLevenshtein := levenshteinDistance <= cmd.SuggestionsMinimumDistance
			suggestByPrefix := strings.HasPrefix(strings.ToLower(help), strings.ToLower(typedName))
			if suggestByLevenshtein || suggestByPrefix {
				suggestions = append(suggestions, help)
			}
		}
	}

	var suggestionsMsg string
	if len(suggestions) > 0 {
		suggestionsMsg += "Did you mean this?\n"
		for _, s := range suggestions {
			suggestionsMsg += fmt.Sprintf("\t%v\n", s)
		}
	}

	if suggestionsMsg != "" {
		requireMsg = fmt.Sprintf("%s. %s", requireMsg, suggestionsMsg)
	}

	return fmt.Errorf(requireMsg, cmd.Name())
}

I think some of the code could be extracted/factorized directly in cobra's SuggestFor(), and give an helper for require/not requiring subcommands.

The only parts not fixed are "Changing usage" and "No suggestion for flags" by the code above. Integrating something around this in cobra directly will enable to prevent code and string duplication and not having to copy the "ld()" function directly. I'm happy to work on another MP for this is that makes sense.

Here is the complete example:

package main

import (
	"fmt"
	"os"
	"strings"

	"github.com/spf13/cobra"
)

var (
	rootCmd = &cobra.Command{
		Use:           "foo COMMAND",
		SilenceErrors: true,
		Args:          subcommandsRequiredWithSuggestions,
		Run:           func(cmd *cobra.Command, args []string) {},
	}

	subCmd1 = &cobra.Command{
		Use:     "sub1 COMMAND",
		Aliases: []string{"alias1"},
		Args:    subcommandsRequiredWithSuggestions,
		Run:     func(cmd *cobra.Command, args []string) {},
	}

	subCmdA = &cobra.Command{
		Use: "subsubA",
		Run: func(cmd *cobra.Command, args []string) {},
	}

	subCmdB = &cobra.Command{
		Use: "subsubB",
		Run: func(cmd *cobra.Command, args []string) {},
	}

	subCmd2 = &cobra.Command{
		Use: "sub2",
		Run: func(cmd *cobra.Command, args []string) {},
	}

	subCmd3 = &cobra.Command{
		Use:    "sub3",
		Hidden: true,
		Run:    func(cmd *cobra.Command, args []string) {},
		Args:   subcommandsRequiredWithSuggestions,
	}

	subCmdC = &cobra.Command{
		Use: "subsubC",
		Run: func(cmd *cobra.Command, args []string) {},
	}
)

func init() {
	rootCmd.AddCommand(subCmd1, subCmd2)
	rootCmd.AddCommand(subCmd3)
	rootCmd.PersistentFlags().Bool("verbose", false, "")
	rootCmd.Flags().Bool("baz", false, "")
	subCmd1.AddCommand(subCmdA, subCmdB)
	subCmd3.AddCommand(subCmdC)
	subCmd3.Flags().Bool("bar", false, "")
	subCmdC.Flags().Bool("other", false, "")
}

// subcommandsRequiredWithSuggestions will ensure we have a subcommand provided by the user and augments it with
// suggestion for commands, alias and help on root command.
func subcommandsRequiredWithSuggestions(cmd *cobra.Command, args []string) error {
	requireMsg := "%s requires a valid subcommand"
	// This will be triggered if cobra didn't find any subcommands.
	// Find some suggestions.
	var suggestions []string

	if len(args) != 0 && !cmd.DisableSuggestions {
		typedName := args[0]
		if cmd.SuggestionsMinimumDistance <= 0 {
			cmd.SuggestionsMinimumDistance = 2
		}
		// subcommand suggestions
		suggestions = append(cmd.SuggestionsFor(args[0]))

		// subcommand alias suggestions (with distance, not exact)
		for _, c := range cmd.Commands() {
			if c.IsAvailableCommand() {
				for _, alias := range c.Aliases {
					levenshteinDistance := ld(typedName, alias, true)
					suggestByLevenshtein := levenshteinDistance <= cmd.SuggestionsMinimumDistance
					suggestByPrefix := strings.HasPrefix(strings.ToLower(alias), strings.ToLower(typedName))
					if suggestByLevenshtein || suggestByPrefix {
						suggestions = append(suggestions, alias)
					}
				}
			}
		}

		// help for root command
		if !cmd.HasParent() {
			help := "help"
			levenshteinDistance := ld(typedName, help, true)
			suggestByLevenshtein := levenshteinDistance <= cmd.SuggestionsMinimumDistance
			suggestByPrefix := strings.HasPrefix(strings.ToLower(help), strings.ToLower(typedName))
			if suggestByLevenshtein || suggestByPrefix {
				suggestions = append(suggestions, help)
			}
		}
	}

	var suggestionsMsg string
	if len(suggestions) > 0 {
		suggestionsMsg += "Did you mean this?\n"
		for _, s := range suggestions {
			suggestionsMsg += fmt.Sprintf("\t%v\n", s)
		}
	}

	if suggestionsMsg != "" {
		requireMsg = fmt.Sprintf("%s. %s", requireMsg, suggestionsMsg)
	}

	return fmt.Errorf(requireMsg, cmd.Name())
}

func main() {
	if err := rootCmd.Execute(); err != nil {
		fmt.Println(err)
		os.Exit(1)
	}
}

// ld compares two strings and returns the levenshtein distance between them.
func ld(s, t string, ignoreCase bool) int {
	if ignoreCase {
		s = strings.ToLower(s)
		t = strings.ToLower(t)
	}
	d := make([][]int, len(s)+1)
	for i := range d {
		d[i] = make([]int, len(t)+1)
	}
	for i := range d {
		d[i][0] = i
	}
	for j := range d[0] {
		d[0][j] = j
	}
	for j := 1; j <= len(t); j++ {
		for i := 1; i <= len(s); i++ {
			if s[i-1] == t[j-1] {
				d[i][j] = d[i-1][j-1]
			} else {
				min := d[i-1][j]
				if d[i][j-1] < min {
					min = d[i][j-1]
				}
				if d[i-1][j-1] < min {
					min = d[i-1][j-1]
				}
				d[i][j] = min + 1
			}
		}

	}
	return d[len(s)][len(t)]
}

[umarcor]

Hi @didrocks and thanks for this very detailed issue! It seems that there's content for a bunch of PRs here... let's go step by step.

I think that the base code you provided should be used for a set of tests. Then, each of the sections/issues that you commented would fit as a specific test. For now, we can only validate supported features, and fail speculatively (i.e. assuming that additional content with a specific format will be added).

I'm the author of #842, which is based on #841. I think that #841 is ready to be merged, and it provides the foundation to implement proper detection of commands with/without flags/args/subcommands. Hence, I'd be glad if we could create the set of tests on top of #841. On the one hand, it would probably fix some of the issues related to ValidArgs. On the other hand, that'd allow me to rethink/reimplement #841 to account for probably uncovered cases.

From there on, we might see how much of addSubcommandsToValidArgs and subcommandsRequiredWithSuggestions needs to be implemented in cobra. What do you think?

[github-actions]

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

[vinamra28]

Hi @umarcor , just wanted to know what's the status of this issue and when can we expect in cobra ?
Thanks

[marckhouzam]

For completion, there was progress.

• completion of the help command
  This was just fixed in Add completion for help command #1136.
• completion for commands aliases
  Commands aliases are not completed, and I believe this was done on purpose. However, it has been suggested that if no other completion is possible then a command's alias should be completed. No progress on implementing this as far as I know although there was an attempt for zsh, see details in Add Aliases completions on ZSH #1014 (comment) This suggested behavior is the behavior for argument aliases (ArgAliases) so has a precedent.
• completion for hidden commands
  I haven't tested but I believe this works for Fish completion already and will work for Zsh completion once Zsh completion V2 #1070 is merged. I think we will need to discuss aligning bash completion (I'm currently working on a proposal).

[vinamra28]

Hi @marckhouzam , thank you for the information. But I wanted to know the status of subcommands suggestions as in the current cobra version we don't get the subcommands suggestions. example:-

Current situation :-

$ ./foo sub1 s
Usage:
  foo sub1 COMMAND [flags]
  foo sub1 [command]

Aliases:
  sub1, alias1

Available Commands:
  subsubA     
  subsubB     

Flags:
  -h, --help   help for sub1

Use "foo sub1 [command] --help" for more information about a command.

Expected:-

invalid argument "s" for "foo sub1"

Did you mean this?
        subsubA
        subsubB

[umarcor]

Hi @umarcor , just wanted to know what's the status of this issue and when can we expect in cobra ?
Thanks

#841 is ready for merging since March 2019. Two users did already review it, and a maintainer was requested to do so in June 2019: #841 (comment). If/when #841 is merged (some day), the work in #842 may continue.

[vinamra28]

@umarcor Thanks for the update 🙂

[caarlos0]

FWIW, it seems that setting TraverseChildren on the root command breaks suggestions on root as well... (it doesn't make much sense to set it in the root command anyway, but this is an unexpected side effect)

[Dentrax]

Any updates on this? Found the following workaround:

// https://github.com/containerd/nerdctl/blob/242e6fc6e861b61b878bd7df8bf25e95674c036d/cmd/nerdctl/main.go#L401-L418
func unknownSubcommandAction(cmd *cobra.Command, args []string) error {
	if len(args) == 0 {
		return cmd.Help()
	}
	err := fmt.Sprintf("unknown subcommand %q for %q", args[0], cmd.Name())
	if suggestions := cmd.SuggestionsFor(args[0]); len(suggestions) > 0 {
		err += "\n\nDid you mean this?\n"
		for _, s := range suggestions {
			err += fmt.Sprintf("\t%v\n", s)
		}
	}
	return fmt.Errorf(err)
}

And use:

cmd := &cobra.Command{
		Use:           "test",
		RunE:          unknownSubcommandAction,
		SilenceUsage:  true,
		SilenceErrors: true,
	}

[avamsi]

Related: #706.