Add positional argument type

[stevegt]

Add the ability to accept positional arguments by adding a parser.Positional() argument type that simply collects all of the remaining unparsed arguments and returns them as a list of strings. Example code in examples/positionals/positionals.go. Still needs tests.

The Nargs additions that @jokelyo made in http://github.com/jokelyo/argparse also look promising; I considered extending that to get positionals. The upside there would be the ability to handle more positional types than just strings; the downside would be a much larger code delta.

[akamensky]

@stevegt thanks, this is awesome, this library needed to be able to handle positional args for awhile and I did not have time to look into that recently.

I left a comment there for a small change if you don't mind to add more description on behavior of positional args in here. I would like the documentation to be as descriptive as possible to save people struggles in understanding how things work.

Also from what I see in this implementation it will allow any ordering of args, which I think can lead to unexpected results. I.e. someone uses this library in a tool called tool, and a user of a tool forgets to quote the string with spaces, this will attempt to interpret any words following first one as positionals.

I think it would probably be better to allow positional args only after all other arguments. Imagine the code (from the example you generously added):

parser := argparse.NewParser("positionals", "Prints provided positional strings to stdout")

s := parser.String("s", "string", &argparse.Options{Required: true, Help: "String to print"})
p := parser.Positional("positional", &argparse.Options{Help: "More strings to print"})

With the logic I described above, calling tool positional-arg -s string would fail, and calling tool -s string positional-arg would succeed with positional-arg being assigned to p.

Also I think we could expand the implementation to not only strings. We can rename .Positional to .PosString and add .PosInt for number and more for other types.

[stevegt]

Thanks for the feedback, and thanks for a fantastic library -- I'm a python refugee and was really glad to find your code.

I'm still only around 70% convinced that the approach in this PR is the right way to go -- if we wanted to allow for positional arguments other than strings, for instance, then @jokelyo's work might be better to build upon, since he's already handling nargs with the various types. But to do that right, we'd want to mimic how python's argparse handles positionals. It's the lack of a leading '-' in lname that causes python's argparse to treat an argument as a positional. To implement that, we'd have to require that users specify the leading '-' characters in sname and lname for non-positional args. But at this point that would be a breaking change. To work around that, I started adding a 'Positional' boolean to Options; you can see what that would look like in https://github.com/stevegt/argparse/blob/nargs-positionals/examples/nargs/nargs.go

Still thinking. If you were to merge in the nargs branch from github.com/jokelyo/argparse, for instance, that would sway my decision toward just building on his code and adding the 'Positional' boolean. The nargs branch is about 350 lines of changes and about 1200 lines of tests though; I was concerned about whether you'd be interested in something that massive.

I thought about argument ordering, but since a convention in UNIX has always been to accept positional arguments before, after, or interspersed among other flags, I decided not to make the code more complex by trying to constrain it. Consider that these two commands are equivalent, for instance:

cat -n /etc/hosts 
cat /etc/hosts -n

[akamensky]

If you were to merge in the nargs branch from github.com/jokelyo/argparse, for instance, that would sway my decision toward just building on his code and adding the 'Positional' boolean. The nargs branch is about 350 lines of changes and about 1200 lines of tests though; I was concerned about whether you'd be interested in something that massive.

nargs are easily implemented without a massive rewrite of the code base, look at .Strings implementation and basically what you did there with argparse.Options.Nargs.

The thing is Go and Python are very different languages. Strict typing in Go prevents many features from working the same way as they do in Python. As in case of nargs, we could return everything as interface{} or string and let user deal with that, but that is not necessary I believe.

Positional arguments hold much more value than nargs in my opinion, and I would prefer first getting this feature onboard first before thinking about nargs.

It's the lack of a leading '-' in lname that causes python's argparse to treat an argument as a positional. To implement that, we'd have to require that users specify the leading '-' characters in sname and lname for non-positional args. But at this point that would be a breaking change.

You are right and that is absolutely unnecessary. You already provided implementation of this that does not break this functionality. I think just would be great to expand more on this and to actually ensure this deals with two items -- ordering and types.

To explain a bit check below code and execution:

posString := parser.PosString(&argparse.Options{Help: "More strings to print"})
posInt := parser.PosInt(&argparse.Options{Help: "More strings to print"})

$ tool "is a string" 3 # suceeds, because 1st positional is a string and 2nd is a number
$ tool 3 "is a string" # fails, because 1st is interpreted as a string, but 2nd is not a number

[stevegt]

I've renamed Positional to PosString, but would be cautious about trying to implement non-string positionals without including something like what @jokelyo at Rackspace has already done in http://github.com/jokelyo/argparse. Likewise, mixing string and non-string positionals is going to need much of the same code that nargs needs, and he's already done it.

nargs are easily implemented without a massive rewrite of the code base, look at .Strings implementation and basically what you did there with argparse.Options.Nargs.

That wasn't me -- that was @jokelyo. That argparse.Options.Nargs construct is part of his hundreds of lines of changes. I started my 'nargs-positionals' branch by merging his 'nargs' branch with your 'master', then began adding the code to support argparse.Options.Positional. The only reason I stopped working on that branch was simply because of the size of his changes, and the amount of work I'd need to do to add the positional code for all of the various types. While it's possible that he wrote more than was needed, I haven't grokked all of what he wrote, so can't judge. The fact that he added 4x more lines of test code than app code is encouraging though.

If you want to eventually implement mixed-type positionals and nargs, it might be better if we reject this PR and continue work on that 'nargs-positionals' branch instead. The major difference is that, instead of adding the dedicated PosString() function in this PR, the 'nargs-positionals' branch implements positionals using the 'Positional' boolean in Options. I'm also thinking that once we go down one or the other path we're pretty much committed to that API.

