-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support validation of positional arguments #284
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,98 @@ | ||
package cobra | ||
|
||
import ( | ||
"fmt" | ||
) | ||
|
||
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()) | ||
} | ||
return nil | ||
} | ||
|
||
// OnlyValidArgs returns an error if any args are not in the list of ValidArgs | ||
func OnlyValidArgs(cmd *Command, args []string) error { | ||
if len(cmd.ValidArgs) > 0 { | ||
for _, v := range args { | ||
if !stringInSlice(v, cmd.ValidArgs) { | ||
return fmt.Errorf("invalid argument %q for %q%s", v, cmd.CommandPath(), cmd.findSuggestions(args[0])) | ||
} | ||
} | ||
} | ||
return nil | ||
} | ||
|
||
func stringInSlice(a string, list []string) bool { | ||
for _, b := range list { | ||
if b == a { | ||
return true | ||
} | ||
} | ||
return false | ||
} | ||
|
||
// ArbitraryArgs never returns an error | ||
func ArbitraryArgs(cmd *Command, args []string) error { | ||
return nil | ||
} | ||
|
||
// MinimumNArgs returns an error if there is not at least N args | ||
func MinimumNArgs(n int) PositionalArgs { | ||
return func(cmd *Command, args []string) error { | ||
if len(args) < n { | ||
return fmt.Errorf("requires at least %d arg(s), only received %d", n, len(args)) | ||
} | ||
return nil | ||
} | ||
} | ||
|
||
// MaximumNArgs returns an error if there are more than N args | ||
func MaximumNArgs(n int) PositionalArgs { | ||
return func(cmd *Command, args []string) error { | ||
if len(args) > n { | ||
return fmt.Errorf("accepts at most %d arg(s), received %d", n, len(args)) | ||
} | ||
return nil | ||
} | ||
} | ||
|
||
// ExactArgs returns an error if there are not exactly n args | ||
func ExactArgs(n int) PositionalArgs { | ||
return func(cmd *Command, args []string) error { | ||
if len(args) != n { | ||
return fmt.Errorf("accepts %d arg(s), received %d", n, len(args)) | ||
} | ||
return nil | ||
} | ||
} | ||
|
||
// RangeArgs returns an error if the number of args is not within the expected range | ||
func RangeArgs(min int, max int) PositionalArgs { | ||
return func(cmd *Command, args []string) error { | ||
if len(args) < min || len(args) > max { | ||
return fmt.Errorf("accepts between %d and %d arg(s), received %d", min, max, len(args)) | ||
} | ||
return nil | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,6 +36,7 @@ var cmdHidden = &Command{ | |
|
||
var cmdPrint = &Command{ | ||
Use: "print [string to print]", | ||
Args: MinimumNArgs(1), | ||
Short: "Print anything to the screen", | ||
Long: `an absolutely utterly useless command for testing.`, | ||
Run: func(cmd *Command, args []string) { | ||
|
@@ -75,6 +76,7 @@ var cmdDeprecated = &Command{ | |
Deprecated: "Please use echo instead", | ||
Run: func(cmd *Command, args []string) { | ||
}, | ||
Args: NoArgs, | ||
} | ||
|
||
var cmdTimes = &Command{ | ||
|
@@ -88,6 +90,8 @@ var cmdTimes = &Command{ | |
Run: func(cmd *Command, args []string) { | ||
tt = args | ||
}, | ||
Args: OnlyValidArgs, | ||
ValidArgs: []string{"one", "two", "three", "four"}, | ||
} | ||
|
||
var cmdRootNoRun = &Command{ | ||
|
@@ -105,6 +109,16 @@ var cmdRootSameName = &Command{ | |
Long: "The root description for help", | ||
} | ||
|
||
var cmdRootTakesArgs = &Command{ | ||
Use: "root-with-args [random args]", | ||
Short: "The root can run it's own function and takes args!", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
Long: "The root description for help, and some args", | ||
Run: func(cmd *Command, args []string) { | ||
tr = args | ||
}, | ||
Args: ArbitraryArgs, | ||
} | ||
|
||
var cmdRootWithRun = &Command{ | ||
Use: "cobra-test", | ||
Short: "The root can run its own function", | ||
|
@@ -458,6 +472,63 @@ func TestUsage(t *testing.T) { | |
checkResultOmits(t, x, cmdCustomFlags.Use+" [flags]") | ||
} | ||
|
||
func TestRootTakesNoArgs(t *testing.T) { | ||
c := initializeWithSameName() | ||
c.AddCommand(cmdPrint, cmdEcho) | ||
result := simpleTester(c, "illegal") | ||
|
||
if result.Error == nil { | ||
t.Fatal("Expected an error") | ||
} | ||
|
||
expectedError := `unknown command "illegal" for "print"` | ||
if !strings.Contains(result.Error.Error(), expectedError) { | ||
t.Errorf("exptected %v, got %v", expectedError, result.Error.Error()) | ||
} | ||
} | ||
|
||
func TestRootTakesArgs(t *testing.T) { | ||
c := cmdRootTakesArgs | ||
result := simpleTester(c, "legal") | ||
|
||
if result.Error != nil { | ||
t.Errorf("expected no error, but got %v", result.Error) | ||
} | ||
} | ||
|
||
func TestSubCmdTakesNoArgs(t *testing.T) { | ||
result := fullSetupTest("deprecated", "illegal") | ||
|
||
if result.Error == nil { | ||
t.Fatal("Expected an error") | ||
} | ||
|
||
expectedError := `unknown command "illegal" for "cobra-test deprecated"` | ||
if !strings.Contains(result.Error.Error(), expectedError) { | ||
t.Errorf("expected %v, got %v", expectedError, result.Error.Error()) | ||
} | ||
} | ||
|
||
func TestSubCmdTakesArgs(t *testing.T) { | ||
noRRSetupTest("echo", "times", "one", "two") | ||
if strings.Join(tt, " ") != "one two" { | ||
t.Error("Command didn't parse correctly") | ||
} | ||
} | ||
|
||
func TestCmdOnlyValidArgs(t *testing.T) { | ||
result := noRRSetupTest("echo", "times", "one", "two", "five") | ||
|
||
if result.Error == nil { | ||
t.Fatal("Expected an error") | ||
} | ||
|
||
expectedError := `invalid argument "five"` | ||
if !strings.Contains(result.Error.Error(), expectedError) { | ||
t.Errorf("expected %v, got %v", expectedError, result.Error.Error()) | ||
} | ||
} | ||
|
||
func TestFlagLong(t *testing.T) { | ||
noRRSetupTest("echo", "--intone=13", "something", "--", "here") | ||
|
||
|
@@ -672,9 +743,9 @@ func TestPersistentFlags(t *testing.T) { | |
} | ||
|
||
// persistentFlag should act like normal flag on its own command | ||
fullSetupTest("echo", "times", "-s", "again", "-c", "-p", "test", "here") | ||
fullSetupTest("echo", "times", "-s", "again", "-c", "-p", "one", "two") | ||
|
||
if strings.Join(tt, " ") != "test here" { | ||
if strings.Join(tt, " ") != "one two" { | ||
t.Errorf("flags didn't leave proper args remaining. %s given", tt) | ||
} | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can a noarg command have subcommands? This seems like the trigger I wanted in #285, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, subcommand lookup happens before this is called in
Find
, so these arg handling functions only take care of non-subcommand args. I believe thisNoArgs
option is what you want in #285.