Skip to content

Commit

Permalink
feat: handle negated option errors
Browse files Browse the repository at this point in the history
  • Loading branch information
erezrokah committed Feb 20, 2022
1 parent 91ab6b1 commit 39eba86
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 1 deletion.
13 changes: 12 additions & 1 deletion lib/command.js
Expand Up @@ -1598,6 +1598,14 @@ Expecting one of '${allowedValues.join("', '")}'`);
* @api private
*/
_conflictingOption(option, conflictingOption) {
const getNegatedOption = (option) => {
const optionKey = option.attributeName();
const negatedLongFlag = option.long && `--no-${optionKey}`;
const negatedOption = negatedLongFlag && this._findOption(negatedLongFlag);

return negatedOption;
};

const getErrorMessage = (option) => {
const optionKey = option.attributeName();
const source = this.getOptionValueSource(optionKey);
Expand All @@ -1607,7 +1615,10 @@ Expecting one of '${allowedValues.join("', '")}'`);
return `option '${option.flags}'`;
};

const message = `error: ${getErrorMessage(option)} cannot be used with ${getErrorMessage(conflictingOption)}`;
const originOption = getNegatedOption(option) || option;

This comment has been minimized.

Copy link
@shadowspawn

shadowspawn Feb 20, 2022

Collaborator

The error message is not exactly the same as in your example for simplicity of implementation.
Please let me know what you think.

This looks like it is just prioritising the negated option over the (positive) option, which I don't think is a step forward in clarity?

program
  .addOption(new Option('--red'))
  .addOption(new Option('--dual').conflicts('red'))
  .addOption(new Option('--no-dual'))
% node index.js --red --dual
error: option '--no-dual' cannot be used with option '--red'
% node index.js --red --no-dual
error: option '--no-dual' cannot be used with option '--red'
const originConflictingOption = getNegatedOption(conflictingOption) || conflictingOption;

const message = `error: ${getErrorMessage(originOption)} cannot be used with ${getErrorMessage(originConflictingOption)}`;
this.error(message, { code: 'commander.conflictingOption' });
}

Expand Down
31 changes: 31 additions & 0 deletions tests/command.conflicts.test.js
Expand Up @@ -24,6 +24,7 @@ describe('command with conflicting options', () => {
beforeEach(() => {
delete process.env.SILENT;
delete process.env.JSON;
delete process.env.NO_COLOR;
});

test('should call action if there are no explicit conflicting options set', () => {
Expand Down Expand Up @@ -92,4 +93,34 @@ describe('command with conflicting options', () => {
expect(actionMock).toHaveBeenCalledTimes(1);
expect(actionMock).toHaveBeenCalledWith({ debug: true, silent: true }, expect.any(Object));
});

test('should report conflict on negated option flag', () => {
const { program } = makeProgram();

program
.command('bar')
.addOption(new commander.Option('--red').conflicts(['color']))
.addOption(new commander.Option('--color'))
.addOption(new commander.Option('-N, --no-color'));

expect(() => {
program.parse('node test.js bar --red -N'.split(' '));
}).toThrow("error: option '--red' cannot be used with option '-N, --no-color'");
});

test('should report conflict on negated option env variable', () => {
const { program } = makeProgram();

process.env.NO_COLOR = true;

program
.command('bar')
.addOption(new commander.Option('--red').conflicts(['color']))
.addOption(new commander.Option('--color'))
.addOption(new commander.Option('-N, --no-color').env('NO_COLOR'));

expect(() => {
program.parse('node test.js bar --red'.split(' '));
}).toThrow("error: option '--red' cannot be used with environment variable 'NO_COLOR'");
});
});

0 comments on commit 39eba86

Please sign in to comment.