[stevegt]

Mentioning #20 and #22 so readers of those issues can find the discussion in this PR.

[akamensky]

Thanks for the code update, after I read description seems i missed the point of this implementation. My apologies, I should have paid attention to that earlier.

I feel this approach is not very convenient for the developers to use. I can't recall now how positionals work in Python's argparse (have not touched Python for awhile), however, for this library I feel best fit would be implementation where we would have one variable per argument (disregarding nargs for now).

Currently, if there are still any arguments left unparsed/unprocessed, library would return an error. I think, instead of collecting all unparsed arguments into one list and returning that list we could add one more parsing stage that will take the list of unparsed args and compare it with a list of defined positional args. I think this would allow for relatively simple implementation of positionals with types and strictly defined list of positional arguments (for each command).

So that API would look something like:

func PosString(opts *Options) *string {
    ...
}

func PostInt(opts *Options) *int {
    ...
}

One detail for this sort of implementation is ordering when using sub-commands. Namely (in my opinion) implementation will need to strictly define that positionals of command come before positionals of sub-command.

And a small complication with this implementation is how to deal with optional positionals parsing.

Consider a tool that can do operations on a file:

parser := argparse.NewParser("tool", "Tool to do some operations on a file")

// Top level positional. Required.
posString := parser.PosString(&argparse.Options{Help: "Path to file", Required: true})

substr := parser.NewCommand("substr", "Print a exempt from file")

// Offset to start at. Optional, int.
offset := substr.PosInt(&argparse.Options{Help: "Offset at which to start the exempt in bytes", Required: false, Default: 0})

// Length of the exempt. Required, int.
length := substr.PosInt(&argparse.Options{Help: "Length of the exempt in bytes", Required: true})

Given the code above, at the time of parsing it would be hard to determine whether 2nd positional argument is an offset or length. (in this specific example it is easy, but in more complex ones with more positionals it will be truly hard).

I think in this case all positionals must be required. But not sure whether this is a good idea.

[hartwork]

It would be great to have this rebased and merged in 2020. Thank you!

[jhughes1153]

It would be great to have this rebased and merged in 2020. Thank you!

Working on it, although feel free to take it off my hands if you need it now.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed soon if no further activity occurs. Thank you for your contributions. Feel free to comment or otherwise update to keep it alive.

[stevegt]

Bump to keep this from closing -- if anyone else has a chance to rebase, I'll keep an eye out for pull requests at https://github.com/stevegt/argparse/tree/positionals. (I've been instead using https://github.com/docopt/docopt.go lately, though it has other issues.)

[vsachs]

Is someone looking at this? I'll do the necessary rebase if someone will merge it afterwards.

[akamensky]

@vsachs If you would like to help with this issue, please go over the conversation above I think the proposed implementation was a bit too complex than necessary for this functionality.

[vsachs]

@akamensky Would it be sufficient to implement a solution which:

• allows positional arguments only on the leaf (sub)-command
• returns an error if the program attempts to configure positionals for non-leaf commands
• returns an error if the program adds sub commands to something with positionals defined
  This could be relaxed over time, as it starts with a fairly strict design.

I'm not clear, from reading the above, whether you would prefer:

1. Setting positional as a boolean in the argparse.Options (implying Required: True)
2. Having a completely new set of functions, command.PosString(), command.PosInt(), etc

Internally it seems like the best implementation is to collect unparsed values into a list then check the defined positional args against that list.

Personally I would very much like to have support for the ability to do:
tool command --flag1 value positional --flag3

Secondarily I don't see a use case of optional positional arguments, I think it would be best to block them. In previous CLI implementations I've done optional positional arguments are always a source of trouble and never particularly valuable. We already have flag support, that should be sufficient for optionals.

Let me know what you think.

[akamensky]

@vsachs Thanks for looking into this issue.

allows positional arguments only on the leaf (sub)-command

I think it is possible solution, not ideal, but if that will make at least initial implementation easier, then could go with that initially, though I think ideal state for this would be to be able to add positional arguments on any level. I think the order in which those arguments are provided on CLI could determine which level it belong to. For example:

cmd - has 1 pos arg defined
subcmd - has 2 pos args defined
subsubcmd - has 1 pos arg defined

cmd subcmd1 subcmd2 --someflag posarg1 posarg2 posarg3 posarg4 posarg5

after parsing it would conclude that:
- posarg1 is for cmd
- posarg2/3 are for subcmd
- posarg4 is for subsubcmd
- posarg5 does not belong to any of them, so throw an error

Does not mean initial implementation has to adhere to this, it just describes my thought of how it ideally would work.

I'm not clear, from reading the above, whether you would prefer:

Setting positional as a boolean in the argparse.Options (implying Required: True)
Having a completely new set of functions, command.PosString(), command.PosInt(), etc

I was thinking of completely new set of functions for this purpose, that would make it explicit. It will likely require new internal type i.e. posarg to logically separate them from other arguments. Though this is implementation specific and how I would approach this.

Internally it seems like the best implementation is to collect unparsed values into a list then check the defined positional args against that list.

Agree, I think this would be easiest to implement.

Personally I would very much like to have support for the ability to do:
tool command --flag1 value positional --flag3

I think if go with collecting unparsed values, that would be doable as well.

Secondarily I don't see a use case of optional positional arguments

I think there are valid use cases for optional positional arguments. But I think the logic would need to be overly complicated for this. So it is easier and more transparent to implement all positional arguments as required (non-optional). And as you mentioned if anything optional need to be provided it can still be easily done with normal arguments.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed soon if no further activity occurs. Thank you for your contributions. Feel free to comment or otherwise update to keep it alive.

[stale]

Closing due to old age. Feel free to re-open or ping maintainers.