cobra icon indicating copy to clipboard operation
cobra copied to clipboard

subcommand handling is inconsistent

Open didrocks opened this issue 5 years ago • 11 comments

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 avatar Oct 25 '19 10:10 didrocks