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

Add .strictOptionalOptionArguments() for POSIX-style handling of options with optional option-arguments #1951

Closed
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Prevent unusable options
- Check if new option is usable with current settings before registering
- Check if registered options remain usable when updating settings

_registerOption() has been borrowed from deb3bdf in #1923.
  • Loading branch information
aweebit committed Aug 7, 2023
commit 733d9cdbaddceb575d08593adb98fe287f57e0f6
56 changes: 52 additions & 4 deletions lib/command.js
Original file line number Diff line number Diff line change
Expand Up @@ -502,13 +502,28 @@ Expecting one of '${allowedValues.join("', '")}'`);
return new Option(flags, description);
}

/**
* Register option if possible with current settings.
* Throw otherwise.
*
* @param {Option} option
* @api private
*/

_registerOption(option) {
this._checkForBrokenOptionalOptionArguments(option);
this.options.push(option);
}

/**
* Add an option.
*
* @param {Option} option
* @return {Command} `this` command for chaining
*/
addOption(option) {
this._registerOption(option);

const oname = option.name();
const name = option.attributeName();

Expand All @@ -523,9 +538,6 @@ Expecting one of '${allowedValues.join("', '")}'`);
this.setOptionValueWithSource(name, option.defaultValue, 'default');
}

// register the option
this.options.push(option);

// handler for cli and env supplied values
const handleOptionValue = (val, invalidValueMessage, valueSource) => {
// val is null for optional option used without an optional-argument.
Expand Down Expand Up @@ -677,6 +689,40 @@ Expecting one of '${allowedValues.join("', '")}'`);
return this._optionEx({ mandatory: true }, flags, description, fn, defaultValue);
}

/**
* Check if the passed option or any registered option is unusable due to the current `strictOptionalOptionArguments` setting.
* Throw if the check result is positive.
*
* An option is deemed unusable when
* - `strictOptionalOptionArguments` is on,
* - and the option has an optional option-argument,
* - and one of the following holds true:
* - `combineFlagAndOptionalValue` is off,
* - or the option is variadic.
*
* @param {Option} [option]
* @api private
*/

_checkForBrokenOptionalOptionArguments(option) {
if (this._strictOptionalOptionArguments) {
const options = option ? [option] : this.options;
if (!this._combineFlagAndOptionalValue) {
options.forEach((option) => {
if (option.optional && !option.long) {
throw new Error(`Option '${option.flags}' is incompatible with strictOptionalOptionArguments${this._name ? ` enabled for '${this._name}'` : ''} when combineFlagAndOptionalValue is off
- must have a long flag`);
}
});
}
options.forEach((option) => {
if (option.optional && option.variadic) {
throw new Error(`Variadic option '${option.flags}' is incompatible with strictOptionalOptionArguments${this._name ? ` enabled for '${this._name}'` : ''}`);
}
});
}
}

/**
* When set to `true`, handle options with optional option-arguments as prescribed by POSIX,
* i.e. only allow providing values for such option-arguments by combining them with a flag
Expand All @@ -688,6 +734,7 @@ Expecting one of '${allowedValues.join("', '")}'`);

strictOptionalOptionArguments(strict = true) {
this._strictOptionalOptionArguments = !!strict;
this._checkForBrokenOptionalOptionArguments();
return this;
}

Expand All @@ -703,6 +750,7 @@ Expecting one of '${allowedValues.join("', '")}'`);
*/
combineFlagAndOptionalValue(combine = true) {
this._combineFlagAndOptionalValue = !!combine;
this._checkForBrokenOptionalOptionArguments();
return this;
}

Expand Down Expand Up @@ -1842,7 +1890,7 @@ Expecting one of '${allowedValues.join("', '")}'`);
description = description || 'output the version number';
const versionOption = this.createOption(flags, description);
this._versionOptionName = versionOption.attributeName();
this.options.push(versionOption);
this._registerOption(versionOption);
this.on('option:' + versionOption.name(), () => {
this._outputConfiguration.writeOut(`${str}\n`);
this._exit(0, 'commander.version', str);
Expand Down