feat!: switch type:string option arguments to greedy, but with error for suspect cases in strict mode

[shadowspawn]

Should we make options expecting arguments greedy or choosy? See #79 for survey of behaviours.

How about greedy, but with explanatory error for arguments which start with a dash when in strict mode?! See https://github.com/pkgjs/parseargs/issues/77#issuecomment-1086534461 Closes #77

Greedy is consistent with POSIX and Commander when strict:false and simple to describe. In strict mode we throw an error due to the possible mistake by end user omitting argument with an explanation of how to proceed if not a mistake.

These arguments will be eaten in strict:false and throw a helpful error in strict:true.

• --foo --
• --foo -a
• --foo -2
• --foo --bar

Errors:

$ node index.js --with --foo
/Users/john/Documents/Sandpits/parseargs/my-parseargs/index.js:60
    throw new ERR_PARSE_ARGS_INVALID_OPTION_VALUE(errorMessage);
    ^

ERR_PARSE_ARGS_INVALID_OPTION_VALUE [Error]: Option '--with' argument is ambiguous.
Did you forget to specify the option argument for '--with'?
Or to specify an option argument starting with a dash use '--with=-XYZ'.
...stack...

$ node index.js -w --foo
/Users/john/Documents/Sandpits/parseargs/my-parseargs/index.js:60
    throw new ERR_PARSE_ARGS_INVALID_OPTION_VALUE(errorMessage);
    ^

ERR_PARSE_ARGS_INVALID_OPTION_VALUE [Error]: Option '-w' argument is ambiguous.
Did you forget to specify the option argument for '-w'?
Or to specify an option argument starting with a dash use '--with=-XYZ' or '-w-XYZ'.
...stack...

Notes:

• construction of error object follows format decided in #74
• suggest short form only if user used short form
• not using user value in example as it may require quoting! e.g. --foo "--bar alpha *test"

[shadowspawn]

I wanted to take a serious look at how we handle --foo --bar and why. Originally asked the question quite casually in #25 when thinking about string used as boolean, then went deep on choosy vs greedy in #79 and #85.

I think it is a coin-flip for greedy vs choosy, in trade-offs and implementations, and the feedback was split. Because we default to type:'boolean' in zero-config, we are not pushed towards choosy to disambiguate zero-config situations.

I think this PR is a good compromise of POSIX compatible behaviour when strict:false and informative error for possible usage errors when strict:true. Greedy with safety rails, it you use them.

(Full disclosure, we could still do the fancy error message if we stuck with choosy mode!)

[shadowspawn]

Hmm, the initial "error message is not very error like. Perhaps have an error first, more like:

ERR_PARSE_ARGS_INVALID_OPTION_VALUE [Error]: Ambiguous argument for  '-w'.
Did you forget to specify the option argument for '-w'?
To specify an option argument starting with a dash use '--with=-XYZ' or '-w-XYZ'.

[shadowspawn]

Bumping this for consideration in case being ignored because I never put it forward for MVP.

I proposed this behaviour in (https://github.com/pkgjs/parseargs/issues/77#issuecomment-1086534461) for feedback before pushing it for MVP, and then the node PR got opened and things got busy. :-)

Feedback was 3 x 👍 (including me).

[bakkot]

I think this is probably the best behavior, though any of the possibilities seem OK. +1 from me.

not using user value in example as it may require quoting! e.g. --foo "--bar alpha *test"

I think

const arg = /^[a-zA-Z0-9\-_]+$/.test(value) ? value : `'${value.replace(/[\\']/g, '\\$&')}'`;

will omit quotes in cases where they're obviously unnecessary and include them in the remaining cases.

[bcoe]

@shadowspawn do we want to land this for MVP?

[shadowspawn]

I would like to. It would be a breaking change in future as changes the parsing behaviour in strict:false for the greedy vs choosy cases.

But it isn't essential.

[shadowspawn]

Moved high-level greedy tests into index.js for sharing upstream.