Better support for options with multiple arguments

[tswistak]

Is there an existing issue that is already proposing this?

• I have searched the existing issues

Is your feature request related to a problem? Please describe it

In my app, I need to have an option that can take varying number of arguments. If the user hasn't provided any, I should get a list of default args, otherwise take these from the user.

In commander.js, I can do something like:

const { program } = require('commander');
program
  .command('write')
  .option('-m, --multiple [texts...]', 'Some values', ['a'])
  .action(({ multiple }) => {
    console.log(multiple);
  });
program.parse();

Then it works like this, what's something I'd like to achieve:

$ node index.js write
[ 'a' ]
$ node index.js write --multiple b c d
[ 'b', 'c', 'd' ]

However, in nest-commander I have to write a custom parser every time. Unfortunately, that parser works in a "reduce"-way (https://nest-commander.jaymcdoniel.dev/en/features/commander/#variadic-options) so I have no ability to set the default value.

For example, if I've a code:

interface WriteCommandOptions {
  multiple?: string[];
}

@Command({ name: 'write' })
export class WriteCommand extends CommandRunner {
  async run(passedParam: string[], options?: WriteCommandOptions): Promise<void> {
    console.log(options.multiple);
  }

  @Option({
    flags: '-m, --multiple [texts...]',
    description: 'Some values',
    defaultValue: ['a'] as any
  })
  parseMultiple(value: string, previous: string[] = []): string[] {
    return previous.concat([value]);
  }
}

it will always start the array with 'a', so it will work in a following way:

$ npm run cli write

> [email protected] cli
> node dist/main write

[ 'a' ]
$ npm run cli write -- -m b c d

> [email protected] cli
> node dist/main write -m b c d

[ 'a', 'b', 'c', 'd' ]

Describe the solution you'd like

I see two solutions for this problem:

1. Add a possibility to define an option without parser function. This way we will keep the default way how commander.js works, so variadic options would work just fine in most cases. However, I understand that it will ruin the concept of defining everything with decorators.
2. Add a proper default value support for variadic options. But here I understand it won't be such an easy thing due to the fact, that this behavior is inherited from commander.js itself (https://github.com/tj/commander.js#custom-option-processing).

Honestly I'd prefer the option 1, because it will simplify not only this case, but also all other cases, where we need to write every time a dummy parser for commands, when we don't parse them in any way. But I absolutely understand if you won't agree, since it's against the decorator-based design of the nest-commander.

BTW. It would also be nice to change the typing of defaultValue, so I don't need to cast to any to provide an array 🙂.

Teachability, documentation, adoption, migration strategy

What I'd like to write here is not exactly how users would be able to use this or anything like this, because I think it's pretty explanatory from what I wrote before. However, I'd like to share with you a trick that I did to finish my project before making this issue.

In case someone needs to have the default behavior from commander.js here's a little workaround using an external array, but I can't assure you, that it will always work properly (at least in mine it works):

@Command({ name: 'write' })
export class WriteCommand extends CommandRunner {
  private multipleValues: string[] = []; // temporary array

  async run(passedParam: string[], options?: WriteCommandOptions): Promise<void> {
    this.multipleValues = []; // clearing temporary array while executing command
    console.log(options.multiple);
  }

  @Option({
    flags: '-m, --multiple [texts...]',
    description: 'Some values',
    defaultValue: ['a'] as any
  })
  parseMultiple(value: string): string[] {
    this.multipleValues.push(value); // storing value in a temporary array
    return [...this.multipleValues]; // returning the copy of a temp array
  }
}

Then it works the same as default behavior of commander.js:

$ npm run cli write

> [email protected] cli
> node dist/main write

[ 'a' ]
$ npm run cli write -- -m b c d

> [email protected] cli
> node dist/main write -m b c d

[ 'b', 'c', 'd' ]

What is the motivation / use case for changing the behavior?

The current way of handling multiple values is a bit misleading, especially if someone wants to set the default value for variadic options.

My use case in the app is that I have a text generator that can take as an argument a list of locales in which text can be generated. Otherwise, it will generate all texts in English. So I'd like to use it like:

$ my-app generate 
// some texts generated in English
$ my-app generate --locales de fr pl
// some texts generated in German, French and Polish, no English

[unicornware]

@tswistak to add options without using a decorator, try overriding the setCommand method:

import { Command } from 'commander'

@Command({ name: 'write' })
export class WriteCommand extends CommandRunner {
  private multipleValues: string[] = []; // temporary array

  @Option({
    flags: '-m, --multiple [texts...]',
    description: 'Some values',
    defaultValue: ['a'] as any
  })
  parseMultiple(value: string): string[] {
    this.multipleValues.push(value); // storing value in a temporary array
    return [...this.multipleValues]; // returning the copy of a temp array
  }

  async run(passedParam: string[], options?: WriteCommandOptions): Promise<void> {
    this.multipleValues = []; // clearing temporary array while executing command
    console.log(options.multiple);
  }

  override setCommand(cmd: Command): this {
    // something cool - https://github.com/tj/commander.js/blob/v10.0.0/lib/command.js
    this.command = cmd
    return this
  }
}

i used setCommand to further configure this.command rather than add options without decorators though, so i'm not too sure how that will work out in your project. i also extended the CliUtilityService to encapsulate parsing logic (see here).

for more control over options, however, i ended up patching nest-commander to not only correct type definitions, but make all commander.Option methods accessible via the Option decorator as well.

[jmcdo29]

@unicornware if these are generic enough extensions to the package, would you like to make a PR to update nest-commander? I'd be happy to review it