feat: allow multiple for arg

[mkg20001]

This enables the multiple flag for an argument

The advantage over #61 is that this allows "multiple" arguments right in the middle, although both proposals could co-exist aswell

NOTE: Only one multiple argument is allowed, otherwise they will eat each other's arguments

The strategy to parse it works by checking how many non-multiple arguments should follow

[salesforce-cla[bot]]

Thanks for the contribution! Before we can merge this, we need @mkg20001 to sign the Salesforce.com Contributor License Agreement.

[mkg20001]

NOTE: I have no clue about typescript. I'd appreciate some guiadance

[codecov[bot]]

Codecov Report

Merging #63 into master will decrease coverage by 84.45%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #63       +/-   ##
==========================================
- Coverage   84.45%   0.00%   -84.46%     
==========================================
  Files          11       1       -10     
  Lines         328      29      -299     
  Branches       91      14       -77     
==========================================
- Hits          277       0      -277     
+ Misses         36      29        -7     
+ Partials       15       0       -15     

Impacted Files	Coverage Δ
src/validate.ts	0.00% <0.00%> (-31.43%)	:arrow_down:
src/errors.ts
src/index.ts
src/list.ts
src/deps.ts
src/util.ts
src/flags.ts
src/help.ts
src/screen.ts

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 73c0c4e...d8f27c3. Read the comment docs.

[frangio]

@RasPhilCo Any chance to get this merged?

[RasPhilCo]

I think we should fix this by fixing how parse handles multiple args with static = false set. It solves this without adding api. See https://github.com/oclif/oclif/issues/234#issuecomment-554822683

[mkg20001]

I think we should fix this by fixing how parse handles multiple args with static = false set. It solves this without adding api. See oclif/oclif#234 (comment)

The advantage over #61 is that this allows "multiple" arguments right in the middle, although both proposals could co-exist aswell

@RasPhilCo This extra API would be extremly convinient for some usecases.

Like parsing "<src <src...>> <dst>" (like cp does it), automatically

[RasPhilCo]

On further thought, I'm on board with this. I'll make a few typescript comments. It'll need tests.

[tiagonapoli]

I'm in need of this feature too, @mkg20001 if you allow me I can try to contribute on this PR, at least trying to make some tests

[mkg20001]

@tiagonapoli You've got push access now: https://github.com/mkg20001/parser/invitations

[G-Rath]

@mkg20001 if you could rebase this, I'm happy to help write tests :)

[mkg20001]

@G-Rath I've added you as a collaborator: https://github.com/mkg20001/parser/invitations

[G-Rath]

@mkg20001 I've had a play with this, but it doesn't seem to work anymore?

Am I missing something:

export default class MyCommand extends Command {
  public static args: Parser.args.IArg[] = [
    {
      name: 'targetPath',
      required: true,
      // @ts-ignore
      multiple: true
    }
  ];

  public async run(): Promise<void> {
    console.log(this.parse(MyCommand));
  }
}

g-rath:terraport$ nrun cli mycommand 1 2 3
> nrun cli
> node -r tsconfig-paths/register bin/terraport "mycommand" "1" "2" "3"
 ›   Error: Unexpected arguments: 2, 3
 ›   See more help with --help

[mkg20001]

Hrm. I changed multiple: Boolean to multiple?: false but I don't think that's what broke it.

[G-Rath]

I changed multiple: Boolean to multiple?: false but I don't think that's what broke it.

Correct, that won't have changed the behaviour. Types exist at compile only - they never affect the emitted js (that's one of the non-goals of TS) :)

[G-Rath]

@mkg20001 ah it's because now the strict property has to be set to false. Without it, the parser won't parse the rest of the arguments.

I feel like this makes strict a bit redundant - it probably should be calculated based off args:

  static get strict() {
    return !this.args.some(arg => arg.multiple);
  }

[G-Rath]

@mkg20001 This doesn't seem to be working as expected still: it's not properly handling args after the multiple:

export default class MyCommand extends Command {
  public static args: Parser.args.IArg[] = [
    {
      name: 'targetPath',
      required: true
      // @ts-ignore
      multiple: true
    },
    {
      name: 'again',
      required: true,
      // @ts-ignore
      // multiple: true
    }
  ];

  static strict = false;

  public async run(): Promise<void> {
    console.log(this.parse(MyCommand));
  }
}

