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

Support options with only a short flag #1256

Merged
merged 5 commits into from
May 26, 2020
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ $ ./examples/pizza -V
```

You may change the flags and description by passing additional parameters to the `version` method, using
the same syntax for flags as the `option` method. The version flags can be named anything, but a long name is required.
the same syntax for flags as the `option` method.

```js
program.version('0.0.1', '-v, --vers', 'output the current version');
Expand Down
51 changes: 38 additions & 13 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,13 @@ class Option {
this.required = flags.includes('<'); // A value must be supplied when the option is specified.
this.optional = flags.includes('['); // A value is optional when the option is specified.
this.mandatory = false; // The option must have a value after parsing, which usually means it must be specified on command line.
this.negate = flags.includes('-no-');
const flagParts = flags.split(/[ ,|]+/);
if (flagParts.length > 1 && !/^[[<]/.test(flagParts[1])) this.short = flagParts.shift();
this.long = flagParts.shift();
const optionFlags = _parseOptionFlags(flags);
this.short = optionFlags.shortFlag;
this.long = optionFlags.longFlag;
this.negate = false;
if (this.long) {
this.negate = this.long.startsWith('--no-');
}
this.description = description || '';
this.defaultValue = undefined;
}
Expand All @@ -39,7 +42,10 @@ class Option {
*/

name() {
return this.long.replace(/^--/, '');
if (this.long) {
return this.long.replace(/^--/, '');
}
return this.short.replace(/^-/, '');
};

/**
Expand Down Expand Up @@ -1187,9 +1193,9 @@ class Command extends EventEmitter {
flags = flags || '-V, --version';
description = description || 'output the version number';
const versionOption = new Option(flags, description);
this._versionOptionName = versionOption.long.substr(2) || 'version';
this._versionOptionName = versionOption.attributeName();
this.options.push(versionOption);
this.on('option:' + this._versionOptionName, () => {
this.on('option:' + versionOption.name(), () => {
process.stdout.write(str + '\n');
this._exit(0, 'commander.version', str);
});
Expand Down Expand Up @@ -1552,12 +1558,9 @@ class Command extends EventEmitter {
this._helpFlags = flags || this._helpFlags;
this._helpDescription = description || this._helpDescription;

const splitFlags = this._helpFlags.split(/[ ,|]+/);

this._helpShortFlag = undefined;
if (splitFlags.length > 1) this._helpShortFlag = splitFlags.shift();

this._helpLongFlag = splitFlags.shift();
const helpFlags = _parseOptionFlags(this._helpFlags);
this._helpShortFlag = helpFlags.shortFlag;
this._helpLongFlag = helpFlags.longFlag;

return this;
};
Expand Down Expand Up @@ -1708,6 +1711,28 @@ function humanReadableArgName(arg) {
: '[' + nameOutput + ']';
}

/**
* Parse the short and long flag out of something like '-m,--mixed <value>'
*
* @api private
*/

function _parseOptionFlags(flags) {
let shortFlag;
let longFlag;
// Use original very loose parsing to maintain backwards compatibility for now,
// which allowed for example unintended `-sw, --short-word` [sic].
const flagParts = flags.split(/[ |,]+/);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this pattern seems buggy if the comment is still accurate, did you intend to say /( |,)+/ there? or just /[ ,]+/ to match space or comma, or really wanted to add a pipe | to the mix?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pipe is really in the mix. From the README:

Each option can have a short flag (single character) and a long name, separated by a comma or space or vertical bar ('|').

The comment is about -sw which is neither a short flag nor a long name.

if (flagParts.length > 1 && !/^[[<]/.test(flagParts[1])) shortFlag = flagParts.shift();
longFlag = flagParts.shift();
// Add support for lone short flag without significantly changing parsing!
if (!shortFlag && /^-[^-]$/.test(longFlag)) {
shortFlag = longFlag;
longFlag = undefined;
}
return { shortFlag, longFlag };
}

/**
* Scan arguments and increment port number for inspect calls (to avoid conflicts when spawning new command).
*
Expand Down
80 changes: 62 additions & 18 deletions tests/command.helpOption.test.js
Original file line number Diff line number Diff line change
@@ -1,22 +1,66 @@
const commander = require('../');

test('when helpOption has custom flags then custom flag invokes help', () => {
// Optional. Suppress normal output to keep test output clean.
const writeSpy = jest.spyOn(process.stdout, 'write').mockImplementation(() => { });
const program = new commander.Command();
program
.exitOverride()
.helpOption('--custom-help', 'custom help output');
expect(() => {
program.parse(['node', 'test', '--custom-help']);
}).toThrow('(outputHelp)');
writeSpy.mockClear();
});
describe('helpOption', () => {
let writeSpy;

beforeAll(() => {
// Optional. Suppress normal output to keep test output clean.
writeSpy = jest.spyOn(process.stdout, 'write').mockImplementation(() => { });
});

afterEach(() => {
writeSpy.mockClear();
});

afterAll(() => {
writeSpy.mockRestore();
});

test('when helpOption has custom flags then custom short flag invokes help', () => {
const program = new commander.Command();
program
.exitOverride()
.helpOption('-c,--custom-help', 'custom help output');
expect(() => {
program.parse(['-c'], { from: 'user' });
}).toThrow('(outputHelp)');
});

test('when helpOption has custom flags then custom long flag invokes help', () => {
const program = new commander.Command();
program
.exitOverride()
.helpOption('-c,--custom-help', 'custom help output');
expect(() => {
program.parse(['--custom-help'], { from: 'user' });
}).toThrow('(outputHelp)');
});

test('when helpOption has just custom short flag then custom short flag invokes help', () => {
const program = new commander.Command();
program
.exitOverride()
.helpOption('-c', 'custom help output');
expect(() => {
program.parse(['-c'], { from: 'user' });
}).toThrow('(outputHelp)');
});

test('when helpOption has just custom long flag then custom long flag invokes help', () => {
const program = new commander.Command();
program
.exitOverride()
.helpOption('--custom-help', 'custom help output');
expect(() => {
program.parse(['--custom-help'], { from: 'user' });
}).toThrow('(outputHelp)');
});

test('when helpOption has custom description then helpInformation include custom description', () => {
const program = new commander.Command();
program
.helpOption('-C,--custom-help', 'custom help output');
const helpInformation = program.helpInformation();
expect(helpInformation).toMatch(/-C,--custom-help +custom help output/);
test('when helpOption has custom description then helpInformation include custom description', () => {
const program = new commander.Command();
program
.helpOption('-C,--custom-help', 'custom help output');
const helpInformation = program.helpInformation();
expect(helpInformation).toMatch(/-C,--custom-help +custom help output/);
});
});
30 changes: 15 additions & 15 deletions tests/helpwrap.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,16 @@ test('when long option description then wrap and indent', () => {
process.stdout.columns = 80;
const program = new commander.Command();
program
.option('-x -extra-long-option-switch', 'kjsahdkajshkahd kajhsd akhds kashd kajhs dkha dkh aksd ka dkha kdh kasd ka kahs dkh sdkh askdh aksd kashdk ahsd kahs dkha skdh');
.option('-x --extra-long-option-switch', 'kjsahdkajshkahd kajhsd akhds kashd kajhs dkha dkh aksd ka dkha kdh kasd ka kahs dkh sdkh askdh aksd kashdk ahsd kahs dkha skdh');

const expectedOutput =
`Usage: [options]

Options:
-x -extra-long-option-switch kjsahdkajshkahd kajhsd akhds kashd kajhs dkha
dkh aksd ka dkha kdh kasd ka kahs dkh sdkh
askdh aksd kashdk ahsd kahs dkha skdh
-h, --help display help for command
-x --extra-long-option-switch kjsahdkajshkahd kajhsd akhds kashd kajhs dkha
dkh aksd ka dkha kdh kasd ka kahs dkh sdkh
askdh aksd kashdk ahsd kahs dkha skdh
-h, --help display help for command
`;

expect(program.helpInformation()).toBe(expectedOutput);
Expand All @@ -29,15 +29,15 @@ test('when long option description and default then wrap and indent', () => {
process.stdout.columns = 80;
const program = new commander.Command();
program
.option('-x -extra-long-option <value>', 'kjsahdkajshkahd kajhsd akhds', 'aaa bbb ccc ddd eee fff ggg');
.option('-x --extra-long-option <value>', 'kjsahdkajshkahd kajhsd akhds', 'aaa bbb ccc ddd eee fff ggg');

const expectedOutput =
`Usage: [options]

Options:
-x -extra-long-option <value> kjsahdkajshkahd kajhsd akhds (default: "aaa
bbb ccc ddd eee fff ggg")
-h, --help display help for command
-x --extra-long-option <value> kjsahdkajshkahd kajhsd akhds (default: "aaa
bbb ccc ddd eee fff ggg")
-h, --help display help for command
`;

expect(program.helpInformation()).toBe(expectedOutput);
Expand All @@ -49,20 +49,20 @@ test('when long command description then wrap and indent', () => {
process.stdout.columns = 80;
const program = new commander.Command();
program
.option('-x -extra-long-option-switch', 'x')
.option('-x --extra-long-option-switch', 'x')
.command('alpha', 'Lorem mollit quis dolor ex do eu quis ad insa a commodo esse.');

const expectedOutput =
`Usage: [options] [command]

Options:
-x -extra-long-option-switch x
-h, --help display help for command
-x --extra-long-option-switch x
-h, --help display help for command

Commands:
alpha Lorem mollit quis dolor ex do eu quis ad insa
a commodo esse.
help [command] display help for command
alpha Lorem mollit quis dolor ex do eu quis ad
insa a commodo esse.
help [command] display help for command
`;

expect(program.helpInformation()).toBe(expectedOutput);
Expand Down
12 changes: 10 additions & 2 deletions tests/options.flags.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,23 @@ const commander = require('../');

// Test the various ways flags can be specified in the first parameter to `.option`

test('when only short flag defined and not specified then value is undefined', () => {
const program = new commander.Command();
program
.option('-p', 'add pepper');
program.parse(['node', 'test']);
expect(program.p).toBeUndefined();
});

// Sanity check that pepper is not true normally, as otherwise all the following tests would pass for thr wrong reasons!
test('when only short flag defined and specified then value is true', () => {
const program = new commander.Command();
program
.option('-p', 'add pepper');
program.parse(['node', 'test', '-p']);
expect(program.P).toBe(true);
expect(program.p).toBe(true);
});

// Sanity check that pepper is not true normally, as otherwise all the following tests would pass for thr wrong reasons!
test('when only long flag defined and not specified then value is undefined', () => {
const program = new commander.Command();
program
Expand Down
24 changes: 24 additions & 0 deletions tests/options.version.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,18 @@ describe('.version', () => {
}).toThrow(myVersion);
});

test('when specify just custom short flag then display version', () => {
const myVersion = '1.2.3';
const program = new commander.Command();
program
.exitOverride()
.version(myVersion, '-r');

expect(() => {
program.parse(['node', 'test', '-r']);
}).toThrow(myVersion);
});

test('when specify custom long flag then display version', () => {
const myVersion = '1.2.3';
const program = new commander.Command();
Expand All @@ -101,6 +113,18 @@ describe('.version', () => {
}).toThrow(myVersion);
});

test('when specify just custom long flag then display version', () => {
const myVersion = '1.2.3';
const program = new commander.Command();
program
.exitOverride()
.version(myVersion, '--revision');

expect(() => {
program.parse(['node', 'test', '--revision']);
}).toThrow(myVersion);
});

test('when custom .version then helpInformation includes custom version help', () => {
const myVersion = '1.2.3';
const myVersionFlags = '-r, --revision';
Expand Down