feat: add 'multipleNonGreedy' flag option to assign only one value per multiple flag

[royra]

This is a port of oclif/parser#79 by @cxam

Original description:

This feature aims to address the issue outlined in oclif/oclif#190 to only allow a single value to be assigned for each flag that has the multiple option set. It's a non-breaking change that requires you to include the singleValue boolean in your flag options.

In the command cmd -a foo bar, it's generally accepted that bar becomes a positional argument and should be the default behaviour. Unfortunately, the PR that broke default behaviour has been part of this project for a while now and I feel providing the user an option to change this back would be the best course of action unless a breaking release is planned for oclif.

If this goes through, I'll update and push the relevant changes to the docs repository.

[salesforce-cla]

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

[mdonnalley]

@royra Thanks for the PR! Can you help me understand why someone would use multiple: true and singleValue: true? Seems to me that they would just negate each other - why not just have a normal flag that doesn't set multiple: true?

[royra]

Can you help me understand why someone would use multiple: true and singleValue: true?

This solves the problem with combining { multiple: true } flags with args, as described in oclif/oclif#190:

Example: assuming m is a { multiple: true } flag and an arg argOne is defined:

command -m val arg1

should parse into

{ 
  flags: { m: ['val'] }, 
  args: { argOne: 'arg1' },
}

but instead parses arg1 as the 2nd value of m.

I simply ported the original PR, and I agree singleValue isn't very descriptive. Maybe multipleNonGreedy?

[royra]

@mdonnalley I tried to sign the CLA and got some kind of server error. When trying again I'm getting a "already signed the CLA" message, but the GH check hasn't passed. Is there anything I can do to fix it?

[royra]

Related: #496

[mdonnalley]

@royra Thanks but I'm still not sure I understand how this new property solves anything

I understand the problem though. You want to be able to write a command that takes arguments and multi flags and have it parsed properly.

But if you're adding a new property to force the parser to stop looking for flag values after it finds the first one, then why not just define the flag as non multi flag? As I understand it these two flag definitions are the exactly the same:

flag1: Flags.string({
  multiple: true,
  singleValue: true,
})

flag1: Flags.string()

Because of that, I'm not inclined to move forward with this PR as it is.

My recommendations are:

• Avoid mixing arguments and multi flags (this was my recommendation in argument not read, if after a flag with multiple: true #496)
• If you must support mutli flags with arguments, you could create a custom flag that accepts an array of comma separated values:

export const arrayFlag = Flags.custom<string[]>({
  delimiter: ',',
})

[royra]

@royra Thanks but I'm still not sure I understand how this new property solves anything

I understand the problem though. You want to be able to write a command that takes arguments and multi flags and have it parsed properly.

But if you're adding a new property to force the parser to stop looking for flag values after it finds the first one, then why not just define the flag as non multi flag? As I understand it these two flag definitions are the exactly the same:

flag1: Flags.string({
  multiple: true,
  singleValue: true,
})

flag1: Flags.string()

This definition doesn't allow for specifying the flag more than once, e.g, cmd --m val1 --m val2 - it results the error Flag --m can only be specified once.

Specifying a flag multiple times is a common pattern; unix classics like sed and find allow it, and modern CLIs like docker too. Having an option for comma-separated values is a useful alternative, and I would like to have the option to parse both. I think this fix is a good compromise, since it allows this while not breaking backwards compat.

[mdonnalley]

@royra Okay, I understand now. You want to allow -m val1 -m val2 but not allow -m val1 val2.

In that case, I'd like for singleValue to be renamed to something more clear. You're earlier suggestion of multipleNonGreedy works for me. I also like multipleStrict, multipleExplicit - whatever you like best

Thanks!

[royra]

In that case, I'd like for singleValue to be renamed to something more clear. You're earlier suggestion of multipleNonGreedy works for me. I also like multipleStrict, multipleExplicit - whatever you like best

@mdonnalley Done. Thank you!