From fec0a0b1ff4732acb10b5059c432160d372a949b Mon Sep 17 00:00:00 2001 From: umarcor Date: Tue, 19 Mar 2019 19:41:09 +0100 Subject: [PATCH] feat: add '[args]' and 'Valid Args:' to --help --- args.go | 22 +-- args_test.go | 458 ++++++++++++++++++++++++++++++++++++++++-------- command.go | 134 +++++++++----- command_test.go | 42 ++--- 4 files changed, 503 insertions(+), 153 deletions(-) diff --git a/args.go b/args.go index 099d0a3950..bf18e02c19 100644 --- a/args.go +++ b/args.go @@ -6,27 +6,13 @@ import ( type PositionalArgs func(cmd *Command, args []string) error -// Legacy arg validation has the following behaviour: -// - root commands with no subcommands can take arbitrary arguments -// - root commands with subcommands will do subcommand validity checking -// - subcommands will always accept arbitrary arguments -func legacyArgs(cmd *Command, args []string) error { - // no subcommand, always take args - if !cmd.HasSubCommands() { - return nil - } - - // root command with subcommands, do subcommand checking. - if !cmd.HasParent() && len(args) > 0 { - return fmt.Errorf("unknown command %q for %q%s", args[0], cmd.CommandPath(), cmd.findSuggestions(args[0])) - } - return nil -} - // NoArgs returns an error if any args are included. func NoArgs(cmd *Command, args []string) error { if len(args) > 0 { - return fmt.Errorf("unknown command %q for %q", args[0], cmd.CommandPath()) + if cmd.HasAvailableSubCommands() { + return fmt.Errorf("unknown command %q for %q", args[0], cmd.CommandPath()) + } + return fmt.Errorf("\"%s\" rejected; %q does not accept args", args[0], cmd.CommandPath()) } return nil } diff --git a/args_test.go b/args_test.go index 2730e0a952..35a00cd37b 100644 --- a/args_test.go +++ b/args_test.go @@ -1,19 +1,29 @@ package cobra import ( + "bytes" "strings" "testing" ) -func newCommand(args PositionalArgs, withValid bool) *Command { +func newCmd(args PositionalArgs, wValid, wRun bool) *Command { c := &Command{ - Use: "c", - Args: args, - Run: emptyRun, + Use: "c", + Short: "A generator", + Long: `Cobra is a CLI ...`, + //Run: emptyRun, } - if withValid { + if args != nil { + c.Args = args + } + if wValid { c.ValidArgs = []string{"one", "two", "three"} } + if wRun { + c.Run = func(cmd *Command, args []string) { + //fmt.Println("RUN", args) + } + } return c } @@ -31,177 +41,217 @@ func expectError(err error, t *testing.T, ex string) { t.Fatal("Expected an error") } expected := map[string]string{ - "valid": `invalid argument "a" for "c"`, - "no": `unknown command "one" for "c"`, - "min": "requires at least 2 arg(s), only received 1", - "max": "accepts at most 2 arg(s), received 3", - "exact": "accepts 2 arg(s), received 3", - "range": "accepts between 2 and 4 arg(s), received 1", + "valid": `invalid argument "a" for "c"`, + "run": `command "c" is not runnable`, + "runsub": `command "c" is not runnable; please provide a subcmd`, + "known": `unknown command "one" for "c"`, + "no": `"one" rejected; "c" does not accept args`, + "min": "requires at least 2 arg(s), only received 1", + "max": "accepts at most 2 arg(s), received 3", + "exact": "accepts 2 arg(s), received 3", + "range": "accepts between 2 and 4 arg(s), received 1", }[ex] if got := err.Error(); got != expected { t.Errorf("Expected: %q, got: %q", expected, got) } } +func executeUsage(c *Command) (string, error) { + buf := new(bytes.Buffer) + c.SetOutput(buf) + err := c.Usage() + return buf.String(), err +} + +func checkOutput(o string, t *testing.T, i string) { + str := map[rune]string{ + 'u': "Usage:", + 'h': "Run 'c --help' for usage", + 'c': "c [command]", + 'v': "Valid Args:", + 'a': "c [flags] [args]", + 'f': "c [flags]", + } + for _, x := range "uhcva" { + b := strings.Contains(i, string(x)) + if s := str[x]; b != strings.Contains(o, s) { + m := "Did not expect" + if b { + m = "Expected" + } + t.Errorf("%s to find '%s' in the output", m, s) + continue + } + if (x == 'a') && b { + return + } + } +} + +func expectErrorAndCheckOutput(t *testing.T, err error, err_k, o, i string) { + expectError(err, t, err_k) + checkOutput(o, t, i) +} + // NoArgs -func TestNoArgs(t *testing.T) { - o, e := executeCommand(newCommand(NoArgs, false)) +func TestArgs_No(t *testing.T) { + o, e := executeCommand(newCmd(NoArgs, false, true)) expectSuccess(o, e, t) } -func TestNoArgsWithArgs(t *testing.T) { - _, e := executeCommand(newCommand(NoArgs, false), "one") +func TestArgs_No_wArgs(t *testing.T) { + _, e := executeCommand(newCmd(NoArgs, false, true), "one") expectError(e, t, "no") } -func TestNoArgsWithArgsWithValid(t *testing.T) { - _, e := executeCommand(newCommand(NoArgs, true), "one") +func TestArgs_No_wArgsWithValid(t *testing.T) { + _, e := executeCommand(newCmd(NoArgs, true, true), "one") expectError(e, t, "no") } // ArbitraryArgs -func TestArbitraryArgs(t *testing.T) { - o, e := executeCommand(newCommand(ArbitraryArgs, false), "a", "b") +func TestArgs_ArbValid(t *testing.T) { + o, e := executeCommand(newCmd(ArbitraryArgs, false, true), "a", "b") expectSuccess(o, e, t) } -func TestArbitraryArgsWithValid(t *testing.T) { - o, e := executeCommand(newCommand(ArbitraryArgs, true), "one", "two") +func TestArgs_ArbValid_wValid(t *testing.T) { + o, e := executeCommand(newCmd(ArbitraryArgs, true, true), "one", "two") expectSuccess(o, e, t) } -func TestArbitraryArgsWithValidWithInvalidArgs(t *testing.T) { - _, e := executeCommand(newCommand(ArbitraryArgs, true), "a") +func TestArgs_ArbValid_wValid_InvalidArgs(t *testing.T) { + _, e := executeCommand(newCmd(ArbitraryArgs, true, true), "a") expectError(e, t, "valid") } // MinimumNArgs -func TestMinimumNArgs(t *testing.T) { - o, e := executeCommand(newCommand(MinimumNArgs(2), false), "a", "b", "c") +func TestArgs_MinimumN(t *testing.T) { + o, e := executeCommand(newCmd(MinimumNArgs(2), false, true), "a", "b", "c") expectSuccess(o, e, t) } -func TestMinimumNArgsWithValid(t *testing.T) { - o, e := executeCommand(newCommand(MinimumNArgs(2), true), "one", "three") +func TestArgs_MinimumN_wValid(t *testing.T) { + o, e := executeCommand(newCmd(MinimumNArgs(2), true, true), "one", "three") expectSuccess(o, e, t) } -func TestMinimumNArgsWithValidWithInvalidArgs(t *testing.T) { - _, e := executeCommand(newCommand(MinimumNArgs(2), true), "a", "b") +func TestArgs_MinimumN_wValid_InvalidArgs(t *testing.T) { + _, e := executeCommand(newCmd(MinimumNArgs(2), true, true), "a", "b") expectError(e, t, "valid") } -func TestMinimumNArgsWithLessArgs(t *testing.T) { - _, e := executeCommand(newCommand(MinimumNArgs(2), false), "a") +func TestArgs_MinimumN_LessArgs(t *testing.T) { + _, e := executeCommand(newCmd(MinimumNArgs(2), false, true), "a") expectError(e, t, "min") } -func TestMinimumNArgsWithLessArgsWithValid(t *testing.T) { - _, e := executeCommand(newCommand(MinimumNArgs(2), true), "one") +func TestArgs_MinimumN_wValid_LessArgs(t *testing.T) { + _, e := executeCommand(newCmd(MinimumNArgs(2), true, true), "one") expectError(e, t, "min") } -func TestMinimumNArgsWithLessArgsWithValidWithInvalidArgs(t *testing.T) { - _, e := executeCommand(newCommand(MinimumNArgs(2), true), "a") +func TestArgs_MinimumN_wValid_LessArgsInvalidArgs(t *testing.T) { + _, e := executeCommand(newCmd(MinimumNArgs(2), true, true), "a") expectError(e, t, "valid") } // MaximumNArgs -func TestMaximumNArgs(t *testing.T) { - o, e := executeCommand(newCommand(MaximumNArgs(3), false), "a", "b") +func TestArgs_MaximumN(t *testing.T) { + o, e := executeCommand(newCmd(MaximumNArgs(3), false, true), "a", "b") expectSuccess(o, e, t) } -func TestMaximumNArgsWithValid(t *testing.T) { - o, e := executeCommand(newCommand(MaximumNArgs(2), true), "one", "three") +func TestArgs_MaximumN_wValid(t *testing.T) { + o, e := executeCommand(newCmd(MaximumNArgs(2), true, true), "one", "three") expectSuccess(o, e, t) } -func TestMaximumNArgsWithValidWithInvalidArgs(t *testing.T) { - _, e := executeCommand(newCommand(MaximumNArgs(2), true), "a", "b") +func TestArgs_MaximumN_wValid_InvalidArgs(t *testing.T) { + _, e := executeCommand(newCmd(MaximumNArgs(2), true, true), "a", "b") expectError(e, t, "valid") } -func TestMaximumNArgsWithMoreArgs(t *testing.T) { - _, e := executeCommand(newCommand(MaximumNArgs(2), false), "a", "b", "c") +func TestArgs_MaximumN_MoreArgs(t *testing.T) { + _, e := executeCommand(newCmd(MaximumNArgs(2), false, true), "a", "b", "c") expectError(e, t, "max") } -func TestMaximumNArgsWithMoreArgsWithValid(t *testing.T) { - _, e := executeCommand(newCommand(MaximumNArgs(2), true), "one", "three", "two") +func TestArgs_MaximumN_wValid_MoreArgs(t *testing.T) { + _, e := executeCommand(newCmd(MaximumNArgs(2), true, true), "one", "three", "two") expectError(e, t, "max") } -func TestMaximumNArgsWithMoreArgsWithValidWithInvalidArgs(t *testing.T) { - _, e := executeCommand(newCommand(MaximumNArgs(2), true), "a", "b", "c") +func TestArgs_MaximumN_wValid_MoreArgsInvalidArgs(t *testing.T) { + _, e := executeCommand(newCmd(MaximumNArgs(2), true, true), "a", "b", "c") expectError(e, t, "valid") } // ExactArgs -func TestExactArgs(t *testing.T) { - o, e := executeCommand(newCommand(ExactArgs(3), false), "a", "b", "c") +func TestArgs_Exact(t *testing.T) { + o, e := executeCommand(newCmd(ExactArgs(3), false, true), "a", "b", "c") expectSuccess(o, e, t) } -func TestExactArgsWithValid(t *testing.T) { - o, e := executeCommand(newCommand(ExactArgs(3), true), "three", "one", "two") +func TestArgs_Exact_wValid(t *testing.T) { + o, e := executeCommand(newCmd(ExactArgs(3), true, true), "three", "one", "two") expectSuccess(o, e, t) } -func TestExactArgsWithValidWithInvalidArgs(t *testing.T) { - _, e := executeCommand(newCommand(ExactArgs(3), true), "three", "a", "two") +func TestArgs_Exact_wValid_InvalidArgs(t *testing.T) { + _, e := executeCommand(newCmd(ExactArgs(3), true, true), "three", "a", "two") expectError(e, t, "valid") } -func TestExactArgsWithInvalidCount(t *testing.T) { - _, e := executeCommand(newCommand(ExactArgs(2), false), "a", "b", "c") +func TestArgs_Exact_InvalidN(t *testing.T) { + _, e := executeCommand(newCmd(ExactArgs(2), false, true), "a", "b", "c") expectError(e, t, "exact") } -func TestExactArgsWithInvalidCountWithValid(t *testing.T) { - _, e := executeCommand(newCommand(ExactArgs(2), true), "three", "one", "two") +func TestArgs_Exact_wValid_InvalidN(t *testing.T) { + _, e := executeCommand(newCmd(ExactArgs(2), true, true), "three", "one", "two") expectError(e, t, "exact") } -func TestExactArgsWithInvalidCountWithValidWithInvalidArgs(t *testing.T) { - _, e := executeCommand(newCommand(ExactArgs(2), true), "three", "a", "two") +func TestArgs_Exact_wValid_InvalidNInvalidArgs(t *testing.T) { + _, e := executeCommand(newCmd(ExactArgs(2), true, true), "three", "a", "two") expectError(e, t, "valid") } // RangeArgs -func TestRangeArgs(t *testing.T) { - o, e := executeCommand(newCommand(RangeArgs(2, 4), false), "a", "b", "c") +func TestArgs_Range(t *testing.T) { + o, e := executeCommand(newCmd(RangeArgs(2, 4), false, true), "a", "b", "c") expectSuccess(o, e, t) } -func TestRangeArgsWithValid(t *testing.T) { - o, e := executeCommand(newCommand(RangeArgs(2, 4), true), "three", "one", "two") +func TestArgs_Range_wValid(t *testing.T) { + o, e := executeCommand(newCmd(RangeArgs(2, 4), true, true), "three", "one", "two") expectSuccess(o, e, t) } -func TestRangeArgsWithValidWithInvalidArgs(t *testing.T) { - _, e := executeCommand(newCommand(RangeArgs(2, 4), true), "three", "a", "two") +func TestArgs_Range_wValid_InvalidArgs(t *testing.T) { + _, e := executeCommand(newCmd(RangeArgs(2, 4), true, true), "three", "a", "two") expectError(e, t, "valid") } -func TestRangeArgsWithInvalidCount(t *testing.T) { - _, e := executeCommand(newCommand(RangeArgs(2, 4), false), "a") +func TestArgs_Range_InvalidN(t *testing.T) { + _, e := executeCommand(newCmd(RangeArgs(2, 4), false, true), "a") expectError(e, t, "range") } -func TestRangeArgsWithInvalidCountWithValid(t *testing.T) { - _, e := executeCommand(newCommand(RangeArgs(2, 4), true), "two") +func TestArgs_Range_wValid_InvalidN(t *testing.T) { + _, e := executeCommand(newCmd(RangeArgs(2, 4), true, true), "two") expectError(e, t, "range") } -func TestRangeArgsWithInvalidCountWithValidWithInvalidArgs(t *testing.T) { - _, e := executeCommand(newCommand(RangeArgs(2, 4), true), "a") +func TestArgs_Range_wValid_InvalidNInvalidArgs(t *testing.T) { + _, e := executeCommand(newCmd(RangeArgs(2, 4), true, true), "a") expectError(e, t, "valid") } @@ -246,7 +296,7 @@ func TestChildTakesNoArgs(t *testing.T) { } got := err.Error() - expected := `unknown command "illegal" for "root child"` + expected := `"illegal" rejected; "root child" does not accept args` if !strings.Contains(got, expected) { t.Errorf("expected %q, got %q", expected, got) } @@ -262,3 +312,267 @@ func TestChildTakesArgs(t *testing.T) { t.Fatalf("Unexpected error: %v", err) } } + +// NOTE 'c [command]' is not shown because this command does not have any subcommand +// NOTE 'Valid Args:' is not shown because this command is not runnable +// NOTE 'c [flags]' is not shown because this command is not runnable +func noRunChecks(t *testing.T, err error, err_k, o string) { + expectErrorAndCheckOutput(t, err, err_k, o, "u") +} + +// NoRun (no children) + +func TestArgs_NoRun(t *testing.T) { + o, e := executeCommand(newCmd(nil, false, false)) + noRunChecks(t, e, "run", o) +} + +func TestArgs_NoRun_ArbValid(t *testing.T) { + o, e := executeCommand(newCmd(nil, false, false), "one", "three") + noRunChecks(t, e, "run", o) +} + +func TestArgs_NoRun_Invalid(t *testing.T) { + o, e := executeCommand(newCmd(nil, false, false), "two", "a") + noRunChecks(t, e, "run", o) +} + +// NoRun (with children) +// NOTE 'Valid Args:' is not shown because this command is not runnable +// NOTE 'c [flags]' is not shown because this command is not runnable + +func TestArgs_NoRun_wChild(t *testing.T) { + c := newCmd(nil, false, false) + d := newCmd(nil, false, true) + c.AddCommand(d) + o, e := executeCommand(c) + expectErrorAndCheckOutput(t, e, "runsub", o, "uc") +} + +func TestArgs_NoRun_wChild_ArbValid(t *testing.T) { + c := newCmd(nil, false, false) + d := newCmd(nil, false, true) + c.AddCommand(d) + o, e := executeCommand(c, "one", "three") + expectErrorAndCheckOutput(t, e, "runsub", o, "h") +} + +func TestArgs_NoRun_wChild_Invalid(t *testing.T) { + c := newCmd(nil, false, false) + d := newCmd(nil, false, true) + c.AddCommand(d) + o, e := executeCommand(c, "one", "a") + expectErrorAndCheckOutput(t, e, "runsub", o, "h") +} + +// NoRun Args + +func TestArgs_NoRun_wArgs(t *testing.T) { + o, e := executeCommand(newCmd(ArbitraryArgs, false, false)) + noRunChecks(t, e, "run", o) +} + +func TestArgs_NoRun_wArgs_ArbValid(t *testing.T) { + o, e := executeCommand(newCmd(ArbitraryArgs, false, false), "one", "three") + noRunChecks(t, e, "run", o) +} + +func TestArgs_NoRun_wArgs_Invalid(t *testing.T) { + o, e := executeCommand(newCmd(ArbitraryArgs, false, false), "two", "a") + noRunChecks(t, e, "run", o) +} + +// NoRun ValidArgs + +func TestArgs_NoRun_wValid(t *testing.T) { + o, e := executeCommand(newCmd(nil, true, false)) + noRunChecks(t, e, "run", o) +} + +func TestArgs_NoRun_wValid_ArbValid(t *testing.T) { + o, e := executeCommand(newCmd(nil, true, false), "one", "three") + noRunChecks(t, e, "run", o) +} + +func TestArgs_NoRun_wValid_Invalid(t *testing.T) { + o, e := executeCommand(newCmd(nil, true, false), "two", "a") + noRunChecks(t, e, "run", o) +} + +// NoRun Args ValidArgs + +func TestArgs_NoRun_wArgswValid(t *testing.T) { + o, e := executeCommand(newCmd(ArbitraryArgs, true, false)) + noRunChecks(t, e, "run", o) +} + +func TestArgs_NoRun_wArgswValid_ArbValid(t *testing.T) { + o, e := executeCommand(newCmd(ArbitraryArgs, true, false), "one", "three") + noRunChecks(t, e, "run", o) +} + +func TestArgs_NoRun_wArgswValid_Invalid(t *testing.T) { + o, e := executeCommand(newCmd(ArbitraryArgs, true, false), "two", "a") + noRunChecks(t, e, "run", o) +} + +// Run (no children) +// NOTE 'c [command]' is not shown because this command does not have any subcommand +// NOTE 'Valid Args:' is not shown because ValidArgs is not defined + +func TestArgs_Run(t *testing.T) { + c := newCmd(nil, false, true) + o, e := executeCommand(c) + expectSuccess(o, e, t) + + o, e = executeUsage(c) + checkOutput(o, t, "ua") +} + +func TestArgs_Run_ArbValid(t *testing.T) { + c := newCmd(nil, false, true) + o, e := executeCommand(c, "one", "three") + expectSuccess(o, e, t) + + o, e = executeUsage(c) + checkOutput(o, t, "ua") +} + +func TestArgs_Run_Invalid(t *testing.T) { + c := newCmd(nil, false, true) + o, e := executeCommand(c, "two", "a") + expectSuccess(o, e, t) + + o, e = executeUsage(c) + checkOutput(o, t, "ua") +} + +// Run (with children) +// NOTE 'Valid Args:' is not shown because ValidArgs is not defined + +func TestArgs_Run_wChild(t *testing.T) { + c := newCmd(nil, false, true) + d := newCmd(nil, false, true) + c.AddCommand(d) + o, e := executeCommand(c) + expectSuccess(o, e, t) + + o, e = executeUsage(c) + checkOutput(o, t, "ucf") +} + +func TestArgs_Run_wChild_ArbValid(t *testing.T) { + c := newCmd(nil, false, true) + d := newCmd(nil, false, false) + c.AddCommand(d) + o, e := executeCommand(c, "one", "three") + expectError(e, t, "no") + // NOTE 'c [command]' is not shown because this command does not have any available subcommand + checkOutput(o, t, "uf") +} + +func TestArgs_Run_wChild_Invalid(t *testing.T) { + c := newCmd(nil, false, true) + d := newCmd(nil, false, false) + c.AddCommand(d) + o, e := executeCommand(c, "one", "a") + expectError(e, t, "no") + // NOTE 'c [command]' is not shown because this command does not have any available subcommand + checkOutput(o, t, "uf") +} + +// Run Args +// NOTE 'c [command]' is not shown because this command does not have any subcommand + +func TestArgs_Run_wArgs(t *testing.T) { + c := newCmd(ArbitraryArgs, false, true) + o, e := executeCommand(c) + expectSuccess(o, e, t) + + o, e = executeUsage(c) + checkOutput(o, t, "ua") +} + +func TestArgs_Run_wArgs_ArbValid(t *testing.T) { + c := newCmd(ArbitraryArgs, false, true) + o, e := executeCommand(c, "one", "three") + expectSuccess(o, e, t) + + o, e = executeUsage(c) + checkOutput(o, t, "ua") +} + +func TestArgs_Run_wArgs_Invalid(t *testing.T) { + c := newCmd(ArbitraryArgs, false, true) + o, e := executeCommand(c, "two", "a") + expectSuccess(o, e, t) + + o, e = executeUsage(c) + checkOutput(o, t, "ua") +} + +// Run ValidArgs +// NOTE 'c [command]' is not shown because this command does not have any subcommand + +func TestArgs_Run_wValid(t *testing.T) { + c := newCmd(nil, true, true) + o, e := executeCommand(c) + expectSuccess(o, e, t) + + o, e = executeUsage(c) + checkOutput(o, t, "uva") +} + +func TestArgs_Run_wValid_ArbValid(t *testing.T) { + c := newCmd(nil, true, true) + o, e := executeCommand(c, "one", "three") + expectSuccess(o, e, t) + + o, e = executeUsage(c) + checkOutput(o, t, "uva") +} + +func TestArgs_Run_wValid_Invalid(t *testing.T) { + o, e := executeCommand(newCmd(nil, true, true), "two", "a") + expectError(e, t, "valid") + checkOutput(o, t, "uva") +} + +// Run Args ValidArgs +// NOTE 'c [command]' is not shown because this command does not have any subcommand + +func TestArgs_Run_wArgswValid(t *testing.T) { + c := newCmd(ArbitraryArgs, true, true) + o, e := executeCommand(c) + expectSuccess(o, e, t) + + o, e = executeUsage(c) + checkOutput(o, t, "uva") +} + +func TestArgs_Run_wArgswValid_ArbValid(t *testing.T) { + c := newCmd(ArbitraryArgs, true, true) + o, e := executeCommand(c, "one", "three") + expectSuccess(o, e, t) + + o, e = executeUsage(c) + checkOutput(o, t, "uva") +} + +func TestArgs_Run_wArgswValid_Invalid(t *testing.T) { + o, e := executeCommand(newCmd(ArbitraryArgs, true, true), "two", "a") + expectError(e, t, "valid") + + checkOutput(o, t, "uva") +} + +// + +func TestArgs_Run_wMinimumNArgs_ArbValid(t *testing.T) { + c := newCmd(MinimumNArgs(2), false, true) + o, e := executeCommand(c, "one", "three") + expectSuccess(o, e, t) + + o, e = executeUsage(c) + checkOutput(o, t, "ua") +} diff --git a/command.go b/command.go index 95c7979d26..765798985e 100644 --- a/command.go +++ b/command.go @@ -399,7 +399,10 @@ func (c *Command) UsageTemplate() string { {{.CommandPath}} [command]{{end}}{{if gt (len .Aliases) 0}} Aliases: - {{.NameAndAliases}}{{end}}{{if .HasExample}} + {{.NameAndAliases}}{{end}}{{if (and .HasValidArgs .Runnable)}} + +Valid Args: + {{range .ValidArgs}}{{.}} {{end}}{{end}}{{if .HasExample}} Examples: {{.Example}}{{end}}{{if .HasAvailableSubCommands}} @@ -526,7 +529,7 @@ func isFlagArg(arg string) bool { // Find the target command given the args and command tree // Meant to be run on the highest node. Only searches down. -func (c *Command) Find(args []string) (*Command, []string, error) { +func (c *Command) Find(args []string) (*Command, []string) { var innerfind func(*Command, []string) (*Command, []string) innerfind = func(c *Command, innerArgs []string) (*Command, []string) { @@ -543,11 +546,13 @@ func (c *Command) Find(args []string) (*Command, []string, error) { return c, innerArgs } - commandFound, a := innerfind(c, args) - if commandFound.Args == nil { - return commandFound, a, legacyArgs(commandFound, stripFlags(a, commandFound)) + cF, a := innerfind(c, args) + // if Args is undefined and this is a root command with subcommands, + // do not accept arguments, unless ValidArgs is set + if cF.Args == nil && cF.HasSubCommands() && !cF.HasParent() && (len(cF.ValidArgs) == 0) { + cF.Args = NoArgs } - return commandFound, a, nil + return cF, a } func (c *Command) findSuggestions(arg string) string { @@ -588,7 +593,7 @@ func (c *Command) findNext(next string) *Command { // Traverse the command tree to find the command, and parse args for // each parent. -func (c *Command) Traverse(args []string) (*Command, []string, error) { +func (c *Command) Traverse(args []string) (*Command, []string) { flags := []string{} inFlag := false @@ -618,15 +623,15 @@ func (c *Command) Traverse(args []string) (*Command, []string, error) { cmd := c.findNext(arg) if cmd == nil { - return c, args, nil + return c, args } if err := c.ParseFlags(flags); err != nil { - return nil, args, err + return nil, append([]string{err.Error()}, args...) } return cmd.Traverse(args[i+1:]) } - return c, args, nil + return c, args } // SuggestionsFor provides suggestions for the typedName. @@ -722,7 +727,10 @@ func (c *Command) execute(a []string) (err error) { } if !c.Runnable() { - return flag.ErrHelp + if c.HasAvailableSubCommands() { + return fmt.Errorf(`command "%s" is not runnable; please provide a subcmd`, c.Name()) + } + return fmt.Errorf(`command "%s" is not runnable`, c.Name()) } c.preRun() @@ -732,7 +740,7 @@ func (c *Command) execute(a []string) (err error) { argWoFlags = a } - if err := c.ValidateArgs(argWoFlags); err != nil { + if err := c.validateArgs(argWoFlags); err != nil { return err } @@ -818,36 +826,26 @@ func (c *Command) ExecuteC() (cmd *Command, err error) { c.InitDefaultHelpCmd() args := c.args - // Workaround FAIL with "go test -v" or "cobra.test -test.v", see #155 if c.args == nil && filepath.Base(os.Args[0]) != "cobra.test" { args = os.Args[1:] } var flags []string + f := c.Find if c.TraverseChildren { - cmd, flags, err = c.Traverse(args) - } else { - cmd, flags, err = c.Find(args) + f = c.Traverse } - if err != nil { - // If found parse to a subcommand and then failed, talk about the subcommand - if cmd != nil { - c = cmd - } - if !c.SilenceErrors { - c.Println("Error:", err.Error()) - c.Printf("Run '%v --help' for usage.\n", c.CommandPath()) + if cmd, flags = f(args); cmd != nil { + cmd.commandCalledAs.called = true + if cmd.commandCalledAs.name == "" { + cmd.commandCalledAs.name = cmd.Name() } - return c, err - } - - cmd.commandCalledAs.called = true - if cmd.commandCalledAs.name == "" { - cmd.commandCalledAs.name = cmd.Name() + err = cmd.execute(flags) + } else { + err = fmt.Errorf(flags[0]) } - err = cmd.execute(flags) if err != nil { // Always show help if requested, even if SilenceErrors is in // effect @@ -856,6 +854,11 @@ func (c *Command) ExecuteC() (cmd *Command, err error) { return cmd, nil } + // Check if a shorter help hint should be shown instead of the full Usage() + if herr := helpHint(cmd, flags, err.Error()); herr != nil { + return cmd, err + } + // If root command has SilentErrors flagged, // all subcommands should respect it if !cmd.SilenceErrors && !c.SilenceErrors { @@ -871,10 +874,26 @@ func (c *Command) ExecuteC() (cmd *Command, err error) { return cmd, err } -func (c *Command) ValidateArgs(args []string) error { - if c.Args == nil { - return nil +func helpHint(c *Command, fs []string, e string) error { + if len(fs) > 0 { + f := fs[0] + for _, s := range []string{"please provide a subcmd", "unknown command"} { + if strings.Contains(e, s) { + if s := c.findSuggestions(f); len(s) != 0 { + e += s + } + if !c.SilenceErrors { + c.Printf("Error: %s\n", e) + c.Printf("Run '%v --help' for usage.\n", c.CommandPath()) + } + return fmt.Errorf("%s", e) + } + } } + return nil +} + +func (c *Command) validateArgs(args []string) error { if len(c.ValidArgs) > 0 { for _, v := range args { if !stringInSlice(v, c.ValidArgs) { @@ -882,6 +901,9 @@ func (c *Command) ValidateArgs(args []string) error { } } } + if c.Args == nil { + return nil + } return c.Args(c, args) } @@ -957,9 +979,9 @@ func (c *Command) InitDefaultHelpCmd() { Simply type ` + c.Name() + ` help [path to command] for full details.`, Run: func(c *Command, args []string) { - cmd, _, e := c.Root().Find(args) - if cmd == nil || e != nil { - c.Printf("Unknown help topic %#q\n", args) + cmd, _ := c.Root().Find(args) + if cmd == nil { + c.Printf("Unknown help topic %#q\n", args[1:]) c.Root().Usage() } else { cmd.InitDefaultHelpFlag() // make possible 'help' flag to be shown @@ -1097,9 +1119,35 @@ func (c *Command) UseLine() string { if c.HasAvailableFlags() && !strings.Contains(useline, "[flags]") { useline += " [flags]" } + + useline += useLineArgs(c) + return useline } +// useLineArgs puts out '[args]' if a given command accepts positional args +func useLineArgs(c *Command) (s string) { + s = " [args]" + if c.Args == nil { + if !c.HasAvailableSubCommands() || c.HasParent() { + return + } + // if Args is undefined and this is a root command with subcommands, + // do not accept arguments, unless ValidArgs is set + if !c.HasParent() && c.HasAvailableSubCommands() && (len(c.ValidArgs) > 0) { + return + } + return "" + } + // Check if the Args validator is other than 'NoArgs' + err := c.Args(c, []string{"someUnexpectedIllegalArg"}) + nerr := NoArgs(c, []string{"someUnexpectedIllegalArg"}) + if err == nil || ((nerr != nil) && (err.Error() != nerr.Error())) { + return + } + return "" +} + // DebugFlags used to determine which flags have been assigned to which commands // and which persist. func (c *Command) DebugFlags() { @@ -1191,6 +1239,10 @@ func (c *Command) NameAndAliases() string { return strings.Join(append([]string{c.Name()}, c.Aliases...), ", ") } +func (c *Command) HasValidArgs() bool { + return len(c.ValidArgs) > 0 +} + // HasExample determines if the command has example. func (c *Command) HasExample() bool { return len(c.Example) > 0 @@ -1264,16 +1316,14 @@ func (c *Command) HasHelpSubCommands() bool { // HasAvailableSubCommands determines if a command has available sub commands that // need to be shown in the usage/help default template under 'available commands'. func (c *Command) HasAvailableSubCommands() bool { - // return true on the first found available (non deprecated/help/hidden) - // sub command + // return true on the first found available (non deprecated/help/hidden) subcmd for _, sub := range c.commands { if sub.IsAvailableCommand() { return true } } - - // the command either has no sub commands, or no available (non deprecated/help/hidden) - // sub commands + // the command either has no sub commands, + // or no available (non deprecated/help/hidden) subcmds return false } diff --git a/command_test.go b/command_test.go index 6e483a3ec4..afa441e14d 100644 --- a/command_test.go +++ b/command_test.go @@ -109,9 +109,7 @@ func TestRootExecuteUnknownCommand(t *testing.T) { rootCmd.AddCommand(&Command{Use: "child", Run: emptyRun}) output, _ := executeCommand(rootCmd, "unknown") - expected := "Error: unknown command \"unknown\" for \"root\"\nRun 'root --help' for usage.\n" - if output != expected { t.Errorf("Expected:\n %q\nGot:\n %q\n", expected, output) } @@ -836,11 +834,13 @@ func TestHelpExecutedOnNonRunnableChild(t *testing.T) { rootCmd.AddCommand(childCmd) output, err := executeCommand(rootCmd, "child") - if err != nil { - t.Errorf("Unexpected error: %v", err) + + expected := `command "child" is not runnable` + if err.Error() != expected { + t.Errorf("Expected %q, got %q", expected, err.Error()) } - checkStringContains(t, output, childCmd.Long) + checkStringContains(t, output, "Usage:") } func TestVersionFlagExecuted(t *testing.T) { @@ -1463,9 +1463,9 @@ func TestTraverseWithParentFlags(t *testing.T) { rootCmd.AddCommand(childCmd) - c, args, err := rootCmd.Traverse([]string{"-b", "--str", "ok", "child", "--int"}) - if err != nil { - t.Errorf("Unexpected error: %v", err) + c, args := rootCmd.Traverse([]string{"-b", "--str", "ok", "child", "--int"}) + if c == nil { + t.Errorf("Unexpected error: %s", args[0]) } if len(args) != 1 && args[0] != "--add" { t.Errorf("Wrong args: %v", args) @@ -1483,9 +1483,9 @@ func TestTraverseNoParentFlags(t *testing.T) { childCmd.Flags().String("str", "", "") rootCmd.AddCommand(childCmd) - c, args, err := rootCmd.Traverse([]string{"child"}) - if err != nil { - t.Errorf("Unexpected error: %v", err) + c, args := rootCmd.Traverse([]string{"child"}) + if c == nil { + t.Errorf("Unexpected error: %v", args[0]) } if len(args) != 0 { t.Errorf("Wrong args %v", args) @@ -1504,13 +1504,13 @@ func TestTraverseWithBadParentFlags(t *testing.T) { expected := "unknown flag: --str" - c, _, err := rootCmd.Traverse([]string{"--str", "ok", "child"}) - if err == nil || !strings.Contains(err.Error(), expected) { - t.Errorf("Expected error, %q, got %q", expected, err) - } + c, args := rootCmd.Traverse([]string{"--str", "ok", "child"}) if c != nil { t.Errorf("Expected nil command") } + if !strings.Contains(args[0], expected) { + t.Errorf("Expected error, %q, got %q", expected, args[0]) + } } func TestTraverseWithBadChildFlag(t *testing.T) { @@ -1522,9 +1522,9 @@ func TestTraverseWithBadChildFlag(t *testing.T) { // Expect no error because the last commands args shouldn't be parsed in // Traverse. - c, args, err := rootCmd.Traverse([]string{"child", "--str"}) - if err != nil { - t.Errorf("Unexpected error: %v", err) + c, args := rootCmd.Traverse([]string{"child", "--str"}) + if c == nil { + t.Errorf("Unexpected error: %s", args[0]) } if len(args) != 1 && args[0] != "--str" { t.Errorf("Wrong args: %v", args) @@ -1545,9 +1545,9 @@ func TestTraverseWithTwoSubcommands(t *testing.T) { } subCmd.AddCommand(subsubCmd) - c, _, err := rootCmd.Traverse([]string{"sub", "subsub"}) - if err != nil { - t.Fatalf("Unexpected error: %v", err) + c, args := rootCmd.Traverse([]string{"sub", "subsub"}) + if c == nil { + t.Fatalf("Unexpected error: %v", args[0]) } if c.Name() != subsubCmd.Name() { t.Fatalf("Expected command: %q, got %q", subsubCmd.Name(), c.Name())