Skip to content

Enforce correct option usage and support custom option flag and name extraction in helpOption()#1926

Closed
aweebit wants to merge 5 commits intotj:developfrom
aweebit:feature/strict-options
Closed

Enforce correct option usage and support custom option flag and name extraction in helpOption()#1926
aweebit wants to merge 5 commits intotj:developfrom
aweebit:feature/strict-options

Conversation

@aweebit
Copy link
Copy Markdown
Contributor

@aweebit aweebit commented Aug 2, 2023

Pull Request

Problem

var program = new Command();
var helpOption = program.createOption(program._helpFlags, program._helpDescription);
var helpOptionName = helpOption.attributeName();
console.log(helpOptionName); // 'help'
program.setOptionValue(helpOptionName, 123); // allowed!

var program = new Command();
program.option('--my-help');
program.helpOption('--my-help'); // allowed!

var program = new Command();
program.setOptionValue('myHelp', 123);
program.helpOption('--my-help'); // allowed!

var program = new Command();
program.version('1.0.0');
console.log(program.version()); // '1.0.0'
var versionOption = program.createOption(program._versionOptionName);
var versionOptionName = versionOption.attributeName();
console.log(versionOptionName); // 'version'
program.setOptionValue(versionOptionName, '1.0.1'); // allowed!
console.log(program.version()); // still '1.0.0'

var program = new Command();
program.option('--my-version');
program.version('1.0.0', '--my-version'); // allowed!

var program = new Command();
program.setOptionValue('myVersion', 123);
program.version('1.0.0', '--my-version'); // allowed!

var program = new Command();
program.setOptionValue('foo', 'bar');
console.log(program.getOptionValue('foo')); // 'bar'
console.log(program.opts()); // { foo: 'bar' }
program.storeOptionsAsProperties(); // allowed!
console.log(program.getOptionValue('foo')); // undefined
console.log(program.opts()); // {}

Solution

Throw errors in all demonstrated cases (partially borrowed from a5afe93 in the now-closed #1921).

Additionally, the code for the help option has been refactored to support custom option flag and name extraction mechanisms defined in subclasses. To achieve that, an Option instance returned from a createOption() call is used for extraction in helpOption() (the same is currently done in version()), which is now called in the constructor and copyInheritedSettings().

The first two commits are tiny improvements to the code that have been made while working on the solution and don't really belong anywhere else.

ChangeLog

Changed

  • Breaking: throw errors when trying to set values for the help and version options
  • Breaking: throw error when trying to add help or version option with .opts() name already in use
  • Breaking: throw error when calling .storeOptionsAsProperties() after setting an option value
  • Breaking: use .createOption() in .helpOption() to support custom option flag and name extraction

Fixed

  • falsy parameter value handling in .addHelpCommand()

Peer PRs

@aweebit aweebit changed the title Enforce correct option usage Enforce correct option usage and support custom option flag and name extraction in helpOption() Aug 2, 2023
@aweebit aweebit marked this pull request as draft August 2, 2023 20:33
@aweebit
Copy link
Copy Markdown
Contributor Author

aweebit commented Aug 2, 2023

Help.visibleOptions() implementation suggests that option conflicts should not be considered a problem. Gonna make some changes to take that into consideration.

@aweebit
Copy link
Copy Markdown
Contributor Author

aweebit commented Aug 2, 2023

Superseded by #1927 #1928 #1929 #1930 for now.

As for the main matter of this PR, namely conflicting options: it might be a good idea to consider using console.warn() to inform the user about the conflicts. Update: Implemented in #1931.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant