From 8a5add25ed54ba61356ec9a86c1b7b19c5ebea3e Mon Sep 17 00:00:00 2001 From: Mark Carver Date: Mon, 1 Jan 2018 20:58:46 -0600 Subject: [PATCH 1/7] fix(defaults): Process help and version defaults better Also removes unnecessary "pkginfo" dependency --- lib/index.js | 3 ++- lib/parse.js | 21 ++++++-------------- lib/utils.js | 50 +++++++++++++++++++++++++++-------------------- lib/version.js | 33 +++++++++++++++++++++++++++++++ package-lock.json | 45 ++++++++++++++++++++++++++++++++++++------ package.json | 1 - 6 files changed, 109 insertions(+), 44 deletions(-) create mode 100644 lib/version.js diff --git a/lib/index.js b/lib/index.js index 0890343..2a16243 100644 --- a/lib/index.js +++ b/lib/index.js @@ -10,7 +10,8 @@ const publicMethods = { parse: require('./parse'), example: require('./example'), examples: require('./examples'), - showHelp: require('./help') + showHelp: require('./help'), + showVersion: require('./version') }; function Args() { diff --git a/lib/parse.js b/lib/parse.js index 3c32519..47e7ce6 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -33,24 +33,21 @@ module.exports = function(argv, options) { this.printSubColor = this.printSubColor[this.config.subColor]; } - if (this.config.help) { - // Register default options and commands - this.option('help', 'Output usage information'); - this.command('help', 'Display help', this.showHelp); - } - // Parse arguments using mri this.raw = parser(argv.slice(1), this.config.mri || this.config.minimist); this.binary = path.basename(this.raw._[0]); // If default version is allowed, check for it if (this.config.version) { - this.checkVersion(this.parent); + this.checkVersion(); } - const subCommand = this.raw._[1]; - const helpTriggered = this.raw.h || this.raw.help; + // If default help is allowed, check for it + if (this.config.help) { + this.checkHelp(); + } + const subCommand = this.raw._[1]; const args = {}; const defined = this.isDefined(subCommand, 'commands'); const optionList = this.getOptions(defined); @@ -67,12 +64,6 @@ module.exports = function(argv, options) { return {}; } - // Show usage information if "help" or "h" option was used - // And respect the option related to it - if (this.config.help && helpTriggered) { - this.showHelp(); - } - // Hand back list of options return optionList; }; diff --git a/lib/utils.js b/lib/utils.js index afc61d2..b987ed4 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -269,6 +269,11 @@ module.exports = { details.init = false; } + // If version is disabled, remove initializer + if (details.usage === 'version' && !this.config.version) { + details.init = false; + } + // If command has initializer, call it if (details.init) { const sub = [].concat(this.sub); @@ -293,7 +298,9 @@ module.exports = { if (process.platform === 'win32') { const binaryExt = path.extname(this.binary); - const mainModule = process.env.APPVEYOR ? '_fixture' : process.mainModule.filename; + const mainModule = process.env.APPVEYOR + ? '_fixture' + : process.mainModule.filename; full = `${mainModule}-${subCommand}`; @@ -337,29 +344,25 @@ module.exports = { }); }, - checkVersion(parent) { - // Load parent module - try { - const pkginfo = require('pkginfo'); - pkginfo(parent); - } catch (err) { - // Do nothing, but version could not be aquired - } - - // And get its version property - const version = parent.exports.version || '-/-'; + checkHelp() { + // Register default option and command. + this.option('help', 'Output usage information'); + this.command('help', 'Display help', this.showHelp); - if (version) { - // If it exists, register it as a default option - this.option('version', 'Output the version number'); + // Immediately output if option was provided. + if (this.optionWasProvided('help')) { + this.showHelp(); + } + }, - // And immediately output it if used in command line - if (this.raw.v || this.raw.version) { - console.log(version); + checkVersion(parent) { + // Register default option and command. + this.option('version', 'Output the version number'); + this.command('version', 'Display version', this.showVersion); - // eslint-disable-next-line unicorn/no-process-exit - process.exit(); - } + // Immediately output if option was provided. + if (this.optionWasProvided('version')) { + this.showVersion(parent); } }, @@ -383,5 +386,10 @@ module.exports = { // If nothing matches, item is not defined return false; + }, + + optionWasProvided(name) { + const option = this.isDefined(name, 'options'); + return option && (this.raw[option.usage[0]] || this.raw[option.usage[1]]); } }; diff --git a/lib/version.js b/lib/version.js new file mode 100644 index 0000000..92cc925 --- /dev/null +++ b/lib/version.js @@ -0,0 +1,33 @@ +'use strict'; + +const fs = require('fs'); +const path = require('path'); + +function findPackage(directory) { + const file = path.resolve(directory, 'package.json'); + if (fs.existsSync(file) && fs.statSync(file).isFile()) { + return file; + } + const parent = path.resolve(directory, '..'); + if (parent === directory) { + return null; + } + return findPackage(parent); +} + +module.exports = function(parent) { + // The runCommand method passes the definition's usage, which in this case is "version". + // @todo The "parent" parameter should be removed once BC support is no longer needed. + parent = !parent || parent === 'version' ? this.parent : parent; + + const filename = parent && parent.filename; + const dir = filename && path.dirname(filename); + const file = dir && findPackage(dir); + const pkg = (file && require(file)) || {}; + const version = pkg.version || '-/-'; + + console.log(version); + + // eslint-disable-next-line unicorn/no-process-exit + process.exit(); +}; diff --git a/package-lock.json b/package-lock.json index b0afbec..ff41da4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -340,9 +340,27 @@ "dev": true, "requires": { "arr-exclude": "1.0.0", + "execa": "0.7.0", "has-yarn": "1.0.0", "read-pkg-up": "2.0.0", "write-pkg": "3.1.0" + }, + "dependencies": { + "execa": { + "version": "0.7.0", + "resolved": "https://registry.npmjs.org/execa/-/execa-0.7.0.tgz", + "integrity": "sha1-lEvs00zEHuMqY6n68nrVpl/Fl3c=", + "dev": true, + "requires": { + "cross-spawn": "5.1.0", + "get-stream": "3.0.0", + "is-stream": "1.1.0", + "npm-run-path": "2.0.2", + "p-finally": "1.0.0", + "signal-exit": "3.0.2", + "strip-eof": "1.0.0" + } + } } }, "babel-code-frame": { @@ -4995,11 +5013,6 @@ "find-up": "2.1.0" } }, - "pkginfo": { - "version": "0.4.1", - "resolved": "https://registry.npmjs.org/pkginfo/-/pkginfo-0.4.1.tgz", - "integrity": "sha1-tUGO8EOd5UJfxJlQQtztFPsqhP8=" - }, "plur": { "version": "2.1.2", "resolved": "https://registry.npmjs.org/plur/-/plur-2.1.2.tgz", @@ -5751,7 +5764,27 @@ "version": "1.2.0", "resolved": "https://registry.npmjs.org/term-size/-/term-size-1.2.0.tgz", "integrity": "sha1-RYuDiH8oj8Vtb/+/rSYuJmOO+mk=", - "dev": true + "dev": true, + "requires": { + "execa": "0.7.0" + }, + "dependencies": { + "execa": { + "version": "0.7.0", + "resolved": "https://registry.npmjs.org/execa/-/execa-0.7.0.tgz", + "integrity": "sha1-lEvs00zEHuMqY6n68nrVpl/Fl3c=", + "dev": true, + "requires": { + "cross-spawn": "5.1.0", + "get-stream": "3.0.0", + "is-stream": "1.1.0", + "npm-run-path": "2.0.2", + "p-finally": "1.0.0", + "signal-exit": "3.0.2", + "strip-eof": "1.0.0" + } + } + } }, "text-table": { "version": "0.2.0", diff --git a/package.json b/package.json index b8a780c..983e88f 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,6 @@ "camelcase": "4.1.0", "chalk": "2.1.0", "mri": "1.1.0", - "pkginfo": "0.4.1", "string-similarity": "1.2.0" } } From 3b51d7b31a8fb02d5821c4c7e9f5c701fe718aa5 Mon Sep 17 00:00:00 2001 From: Mark Carver Date: Tue, 2 Jan 2018 02:05:46 -0600 Subject: [PATCH 2/7] fix(command): Pass any options that appear before a command --- lib/utils.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index b987ed4..459e10a 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -288,12 +288,15 @@ module.exports = { : details.usage; let full = this.binary + '-' + subCommand; - const args = process.argv; - let i = 0; - - while (i < 3) { - args.shift(); - i++; + // Remove node and original command. + const args = process.argv.slice(2); + + // Remove the first occurance of subCommand from the args. + for (let i = 0, l = args.length; i < l; i++) { + if (args[i] === subCommand) { + args.splice(i, 1); + break; + } } if (process.platform === 'win32') { From 7cd382b905a2e56a254cc0cfe0d4eefe3810552f Mon Sep 17 00:00:00 2001 From: Mark Carver Date: Tue, 2 Jan 2018 11:45:06 -0600 Subject: [PATCH 3/7] task(version): Remove unnecessary parent parameter introduced in #96 feat(config): Add "exit" booleans for help and version fix(help): Clone objects in array so original data is not altered Fixes #95 --- lib/help.js | 15 +++++++++------ lib/index.js | 3 +-- lib/utils.js | 15 ++++++++++++--- lib/version.js | 14 ++++++-------- 4 files changed, 28 insertions(+), 19 deletions(-) diff --git a/lib/help.js b/lib/help.js index dace7fe..2a34b64 100644 --- a/lib/help.js +++ b/lib/help.js @@ -22,13 +22,14 @@ module.exports = function() { const optionHandle = groups.options ? '[options] ' : ''; const cmdHandle = groups.commands ? '[command]' : ''; - const value = typeof this.config.value === 'string' - ? ' ' + this.config.value - : ''; + const value = + typeof this.config.value === 'string' ? ' ' + this.config.value : ''; parts.push([ '', - `Usage: ${this.printMainColor(name)} ${this.printSubColor(optionHandle + cmdHandle + value)}`, + `Usage: ${this.printMainColor(name)} ${this.printSubColor( + optionHandle + cmdHandle + value + )}`, '' ]); @@ -68,6 +69,8 @@ module.exports = function() { console.log(output); - // eslint-disable-next-line unicorn/no-process-exit - process.exit(); + if (this.config.exit && this.config.exit.help) { + // eslint-disable-next-line unicorn/no-process-exit + process.exit(); + } }; diff --git a/lib/index.js b/lib/index.js index 2a16243..8046d4d 100644 --- a/lib/index.js +++ b/lib/index.js @@ -23,6 +23,7 @@ function Args() { // Configuration defaults this.config = { + exit: { help: true, version: true }, help: true, version: true, usageFilter: null, @@ -34,8 +35,6 @@ function Args() { this.printMainColor = chalk; this.printSubColor = chalk; - - this.parent = module.parent; } // Assign internal helpers diff --git a/lib/utils.js b/lib/utils.js index 459e10a..f361f47 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -177,7 +177,16 @@ module.exports = { generateDetails(kind) { // Get all properties of kind from global scope - const items = typeof kind === 'string' ? this.details[kind] : kind; + const items = []; + + // Clone passed objects so changing them here doesn't affect real data. + const passed = [].concat( + typeof kind === 'string' ? this.details[kind] : kind + ); + for (let i = 0, l = passed.length; i < l; i++) { + items.push({ ...passed[i] }); + } + const parts = []; const isCmd = kind === 'commands'; @@ -358,14 +367,14 @@ module.exports = { } }, - checkVersion(parent) { + checkVersion() { // Register default option and command. this.option('version', 'Output the version number'); this.command('version', 'Display version', this.showVersion); // Immediately output if option was provided. if (this.optionWasProvided('version')) { - this.showVersion(parent); + this.showVersion(); } }, diff --git a/lib/version.js b/lib/version.js index 92cc925..37448e4 100644 --- a/lib/version.js +++ b/lib/version.js @@ -15,12 +15,8 @@ function findPackage(directory) { return findPackage(parent); } -module.exports = function(parent) { - // The runCommand method passes the definition's usage, which in this case is "version". - // @todo The "parent" parameter should be removed once BC support is no longer needed. - parent = !parent || parent === 'version' ? this.parent : parent; - - const filename = parent && parent.filename; +module.exports = function() { + const filename = module.parent && module.parent.filename; const dir = filename && path.dirname(filename); const file = dir && findPackage(dir); const pkg = (file && require(file)) || {}; @@ -28,6 +24,8 @@ module.exports = function(parent) { console.log(version); - // eslint-disable-next-line unicorn/no-process-exit - process.exit(); + if (this.config.exit && this.config.exit.version) { + // eslint-disable-next-line unicorn/no-process-exit + process.exit(); + } }; From 6f3c025416447696f28d84dc54d1cfd3f8621a96 Mon Sep 17 00:00:00 2001 From: Mark Carver Date: Tue, 2 Jan 2018 11:46:03 -0600 Subject: [PATCH 4/7] test(options): Add assertions to ensure "help" and "version" options work properly --- test/index.js | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) diff --git a/test/index.js b/test/index.js index 540def7..4d7b229 100644 --- a/test/index.js +++ b/test/index.js @@ -9,6 +9,25 @@ import execa from 'execa'; import args from '../lib'; import { version } from '../package'; +// Provide a reset function during testing. +args.reset = function() { + this.details = { + options: [], + commands: [], + examples: [] + }; + return this; +}; + +// Provide a helper function for suppressing any output. +args.suppressOutput = function(fn) { + const original = process.stdout.write; + process.stdout.write = () => () => {}; + const result = fn.call(this); + process.stdout.write = original; + return result; +}; + const port = 8000; const argv = [ @@ -25,7 +44,13 @@ const argv = [ 10 ]; -test('options', t => { +// Reset args after each test. +test.afterEach.always(() => { + args.reset(); +}); + +// @todo Each of these options should be broken out into separate tests. +function setupOptions() { args .option('port', 'The port on which the site will run') .option('true', 'Boolean', true) @@ -33,6 +58,12 @@ test('options', t => { .option(['d', 'data'], 'The data that shall be used') .option('duplicated', 'Duplicated first char in option') .options([{ name: 'anotheroption', description: 'another option' }]); + return args; +} + +// @todo Each of these options should be broken out into separate tests. +test('options', t => { + const args = setupOptions(); const config = args.parse(argv); @@ -70,7 +101,48 @@ test('options', t => { } }); +test('help/host: only host is triggered', t => { + args.option('host', 'The host address'); + const config = args.parse(['node', 'foo', '-h', 'http://example.com']); + t.is(config.h, 'http://example.com'); + t.is(config.host, 'http://example.com'); + t.falsy(config.H); + t.falsy(config.help); +}); + +test('help/host: only help is triggered', t => { + args.option('host', 'The host address'); + const config = args.suppressOutput(() => + args.parse(['node', 'foo', '-H'], { exit: { help: false } }) + ); + t.falsy(config.h); + t.falsy(config.host); + t.true(config.H); + t.true(config.help); +}); + +test('version/verbose: only verbose is triggered', t => { + args.option('verbose', 'Verbose output'); + const config = args.parse(['node', 'foo', '-v']); + t.true(config.v); + t.true(config.verbose); + t.falsy(config.H); + t.falsy(config.help); +}); + +test('version/verbose: only version is triggered', t => { + args.option('verbose', 'Verbose output'); + const config = args.suppressOutput(() => + args.parse(['node', 'foo', '-V'], { exit: { version: false } }) + ); + t.falsy(config.v); + t.falsy(config.verbose); + t.true(config.V); + t.true(config.version); +}); + test('usage information', t => { + const args = setupOptions(); const filter = data => data; args.parse(argv, { @@ -85,6 +157,7 @@ test('usage information', t => { }); test('config', t => { + const args = setupOptions(); args.parse(argv, { help: false, errors: false @@ -126,6 +199,7 @@ test('command aliases', async t => { }); test('options propogated to mri', t => { + const args = setupOptions(); args.option('port', 'The port on which the site will run'); const config = args.parse(argv, { mri: { string: 'p' } }); From 32da0281309e32b646b1623736cf35469128deb4 Mon Sep 17 00:00:00 2001 From: Mark Carver Date: Tue, 2 Jan 2018 11:47:14 -0600 Subject: [PATCH 5/7] docs(options): Add ".showVersion()" method and "exit" configuration --- readme.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/readme.md b/readme.md index 21e5e32..1d04776 100644 --- a/readme.md +++ b/readme.md @@ -165,14 +165,19 @@ pizza eat ./directory ### .showHelp() -Outputs the usage information based on the options and comments you've registered so far. +Outputs the usage information based on the options and comments you've registered so far and exits, if configured to do so. + +### .showVersion() + +Outputs the version and exits, if configured to do so. ## Configuration -By default, the module already registers some default options (e.g. "version" and "help"), as well as a command named "help". These things have been implemented to make creating CLIs easier for beginners. However, they can also be disabled by taking advantage of the following properties: +By default, the module already registers some default options and commands (e.g. "version" and "help"). These things have been implemented to make creating CLIs easier for beginners. However, they can also be disabled by taking advantage of the following properties: | Property | Description | Default value | Type | | -------- | ----------- | ------------------ | ---- | +| exit | Automatically exits when help or version is rendered | `{ help: true, version: true }` | Object | | help | Automatically render the usage information when running `help`, `-h` or `--help` | true | Boolean | | name | The name of your program to display in help | Name of script file | String | | version | Outputs the version tag of your package.json | true | Boolean | From e071894cdbc83dc9fc011c7b08e148773da50971 Mon Sep 17 00:00:00 2001 From: Mark Carver Date: Tue, 2 Jan 2018 11:52:25 -0600 Subject: [PATCH 6/7] fix(generateDetails): Use Object.assign instead of spread for older node versions --- lib/utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/utils.js b/lib/utils.js index f361f47..10d1a60 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -184,7 +184,7 @@ module.exports = { typeof kind === 'string' ? this.details[kind] : kind ); for (let i = 0, l = passed.length; i < l; i++) { - items.push({ ...passed[i] }); + items.push(Object.assign({}, passed[i])); } const parts = []; From fa91db7589d4203ac775b9eb6ecea80df07d28e4 Mon Sep 17 00:00:00 2001 From: Mark Carver Date: Tue, 2 Jan 2018 14:14:48 -0600 Subject: [PATCH 7/7] fix(version): Use process.mainModule instead of module.parent --- lib/version.js | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/lib/version.js b/lib/version.js index 37448e4..311f956 100644 --- a/lib/version.js +++ b/lib/version.js @@ -3,24 +3,27 @@ const fs = require('fs'); const path = require('path'); +/** + * Retrieves the main module package.json information. + * + * @param {string} directory + * The directory to start looking in. + * + * @return {Object|null} + * An object containing the package.json contents or NULL if it could not be found. + */ function findPackage(directory) { const file = path.resolve(directory, 'package.json'); if (fs.existsSync(file) && fs.statSync(file).isFile()) { - return file; + return require(file); } const parent = path.resolve(directory, '..'); - if (parent === directory) { - return null; - } - return findPackage(parent); + return parent === directory ? null : findPackage(parent); } module.exports = function() { - const filename = module.parent && module.parent.filename; - const dir = filename && path.dirname(filename); - const file = dir && findPackage(dir); - const pkg = (file && require(file)) || {}; - const version = pkg.version || '-/-'; + const pkg = findPackage(path.dirname(process.mainModule.filename)); + const version = (pkg && pkg.version) || '-/-'; console.log(version);