From 60d1d7d413ebfff428bd8f8d8fdfe06a1628c740 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 25 Apr 2020 17:56:41 +1200 Subject: [PATCH 1/4] Support short flag alone --- index.js | 37 +++++++++++++++++++++++++++---------- tests/helpwrap.test.js | 30 +++++++++++++++--------------- tests/options.flags.test.js | 2 +- 3 files changed, 43 insertions(+), 26 deletions(-) diff --git a/index.js b/index.js index 987b79f0d..2dd223d5d 100644 --- a/index.js +++ b/index.js @@ -24,9 +24,9 @@ class Option { this.optional = flags.indexOf('[') >= 0; // 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.indexOf('-no-') !== -1; - 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.description = description || ''; this.defaultValue = undefined; } @@ -39,7 +39,10 @@ class Option { */ name() { - return this.long.replace(/^--/, ''); + if (this.long) { + return this.long.replace(/^--/, ''); + } + return this.short.replace(/^-/, ''); }; /** @@ -1552,12 +1555,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; }; @@ -1708,6 +1708,23 @@ function humanReadableArgName(arg) { : '[' + nameOutput + ']'; } +/** + * Pulling the flags out of something like '-m,--mixed ' + * @api private + */ + +function _parseOptionFlags(flags) { + let shortFlag; + let longFlag; + const flagParts = flags.split(/[ |,]+/); + // single character after '-' + if (flagParts.length > 0 && /^-[^-]$/.test(flagParts[0])) shortFlag = flagParts.shift(); + // long flag after '--' + if (flagParts.length > 0 && /^--[^-]/.test(flagParts[0])) longFlag = flagParts.shift(); + if (!shortFlag && !longFlag) throw new Error(`option flags not in expected format: ${flags}`); + return { shortFlag, longFlag }; +} + /** * Scan arguments and increment port number for inspect calls (to avoid conflicts when spawning new command). * diff --git a/tests/helpwrap.test.js b/tests/helpwrap.test.js index 22f296693..0184bdd76 100644 --- a/tests/helpwrap.test.js +++ b/tests/helpwrap.test.js @@ -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); @@ -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 ', 'kjsahdkajshkahd kajhsd akhds', 'aaa bbb ccc ddd eee fff ggg'); + .option('-x --extra-long-option ', 'kjsahdkajshkahd kajhsd akhds', 'aaa bbb ccc ddd eee fff ggg'); const expectedOutput = `Usage: [options] Options: - -x -extra-long-option kjsahdkajshkahd kajhsd akhds (default: "aaa - bbb ccc ddd eee fff ggg") - -h, --help display help for command + -x --extra-long-option kjsahdkajshkahd kajhsd akhds (default: "aaa + bbb ccc ddd eee fff ggg") + -h, --help display help for command `; expect(program.helpInformation()).toBe(expectedOutput); @@ -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); diff --git a/tests/options.flags.test.js b/tests/options.flags.test.js index 9c692461b..b76602b8c 100644 --- a/tests/options.flags.test.js +++ b/tests/options.flags.test.js @@ -7,7 +7,7 @@ test('when only short flag defined and specified then value is true', () => { 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! From 43785f2fda0bb9ad4951520ddf964c0fd496b006 Mon Sep 17 00:00:00 2001 From: John Gee Date: Mon, 27 Apr 2020 17:28:47 +1200 Subject: [PATCH 2/4] Weaken option parsing for backwards compatibility --- index.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/index.js b/index.js index 2dd223d5d..5f582c24f 100644 --- a/index.js +++ b/index.js @@ -1709,19 +1709,24 @@ function humanReadableArgName(arg) { } /** - * Pulling the flags out of something like '-m,--mixed ' + * Parse the short and long flag out of something like '-m,--mixed ' + * * @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(/[ |,]+/); - // single character after '-' - if (flagParts.length > 0 && /^-[^-]$/.test(flagParts[0])) shortFlag = flagParts.shift(); - // long flag after '--' - if (flagParts.length > 0 && /^--[^-]/.test(flagParts[0])) longFlag = flagParts.shift(); - if (!shortFlag && !longFlag) throw new Error(`option flags not in expected format: ${flags}`); + 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 }; } From 08b21e19989b85a04c135fc1f43d99d842bdcfef Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 2 May 2020 16:55:37 +1200 Subject: [PATCH 3/4] Have .version allow short only flag --- Readme.md | 2 +- index.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Readme.md b/Readme.md index fcb995ab3..13015ab71 100644 --- a/Readme.md +++ b/Readme.md @@ -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'); diff --git a/index.js b/index.js index 5f582c24f..36d8ec2f2 100644 --- a/index.js +++ b/index.js @@ -1190,9 +1190,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); }); From af45c33ee1f5ee415b191b2252f8c05f4d0d919e Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 24 May 2020 13:23:15 +1200 Subject: [PATCH 4/4] Add tests for lone short and long flags --- tests/command.helpOption.test.js | 80 +++++++++++++++++++++++++------- tests/options.flags.test.js | 10 +++- tests/options.version.test.js | 24 ++++++++++ 3 files changed, 95 insertions(+), 19 deletions(-) diff --git a/tests/command.helpOption.test.js b/tests/command.helpOption.test.js index f8b96b677..8cda1f20b 100644 --- a/tests/command.helpOption.test.js +++ b/tests/command.helpOption.test.js @@ -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/); + }); }); diff --git a/tests/options.flags.test.js b/tests/options.flags.test.js index b76602b8c..c7f1e581c 100644 --- a/tests/options.flags.test.js +++ b/tests/options.flags.test.js @@ -2,6 +2,15 @@ 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 @@ -10,7 +19,6 @@ test('when only short flag defined and specified then value is 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 diff --git a/tests/options.version.test.js b/tests/options.version.test.js index d636bfdf4..e2c7ed3ef 100644 --- a/tests/options.version.test.js +++ b/tests/options.version.test.js @@ -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(); @@ -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';