Skip to content
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

Initial implementation of positional arguments #102

Merged
merged 12 commits into from Aug 8, 2022
23 changes: 21 additions & 2 deletions README.md
Expand Up @@ -66,6 +66,12 @@ String will allow you to get a string from arguments, such as `$ progname --stri
var myString *string = parser.String("s", "string", ...)
```

Positional arguments can be used like this `$ progname value1`.
See [Basic Option Structure](#basic-option-structure) and [Positionals](#positionals).
```go
var myString *string = parser.String("", "posarg", &Options{Positional: true})
```

Selector works same as a string, except that it will only allow specific values.
For example like this `$ progname --debug-level WARN`
```go
Expand Down Expand Up @@ -134,6 +140,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 implies that a subcommand is required.
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!

Expand All @@ -153,19 +162,21 @@ type Options struct {
Validate func(args []string) error
Help string
Default interface{}
Positional bool
}
```

You can Set `Required` to let it know if it should ask for arguments.
You can set `Required` to let it know if it should ask for arguments.
Or you can set `Validate` as a lambda function to make it know while value is valid.
Or you can set `Help` for your beautiful help document.
Or you can set `Default` will set the default value if user does not provide a value.
You can set `Positional` to indicate an arg is required in a specific position but shouldn't include "--name". Positionals are required in the order they are added. Flags can precede or follow positionals.

Example:
```
dirpath := parser.String("d", "dirpath",
&argparse.Options{
Require: false,
Required: false,
Help: "the input files' folder path",
Default: "input",
})
Expand All @@ -183,6 +194,14 @@ There are a few caveats (or more like design choices) to know about:
* `parser.Parse()` returns error in case of something going wrong, but it is not expected to cover ALL cases
* Any arguments that left un-parsed will be regarded as error

##### Positionals
* `Positional` has a set of effects and conditions:
* It will always set Required=True and Default=nil
* It will ignore the shortname
* It will fail (and app will panic) to add the argument if it is one of:
* Flag
* StringList, IntList, FloatList, FileList


#### Contributing

Expand Down
15 changes: 11 additions & 4 deletions argparse.go
Expand Up @@ -80,6 +80,11 @@ type Parser struct {
// Options are specific options for every argument. They can be provided if necessary.
// Possible fields are:
//
// 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=""
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, interesting, while Default == nil makes sense (positional either provided or it is not), I am not so sure about Required == true.

Assuming for a sub-command tree we defined 5 positionals: progname cmd1 cmd2 prognamePositional1 prognamePositional2 cmd1Positional1 cmd1Positional2 cmd2Positional, and input provided on CLI progname cmd1 cmd2 somestr1 somestr2 somestr3, the only way we can match positional arguments is in the order they provided. Hence:

  • prognamePositional1 == somestr1
  • prognamePositional2 == somestr2
  • cmd1Positional1 == somestr3
  • cmd1Positional2 == nil
  • cmd2Positional == nil

Which would make it possible to define "optional" positionals, as in -- if it was not provided, it is not set to any value, meantime any that provided will be matched to their corresponding positional in exactly same order (that is in example above prognamePositional1 should always be matched to somestr1

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

progname cmd1 cmd2 prognamePositional1 prognamePositional2 cmd1Positional1 cmd1Positional2 cmd2Positional
----->
progname cmd1 cmd2 somestr1 somestr2 somestr3
----->
prognamePositional1 == somestr1
prognamePositional2 == somestr2
cmd1Positional1 == somestr3
cmd1Positional2 == nil
cmd2Positional == nil

I believe I can allow this relatively easily but it's going to open a can of worms. What if the only optional positionals are actually prognamePositional1 and prognamePositional2? We can't distinguish which positionals the user is trying to fill (unless we actually allow naming of the positionals on the CLI, which... basically defeats the whole point), so we will still end up with the same result above except that the last two are required and nil, giving us an error. Effectively they are all required in this scenario.

Do we want to require that any optional positionals be at the tail end of added arguments? If so this has the implication:

  • That all sub-commands of any command with optionals can now only add optional positionals.

// 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.
//
Expand All @@ -95,10 +100,11 @@ type Parser struct {
// in case if this argument was not supplied on command line. File default value is a string which it will be open with
// provided options. In case if provided value type does not match expected, the error will be returned on run-time.
type Options struct {
Required bool
Validate func(args []string) error
Help string
Default interface{}
Positional bool
Required bool
Validate func(args []string) error
Help string
Default interface{}
}

// NewParser creates new Parser object that will allow to add arguments for parsing
Expand Down Expand Up @@ -700,6 +706,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, " "))
}
Expand Down
250 changes: 250 additions & 0 deletions argparse_test.go
Expand Up @@ -2751,3 +2751,253 @@ 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 &&
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In its current form this test will give false result (test Pass) if there is an error, but error message is different.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should fail in that case, I don't see how the current if statement could cause that problem. However I did realize it will pass if the error is nil which is definitely incorrect. Fixed that.

err.Error() != "[sub]Command required" {
t.Error(err.Error())
} else if *strval != "" {
t.Error("Strval did not match expected")
} else if *intval != 0 {
t.Error("intval did not match expected")
}
}

func TestPos5(t *testing.T) {
errStr := "unable to add Flag: argument type cannot be positional"
parser := NewParser("pos", "")
var boolval *bool
// Catch the panic
defer func() {
err := recover()
if err.(error).Error() != errStr {
t.Error(err.(error).Error())
} else if boolval != nil {
t.Error("Boolval was set")
}
}()
boolval = parser.Flag("", "booly", &Options{Positional: true})
}

func TestPos6(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())
vsachs marked this conversation as resolved.
Show resolved Hide resolved
} else if *strval != "abc" {
t.Error("Strval did not match expected")
} else if *intval != 2 {
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")
}
}

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", "second"}
// Error cases
errArgs1 := []string{"pos", "subcommand2", "-i", "1"}
errArgs2 := []string{"pos", "subcommand2", "-i", "1", "abc"}
errArgs3 := []string{"pos", "subcommand3", "abc"}

newParser := func() *Parser {
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.Selector("", "select", []string{"first", "second"},
&Options{Positional: true})
return parser
}

if err := newParser().Parse(testArgs1); err != nil {
t.Error(err.Error())
}
if err := newParser().Parse(testArgs2); err != nil {
t.Error(err.Error())
}
if err := newParser().Parse(testArgs3); err != nil {
t.Error(err.Error())
}
if err := newParser().Parse(testArgs4); err != nil {
t.Error(err.Error())
}
if err := newParser().Parse(testArgs5); err != nil {
t.Error(err.Error())
}
if err := newParser().Parse(testArgs6); err != nil {
t.Error(err.Error())
}
if err := newParser().Parse(testArgs7); err != nil {
t.Error(err.Error())
}
if err := newParser().Parse(testArgs8); err != nil {
t.Error(err.Error())
}
if err := newParser().Parse(testArgs9); err != nil {
t.Error(err.Error())
}

if err := newParser().Parse(errArgs1); err == nil {
t.Error("Expected error")
}
if err := newParser().Parse(errArgs2); err == nil {
t.Error("Expected error")
}
if err := newParser().Parse(errArgs3); err == nil {
t.Error("Expected error")
}
}