From 5e355311bf9bf976342a8a5799a8e1a30c1e7466 Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Mon, 7 Aug 2023 03:26:00 +0300 Subject: [PATCH 1/5] Correctly handle allowUnknownOption in parseOptions() Partially borrowed from #1934 --- lib/command.js | 59 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 19 deletions(-) diff --git a/lib/command.js b/lib/command.js index 590a271dd..4b8486512 100644 --- a/lib/command.js +++ b/lib/command.js @@ -1445,6 +1445,8 @@ Expecting one of '${allowedValues.join("', '")}'`); // parse options let activeVariadicOption = null; + let onlyKnownOptionsSoFar = true; + let reprocessedBySubcommand = false; while (args.length) { const arg = args.shift(); @@ -1510,31 +1512,50 @@ Expecting one of '${allowedValues.join("', '")}'`); } } - // Not a recognised option by this command. - // Might be a command-argument, or subcommand option, or unknown option, or help command or option. - - // An unknown option means further arguments also classified as unknown so can be reprocessed by subcommands. - if (maybeOption(arg)) { - dest = unknown; - } - - // If using positionalOptions, stop processing our options at subcommand. - if ((this._enablePositionalOptions || this._passThroughOptions) && operands.length === 0 && unknown.length === 0) { + // Not a recognised option by this command. Might be + // - a subcommand, + // - the help option encountered after a subcommand (considered unknown), + // - any other option unknown to this command, + // - or a command-argument. + + if (onlyKnownOptionsSoFar && !reprocessedBySubcommand) { + reprocessedBySubcommand = true; // reset to false later if not entering subcommand + const stopAtSubcommand = ( + this._enablePositionalOptions || this._passThroughOptions + ); if (this._findCommand(arg)) { - operands.push(arg); - if (args.length > 0) unknown.push(...args); - break; + if (stopAtSubcommand) { + operands.push(arg); + unknown.push(...args); + break; + } } else if (arg === this._helpCommandName && this._hasImplicitHelpCommand()) { - operands.push(arg); - if (args.length > 0) operands.push(...args); - break; + if (stopAtSubcommand) { + operands.push(arg, ...args); + break; + } } else if (this._defaultCommandName) { - unknown.push(arg); - if (args.length > 0) unknown.push(...args); - break; + if (stopAtSubcommand) { + unknown.push(arg, ...args); + break; + } + } else { + reprocessedBySubcommand = false; } } + onlyKnownOptionsSoFar = false; + + // Respect the fact that the working principle of allowUnknownOption is to treat unknown options as command-arguments. + const treatUnknownOptionAsCommandArgument = ( + this._allowUnknownOption && !reprocessedBySubcommand + ); + + // An unknown option means further arguments also classified as unknown so can be reprocessed by subcommands. + if (maybeOption(arg) && !treatUnknownOptionAsCommandArgument) { + dest = unknown; + } + // If using passThroughOptions, stop processing options at first command-argument. if (this._passThroughOptions) { dest.push(arg); From ab9b9309fda4026f3c07425e63b70816c6f2222f Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Mon, 7 Aug 2023 03:27:10 +0300 Subject: [PATCH 2/5] Add test for unknown option parsing when using allowUnknownOption --- tests/command.allowUnknownOption.test.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/command.allowUnknownOption.test.js b/tests/command.allowUnknownOption.test.js index 6bdd72eed..213dc3b75 100644 --- a/tests/command.allowUnknownOption.test.js +++ b/tests/command.allowUnknownOption.test.js @@ -92,4 +92,12 @@ describe('allowUnknownOption', () => { program.parse(['node', 'test', 'sub', '-m']); }).not.toThrow(); }); + + test('when specify unknown program option and allowUnknownOption then unknown option parsed as operand', () => { + const program = new commander.Command(); + program + .allowUnknownOption(); + const result = program.parseOptions(['-m']); + expect(result).toEqual({ operands: ['-m'], unknown: [] }); + }); }); From 83ca473218a1c216af2799d441dae003e85a51ca Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Mon, 7 Aug 2023 03:30:20 +0300 Subject: [PATCH 3/5] Remove incorrect unknown option test --- tests/command.helpCommand.test.js | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/tests/command.helpCommand.test.js b/tests/command.helpCommand.test.js index bbf9f4502..272ace4cf 100644 --- a/tests/command.helpCommand.test.js +++ b/tests/command.helpCommand.test.js @@ -21,22 +21,6 @@ describe('help command listed in helpInformation', () => { expect(helpInformation).toMatch(/help \[command\]/); }); - test('when program has subcommands and specify only unknown option then display help', () => { - const program = new commander.Command(); - program - .configureHelp({ formatHelp: () => '' }) - .exitOverride() - .allowUnknownOption() - .command('foo'); - let caughtErr; - try { - program.parse(['--unknown'], { from: 'user' }); - } catch (err) { - caughtErr = err; - } - expect(caughtErr.code).toEqual('commander.help'); - }); - test('when program has subcommands and suppress help command then no help command', () => { const program = new commander.Command(); program.addHelpCommand(false); From 3a0f30720e09ef24134c4580541814af8ec1b53d Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Mon, 7 Aug 2023 03:40:53 +0300 Subject: [PATCH 4/5] Remove comment detail only relevant in #1934 --- lib/command.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/command.js b/lib/command.js index 4b8486512..e9a397db3 100644 --- a/lib/command.js +++ b/lib/command.js @@ -1514,8 +1514,7 @@ Expecting one of '${allowedValues.join("', '")}'`); // Not a recognised option by this command. Might be // - a subcommand, - // - the help option encountered after a subcommand (considered unknown), - // - any other option unknown to this command, + // - an option unknown to this command, // - or a command-argument. if (onlyKnownOptionsSoFar && !reprocessedBySubcommand) { From ef6acf4258beb5a5559c885c13ced35d53a62190 Mon Sep 17 00:00:00 2001 From: Wee Bit Date: Mon, 7 Aug 2023 04:32:10 +0300 Subject: [PATCH 5/5] Refactor parseOptions() Export check to variable for clarity and potential future uses --- lib/command.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/command.js b/lib/command.js index e9a397db3..ffb19844d 100644 --- a/lib/command.js +++ b/lib/command.js @@ -1542,16 +1542,18 @@ Expecting one of '${allowedValues.join("', '")}'`); reprocessedBySubcommand = false; } } - onlyKnownOptionsSoFar = false; // Respect the fact that the working principle of allowUnknownOption is to treat unknown options as command-arguments. const treatUnknownOptionAsCommandArgument = ( this._allowUnknownOption && !reprocessedBySubcommand ); + const isUnknownOption = ( + maybeOption(arg) && !treatUnknownOptionAsCommandArgument + ); // An unknown option means further arguments also classified as unknown so can be reprocessed by subcommands. - if (maybeOption(arg) && !treatUnknownOptionAsCommandArgument) { + if (isUnknownOption) { dest = unknown; }