util: support negative options for `parseArgs`

[kylo5aby]

This PR tries to support negative options like the format --no-foo for parseArgs by adding a flag allowNegative in the config of parseArgs. It works for general CLI flag and options passed to parseArgs .

By default, allowNegative is false in order to bring a breaking change.

Refs: #53095

[avivkeller]

IMO there should be a flag, something like allowNegativeArguments in the options (My name probably needs some work). Someone may be be parsing a --no- prefixed argument for the general CLI args.

[!TIP] I am not a core collaborator, and this is only a suggestion.

[bakkot]

Turning this on by default would be a breaking change. Might make more sense to make it opt-in per option, as in { 'foo': { type: 'boolean', allowNegation: true } }.

Also I'm not sure it makes sense to support this for multiple: true boolean options. That's intended to be for stuff like -vvv, whereas this is for single flags which can be on or off.

If the argument is passed with the --no- prefix, the value of the argument will be set to the opposite of its default value.

That's definitely wrong. --no-foo means foo is false, regardless of what foo defaults to.

[kylo5aby]

IMO there should be a flag, something like allowNegativeArguments in the options (My name probably needs some work). Someone may be be parsing a --no- prefixed argument for the general CLI args.

Tip

I am not a core collaborator, and this is only a suggestion.

Thanks for the advise!, I believe a flag like allowNegativeArguments or something is better, which will not bring a breaking change. And I think the flag should in the config just like strict or allowPositionals instead of in every options, this will be friendly towards the general CLI args like process.argv.

[kylo5aby]

Turning this on by default would be a breaking change. Might make more sense to make it opt-in per option, as in { 'foo': { type: 'boolean', allowNegation: true } }.

Also I'm not sure it makes sense to support this for multiple: true boolean options. That's intended to be for stuff like -vvv, whereas this is for single flags which can be on or off.

If the argument is passed with the --no- prefix, the value of the argument will be set to the opposite of its default value.

That's definitely wrong. --no-foo means foo is false, regardless of what foo defaults to.

That's definitely wrong. --no-foo means foo is false, regardless of what foo defaults to.

Thanks for the reminder! I have misunderstanding it before and thought --no- prefix should just opposite the default value.

Turning this on by default would be a breaking change. Might make more sense to make it opt-in per option, as in { 'foo': { type: 'boolean', allowNegation: true } }.

I believe add a new flag in the config to determine whether allow bidirectional arguments will be better for avoiding the breaking change.

Also I'm not sure it makes sense to support this for multiple: true boolean options. That's intended to be for stuff like -vvv, whereas this is for single flags which can be on or off.

I think that the --no- prefix and multiple: true are independent of each other. For type: 'boolean' option foo, since ['--foo', '--foo'] is allowed, ['--foo', '--no-foo'] should also be allowed.

[kylo5aby]

I don't think it should be set to the opposite of the default value, but to false.

Thanks for the reminder. I have misunderstanding it before and thought --no- prefix should just opposite the default value. In order to avoiding the breaking change, add a new flag in config for parseArgs to control whether allow bidirectional arguments, what do you think?

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/59381/

[bakkot]

If landing this, it would be good to also update the docs which use this precise case as an example of the tokens array:

For example to use the returned tokens to add support for a negated option like --no-color, the tokens can be reprocessed to change the value stored for the negated option.

This example should either be replaced or should at least mention that you can do this automatically with the allowNegative option. Maybe something like:

For example, you can replicate the behavior of the allowNegative option (which adds support for a negated option like --no-color) by reprocessing the tokens array to change the value stored for the negated option.

[shadowspawn]

I was thinking about that too. The example in the docs support --no-* for a "string" option too, so is doing something different and not completely obsoleted by allowNegative. Say:

For example to use the returned tokens to add support for a negated option like --no-color independent of the option type, the tokens can be reprocessed to change the value stored for the negated option.

[shadowspawn]

For interest, I was wondering about a short option for negation, and current state of PR does allow this:

