From 49423a288b6561190461bf91231a18085e60dad4 Mon Sep 17 00:00:00 2001 From: John Gee Date: Fri, 27 Dec 2024 14:51:31 +1300 Subject: [PATCH] Add save/restore state to allow multiple calls to parse (#2299) --- Readme.md | 2 - lib/command.js | 55 +++++++++ tests/command.parse.test.js | 218 ++++++++++++++++++++++++++++++++++++ typings/index.d.ts | 18 +++ typings/index.test-d.ts | 4 + 5 files changed, 295 insertions(+), 2 deletions(-) diff --git a/Readme.md b/Readme.md index 91c1d2761..6cf55910f 100644 --- a/Readme.md +++ b/Readme.md @@ -959,8 +959,6 @@ program.parse(['--port', '80'], { from: 'user' }); // just user supplied argumen Use parseAsync instead of parse if any of your action handlers are async. -If you want to parse multiple times, create a new program each time. Calling parse does not clear out any previous state. - ### Parsing Configuration If the default parsing does not suit your needs, there are some behaviours to support other usage patterns. diff --git a/lib/command.js b/lib/command.js index e27d2e53f..858e13e29 100644 --- a/lib/command.js +++ b/lib/command.js @@ -55,6 +55,7 @@ class Command extends EventEmitter { /** @type {(boolean | string)} */ this._showHelpAfterError = false; this._showSuggestionAfterError = true; + this._savedState = null; // used in save/restoreStateBeforeParse // see configureOutput() for docs this._outputConfiguration = { @@ -1069,6 +1070,7 @@ Expecting one of '${allowedValues.join("', '")}'`); */ parse(argv, parseOptions) { + this._prepareForParse(); const userArgs = this._prepareUserArgs(argv, parseOptions); this._parseCommand([], userArgs); @@ -1097,12 +1099,62 @@ Expecting one of '${allowedValues.join("', '")}'`); */ async parseAsync(argv, parseOptions) { + this._prepareForParse(); const userArgs = this._prepareUserArgs(argv, parseOptions); await this._parseCommand([], userArgs); return this; } + _prepareForParse() { + if (this._savedState === null) { + this.saveStateBeforeParse(); + } else { + this.restoreStateBeforeParse(); + } + } + + /** + * Called the first time parse is called to save state and allow a restore before subsequent calls to parse. + * Not usually called directly, but available for subclasses to save their custom state. + * + * This is called in a lazy way. Only commands used in parsing chain will have state saved. + */ + saveStateBeforeParse() { + this._savedState = { + // name is stable if supplied by author, but may be unspecified for root command and deduced during parsing + _name: this._name, + // option values before parse have default values (including false for negated options) + // shallow clones + _optionValues: { ...this._optionValues }, + _optionValueSources: { ...this._optionValueSources }, + }; + } + + /** + * Restore state before parse for calls after the first. + * Not usually called directly, but available for subclasses to save their custom state. + * + * This is called in a lazy way. Only commands used in parsing chain will have state restored. + */ + restoreStateBeforeParse() { + if (this._storeOptionsAsProperties) + throw new Error(`Can not call parse again when storeOptionsAsProperties is true. +- either make a new Command for each call to parse, or stop storing options as properties`); + + // clear state from _prepareUserArgs + this._name = this._savedState._name; + this._scriptPath = null; + this.rawArgs = []; + // clear state from setOptionValueWithSource + this._optionValues = { ...this._savedState._optionValues }; + this._optionValueSources = { ...this._savedState._optionValueSources }; + // clear state from _parseCommand + this.args = []; + // clear state from _processArguments + this.processedArgs = []; + } + /** * Throw if expected executable is missing. Add lots of help for author. * @@ -1283,6 +1335,7 @@ Expecting one of '${allowedValues.join("', '")}'`); const subCommand = this._findCommand(commandName); if (!subCommand) this.help({ error: true }); + subCommand._prepareForParse(); let promiseChain; promiseChain = this._chainOrCallSubCommandHook( promiseChain, @@ -1660,6 +1713,8 @@ Expecting one of '${allowedValues.join("', '")}'`); * Parse options from `argv` removing known options, * and return argv split into operands and unknown arguments. * + * Side effects: modifies command by storing options. Does not reset state if called again. + * * Examples: * * argv => operands, unknown diff --git a/tests/command.parse.test.js b/tests/command.parse.test.js index d0fbb1455..5bbd9c3fd 100644 --- a/tests/command.parse.test.js +++ b/tests/command.parse.test.js @@ -199,3 +199,221 @@ describe('parseAsync parameter is treated as readonly, per TypeScript declaratio expect(program.rawArgs).toEqual(oldRawArgs); }); }); + +describe('.parse() called multiple times', () => { + test('when use boolean options then option values reset', () => { + const program = new commander.Command().option('--black').option('--white'); + + program.parse(['--black'], { from: 'user' }); + expect(program.opts()).toEqual({ black: true }); + + program.parse(['--white'], { from: 'user' }); + expect(program.opts()).toEqual({ white: true }); + }); + + test('when use options with option-argument then option values and sources reset', () => { + const program = new commander.Command() + .option('-f, --foo ') + .option('-b, --bar '); + + program.parse(['--foo', 'FOO'], { from: 'user' }); + expect(program.opts()).toEqual({ foo: 'FOO' }); + expect(program.getOptionValueSource('foo')).toEqual('cli'); + expect(program.getOptionValueSource('bar')).toBeUndefined(); + + program.parse(['--bar', 'BAR'], { from: 'user' }); + expect(program.opts()).toEqual({ bar: 'BAR' }); + expect(program.getOptionValueSource('foo')).toBeUndefined(); + expect(program.getOptionValueSource('bar')).toEqual('cli'); + }); + + test('when use options with option-argument and default then option values and sources reset', () => { + const program = new commander.Command() + .option('-f, --foo ', 'description', 'default-FOO') + .option('-b, --bar ', 'description', 'default-BAR'); + + program.parse(['--foo', 'FOO'], { from: 'user' }); + expect(program.opts()).toEqual({ foo: 'FOO', bar: 'default-BAR' }); + expect(program.getOptionValueSource('foo')).toEqual('cli'); + expect(program.getOptionValueSource('bar')).toEqual('default'); + + program.parse(['--bar', 'BAR'], { from: 'user' }); + expect(program.opts()).toEqual({ foo: 'default-FOO', bar: 'BAR' }); + expect(program.getOptionValueSource('foo')).toEqual('default'); + expect(program.getOptionValueSource('bar')).toEqual('cli'); + }); + + test('when use negated options then option values reset', () => { + const program = new commander.Command() + .option('--no-foo') + .option('--no-bar'); + + program.parse(['--no-foo'], { from: 'user' }); + expect(program.opts()).toEqual({ foo: false, bar: true }); + + program.parse(['--no-bar'], { from: 'user' }); + expect(program.opts()).toEqual({ foo: true, bar: false }); + }); + + test('when use variadic option then option values reset', () => { + const program = new commander.Command().option('--var '); + + program.parse(['--var', 'a', 'b'], { from: 'user' }); + expect(program.opts()).toEqual({ var: ['a', 'b'] }); + + program.parse(['--var', 'c'], { from: 'user' }); + expect(program.opts()).toEqual({ var: ['c'] }); + }); + + test('when use collect example then option value resets', () => { + function collect(value, previous) { + return previous.concat([value]); + } + const program = new commander.Command(); + program.option('-c, --collect ', 'repeatable value', collect, []); + + program.parse(['-c', 'a', '-c', 'b'], { from: 'user' }); + expect(program.opts()).toEqual({ collect: ['a', 'b'] }); + + program.parse(['-c', 'c'], { from: 'user' }); + expect(program.opts()).toEqual({ collect: ['c'] }); + }); + + test('when use increaseVerbosity example then option value resets', () => { + function increaseVerbosity(dummyValue, previous) { + return previous + 1; + } + const program = new commander.Command(); + program.option( + '-v, --verbose', + 'verbosity that can be increased', + increaseVerbosity, + 0, + ); + + program.parse(['-vvv'], { from: 'user' }); + expect(program.opts()).toEqual({ verbose: 3 }); + program.parse(['-vv'], { from: 'user' }); + + expect(program.opts()).toEqual({ verbose: 2 }); + program.parse([], { from: 'user' }); + expect(program.opts()).toEqual({ verbose: 0 }); + }); + + test('when use parse and parseAsync then option values reset', async () => { + const program = new commander.Command().option('--black').option('--white'); + + program.parse(['--black'], { from: 'user' }); + expect(program.opts()).toEqual({ black: true }); + await program.parseAsync(['--white'], { from: 'user' }); + expect(program.opts()).toEqual({ white: true }); + }); + + test('when call subcommand then option values reset (program and subcommand)', () => { + const program = new commander.Command().option('--black').option('--white'); + const subcommand = program.command('sub').option('--red').option('--green'); + + program.parse(['--black', 'sub', '--red'], { from: 'user' }); + expect(subcommand.optsWithGlobals()).toEqual({ black: true, red: true }); + + program.parse(['--white', 'sub', '--green'], { from: 'user' }); + expect(subcommand.optsWithGlobals()).toEqual({ white: true, green: true }); + }); + + test('when call different subcommand then no reset because lazy', () => { + // This is not a required behaviour, but is the intended behaviour. + const program = new commander.Command(); + const sub1 = program.command('sub1').option('--red'); + const sub2 = program.command('sub2').option('--green'); + + program.parse(['sub1', '--red'], { from: 'user' }); + expect(sub1.opts()).toEqual({ red: true }); + expect(sub2.opts()).toEqual({}); + + program.parse(['sub2', '--green'], { from: 'user' }); + expect(sub1.opts()).toEqual({ red: true }); + expect(sub2.opts()).toEqual({ green: true }); + }); + + test('when parse with different implied program name then name changes', () => { + const program = new commander.Command(); + + program.parse(['node', 'script1.js']); + expect(program.name()).toEqual('script1'); + + program.parse(['electron', 'script2.js']); + expect(program.name()).toEqual('script2'); + }); + + test('when parse with different arguments then args change', () => { + // weak test, would work without store/reset! + const program = new commander.Command() + .argument('') + .argument('[second]'); + + program.parse(['one', 'two'], { from: 'user' }); + expect(program.args).toEqual(['one', 'two']); + + program.parse(['alpha'], { from: 'user' }); + expect(program.args).toEqual(['alpha']); + }); + + test('when parse with different arguments then rawArgs change', () => { + // weak test, would work without store/reset! + const program = new commander.Command() + .argument('') + .option('--white') + .option('--black'); + + program.parse(['--white', 'one'], { from: 'user' }); + expect(program.rawArgs).toEqual(['--white', 'one']); + + program.parse(['--black', 'two'], { from: 'user' }); + expect(program.rawArgs).toEqual(['--black', 'two']); + }); + + test('when parse with different arguments then processedArgs change', () => { + // weak test, would work without store/reset! + const program = new commander.Command().argument( + '', + 'first arg', + parseFloat, + ); + + program.parse([123], { from: 'user' }); + expect(program.processedArgs).toEqual([123]); + + program.parse([456], { from: 'user' }); + expect(program.processedArgs).toEqual([456]); + }); + + test('when parse subcommand then reset state before preSubcommand hook called', () => { + let hookCalled = false; + const program = new commander.Command().hook( + 'preSubcommand', + (thisCommand, subcommand) => { + hookCalled = true; + expect(subcommand.opts()).toEqual({}); + }, + ); + const subcommand = program.command('sub').option('--red').option('--green'); + + hookCalled = false; + program.parse(['sub', '--red'], { from: 'user' }); + expect(hookCalled).toBe(true); + expect(subcommand.opts()).toEqual({ red: true }); + + hookCalled = false; + program.parse(['sub', '--green'], { from: 'user' }); + expect(hookCalled).toBe(true); + expect(subcommand.opts()).toEqual({ green: true }); + }); + + test('when using storeOptionsAsProperties then throw on second parse', () => { + const program = new commander.Command().storeOptionsAsProperties(); + program.parse(); + expect(() => { + program.parse(); + }).toThrow(); + }); +}); diff --git a/typings/index.d.ts b/typings/index.d.ts index 25518b151..b31fad7c5 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -821,10 +821,28 @@ export class Command { parseOptions?: ParseOptions, ): Promise; + /** + * Called the first time parse is called to save state and allow a restore before subsequent calls to parse. + * Not usually called directly, but available for subclasses to save their custom state. + * + * This is called in a lazy way. Only commands used in parsing chain will have state saved. + */ + saveStateBeforeParse(): void; + + /** + * Restore state before parse for calls after the first. + * Not usually called directly, but available for subclasses to save their custom state. + * + * This is called in a lazy way. Only commands used in parsing chain will have state restored. + */ + restoreStateBeforeParse(): void; + /** * Parse options from `argv` removing known options, * and return argv split into operands and unknown arguments. * + * Side effects: modifies command by storing options. Does not reset state if called again. + * * argv => operands, unknown * --known kkk op => [op], [] * op --known kkk => [op], [] diff --git a/typings/index.test-d.ts b/typings/index.test-d.ts index 123e696f2..9c03cf2cd 100644 --- a/typings/index.test-d.ts +++ b/typings/index.test-d.ts @@ -370,6 +370,10 @@ expectType<{ operands: string[]; unknown: string[] }>( program.parseOptions(['node', 'script.js', 'hello']), ); +// save/restore state +expectType(program.saveStateBeforeParse()); +expectType(program.restoreStateBeforeParse()); + // opts const opts = program.opts(); expectType(opts);