Allow dynamically updating version number and version option flags and description by aweebit · Pull Request #1933 · tj/commander.js

aweebit · 2023-08-03T20:57:43Z

Problem

// demo.js
const { Command } = require('commander');

const flags = '-v, --my-version';
const description = 'custom version option description';

var program = new Command();
program
  .version('1.0.0')
  .version('2.0.0', flags, description); // expectation: the version number and option's flags and description are updated
program.outputHelp(); // reality: a new option is added

var program = new Command();
program
  .version('1.0.0')
  .version('2.0.0'); // expectation: the version number is updated
program.outputHelp(); // reality: a new option is added

var program = new Command();
program
  .version('1.0.0')
  .version(program._version, flags); // expectation: the version option's flags are updated
program.outputHelp(); // reality: a new option is added

var program = new Command();
program
  .version('1.0.0')
  .version(program._version, undefined, description); // expectation: the version option's description is updated
program.outputHelp(); // reality: a new option is added

var program = new Command();
const result = program
  .version('1.0.0')
  .version(undefined, flags, description); // expectation: the version option's flags and description are updated
program.outputHelp(); // reality: the flags are not updated
console.log(result); // the version number is returned instead

// A real-world example
var program = new Command();
program.version('1.0.0');
if ('EARLY_ADOPTER' in process.env) {
  program.version('2.0.0'); // expectation: the version number is simply updated
}
program.outputHelp(); // reality: contains version option twice if early adopter
program.parse(['--version'], { from: 'user' }); // output is always '1.0.0'

EARLY_ADOPTER= node demo.js

Solution

Rework version() to make all the above examples behave as expected.

Includes various improvements to the code for the version option, including the type fix from #1930. For reasoning behind those improvements, see the commit descriptions.

ChangeLog

Added

support for dynamically updating version number and version option flags and description via .version()

Fixed

(from #1930) .version() optional parameter type

Peer PRs

…solving similar problems

Use createOption() in helpOption() to support custom option flag extraction (+ various improvements) #1929 and it's extensions (storing the help option instance in _helpOption instead of only the individual flags in _helpShortFlag and _helpLongFlag)

Borrowed from a3f0e28 that was supposed to land in the now-closed tj#1921.

The _version and _versionOptionName properties are initialized to undefined in the constructor. The reasoning is - to make them visible when the Command instance is logged into the console before .version() is called, and not only appear after suddenly - to not break anything if the proxy use for consistent option value handling in the legacy setOptionValueWithSource mode suggested in tj#1919 (and previously in tj#1921) is adopted

Storing the Option instance of the version option instead of just its .attributeName() (_versionOptionName) is meaningful for cases when other properties of the instance need to be accessed (not currently the case, but could be in the future).

Analogous to 3c94dfd in tj#1933 for version option. Storing the Option instance of the help option instead of just its flags (_helpShortFlag, _helpLongFlag) is meaningful for cases when other properties of the instance need to be accessed (not currently the case, but could be in the future).

shadowspawn · 2023-08-04T01:09:30Z

A quick reaction. I don't think being able to call .version() twice is important. On a related note in terms of early detection by author that it does not work that way, there is the PR open to add an error for overlapping option flags.

aweebit · 2023-08-04T01:43:34Z

I don't think being able to call .version() twice is important.

The last example in the demo code shows it could be important.

Being able to call .version() twice with new flags and description is not that important, but added for completeness and to provide meaning to calls that currently don't have any well-defined meaning at all.

On a related note in terms of early detection by author that it does not work that way, there is the PR open to add an error for overlapping option flags.

If you mean #1923, I updated it a few hours ago so that version option flag conflicts are handled, too. But it solves a different problem.

shadowspawn · 2023-08-10T04:22:10Z

lib/option.js

-    this.optional = flags.includes('['); // A value is optional when the option is specified.
-    // variadic test ignores <value,...> et al which might be used to describe custom splitting of single argument
-    this.variadic = /\w\.\.\.[>\]]$/.test(flags); // The option can take multiple values.
+    this.setFlagsAndDescription(flags, description);


I was quite surprised this refactor did not break the typescript-checkJS run-script. At one time I am pretty sure we were getting errors for JavaScript properties which were not declared/set in the constructor.

TypeScript may have removed that check as too strict for real-world JavaScript.

microsoft/TypeScript#22896

shadowspawn · 2023-08-10T04:46:12Z

I do not currently want this functionality or refactor of Option, thanks. The .version() support is a simple convenience to add a version option with an event handler to display the version.

I see one commit was initialising this._version and this._versionOption in the constructor. That is consistent with our normal pattern if you want to suggest that separately.

(.version() started out looking like a getter/setter for a "version" property, including returning it in .opts(). Now I don't think it should be acting as a getter and I don't want to store _version or _versionOption, so I should declare that behaviour deprecated!)

shadowspawn · 2023-08-10T04:48:33Z

I see one commit was initialising this._version and this._versionOption in the constructor. That is consistent with our normal pattern if you want to suggest that separately.

Actually, no. I'll leave it undeclared and deprecate it as moving in the preferred direction.

[edit] Opened #1954

aweebit added 4 commits August 3, 2023 13:04

Fix version() parameter type

e8bea4a

Borrowed from a3f0e28 that was supposed to land in the now-closed tj#1921.

Allow version number and option's flags and description updates

2f3c4c3

This was referenced Aug 3, 2023

Use createOption() in helpOption() to support custom option flag extraction (+ various improvements) #1929

Closed

Warn about obscured help option #1931

Closed

shadowspawn added the on hold label Aug 8, 2023

shadowspawn reviewed Aug 10, 2023

View reviewed changes

shadowspawn removed the on hold label Aug 10, 2023

shadowspawn closed this Aug 10, 2023

This was referenced Aug 12, 2023

Deprecate version retrieval #1954

Closed

Clean up state from previous parse call when calling parse() / parseAsync() #1919

Closed

Pull request catalog by @aweebit #1964

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Allow dynamically updating version number and version option flags and description#1933

Allow dynamically updating version number and version option flags and description#1933
aweebit wants to merge 4 commits intotj:developfrom
aweebit:feature/version

aweebit commented Aug 3, 2023 •

edited

Loading

Uh oh!

shadowspawn commented Aug 4, 2023

Uh oh!

aweebit commented Aug 4, 2023

Uh oh!

shadowspawn Aug 10, 2023

Uh oh!

shadowspawn commented Aug 10, 2023

Uh oh!

shadowspawn commented Aug 10, 2023 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

aweebit commented Aug 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

ChangeLog

Added

Fixed

Peer PRs

…solving similar problems

Uh oh!

shadowspawn commented Aug 4, 2023

Uh oh!

aweebit commented Aug 4, 2023

Uh oh!

shadowspawn Aug 10, 2023

Choose a reason for hiding this comment

Uh oh!

shadowspawn commented Aug 10, 2023

Uh oh!

shadowspawn commented Aug 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aweebit commented Aug 3, 2023 •

edited

Loading

shadowspawn commented Aug 10, 2023 •

edited

Loading