Skip to content

Commit

Permalink
Refactor parseOptions() to a good library function
Browse files Browse the repository at this point in the history
Add a displayHelp property to the returned object indicating whether a
help flag had been found before encountering a subcommand, or before
giving up option processing in favor of subcommands due to passThroughOptions being enabled.

Before encountering a subcommand, the help option is now consumed in
exactly the same cases any other option would be consumed.
  • Loading branch information
aweebit committed Aug 4, 2023
1 parent f511028 commit 87ed7ea
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 49 deletions.
41 changes: 23 additions & 18 deletions lib/command.js
Original file line number Diff line number Diff line change
Expand Up @@ -1257,7 +1257,14 @@ Expecting one of '${allowedValues.join("', '")}'`);
*/

_parseCommand(operands, unknown) {
const parsed = this.parseOptions(unknown);
const { displayHelp, ...parsed } = this.parseOptions(unknown);
if (displayHelp) {
// Could simply call .help() if not for the legacy 'commander.helpDisplayed' error code.
this.outputHelp();
// (Do not have all displayed text available so only passing placeholder.)
this._exit(0, 'commander.helpDisplayed', '(outputHelp)');
}

this._parseOptionsEnv(); // after cli, so parseArg not called on both cli and env
this._parseOptionsImplied();
operands = operands.concat(parsed.operands);
Expand Down Expand Up @@ -1417,23 +1424,25 @@ Expecting one of '${allowedValues.join("', '")}'`);

/**
* Parse options from `argv` removing known options,
* and return argv split into operands and unknown arguments.
*
* Examples:
* and return argv split into operands and unknown arguments,
* as well as a boolean indicating whether a help flag had been found before encountering a subcommand.
*
* argv => operands, unknown
* --known kkk op => [op], []
* op --known kkk => [op], []
* sub --unknown uuu op => [sub], [--unknown uuu op]
* sub -- --unknown uuu op => [sub --unknown uuu op], []
* argv => operands, unknown, displayHelp
* --known kkk op => [op], [], false
* op --known kkk => [op], [], false
* sub --unknown uuu op => [sub], [--unknown uuu op], false
* sub -- --unknown uuu op => [sub --unknown uuu op], [], false
* --help => [], [], true
*
* @param {String[]} argv
* @return {{operands: String[], unknown: String[]}}
* @return {{operands: String[], unknown: String[], displayHelp: boolean}}
*/

parseOptions(argv) {
const operands = []; // operands, not options or values
const unknown = []; // first unknown option and remaining unknown args
let displayHelp = false; // whether a help flag had been found before encountering a subcommand

let dest = operands;
const args = argv.slice();

Expand Down Expand Up @@ -1465,13 +1474,9 @@ Expecting one of '${allowedValues.join("', '")}'`);
// Options added via .option(), .addOption() and .version() have precedence over help option.
const option = this._findOption(arg) ?? (isHelpOption && this._helpOption);
if (option === this._helpOption) {
// Help option is always positional, skip when encountered after subcommand.
if (!subcommandEncountered) {
// Could simply call .help() if not for the legacy 'commander.helpDisplayed' error code.
this.outputHelp();
// (Do not have all displayed text available so only passing placeholder.)
this._exit(0, 'commander.helpDisplayed', '(outputHelp)');
}
// Help option is always positional, consider unknown when encountered after subcommand.
displayHelp = !subcommandEncountered;
if (displayHelp) continue;
} else if (option) {
// recognised option, call listener to assign value with possible custom processing
if (option.required) {
Expand Down Expand Up @@ -1567,7 +1572,7 @@ Expecting one of '${allowedValues.join("', '")}'`);
dest.push(arg);
}

return { operands, unknown };
return { operands, unknown, displayHelp };
}

