Provide usefull errors when using invalid sub command

[mandelsoft]

If the TraverseChildren mode is active, the Traverse function returns the last accepted command along the requested sub command chain. This command is then used to parse the arguments. This will for sure fail, if the traversing was aborted because an invalid sub command was given. As a result, cobra complains about invalid flags instead of an invalid sub command.

This behaviour might be useful, if the intermediate command is Runnable(), but it for sure makes no sense, if it isn't.

This PR fixes this by providing a legacyArgs() error in case of not runnable intermediate commands.

---

Remark: It solves the obvious problematic behaviour, but basically the general scenario is more complex.
If the intermediate command is Runnable() in addition to providing additional sub commands, then it should be possible to forward the control about the error message to the command to be a able to distinguish between these two cases according to its needs. This is basically already possible by declaring a FlagErrorFunc(). But unfortunately it only gets the command and error as arguments, but not the actual set of arguments. Therefore it has no chance to detect the actual problem and provide appropriate error feedback.

Solving this is more invasive, because it would require, either to change the interface, or add another more general error handler option.

[CLAassistant]

All committers have signed the CLA.

[Skarlso]

Hello @marckhouzam :)

Sorry for the ping, just raising visibility. Is there any chance for anyone to review this change? Pretty please? :)

The code is concise and contains a small, but effective fix for a relative obscure behaviour but leaves a really nice error for the user.

[marckhouzam]

Thanks @mandelsoft. To make the review easier, could you provide a simple test program and the steps to reproduce the problem? Ideally you would open an issue with that information and refer this PR to that issue.

Thank you.

[Skarlso]

Thank you, Marc!

[mandelsoft]

Hello @marckhouzam, sorry, for the delay. I could open an issue, also, if you want.

The problem can be reproduced with the following program:

package main

import (
	"fmt"

	"github.com/spf13/cobra"
)

var opt string

func main() {
	cmd := &cobra.Command{
		Use:                   "test",
		Short:                 "test program",
		Version:               "v1",
		TraverseChildren:      true,
		SilenceUsage:          true,
		DisableFlagsInUseLine: true,
		PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
			return nil
		},
	}

	sub := &cobra.Command{
		Use:                   "sub",
		Short:                 "sub command",
		TraverseChildren:      true,
		SilenceUsage:          true,
		DisableFlagsInUseLine: true,
		RunE: func(cmd *cobra.Command, args []string) error {
			fmt.Printf("option is %q\n", opt)
			return nil
		},
	}
	flagset := sub.Flags()
	flagset.StringVarP(&opt, "opt", "", "", "some option")

	cmd.AddCommand(sub)
	cmd.InitDefaultHelpCmd()
	cmd.Execute()
}

It uses a sub command subwith the option --opt. If run with

$ cmd sub --opt test

It prints the option value.

If called with a misspelled sub command

$ cmd subb --opt test

It provides the following error message:

Error: unknown flag: --opt

This is not directly wrong, but misleading, the problem is not the option, but the misspelled sub command and the main command does not provide a runner (meaning it cannot be executed without a sub command).

With the provided fix, the error output correctly is

Error: unknown command "subb" for "test"

Did you mean this?
        sub

Run 'test --help' for usage.

[Skarlso]

@marckhouzam Hello. :) 👋

Are you happy with the added information? :)

Thanks! 🙏

[Skarlso]

@marckhouzam Hello. :) 👋

Are you happy with the added information? :)

Thanks! 🙏