From d3279fadfe696378f5de708e236c5c9a265c2d31 Mon Sep 17 00:00:00 2001 From: John Gee Date: Fri, 25 Mar 2022 19:50:24 +1300 Subject: [PATCH 1/5] Rename test file --- tests/{command.conflicts.test.js => options.conflicts.test.js} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/{command.conflicts.test.js => options.conflicts.test.js} (100%) diff --git a/tests/command.conflicts.test.js b/tests/options.conflicts.test.js similarity index 100% rename from tests/command.conflicts.test.js rename to tests/options.conflicts.test.js From 43fb9aa0184a57cbdf2a9281d76ac05bfb3f054f Mon Sep 17 00:00:00 2001 From: John Gee Date: Fri, 25 Mar 2022 20:13:49 +1300 Subject: [PATCH 2/5] Add test for confilcts in parent command of called command --- tests/options.conflicts.test.js | 54 +++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/tests/options.conflicts.test.js b/tests/options.conflicts.test.js index 8e2ce9860..a43dfdd1c 100644 --- a/tests/options.conflicts.test.js +++ b/tests/options.conflicts.test.js @@ -1,3 +1,4 @@ +const path = require('path'); const commander = require('../'); describe('command with conflicting options', () => { @@ -253,4 +254,57 @@ describe('command with conflicting options', () => { program.parse('node test.js bar --a --b'.split(' ')); }).toThrow("error: option '--b' cannot be used with option '--a'"); }); + + test('when conflict on program calling action subcommand then throw conflict', () => { + const { program } = makeProgram(); + let exception; + + program + .addOption(new commander.Option('--black')) + .addOption(new commander.Option('--white').conflicts('black')); + + try { + program.parse('--white --black foo'.split(' '), { from: 'user' }); + } catch (err) { + exception = err; + } + expect(exception).not.toBeUndefined(); + expect(exception.code).toBe('commander.conflictingOption.'); + }); + + test('when conflict on program calling action subcommand with help then show help', () => { + const { program } = makeProgram(); + let exception; + + program + .addOption(new commander.Option('--black')) + .addOption(new commander.Option('--white').conflicts('black')); + + try { + program.parse('--white --black foo --help'.split(' '), { from: 'user' }); + } catch (err) { + exception = err; + } + expect(exception).not.toBeUndefined(); + expect(exception.code).toBe('commander.helpDisplayed'); + }); + + test('when conflict on program calling external subcommand then throw conflict', () => { + const { program } = makeProgram(); + let exception; + + program + .addOption(new commander.Option('--black')) + .addOption(new commander.Option('--white').conflicts('black')); + const pm = path.join(__dirname, './fixtures/pm'); + program.command('ext', 'external command', { executableFile: pm }); + + try { + program.parse('--white --black ext'.split(' '), { from: 'user' }); + } catch (err) { + exception = err; + } + expect(exception).not.toBeUndefined(); + expect(exception.code).toBe('commander.conflictingOption.'); + }); }); From b238e81347111ea654590fc2d858a013cff34fcd Mon Sep 17 00:00:00 2001 From: John Gee Date: Fri, 25 Mar 2022 20:24:57 +1300 Subject: [PATCH 3/5] Check for conflicts up hierarchy of commands --- lib/command.js | 20 +++++++++++++++++--- tests/options.conflicts.test.js | 4 ++-- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/lib/command.js b/lib/command.js index cd6614f0c..cb186de34 100644 --- a/lib/command.js +++ b/lib/command.js @@ -946,6 +946,7 @@ Expecting one of '${allowedValues.join("', '")}'`); // Not checking for help first. Unlikely to have mandatory and executable, and can't robustly test for help flags in external command. this._checkForMissingMandatoryOptions(); + this._checkForConflictingOptions(); // executableFile and executableDir might be full path, or just a name let executableFile = subcommand._executableFile || `${this._name}-${subcommand._name}`; @@ -1294,7 +1295,7 @@ Expecting one of '${allowedValues.join("', '")}'`); /** * Display an error message if a mandatory option does not have a value. - * Lazy calling after checking for help flags from leaf subcommand. + * Called after checking for help flags in leaf subcommand. * * @api private */ @@ -1311,11 +1312,11 @@ Expecting one of '${allowedValues.join("', '")}'`); } /** - * Display an error message if conflicting options are used together. + * Display an error message if conflicting options are used together in this. * * @api private */ - _checkForConflictingOptions() { + _checkForConflictingLocalOptions() { const definedNonDefaultOptions = this.options.filter( (option) => { const optionKey = option.attributeName(); @@ -1340,6 +1341,19 @@ Expecting one of '${allowedValues.join("', '")}'`); }); } + /** + * Display an error message if conflicting options are used together. + * Called after checking for help flags in leaf subcommand. + * + * @api private + */ + _checkForConflictingOptions(cmd) { + // Walk up hierarchy so can call in subcommand after checking for displaying help. + for (let cmd = this; cmd; cmd = cmd.parent) { + cmd._checkForConflictingLocalOptions(); + } + } + /** * Parse options from `argv` removing known options, * and return argv split into operands and unknown arguments. diff --git a/tests/options.conflicts.test.js b/tests/options.conflicts.test.js index a43dfdd1c..22d4c7688 100644 --- a/tests/options.conflicts.test.js +++ b/tests/options.conflicts.test.js @@ -269,7 +269,7 @@ describe('command with conflicting options', () => { exception = err; } expect(exception).not.toBeUndefined(); - expect(exception.code).toBe('commander.conflictingOption.'); + expect(exception.code).toBe('commander.conflictingOption'); }); test('when conflict on program calling action subcommand with help then show help', () => { @@ -305,6 +305,6 @@ describe('command with conflicting options', () => { exception = err; } expect(exception).not.toBeUndefined(); - expect(exception.code).toBe('commander.conflictingOption.'); + expect(exception.code).toBe('commander.conflictingOption'); }); }); From 54f09093ecbf0aaa75c798c4c8557018d2f15ca7 Mon Sep 17 00:00:00 2001 From: John Gee Date: Fri, 25 Mar 2022 20:36:07 +1300 Subject: [PATCH 4/5] Suppress help output from test --- tests/options.conflicts.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/options.conflicts.test.js b/tests/options.conflicts.test.js index 22d4c7688..72884b802 100644 --- a/tests/options.conflicts.test.js +++ b/tests/options.conflicts.test.js @@ -8,7 +8,8 @@ describe('command with conflicting options', () => { program .exitOverride() .configureOutput({ - writeErr: () => {} + writeErr: () => {}, + writeOut: () => {} }) .command('foo') .addOption(new commander.Option('-s, --silent', "Don't print anything").env('SILENT')) From 31d6f94e560d90d52a5ef3a1927f1e9893cc178e Mon Sep 17 00:00:00 2001 From: John Gee Date: Fri, 25 Mar 2022 21:02:33 +1300 Subject: [PATCH 5/5] Remove bogus parameter --- lib/command.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/command.js b/lib/command.js index cb186de34..1a1424c5e 100644 --- a/lib/command.js +++ b/lib/command.js @@ -1347,7 +1347,7 @@ Expecting one of '${allowedValues.join("', '")}'`); * * @api private */ - _checkForConflictingOptions(cmd) { + _checkForConflictingOptions() { // Walk up hierarchy so can call in subcommand after checking for displaying help. for (let cmd = this; cmd; cmd = cmd.parent) { cmd._checkForConflictingLocalOptions();