diff --git a/index.js b/index.js index 3efcf38e2..ce38da26b 100644 --- a/index.js +++ b/index.js @@ -324,6 +324,7 @@ Command.prototype.action = function(fn) { // Output help if necessary outputHelpIfNecessary(self, parsed.unknown); + self._checkForMissingMandatoryOptions(); // If there are still any unknown options, then we simply // die, unless someone asked for help, in which case we give it @@ -563,10 +564,17 @@ Command.prototype.parse = function(argv) { if (args[0] === 'help' && args.length === 1) this.help(); + // Note for future: we could return early if we found an action handler in parseArgs, as none of following code needed? + // --help if (args[0] === 'help') { args[0] = args[1]; args[1] = this._helpLongFlag; + } else { + // If calling through to executable subcommand we could check for help flags before failing, + // but a somewhat unlikely case since program options not passed to executable subcommands. + // Wait for reports to see if check needed and what usage pattern is. + this._checkForMissingMandatoryOptions(); } // executable sub-commands @@ -832,12 +840,14 @@ Command.prototype.optionFor = function(arg) { */ Command.prototype._checkForMissingMandatoryOptions = function() { - const self = this; - this.options.forEach((anOption) => { - if (anOption.mandatory && (self[anOption.attributeName()] === undefined)) { - self.missingMandatoryOptionValue(anOption); - } - }); + // Walk up hierarchy so can call from action handler after checking for displaying help. + for (var cmd = this; cmd; cmd = cmd.parent) { + cmd.options.forEach((anOption) => { + if (anOption.mandatory && (cmd[anOption.attributeName()] === undefined)) { + cmd.missingMandatoryOptionValue(anOption); + } + }); + } }; /** @@ -916,8 +926,6 @@ Command.prototype.parseOptions = function(argv) { args.push(arg); } - this._checkForMissingMandatoryOptions(); - return { args: args, unknown: unknownOptions }; }; diff --git a/tests/options.mandatory.test.js b/tests/options.mandatory.test.js index 0fe633592..87b725473 100644 --- a/tests/options.mandatory.test.js +++ b/tests/options.mandatory.test.js @@ -1,6 +1,7 @@ const commander = require('../'); // Assuming mandatory options behave as normal options apart from the mandatory aspect, not retesting all behaviour. +// Likewise, not redoing all tests on subcommand after testing on program. describe('required program option with mandatory value specified', () => { test('when program has required value specified then value as specified', () => { @@ -224,3 +225,54 @@ describe('required command option with mandatory value not specified', () => { }).not.toThrow(); }); }); + +describe('missing mandatory option but help requested', () => { + // Optional. Use internal knowledge to suppress output to keep test output clean. + let writeSpy; + + beforeAll(() => { + writeSpy = jest.spyOn(process.stdout, 'write').mockImplementation(() => { }); + }); + + afterEach(() => { + writeSpy.mockClear(); + }); + + afterAll(() => { + writeSpy.mockRestore(); + }); + + test('when program has required option not specified and --help then help', () => { + const program = new commander.Command(); + program + .exitOverride() + .requiredOption('--cheese ', 'cheese type'); + + let caughtErr; + try { + program.parse(['node', 'test', '--help']); + } catch (err) { + caughtErr = err; + } + + expect(caughtErr.code).toEqual('commander.helpDisplayed'); + }); + + test('when program has required option not specified and subcommand --help then help', () => { + const program = new commander.Command(); + program + .exitOverride() + .requiredOption('--cheese ', 'cheese type') + .command('sub') + .action(() => {}); + + let caughtErr; + try { + program.parse(['node', 'test', 'sub', '--help']); + } catch (err) { + caughtErr = err; + } + + expect(caughtErr.code).toEqual('commander.helpDisplayed'); + }); +});