Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow multiple arguments (array) for individual key #1318

Open
plroebuck opened this issue Mar 13, 2019 · 8 comments
Open

Disallow multiple arguments (array) for individual key #1318

plroebuck opened this issue Mar 13, 2019 · 8 comments

Comments

@plroebuck
Copy link

Description

"duplicate-arguments-array" exists as a "yargs-parser" option to set this globally.
But I don't see how to leave it enabled globally and disable it on key-by-key basis (or vice versa).
Need both behaviors.

Desired Outcome

Example has two arguments:

  • collection: Value is an array of all provided values
  • scalar: Value should be last given
$ node examples/reflect.js --collection "foo" --collection "bar" --scalar 5 --scalar 6
{ _: [], collection: [ "foo", "bar" ], scalar: 6, '$0': 'examples/reflect.js' }

Related

#229 (Disallow multiple arguments (array) for a key) [original issue]
#530 (Disable "duplicates" behaviour)
#586 (Add coerce API)

@juergba
Copy link
Contributor

juergba commented Mar 21, 2019

Within "yargs-parser" you can define collection as an array: array: ['collection'].
duplicate-arguments-array has no effect on yargs type array.

@juergba
Copy link
Contributor

juergba commented Mar 21, 2019

duplicate-arguments-array is not working consistently. The output is different for number type than for string type:

'use strict';
var parse = require('yargs-parser');

var args = parse(
    ['--timeout', '2s', '--timeout', '1s', '--timeout', '800', '--file', '2', '--file', '6'], {
        array: [{key: 'file', number: true}],
        string: ['timeout'],
        configuration: {
            'duplicate-arguments-array': false,
            'camel-case-expansion': false
        }
});
console.log(args);         // { _: [], timeout: '800', file: [ 2, 6 ] }

// with "array: [{key: 'file', string: true}]," the output is:  
//                            { _: [], timeout: '800', file: [ '6' ] }

@nylen
Copy link
Contributor

nylen commented Apr 5, 2019

I found this quite confusing as well. I'm seeing that by default, if I specify a type like number or string, or even set a choice of multiple string values, my options can end up as arrays, even if I enable strict parsing mode.

I would have expected any option key that specifies a type or choice, but does not specify array: true, to throw an error if the option is passed multiple times. If I give a primitive type, then I mean I want a single value for that option!

Due to compatibility, maybe it would be more feasible to only enable this behavior in strict parsing mode, or perhaps also require setting array: false for individual options? @bcoe let me know if you think one of these options is worth pursuing and I will try to write a PR.

In the meantime people need to be aware of this gotcha, or arguments will not have their expected types under all circumstances, even in strict mode. Depending on what the program does with the argument, this could be pretty bad. Here's what I am currently doing for additional validation:

const program = require( 'yargs' )
    .usage( ... )
    .option( 'string-arg1', { type: 'string' } )
    ...
    .strict();

const argv = program.argv;

function usageAndExit( errorMessage ) {
    if ( errorMessage ) {
        console.error( 'Error: %s', errorMessage );
        console.error();
    }
    program.showHelp();
    process.exit( 1 );
}

[ 'stringArg1', 'stringArg2', 'numberArg', 'choiceArg' ].forEach( arg => {
    if ( argv[ arg ] && Array.isArray( argv[ arg ] ) ) {
        usageAndExit( util.format(
            'Invalid value for --%s: %o',
            arg,
            argv[ arg ]
        ) );
    }
} );

@danielvy
Copy link

@nylen

You can use .check() to validate count of arguments. It will make code more clear and concise.

const program = require( 'yargs' )
  .usage( ... )
  .option( 'string-arg1', { type: 'string' } )
  .check(args => {
    return Array.isArray(args.string-arg1) ? "Too many arguments" : true;
  })
  ...
  .strict();

