Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Display help despite missing required option #1091

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 16 additions & 8 deletions index.js
Expand Up @@ -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
Expand Down Expand Up @@ -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?

// <cmd> --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
Expand Down Expand Up @@ -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);
}
});
}
};

/**
Expand Down Expand Up @@ -916,8 +926,6 @@ Command.prototype.parseOptions = function(argv) {
args.push(arg);
}

this._checkForMissingMandatoryOptions();

return { args: args, unknown: unknownOptions };
};

Expand Down
52 changes: 52 additions & 0 deletions 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', () => {
Expand Down Expand Up @@ -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 <type>', '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 <type>', 'cheese type')
.command('sub')
.action(() => {});

let caughtErr;
try {
program.parse(['node', 'test', 'sub', '--help']);
} catch (err) {
caughtErr = err;
}

expect(caughtErr.code).toEqual('commander.helpDisplayed');
});
});