spf13 / cobra

A Commander for modern Go CLI interactions
https://cobra.dev
Apache License 2.0
37.51k stars 2.83k forks source link

Need an error when the command is not found in the subcommand #2130

Open timidsmile opened 4 months ago

timidsmile commented 4 months ago

hi

Description

When I execute a job on our task center, I need to get the final result of the execution, for example:

myapp job1 args1

When job1 doesn't exist, I can get the error:

Error: unknown command "job1" for ""
Run ' --help' for usage.

Then the task center can be accessed by echo $? to get the final execution result. However, when I use a subcommand, I can't get the error.

How to reproduce

For example :

package main

import (
    "github.com/spf13/cobra"
)

func main() {
    var rootCmd = &cobra.Command{}

    configCmd := cobra.Command{
        Use: "config",
    }

    getCmd := cobra.Command{
        Use: "set",
        Run: func(cmd *cobra.Command, args []string) {
            println("this is the config set")
        },
    }
    setCmd := cobra.Command{
        Use: "get",
        Run: func(cmd *cobra.Command, args []string) {
            println("this is the config get")
        },
    }
    configCmd.AddCommand(&getCmd)
    configCmd.AddCommand(&setCmd)
    rootCmd.AddCommand(&configCmd)

    if err := rootCmd.Execute(); err != nil {
        panic(err.Error())
    }
}

When I execute:

myapp config xxx

I can get the following prompt without an error.

Usage:
   config [command]

Available Commands:
  get         
  set         

Flags:
  -h, --help   help for config

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

Expectations

Need to return error


Is there some special consideration here? https://github.com/spf13/cobra/blob/main/command.go#L1123

marckhouzam commented 4 months ago

You can play around with adding the Args field.

If you search issues you’ll find there have been multiple discussions about this.

andremueller commented 2 months ago

I think the main problem is here:

https://github.com/spf13/cobra/blob/e94f6d0dd9a5e5738dca6bce03c4b1207ffbc0ec/command.go#L925-L927

If we could just return another error code instead of flag.ErrHelp lets say ErrUnknownSubcommand then the outer error would be forwarded in the lines @timidsmile mentioned, and the user at the outside call of e.g. rootCmd.Execute() could handle that error and print e.g. help or call os.exit(1) - that would solve many problems here. When automating things with scripts we have otherwise no idea when a sub-command is not even implemented! Would this really break the API for the others? Should I pose a merge request with that patch?

In my opinion it must be possible to differentiate between calling help and an unknown subcommand! So a few lines above the help command uses the same error code!

https://github.com/spf13/cobra/blob/e94f6d0dd9a5e5738dca6bce03c4b1207ffbc0ec/command.go#L905-L907

marckhouzam commented 2 months ago

and the user at the outside call of e.g. rootCmd.Execute() could handle that error and print e.g. help or call os.exit(1)

This requires the caller to change their code to keep the current behavior, which breaks backwards compatibility. If this is important enough, then we could add this as an optional opt-in behavior.

andremueller commented 2 months ago

Ok. What about a further flag in the Command struct such like

type Command struct {
   // everything else ...
   ErrorOnUnknownSubcommand bool
}

Default is false - and we would then change the lines in Execute like this

    if !c.Runnable() {
               if c.ErrorOnUnknownSubcommand {
                  return ErrUnknownSubcommand
               } else {
           return flag.ErrHelp
              }
    }

would that be fine? All users who really want this feature can just activate it - and all others will not care.

marckhouzam commented 2 months ago

That seems reasonable