From 9d985bfae755d4de4dfc0594a7c63ff0c1b80606 Mon Sep 17 00:00:00 2001 From: vsachs Date: Thu, 28 Jul 2022 17:15:03 -0700 Subject: [PATCH] Unit tests and minor changes --- README.md | 3 + argparse.go | 3 + argparse_test.go | 212 +++++++++++++++++++++++++++++++++++++++++++++++ argument.go | 11 ++- command.go | 9 +- 5 files changed, 231 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 7404f6b..ab8db14 100644 --- a/README.md +++ b/README.md @@ -134,6 +134,9 @@ var myLogFiles *[]os.File = parser.FileList("l", "log-file", os.O_RDWR, 0600, .. ``` You can implement sub-commands in your CLI using `parser.NewCommand()` or go even deeper with `command.NewCommand()`. +Addition of a sub-command does not prevent usage of the parent command with or without flags. +Sub-commands are always parsed before arguments. +If a command has positional arguments and sub-commands then sub-commands take precedence. Since parser inherits from command, every command supports exactly same options as parser itself, thus allowing to add arguments specific to that command or more global arguments added on parser itself! diff --git a/argparse.go b/argparse.go index d41345c..cbb4383 100644 --- a/argparse.go +++ b/argparse.go @@ -82,6 +82,8 @@ type Parser struct { // // Options.Positional - tells Parser that the argument is positional (implies Required) // Positional arguments do not require the flag name to precede them and must come in a specific order. +// Positional sets Required=true, Default=nil, Shortname="" +// Existence of a positional means that any flags preceding the positional must use `=` to pair with their value. // // Options.Required - tells Parser that this argument is required to be provided. // useful when specific Command requires some data provided. @@ -693,6 +695,7 @@ func (o *Parser) Parse(args []string) error { unparsed = append(unparsed, v) } } + if result == nil && len(unparsed) > 0 { return errors.New("unknown arguments " + strings.Join(unparsed, " ")) } diff --git a/argparse_test.go b/argparse_test.go index b247044..d511d58 100644 --- a/argparse_test.go +++ b/argparse_test.go @@ -2746,3 +2746,215 @@ func TestCommandHelpSetSnameOnly(t *testing.T) { t.Error("Help arugment names should have defaulted") } } + +func TestCommandPositional(t *testing.T) { + testArgs1 := []string{"pos", "heyo"} + parser := NewParser("pos", "") + strval := parser.String("s", "string", &Options{Positional: true}) + + if err := parser.Parse(testArgs1); err != nil { + t.Error(err.Error()) + } else if *strval != "heyo" { + t.Errorf("Strval did not match expected") + } +} + +func TestCommandPositionalErr(t *testing.T) { + errArgs1 := []string{"pos"} + parser := NewParser("pos", "") + strval := parser.String("s", "beep", &Options{Positional: true}) + + if err := parser.Parse(errArgs1); err == nil { + t.Errorf("Positional required") + } else if err.Error() != "[beep] is required" { + // This is an admittedly slightly confusing error message + // but it basically means the parser bailed because the arg list + // is empty...? + t.Error(err.Error()) + } else if *strval != "" { + t.Errorf("Strval nonempty") + } +} + +func TestCommandPositionals(t *testing.T) { + testArgs1 := []string{"posint", "5", "abc", "1.0"} + parser := NewParser("posint", "") + intval := parser.Int("i", "integer", &Options{Positional: true}) + strval := parser.String("s", "string", &Options{Positional: true}) + floatval := parser.Float("f", "floats", &Options{Positional: true}) + + if err := parser.Parse(testArgs1); err != nil { + t.Error(err.Error()) + } else if *intval != 5 { + t.Error("Intval did not match expected") + } else if *strval != "abc" { + t.Error("Strval did not match expected") + } else if *floatval != 1.0 { + t.Error("Floatval did not match expected") + } +} + +func TestCommandPositionalsErr(t *testing.T) { + errArgs1 := []string{"posint", "abc", "abc", "1.0"} + parser := NewParser("posint", "") + _ = parser.Int("i", "cool", &Options{Positional: true}) + _ = parser.String("s", "string", &Options{Positional: true}) + _ = parser.Float("f", "floats", &Options{Positional: true}) + + if err := parser.Parse(errArgs1); err == nil { + t.Error("String argument accepted for integer") + } else if err.Error() != "[cool] bad integer value [abc]" { + t.Error(err.Error()) + } +} + +func TestPos1(t *testing.T) { + testArgs1 := []string{"pos", "subcommand1", "-i=2", "abc"} + parser := NewParser("pos", "") + + strval := parser.String("s", "string", &Options{Positional: true}) + com1 := parser.NewCommand("subcommand1", "beep") + intval := com1.Int("i", "integer", nil) + + if err := parser.Parse(testArgs1); err != nil { + t.Error(err.Error()) + } else if *strval != "abc" { + t.Error("Strval did not match expected") + } else if *intval != 2 { + t.Error("intval did not match expected") + } +} + +func TestPos2(t *testing.T) { + testArgs1 := []string{"pos", "subcommand1", "a123"} + parser := NewParser("pos", "") + + strval := parser.String("s", "string", &Options{Positional: true}) + com1 := parser.NewCommand("subcommand1", "beep") + intval := com1.Int("i", "integer", nil) + + if err := parser.Parse(testArgs1); err != nil { + t.Error(err.Error()) + } else if *strval != "a123" { + t.Error("Strval did not match expected") + } else if *intval != 0 { + t.Error("intval did not match expected") + } +} + +func TestPos3(t *testing.T) { + testArgs1 := []string{"pos", "subcommand1", "xyz", "--integer", "3"} + parser := NewParser("pos", "") + + strval := parser.String("s", "string", &Options{Positional: true}) + com1 := parser.NewCommand("subcommand1", "beep") + intval := com1.Int("i", "integer", nil) + + if err := parser.Parse(testArgs1); err != nil { + t.Error(err.Error()) + } else if *strval != "xyz" { + t.Error("Strval did not match expected") + } else if *intval != 3 { + t.Error("intval did not match expected") + } +} + +func TestPos4(t *testing.T) { + testArgs1 := []string{"pos", "abc"} + parser := NewParser("pos", "") + + strval := parser.String("s", "string", &Options{Positional: true}) + com1 := parser.NewCommand("subcommand1", "beep") + intval := com1.Int("i", "integer", nil) + + if err := parser.Parse(testArgs1); err != nil { + t.Error(err.Error()) + } else if *strval != "abc" { + t.Error("Strval did not match expected") + } else if *intval != 0 { + t.Error("intval did not match expected") + } +} +func TestPosErr1(t *testing.T) { + errArgs1 := []string{"pos", "subcommand1"} + parser := NewParser("pos", "") + + strval := parser.String("s", "posy", &Options{Positional: true, Default: "abc"}) + com1 := parser.NewCommand("subcommand1", "beep") + intval := com1.Int("i", "integer", nil) + + if err := parser.Parse(errArgs1); err == nil { + t.Error("Subcommand should be required") + } else if err.Error() != "[posy] is required" { + t.Error(err.Error()) + } else if *strval != "" { + t.Error("strval incorrectly defaulted:" + *strval) + } else if *intval != 0 { + t.Error("intval did not match expected") + } +} + +// Must break up for correct unit testing +// func TestCommandSubcommandPositionals(t *testing.T) { +// testArgs1 := []string{"pos", "subcommand2", "efg"} +// testArgs2 := []string{"pos", "subcommand1"} +// testArgs3 := []string{"pos", "subcommand2", "abc", "-i", "1"} +// testArgs4 := []string{"pos", "subcommand2", "abc", "--integer", "1"} +// testArgs5 := []string{"pos", "subcommand2", "abc", "-i=1"} +// testArgs6 := []string{"pos", "subcommand2", "abc", "--integer=1"} +// // flags before positional must use `=` for values +// testArgs7 := []string{"pos", "subcommand2", "-i=1", "abc"} +// testArgs8 := []string{"pos", "subcommand2", "--integer=1", "abc"} +// testArgs9 := []string{"pos", "subcommand3", "true"} +// // Error cases +// errArgs1 := []string{"pos", "subcommand2", "--i", "1"} +// errArgs2 := []string{"pos", "subcommand2", "--i", "1", "abc"} +// errArgs3 := []string{"pos", "subcommand3", "abc"} +// parser := NewParser("pos", "") + +// _ = parser.NewCommand("subcommand1", "") +// com2 := parser.NewCommand("subcommand2", "") +// com2.String("s", "string", &Options{Positional: true}) +// com2.Int("i", "integer", nil) +// com2.Flag("b", "bool", nil) +// com3 := parser.NewCommand("subcommand3", "") +// com3.Flag("b", "bool", &Options{Positional: true}) + +// if err := parser.Parse(testArgs1); err != nil { +// t.Error(err.Error()) +// } +// if err := parser.Parse(testArgs2); err != nil { +// t.Error(err.Error()) +// } +// if err := parser.Parse(testArgs3); err != nil { +// t.Error(err.Error()) +// } +// if err := parser.Parse(testArgs4); err != nil { +// t.Error(err.Error()) +// } +// if err := parser.Parse(testArgs5); err != nil { +// t.Error(err.Error()) +// } +// if err := parser.Parse(testArgs6); err != nil { +// t.Error(err.Error()) +// } +// if err := parser.Parse(testArgs7); err != nil { +// t.Error(err.Error()) +// } +// if err := parser.Parse(testArgs8); err != nil { +// t.Error(err.Error()) +// } +// if err := parser.Parse(testArgs9); err != nil { +// t.Error(err.Error()) +// } + +// if err := parser.Parse(errArgs1); err == nil { +// t.Error(err.Error()) +// } +// if err := parser.Parse(errArgs2); err == nil { +// t.Error(err.Error()) +// } +// if err := parser.Parse(errArgs3); err == nil { +// t.Error(err.Error()) +// } +// } diff --git a/argument.go b/argument.go index 1f25d32..c3b32e1 100644 --- a/argument.go +++ b/argument.go @@ -110,7 +110,7 @@ func (o *arg) checkShortName(argument string) (int, error) { } // check if argument present. -// check - returns the argumet's number of occurrences and error. +// check - returns the argument's number of occurrences and error. // For long name return value is 0 or 1. // For shorthand argument - 0 if there is no occurrences, or count of occurrences. // Shorthand argument with parametr, mast be the only or last in the argument string. @@ -399,7 +399,11 @@ func (o *arg) parseSomeType(args []string, argCount int) error { } func (o *arg) parsePositional(arg string) error { - return o.parse([]string{arg}, 1) + if err := o.parse([]string{arg}, 1); err != nil { + return err + } + + return nil } func (o *arg) parse(args []string, argCount int) error { @@ -419,6 +423,9 @@ func (o *arg) parse(args []string, argCount int) error { } func (o *arg) name() string { + if o.GetPositional() { + return o.lname + } var name string if o.lname == "" { name = "-" + o.sname diff --git a/command.go b/command.go index 9a4462e..8bcdae5 100644 --- a/command.go +++ b/command.go @@ -53,7 +53,9 @@ func (o *Command) addArg(a *arg) error { a.parent = o if a.GetPositional() { + a.sname = "" a.opts.Required = true + a.opts.Default = nil a.size = 1 // We could allow other sizes in the future } o.args = append(o.args, a) @@ -88,8 +90,8 @@ func (o *Command) parseArguments(inputArgs *[]string) error { if arg == "" { continue } - if oarg.GetPositional() { - // Skip any flags + if !strings.HasPrefix(arg, "-") && oarg.GetPositional() { + // If this arg is a flag we just skip parsing positionals // This has the subtle effect of requiring flags // to use `=` for their value pairing if any // positionals are defined AND are not satisfied yet. @@ -103,9 +105,6 @@ func (o *Command) parseArguments(inputArgs *[]string) error { // it must be for that flag OR // the user made an error // However this is highly ambiguous so best avoided. - if strings.HasPrefix(arg, "-") { - continue - } if err := oarg.parsePositional(arg); err != nil { return err }