/**
Expand Down
48 changes: 24 additions & 24 deletions tests/command.parseOptions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,97 +61,97 @@ describe('parseOptions', () => {
test('when empty args then empty results', () => {
const program = createProgram();
const result = program.parseOptions([]);
expect(result).toEqual({ operands: [], unknown: [] });
expect(result).toEqual({ operands: [], unknown: [], displayHelp: false });
});

test('when only operands then results has all operands', () => {
const program = createProgram();
const result = program.parseOptions('one two three'.split(' '));
expect(result).toEqual({ operands: ['one', 'two', 'three'], unknown: [] });
expect(result).toEqual({ operands: ['one', 'two', 'three'], unknown: [], displayHelp: false });
});

test('when subcommand and operand then results has subcommand and operand', () => {
const program = createProgram();
const result = program.parseOptions('sub one'.split(' '));
expect(result).toEqual({ operands: ['sub', 'one'], unknown: [] });
expect(result).toEqual({ operands: ['sub', 'one'], unknown: [], displayHelp: false });
});

test('when program has flag then option removed', () => {
const program = createProgram();
const result = program.parseOptions('--global-flag'.split(' '));
expect(result).toEqual({ operands: [], unknown: [] });
expect(result).toEqual({ operands: [], unknown: [], displayHelp: false });
});

test('when program has option with value then option removed', () => {
const program = createProgram();
const result = program.parseOptions('--global-value foo'.split(' '));
expect(result).toEqual({ operands: [], unknown: [] });
expect(result).toEqual({ operands: [], unknown: [], displayHelp: false });
});

test('when program has flag before operand then option removed', () => {
const program = createProgram();
const result = program.parseOptions('--global-flag arg'.split(' '));
expect(result).toEqual({ operands: ['arg'], unknown: [] });
expect(result).toEqual({ operands: ['arg'], unknown: [], displayHelp: false });
});

test('when program has flag after operand then option removed', () => {
const program = createProgram();
const result = program.parseOptions('arg --global-flag'.split(' '));
expect(result).toEqual({ operands: ['arg'], unknown: [] });
expect(result).toEqual({ operands: ['arg'], unknown: [], displayHelp: false });
});

test('when program has flag after subcommand then option removed', () => {
const program = createProgram();
const result = program.parseOptions('sub --global-flag'.split(' '));
expect(result).toEqual({ operands: ['sub'], unknown: [] });
expect(result).toEqual({ operands: ['sub'], unknown: [], displayHelp: false });
});

test('when program has unknown option then option returned in unknown', () => {
const program = createProgram();
const result = program.parseOptions('--unknown'.split(' '));
expect(result).toEqual({ operands: [], unknown: ['--unknown'] });
expect(result).toEqual({ operands: [], unknown: ['--unknown'], displayHelp: false });
});

test('when program has unknown option before operands then all unknown in same order', () => {
const program = createProgram();
const result = program.parseOptions('--unknown arg'.split(' '));
expect(result).toEqual({ operands: [], unknown: ['--unknown', 'arg'] });
expect(result).toEqual({ operands: [], unknown: ['--unknown', 'arg'], displayHelp: false });
});

test('when program has unknown option after operand then option returned in unknown', () => {
const program = createProgram();
const result = program.parseOptions('arg --unknown'.split(' '));
expect(result).toEqual({ operands: ['arg'], unknown: ['--unknown'] });
expect(result).toEqual({ operands: ['arg'], unknown: ['--unknown'], displayHelp: false });
});

test('when program has flag after unknown option then flag removed', () => {
const program = createProgram();
const result = program.parseOptions('--unknown --global-flag'.split(' '));
expect(result).toEqual({ operands: [], unknown: ['--unknown'] });
expect(result).toEqual({ operands: [], unknown: ['--unknown'], displayHelp: false });
});

test('when subcommand has flag then flag returned as unknown', () => {
const program = createProgram();
const result = program.parseOptions('sub --sub-flag'.split(' '));
expect(result).toEqual({ operands: ['sub'], unknown: ['--sub-flag'] });
expect(result).toEqual({ operands: ['sub'], unknown: ['--sub-flag'], displayHelp: false });
});

test('when program has literal before known flag then option returned as operand', () => {
const program = createProgram();
const result = program.parseOptions('-- --global-flag'.split(' '));
expect(result).toEqual({ operands: ['--global-flag'], unknown: [] });
expect(result).toEqual({ operands: ['--global-flag'], unknown: [], displayHelp: false });
});

test('when program has literal before unknown option then option returned as operand', () => {
const program = createProgram();
const result = program.parseOptions('-- --unknown uuu'.split(' '));
expect(result).toEqual({ operands: ['--unknown', 'uuu'], unknown: [] });
expect(result).toEqual({ operands: ['--unknown', 'uuu'], unknown: [], displayHelp: false });
});

test('when program has literal after unknown option then literal preserved too', () => {
const program = createProgram();
const result = program.parseOptions('--unknown1 -- --unknown2'.split(' '));
expect(result).toEqual({ operands: [], unknown: ['--unknown1', '--', '--unknown2'] });
expect(result).toEqual({ operands: [], unknown: ['--unknown1', '--', '--unknown2'], displayHelp: false });
});
});

Expand Down Expand Up @@ -236,56 +236,56 @@ describe('Utility Conventions', () => {
test('when program has combo known boolean short flags then arg removed', () => {
const program = createProgram();
const result = program.parseOptions(['-ab']);
expect(result).toEqual({ operands: [], unknown: [] });
expect(result).toEqual({ operands: [], unknown: [], displayHelp: false });
expect(program.opts()).toEqual({ aaa: true, bbb: true });
});

test('when program has combo unknown short flags then arg preserved', () => {
const program = createProgram();
const result = program.parseOptions(['-pq']);
expect(result).toEqual({ operands: [], unknown: ['-pq'] });
expect(result).toEqual({ operands: [], unknown: ['-pq'], displayHelp: false });
expect(program.opts()).toEqual({ });
});

test('when program has combo known short option and required value then arg removed', () => {
const program = createProgram();
const result = program.parseOptions(['-cvalue']);
expect(result).toEqual({ operands: [], unknown: [] });
expect(result).toEqual({ operands: [], unknown: [], displayHelp: false });
expect(program.opts()).toEqual({ ccc: 'value' });
});

test('when program has combo known short option and optional value then arg removed', () => {
const program = createProgram();
const result = program.parseOptions(['-dvalue']);
expect(result).toEqual({ operands: [], unknown: [] });
expect(result).toEqual({ operands: [], unknown: [], displayHelp: false });
expect(program.opts()).toEqual({ ddd: 'value' });
});

test('when program has known combo short boolean flags and required value then arg removed', () => {
const program = createProgram();
const result = program.parseOptions(['-abcvalue']);
expect(result).toEqual({ operands: [], unknown: [] });
expect(result).toEqual({ operands: [], unknown: [], displayHelp: false });
expect(program.opts()).toEqual({ aaa: true, bbb: true, ccc: 'value' });
});

test('when program has known combo short boolean flags and optional value then arg removed', () => {
const program = createProgram();
const result = program.parseOptions(['-abdvalue']);
expect(result).toEqual({ operands: [], unknown: [] });
expect(result).toEqual({ operands: [], unknown: [], displayHelp: false });
expect(program.opts()).toEqual({ aaa: true, bbb: true, ddd: 'value' });
});

test('when program has known long flag=value then arg removed', () => {
const program = createProgram();
const result = program.parseOptions(['--ccc=value']);
expect(result).toEqual({ operands: [], unknown: [] });
expect(result).toEqual({ operands: [], unknown: [], displayHelp: false });
expect(program.opts()).toEqual({ ccc: 'value' });
});

test('when program has unknown long flag=value then arg preserved', () => {
const program = createProgram();
const result = program.parseOptions(['--rrr=value']);
expect(result).toEqual({ operands: [], unknown: ['--rrr=value'] });
expect(result).toEqual({ operands: [], unknown: ['--rrr=value'], displayHelp: false });
expect(program.opts()).toEqual({ });
});

Expand Down
15 changes: 9 additions & 6 deletions typings/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -694,13 +694,15 @@ export class Command {

/**
* Parse options from `argv` removing known options,
* and return argv split into operands and unknown arguments.
* and return argv split into operands and unknown arguments,
* as well as a boolean indicating whether a help flag had been found before encountering a subcommand.
*
* argv => operands, unknown
* --known kkk op => [op], []
* op --known kkk => [op], []
* sub --unknown uuu op => [sub], [--unknown uuu op]
* sub -- --unknown uuu op => [sub --unknown uuu op], []
* argv => operands, unknown, displayHelp
* --known kkk op => [op], [], false
* op --known kkk => [op], [], false
* sub --unknown uuu op => [sub], [--unknown uuu op], false
* sub -- --unknown uuu op => [sub --unknown uuu op], [], false
* --help => [], [], true
*/
parseOptions(argv: string[]): ParseOptionsResult;

Expand Down Expand Up @@ -880,6 +882,7 @@ export interface ExecutableCommandOptions extends CommandOptions {
export interface ParseOptionsResult {
operands: string[];
unknown: string[];
displayHelp: boolean;
}

export function createCommand(name?: string): Command;
Expand Down
4 changes: 3 additions & 1 deletion typings/index.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,9 @@ expectType<Promise<commander.Command>>(program.parseAsync(['--option'], { from:
expectType<Promise<commander.Command>>(program.parseAsync(['node', 'script.js'] as const));

// parseOptions (and ParseOptionsResult)
expectType<{ operands: string[]; unknown: string[] }>(program.parseOptions(['node', 'script.js', 'hello']));
expectType<{
operands: string[]; unknown: string[]; displayHelp: boolean;
}>(program.parseOptions(['node', 'script.js', 'hello']));

// opts
const opts = program.opts();
Expand Down

0 comments on commit 87ed7ea

Please sign in to comment.