75lb / command-line-args

A mature, feature-complete library to parse command-line options.
MIT License
679 stars 107 forks source link

Add support for positional arguments #86

Closed rgov closed 5 years ago

rgov commented 6 years ago

Python's argparse is well-designed, and one thing that it does particularly well is separating optional arguments and positional arguments as distinct concepts.

Suppose I want a command-line tool that divides a number:

$ ./divide 12 2
6

This would use what argparse calls positional arguments, because it parses them according to their position in the argument list. You define them like this:

parser.add_argument('dividend', type=int)
parser.add_argument('divisor', type=int)

command-line-args only allows me to name one default option, which can either be one argument or any number of arguments. This is less flexible; the workaround would be to capture a bunch of default option values and then perform additional validation and parsing on them afterwards.

Plenty of UNIX tools like cp and mv use multiple positional arguments this way.

On the other hand, argparse has optional arguments like --foo and -f and so forth. Optional arguments are only parsed up until an -- argument is encountered (#83). Whereas positional arguments are required, optional arguments can be omitted (unless they are marked required).

command-line-args allows the default option to be set by name or not. If the default option is called file then I can either just write the file name, or write --file a.txt or --file=a.txt. This muddies the distinction between the two types of arguments. It is potentially undesirable, and the test cases which cover it are bizarre; it's unclear what the desirable parse should be. If the user wants it, they should be able to add an optional argument called --file that stores to the same variable as the default option.


In summary, I think it would align better with existing command-line interfaces and user expectations to change the behavior as follows:

75lb commented 6 years ago

True, this lib has no concept of positional args. If we expect to receive args in a fixed order, can't we just use process.argv directly? For example, your divide script could be written:

console.log(process.argv[2] / process.argv[3])
rgov commented 6 years ago

If we expect to receive args in a fixed order, yes, we can just use the argv array directly. But this is inferior in several ways. First, we need to know what index to start looking in argv after all the optional arguments have been handled. Then, we have to correctly subdivide the remaining argv using array slicing, which is not likely to be as readable. We have to do type conversion and validation ourselves, and of course assign it to a named variable, rather than letting the argument parser handle this for us.

I mean, why not parse the whole argument array manually? Because an argument parser lib saves you a lot of hassle by letting you declare what you want.

while (argv.length > 0) {
  var arg = argv.shift()
  if (arg == '--test' || arg == '-t') {
     ...
  } else if (arg == '--foo') {
     var value = argv.shift()
     ...
  }
  ...
}
75lb commented 6 years ago

I'm open to adding support for positional args. Could you post some examples of realistic commands that use both optional and positional args to help me visualise what you have in mind? Also, for your example(s) could you show me how the optionDefinitions array could/should look to make such a command possible?

rgov commented 6 years ago
{ name: 'recursive', alias: 'r', type: Boolean },
{ name: 'sources', positional: true, multiple: true },
{ name: 'target', positional: true }

(Note argparse would have you specify name: '--recursive' for optional arguments, but it's a backwards compatibility issue.)

{ name: 'hostname', positional: true },
{ name: 'command', positional: true }
75lb commented 6 years ago

I agree positional args would be a useful addition - will get back to you soon with next step info.

75lb commented 6 years ago

the test cases which cover it are bizarre; it's unclear what the desirable parse should be.

True, I'm looking at the tests now (while reviewing your PR) and some of them test usage that is far from realistic. Beside the usual realistic tests I often write "bad input" tests checking useful output or exceptions are emitted even in cases where the user has input absolute rubbish..