g-rath:terraport$ nrun cli mycommand 1 2 3
> nrun cli
> node -r tsconfig-paths/register bin/terraport "mycommand" "1" "2" "3"
{
  args: { targetPath: '1', again: [] },
  argv: [ '1', '2', '3' ],
  flags: {},
  raw: [
    { type: 'arg', input: '1' },
    { type: 'arg', input: '2' },
    { type: 'arg', input: '3' }
  ],
  metadata: { flags: {} }
}

I just want to check if this is the intended behaviour?

[G-Rath]

Looking further into this, I think the answer is what @RasPhilCo originally said:

I think we should fix this by fixing how parse handles multiple args with static = false set. It solves this without adding api. See oclif/oclif#234 (comment)

Effectively by having strict as false, the last argument should act as a sponge, socking up all trailing args.

This should provide a simple but powerful api for supporting variadic arguments:

Commands that take at 1 or more args:

export default class FormatFiles extends Command {
  public static args: Parser.args.IArg[] = [
    { name: 'file', required: true },
    { name: 'additionalFiles', required: false }
  ];

  static strict = false;

  public async run(): Promise<void> {
    const {
      args: { file, additionalFiles }
    } = this.parse(FormatFiles);

    const allFiles: string[] = [file].concat(additionalFiles);
  }
}

Commands that take multiple arguments before a final argument (a la cp):

export default class CopyFiles extends Command {
  public static args: Parser.args.IArg[] = [{ name: 'everything' }];

  static strict = false;

  public async run(): Promise<void> {
    const {
      args: { everything }
    } = this.parse(CopyFiles);

    const dest: string = everything.pop();
    const srcs: string[] = everything;
  }
}

I believe the complexity to implement the proposed API above is much smaller than what it would take to implement this PR.

[alexkli]

Any chance of getting this implemented? Any workaround?

Support for array-like arguments (mycli arg ...) seems like a very basic feature. Using flags using multiple: true is not really the same, as mycli -a one -b two is a very different command line UX as cli one two ..., at least to me 😄 .

[nwalters512]

Hey maintainers, can I do anything to help push this forward?

[mkg20001]

I'm no longer interested in the development of this PR. Feel free to take my changes and implement them in another PR

[msabramo]

I am interested in this feature and would be willing to put in some time to get it polished, as long as the maintainers can say that they're willing to merge it.

I think this is a very handy feature for when one wants to have a command like process-image --filter=blur *.jpg or such.

I also think it is very consistent with oclif's current design, because flags already have a multiple property -- https://oclif.io/docs/flags

[msabramo]

I found a workaround, in case anyone else finds this useful:

    static args = [{ name: 'path' }];
    static strict = false  // Allow variable length arguments (https://oclif.io/docs/args)
    ...
    getAllPathArgsArray() {
        const { args } = this.parse(TestCommand)
        const indexOfFirstPath = this.argv.findIndex(arg => arg === args.path)
        return this.argv.slice(indexOfFirstPath)
    }

or perhaps if you're already calling this.parse elsewhere to process other arguments and flags, which likely you are, then maybe this is better:

    const oclifParseResult = this.parse(TestCommand)
    const { flags, args } = oclifParseResult
    const allPathArgsArray = this.getAllPathArgsArray(oclifParseResult)

    getAllPathArgsArray(oclifParseResult) {
        const indexOfFirstPath = this.argv.findIndex(arg => arg === oclifParseResult.args.path)
        return this.argv.slice(indexOfFirstPath)
    }

I do find it surprising that oclif doesn't just support a multiple property for args, as it does for flags.

[NickTomlin]

Inspired by @msabramo I was able to work around this by using the raw property of ParseResult.

$ mycommand --cluster biz example/simple/simple.workflow.json bar biz

Results in a parse result like this:

{
  args: { path: 'example/simple/simple.workflow.json' },
  argv: [ 'example/simple/simple.workflow.json', 'bar', 'biz' ],
  flags: { cluster: 'biz' },
  raw: [
    { type: 'flag', flag: 'cluster', input: 'biz' },
    { type: 'arg', input: 'example/simple/simple.workflow.json' },
    { type: 'arg', input: 'bar' },
    { type: 'arg', input: 'biz' }
  ],
  metadata: { flags: {} }
}

Which I then filtered the raw elements like so:

class MyCommand {

  static flags = { /* your flags */ }
  static args = [{ name: 'path', multiple: true}];
  // allow for passing multiple non typed args through (https://oclif.io/docs/args)
  static strict = false
  run () {
    const parseResult = this.parse(MyCommand);
    const paths = parseResult.raw
      .filter(arg => arg.type == 'arg')
      .map(arg =>  arg.input)
  }
}