microsoft / rushstack

Monorepo for tools developed by the Rush Stack community
https://rushstack.io/
Other
5.83k stars 592 forks source link

[ts-command-line] Officially support positional arguments #2785

Open elliot-nelson opened 3 years ago

elliot-nelson commented 3 years ago

Summary

Regardless of whether Rush ever uses positional arguments, if we want ts-command-line to be usable outside of rushstack (by folks interested in a robust TypeScript-based command line parser), it needs to have positional argument support.

What we'd be interested in is a command syntax like:

bin/my-html-convert --format markdown input.md output.html --github

Where:

These positional arguments should be definable using the existing conventions, defineSomething(), and have defined types just like the existing parameters.

Details

The backing library (argparse) already has robust support for positional arguments, so we won't need to move away from that library to make this change. We only need to decide what additional restrictions we want to put on the defined arguments.

My proposal would be to make the parsing rules simple by allowing flags to exist anywhere (before and after positional arguments), and to allow both required and optional positional arguments; but, as soon as an optional positional argument is specified, all positional arguments after that must also be optional. You are allowed to create List-type positional arguments, but only as the last positional argument defined. This set of restrictions should support almost all possible desired CLI syntaxes, without requiring any "guesswork" (ambiguity about what argument is assigned to what parameter).

I'd like to be able to define positional arguments as a String, StringList, Choice, ChoiceList, Integer, or IntegerList (any existing parameter type except for Flag). Tentatively, I think it would make sense for the user API to just duplicate all existing methods (even though likely we'd just extend existing Parameter types to support positional args instead of parameter names).

For example:

this.defineStringPositional({
    name: 'INPUT_FILE',
    description: 'The contacts file to process'
});

this.defineStringPositional({
    name: 'OUTPUT_FILE',
    description: 'The generated database filename'
});

this.defineChoiceParameter({
    alternatives: ['json', 'csv', 'tab'],
    parameterShortName: '-f',
    description: 'Format of incoming contacts file'
});

An alternative would be to take time to break apart underlying types and parameters/positionals into separate concepts, and define parameters in a new API:

this.definePositional({
    name: 'INPUT_FILE',
    type: CommandLineParameter.STRING,
    description: 'The contacts file to process'
});

This alternative likely has the disadvantages of (1) more complicated type algebra in ts-command-line, and (2) forces users to import various additional enums and classes from the package in order to make their definitions. (An advantage of today's design is that if you stick to using the define and get methods, you never need to import any extra type names at all.)

(Defining parameters and positionals is a big part of the dev experience when using a CLI framework so I'm very interested in additional ideas on improvements!)

Standard questions

Please answer these questions to help us investigate your issue more quickly:

Question Answer
Package name: ts-command-line
Package version? 5.4x.x
Operating system? Mac
Would you consider contributing a PR? Yes!
Node.js version (node -v)? 12.17.0
elliot-nelson commented 3 years ago

Possible negatives / caveats

If we go ahead with https://github.com/microsoft/rushstack/pull/2784 (allow space-separated arguments for list parameters), then list parameters and positional arguments interact in a way that depends on order. For example, if --color is a ChoiceList and your command takes positional arguments inputFile and outputFile:

bin/my-program in.png out.png --color red green
# -> { color: ['red', 'green'], inputFile: 'in.png', outputFile: 'out.png' }

bin/my-program --color red green in.png out.png
# ERROR: `in.png` is not a valid choice for color

There's several ways for the user to specify what they intend, but this conflict does leave the designer of the CLI with no options if they want to avoid the user running into this error.

Possible approaches in this case:

  1. Implement both features, and ignore the issue that users cannot avoid creating ambiguous CLIs if they use positional args.
  2. Do not implement positional args (i.e., reject this proposal altogether).
  3. Implement positional args, but reject https://github.com/microsoft/rushstack/pull/2784 (this would mean you could use Linux-style wildcards for positional args but never for any List param, which to me seems like a step backwards).
  4. Amend https://github.com/microsoft/rushstack/pull/2784 to allow the user to specify whether a List param accepts a single or multiple values at a time. We could then recommend (but not require) that CLI implementers do not use both positional arguments and multi-value List params at the same time.

My vote currently would be Option 4, as I think it gives the user the most flexibility.