const parsedArgs = parseArgs({
    allowNegative: true,
    options: {
        boolean: { type: 'boolean', short: 'b' },
        'no-boolean': { type: 'boolean', short: 'B' },
    }
});
console.log(parsedArgs);

% node index.js 
{ values: [Object: null prototype] {}, positionals: [] }
% node index.js --no-boolean
{
  values: [Object: null prototype] { boolean: false },
  positionals: []
}
% node index.js -B          
{
  values: [Object: null prototype] { boolean: false },
  positionals: []
}

[kylo5aby]

I was thinking about that too. The example in the docs support --no-* for a "string" option too, so is doing something different and not completely obsoleted by allowNegative. Say:

For example to use the returned tokens to add support for a negated option like --no-color independent of the option type, the tokens can be reprocessed to change the value stored for the negated option.

I'm curious if the example of --no-logfile in the document might cause confusion, because it actually change the type of option logfile. So I believe this should be update after introducing allowNegative, or allowNegative also should support options which has type: "string", for example, if --no-logfile specified, option logfile ({type: 'string'}) should have value false?

[kylo5aby]

For interest, I was wondering about a short option for negation, and current state of PR does allow this:

const parsedArgs = parseArgs({
    allowNegative: true,
    options: {
        boolean: { type: 'boolean', short: 'b' },
        'no-boolean': { type: 'boolean', short: 'B' },
    }
});
console.log(parsedArgs);

% node index.js 
{ values: [Object: null prototype] {}, positionals: [] }
% node index.js --no-boolean
{
  values: [Object: null prototype] { boolean: false },
  positionals: []
}
% node index.js -B          
{
  values: [Object: null prototype] { boolean: false },
  positionals: []
}

I have referred the usage of negative options in configure.py, seems there no such usage like a short option for negation, I will update this case

[shadowspawn]

For interest, I was wondering about a short option for negation, and current state of PR does allow this:

To be clear, I think the current PR behaviour is fine. I tried that configuration because it is natural in Commander, which has separate options for the positive and negative configurations, so obvious that can have a short option for the negative. I wanted to see if I could do it in parseArgs and it worked.

[shadowspawn]

I'm curious if the example of --no-logfile in the document might cause confusion, because it actually change the type of option logfile. So I believe this should be update after introducing allowNegative, or allowNegative also should support options which has type: "string", for example, if --no-logfile specified, option logfile ({type: 'string'}) should have value false?

I think it is a simpler and more predictable behaviour for allowNegation to only work with boolean options since it is global configuration and not per-option. Otherwise, all options including strings would possibly return false.

As for whether we need a new example...

[shadowspawn]

There are some other example uses for tokens on the parseArg repo. Perhaps blocking repeated options?

• https://github.com/pkgjs/parseargs/blob/main/examples/no-repeated-options.js

[kylo5aby]

There are some other example uses for tokens on the parseArg repo. Perhaps blocking repeated options?

• https://github.com/pkgjs/parseargs/blob/main/examples/no-repeated-options.js

I have added some checks in storeOption to ensure that if an option starts with --no- and allowNegative is enabled, token.name is corrected to longOption.

[kylo5aby]

If landing this, it would be good to also update the docs which use this precise case as an example of the tokens array:

For example to use the returned tokens to add support for a negated option like --no-color, the tokens can be reprocessed to change the value stored for the negated option.

This example should either be replaced or should at least mention that you can do this automatically with the allowNegative option. Maybe something like:

For example, you can replicate the behavior of the allowNegative option (which adds support for a negated option like --no-color) by reprocessing the tokens array to change the value stored for the negated option.

Updated the doc to mention that allowNegative can do this for options with boolean type. Thanks for the advice

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/59437/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/59455/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/59880/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/59881/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/59885/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/59886/

[nodejs-github-bot]

Landed in 4a72b2f92769753b48934665544d5facec9b2609

[ljharb]

https://github.com/DefinitelyTyped/DefinitelyTyped/discussions/70056