But yeah, an option that should be included in arguments only once is such a trivial case, it makes no sense at all that there's no simple way to specify it. Especially considering that you set the type of option as a simple value (number/string) and then you get it as an array, forcing you to perform unnecessary checks.

@patarapolw
Copy link

Too check all, I resorted to

  .check((args) => {
    const arrayArgs = Object.entries(args)
      .filter(([k, v]) => typeof k === 'string' && /[A-Z]+/i.test(k) && Array.isArray(v))
      .map(([k, _]) => k)

    return arrayArgs.length > 0
      ? `Too many arguments: ${arrayArgs.join(', ')}`
      : true
  })

@mleguen
Copy link
Member

mleguen commented Jan 8, 2020

Some tests with the the current master branch of yargs.

With the default configuration:

const yargs = require('yargs')

console.log(
  yargs
    .option("collection", { array: true })
    .parse()
)
$ node index.js --collection 1 --collection 2 3 --scalar 5 --scalar 6 7
{ _: [ 7 ],
  collection: [ 1, 2, 3 ],
  scalar: [ 5, 6 ],
  '$0': 'index.js' }

If disabling duplicate-arguments-array:

    .parserConfiguration({
      "duplicate-arguments-array": false
    })
$ node index.js --collection 1 --collection 2 3 --scalar 5 --scalar 6 7
{ _: [ 7 ], collection: [ 2, 3 ], scalar: 6, '$0': 'index.js' }

And even if specifying collection as string:

    .option("collection", { array: true, string: true })
$ node index.js --collection 1 --collection 2 3 --scalar 5 --scalar 6 7
{ _: [ 7 ],
  collection: [ '2', '3' ],
  scalar: 6,
  '$0': 'index.js' }

To summarize:

  • with "duplicate-arguments-array": true` (default parser configuration):
    • a repeated option will generate an array (even with array: false)
    • multiple values for a single option occurrence will be parsed only if array: true
  • with "duplicate-arguments-array": false:
    • a repeated option will generate a scalar (even with array: true)
    • multiple values for a single option occurrence will be parsed only if array: true
    • I am unable to reproduce the difference between "string" and "number" reported by @juergba

So:

  • .array() seems to be doing only half of what it is promising in the doc (the "multiple values for a single option occurrence" part)
  • we can no longer have scalar options and array options (with repeated options) in the same cli, which was what @plroebuck initially reported here

IMHO:

  • unless array() is called (or set) for a key, we should only keep the last value of the array provided by yargs-parser
  • we should add a warning in the doc for array() that it will only partially work if "duplicate-arguments-array" is disabled

@bcoe @juergba What is your opinion on this?

@markus456
Copy link

markus456 commented Jan 5, 2021

This seems to also affect variadic positional arguments:

var yargs = require("yargs")

yargs
    .parserConfiguration({
        "duplicate-arguments-array": false,
    })
    .command(
        "test <name> <key> <value> [extras...]",
        "Test positional arguments",
        function (yargs) {},
        function (argv) {
            console.log(argv.name, argv.key, argv.value, argv.extras)
        }
    )
    .parse()
$ node index.js test name key1 value1 key2 value2
name key1 value1 [ 'value2' ]

Removing "duplicate-arguments-array": false from the parser configuration results in expected output:

$ node index.js test name key1 value1 key2 value2
name key1 value1 [ 'key2', 'value2' ]

Explicitly specifying the type of the variadic argument as array with .positional doesn't seem to affect it. This was with yargs 16.2.0.

@the-spyke
Copy link

Yargs 16.2.0 having

.parserConfiguration({
  "duplicate-arguments-array": false,
})
.option(`s`, {
  alias: `source`,
  default: undefined,
  nargs: 1,
  requiresArg: true,
  string: true,
  array: false,
})

and calling mycommand -s asd -s qwe I get

{
  "_": [],
  "s": "qwe",
  "source": [
    "asd",
    "qwe"
  ],
  "$0": "mycommand"
}

Notice that s is okay, but source is still an array.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants