From aca9d642d0c848068a93f1e82a168d7dbabfea56 Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Sat, 21 Jan 2017 17:59:25 -0800 Subject: [PATCH 01/13] feat: rethink how options are inherited by commands --- lib/validation.js | 6 ++- test/command.js | 94 +++++++++++++++++++++++++++++++++++++++++++++++ test/usage.js | 2 +- test/yargs.js | 16 +++++--- yargs.js | 22 ++++++++--- 5 files changed, 126 insertions(+), 14 deletions(-) diff --git a/lib/validation.js b/lib/validation.js index 328d83aff..4413037d1 100644 --- a/lib/validation.js +++ b/lib/validation.js @@ -224,6 +224,7 @@ module.exports = function (yargs, usage, y18n) { self.implies(k, key[k]) }) } else { + yargs.global(key) implied[key] = value } } @@ -290,6 +291,7 @@ module.exports = function (yargs, usage, y18n) { self.conflicts(k, key[k]) }) } else { + yargs.global(key) conflicting[key] = value } } @@ -328,8 +330,10 @@ module.exports = function (yargs, usage, y18n) { implied = objFilter(implied, function (k, v) { return globalLookup[k] }) + conflicting = objFilter(conflicting, function (k, v) { + return globalLookup[k] + }) checks = [] - conflicting = {} return self } diff --git a/test/command.js b/test/command.js index be4f0dca8..8fd99427e 100644 --- a/test/command.js +++ b/test/command.js @@ -839,4 +839,98 @@ describe('Command', function () { argv.sins.should.deep.equal([113993, 112888]) }) }) + + // make sure yargs parsing features configured on + // the top-level apply appropriately to commands. + describe('global', function () { + context('false', function () { + describe('config', function () { + it('does not load config in argv passed to command', function (done) { + yargs('command --foo ./package.json') + .command('command', 'a command', {}, function (argv) { + expect(argv.license).to.equal(undefined) + return done() + }) + .config('foo') + .global('foo', false) + .argv + }) + }) + + describe('validation', function () { + it('resets implies logic for command', function (done) { + yargs('command --foo 99') + .command('command', 'a command', {}, function (argv) { + argv.foo.should.equal(99) + return done() + }) + .implies('foo', 'bar') + .global('foo', false) + .argv + }) + + it('resets conflicts logic for command', function (done) { + yargs('command --foo --bar') + .command('command', 'a command', {}, function (argv) { + argv.foo.should.equal(true) + argv.bar.should.equal(true) + return done() + }) + .conflicts('foo', 'bar') + .global('foo', false) + .argv + }) + }) + }) + + // TODO: tests for: + // strict mode. + // custom checks. + // aliases. + // types (array, boolean, number, choices, count, nargs, normalize) + // coerce. + // defaults. + // demand (demandOption, demandCommand). + // describe. + // groups. + // help, version. + + context('true (default)', function () { + describe('config', function () { + it('loads config for in argv passed to command', function (done) { + yargs('command --foo ./package.json') + .command('command', 'a command', {}, function (argv) { + argv.license.should.equal('MIT') + return done() + }) + .config('foo') + .argv + }) + }) + + describe('validation', function () { + it('applies implies logic for command', function (done) { + yargs('command --foo 99') + .command('command', 'a command', {}, function (argv) {}) + .fail(function (msg) { + msg.should.match(/foo -> bar/) + return done() + }) + .implies('foo', 'bar') + .argv + }) + + it('applies conflicts logic for command', function (done) { + yargs('command --foo --bar') + .command('command', 'a command', {}, function (argv) {}) + .fail(function (msg) { + msg.should.match(/foo -> bar/) + return done() + }) + .conflicts('foo', 'bar') + .argv + }) + }) + }) + }) }) diff --git a/test/usage.js b/test/usage.js index b3f532d76..96d098768 100644 --- a/test/usage.js +++ b/test/usage.js @@ -1598,7 +1598,7 @@ describe('usage tests', function () { ]) }) - it('preserves groups with global keys', function () { + it('allows global option to be disabled', function () { var r = checkUsage(function () { return yargs(['upload', '-h']) .command('upload', 'upload something', function (yargs) { diff --git a/test/yargs.js b/test/yargs.js index b8777e748..3bbca7d3c 100644 --- a/test/yargs.js +++ b/test/yargs.js @@ -1123,6 +1123,7 @@ describe('yargs dsl tests', function () { .config('config', function (path) { return JSON.parse(fs.readFileSync(path)) }) + .global('config', false) .argv argv.foo.should.equal('baz') @@ -1131,7 +1132,8 @@ describe('yargs dsl tests', function () { it('allows key to be specified with option shorthand', function () { var argv = yargs('--config ./test/fixtures/config.json') .option('config', { - config: true + config: true, + global: false }) .argv @@ -1218,7 +1220,8 @@ describe('yargs dsl tests', function () { nargs: 2 }) .option('bar', { - nargs: 2 + nargs: 2, + global: false }) .global('foo') .reset() @@ -1238,7 +1241,8 @@ describe('yargs dsl tests', function () { .option('bar', { nargs: 2, string: true, - demand: true + demand: true, + global: false }) .global('foo') .reset({ @@ -1298,11 +1302,11 @@ describe('yargs dsl tests', function () { it('should expose an options short-hand for declaring global options', function () { var y = yargs('--foo a b c') .option('foo', { - nargs: 2, - global: true + nargs: 2 }) .option('bar', { - nargs: 2 + nargs: 2, + global: false }) .reset() var options = y.getOptions() diff --git a/yargs.js b/yargs.js index bbb3d64c9..b73d2ace1 100644 --- a/yargs.js +++ b/yargs.js @@ -228,6 +228,7 @@ function Yargs (processArgs, cwd, parentRequire) { } key = key || 'config' + self.global(key) self.describe(key, msg || usage.deferY18nLookup('Path to JSON config file')) ;(Array.isArray(key) ? key : [key]).forEach(function (k) { options.config[k] = parseFn || true @@ -436,14 +437,23 @@ function Yargs (processArgs, cwd, parentRequire) { return self } - self.global = function (globals) { - options.global.push.apply(options.global, [].concat(globals)) + self.global = function (globals, isGlobal) { + globals = [].concat(globals) + // if isGlobal isn't provided, assume true. + if (isGlobal || typeof isGlobal === 'undefined') { + globals.forEach(function (g) { + if (options.global.indexOf(g) === -1) options.global.push(g) + }) + } else { + options.global = options.global.filter(function (g) { + return globals.indexOf(g) === -1 + }) + } return self } self.pkgConf = function (key, path) { var conf = null - var obj = pkgUp(path) // If an object exists in the key, add it to options.configObjects @@ -568,9 +578,9 @@ function Yargs (processArgs, cwd, parentRequire) { self.group(key, opt.group) } - if (opt.global) { - self.global(key) - } + // options default to global, this behavior can be disabled with global: false. + const isGlobal = typeof opt.global === 'undefined' ? true : opt.global + self.global(key, isGlobal) if (opt.boolean || opt.type === 'boolean') { self.boolean(key) From 521804063f4b9a163af9f0d2f288757c310a1d38 Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Sat, 21 Jan 2017 18:14:47 -0800 Subject: [PATCH 02/13] fix: fix up a couple more tests --- test/command.js | 2 +- test/yargs.js | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/test/command.js b/test/command.js index 8fd99427e..2243419f0 100644 --- a/test/command.js +++ b/test/command.js @@ -1,4 +1,4 @@ -/* global describe, it, beforeEach */ +/* global context, describe, it, beforeEach */ var yargs = require('../') var expect = require('chai').expect var checkOutput = require('./helpers/utils').checkOutput diff --git a/test/yargs.js b/test/yargs.js index 3bbca7d3c..2c98116d0 100644 --- a/test/yargs.js +++ b/test/yargs.js @@ -222,6 +222,8 @@ describe('yargs dsl tests', function () { .group('foo', 'Group:') .strict() .exitProcess(false) // defaults to true. + .global('foo', false) + .global('qux', false) .env('YARGS') .reset() @@ -1292,7 +1294,7 @@ describe('yargs dsl tests', function () { .implies({ z: 'w' }) - .global(['x']) + .global(['z'], false) .reset() var implied = y.getValidationInstance().getImplied() Object.keys(implied).should.include('x') From 1a7e703c2fb7cd2b1566c8f49e57949b4d902289 Mon Sep 17 00:00:00 2001 From: nexdrew Date: Fri, 16 Dec 2016 17:18:23 -0500 Subject: [PATCH 03/13] feat: allow strict mode to be applied globally --- test/yargs.js | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++- yargs.js | 8 ++++-- 2 files changed, 84 insertions(+), 4 deletions(-) diff --git a/test/yargs.js b/test/yargs.js index 2c98116d0..109f86128 100644 --- a/test/yargs.js +++ b/test/yargs.js @@ -765,7 +765,7 @@ describe('yargs dsl tests', function () { }) }) - // yargs.parse(['foo', '--bar'], function (err, argv, output) {} + // yargs.parse(['foo', '--bar'], function (err, argv, output) {}) context('function passed as second argument to parse', function () { it('does not print to stdout', function () { var r = checkOutput(function () { @@ -1873,6 +1873,84 @@ describe('yargs dsl tests', function () { expect(err).to.exist }) }) + + describe('strict', function () { + it('defaults to false when not called', function () { + var commandCalled = false + yargs('hi') + .command('hi', 'The hi command', function (innerYargs) { + commandCalled = true + innerYargs.getStrict().should.be.false + }) + yargs.getStrict().should.be.false + yargs.argv // parse and run command + commandCalled.should.be.true + }) + + it('can be enabled just for a command', function () { + var commandCalled = false + yargs('hi') + .command('hi', 'The hi command', function (innerYargs) { + commandCalled = true + innerYargs.strict().getStrict().should.be.true + }) + yargs.getStrict().should.be.false + yargs.argv // parse and run command + commandCalled.should.be.true + }) + + it('is true but non-global when called without arguments', function () { + var commandCalled = false + yargs('hi') + .strict() + .command('hi', 'The hi command', function (innerYargs) { + commandCalled = true + innerYargs.getStrict().should.be.false + }) + yargs.getStrict().should.be.true + yargs.argv // parse and run command + commandCalled.should.be.true + }) + + it('is false when passed a value of `false`', function () { + var commandCalled = false + yargs('hi') + .strict(false) + .command('hi', 'The hi command', function (innerYargs) { + commandCalled = true + innerYargs.getStrict().should.be.false + }) + yargs.getStrict().should.be.false + yargs.argv // parse and run command + commandCalled.should.be.true + }) + + it('is true and global when passed a value of `true`', function () { + var commandCalled = false + yargs('hi') + .strict(true) + .command('hi', 'The hi command', function (innerYargs) { + commandCalled = true + innerYargs.getStrict().should.be.true + }) + yargs.getStrict().should.be.true + yargs.argv // parse and run command + commandCalled.should.be.true + }) + + it('allows a command to override global with a value of `false`', function () { + var commandCalled = false + yargs('hi') + .strict(true) + .command('hi', 'The hi command', function (innerYargs) { + commandCalled = true + innerYargs.strict(false).getStrict().should.be.false + }) + yargs.getStrict().should.be.true + yargs.argv // parse and run command + commandCalled.should.be.true + }) + }) }) describe('yargs context', function () { diff --git a/yargs.js b/yargs.js index b73d2ace1..d0f9433f2 100644 --- a/yargs.js +++ b/yargs.js @@ -124,7 +124,7 @@ function Yargs (processArgs, cwd, parentRequire) { command = command ? command.reset() : Command(self, usage, validation) if (!completion) completion = Completion(self, usage, command) - strict = false + if (!strictGlobal) strict = false completionCommand = null output = '' exitError = null @@ -664,8 +664,10 @@ function Yargs (processArgs, cwd, parentRequire) { } var strict = false - self.strict = function () { - strict = true + var strictGlobal = false + self.strict = function (_global) { + strict = _global !== false + strictGlobal = !!_global return self } self.getStrict = function () { From 695e15fc26e52f70ff4486b5e67d999e91bc5ad2 Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Sat, 21 Jan 2017 18:39:22 -0800 Subject: [PATCH 04/13] chore: cherry-picked in @nexdrew's work on strict --- test/command.js | 215 ++++++++++++++++++++++++++++++++---------------- test/yargs.js | 78 ------------------ 2 files changed, 142 insertions(+), 151 deletions(-) diff --git a/test/command.js b/test/command.js index 2243419f0..5df5a4539 100644 --- a/test/command.js +++ b/test/command.js @@ -1,4 +1,4 @@ -/* global context, describe, it, beforeEach */ +/* global describe, it, beforeEach */ var yargs = require('../') var expect = require('chai').expect var checkOutput = require('./helpers/utils').checkOutput @@ -843,46 +843,153 @@ describe('Command', function () { // make sure yargs parsing features configured on // the top-level apply appropriately to commands. describe('global', function () { - context('false', function () { - describe('config', function () { - it('does not load config in argv passed to command', function (done) { - yargs('command --foo ./package.json') - .command('command', 'a command', {}, function (argv) { - expect(argv.license).to.equal(undefined) - return done() - }) - .config('foo') - .global('foo', false) - .argv - }) + describe('config', function () { + it('does not load config for command if global is false', function (done) { + yargs('command --foo ./package.json') + .command('command', 'a command', {}, function (argv) { + expect(argv.license).to.equal(undefined) + return done() + }) + .config('foo') + .global('foo', false) + .argv }) - describe('validation', function () { - it('resets implies logic for command', function (done) { - yargs('command --foo 99') - .command('command', 'a command', {}, function (argv) { - argv.foo.should.equal(99) - return done() - }) - .implies('foo', 'bar') - .global('foo', false) - .argv - }) + it('loads config for command by default', function (done) { + yargs('command --foo ./package.json') + .command('command', 'a command', {}, function (argv) { + argv.license.should.equal('MIT') + return done() + }) + .config('foo') + .argv + }) + }) - it('resets conflicts logic for command', function (done) { - yargs('command --foo --bar') - .command('command', 'a command', {}, function (argv) { - argv.foo.should.equal(true) - argv.bar.should.equal(true) - return done() - }) - .conflicts('foo', 'bar') - .global('foo', false) - .argv - }) + describe('validation', function () { + it('resets implies logic for command if global is false', function (done) { + yargs('command --foo 99') + .command('command', 'a command', {}, function (argv) { + argv.foo.should.equal(99) + return done() + }) + .implies('foo', 'bar') + .global('foo', false) + .argv + }) + + it('applies implies logic for command by default', function (done) { + yargs('command --foo 99') + .command('command', 'a command', {}, function (argv) {}) + .fail(function (msg) { + msg.should.match(/foo -> bar/) + return done() + }) + .implies('foo', 'bar') + .argv + }) + + it('resets conflicts logic for command if global is false', function (done) { + yargs('command --foo --bar') + .command('command', 'a command', {}, function (argv) { + argv.foo.should.equal(true) + argv.bar.should.equal(true) + return done() + }) + .conflicts('foo', 'bar') + .global('foo', false) + .argv + }) + + it('applies conflicts logic for command by default', function (done) { + yargs('command --foo --bar') + .command('command', 'a command', {}, function (argv) {}) + .fail(function (msg) { + msg.should.match(/mutually exclusive/) + return done() + }) + .conflicts('foo', 'bar') + .argv }) }) + describe('strict', function () { + it('defaults to false when not called', function () { + var commandCalled = false + yargs('hi') + .command('hi', 'The hi command', function (innerYargs) { + commandCalled = true + innerYargs.getStrict().should.be.false + }) + yargs.getStrict().should.be.false + yargs.argv // parse and run command + commandCalled.should.be.true + }) + + it('can be enabled just for a command', function () { + var commandCalled = false + yargs('hi') + .command('hi', 'The hi command', function (innerYargs) { + commandCalled = true + innerYargs.strict().getStrict().should.be.true + }) + yargs.getStrict().should.be.false + yargs.argv // parse and run command + commandCalled.should.be.true + }) + + it('is true but non-global when called without arguments', function () { + var commandCalled = false + yargs('hi') + .strict() + .command('hi', 'The hi command', function (innerYargs) { + commandCalled = true + innerYargs.getStrict().should.be.false + }) + yargs.getStrict().should.be.true + yargs.argv // parse and run command + commandCalled.should.be.true + }) + + it('is false when passed a value of `false`', function () { + var commandCalled = false + yargs('hi') + .strict(false) + .command('hi', 'The hi command', function (innerYargs) { + commandCalled = true + innerYargs.getStrict().should.be.false + }) + yargs.getStrict().should.be.false + yargs.argv // parse and run command + commandCalled.should.be.true + }) + + it('is true and global when passed a value of `true`', function () { + var commandCalled = false + yargs('hi') + .strict(true) + .command('hi', 'The hi command', function (innerYargs) { + commandCalled = true + innerYargs.getStrict().should.be.true + }) + yargs.getStrict().should.be.true + yargs.argv // parse and run command + commandCalled.should.be.true + }) + + it('allows a command to override global with a value of `false`', function () { + var commandCalled = false + yargs('hi') + .strict(true) + .command('hi', 'The hi command', function (innerYargs) { + commandCalled = true + innerYargs.strict(false).getStrict().should.be.false + }) + yargs.getStrict().should.be.true + yargs.argv // parse and run command + commandCalled.should.be.true + }) + }) // TODO: tests for: // strict mode. // custom checks. @@ -894,43 +1001,5 @@ describe('Command', function () { // describe. // groups. // help, version. - - context('true (default)', function () { - describe('config', function () { - it('loads config for in argv passed to command', function (done) { - yargs('command --foo ./package.json') - .command('command', 'a command', {}, function (argv) { - argv.license.should.equal('MIT') - return done() - }) - .config('foo') - .argv - }) - }) - - describe('validation', function () { - it('applies implies logic for command', function (done) { - yargs('command --foo 99') - .command('command', 'a command', {}, function (argv) {}) - .fail(function (msg) { - msg.should.match(/foo -> bar/) - return done() - }) - .implies('foo', 'bar') - .argv - }) - - it('applies conflicts logic for command', function (done) { - yargs('command --foo --bar') - .command('command', 'a command', {}, function (argv) {}) - .fail(function (msg) { - msg.should.match(/foo -> bar/) - return done() - }) - .conflicts('foo', 'bar') - .argv - }) - }) - }) }) }) diff --git a/test/yargs.js b/test/yargs.js index 109f86128..be1817f31 100644 --- a/test/yargs.js +++ b/test/yargs.js @@ -1873,84 +1873,6 @@ describe('yargs dsl tests', function () { expect(err).to.exist }) }) - - describe('strict', function () { - it('defaults to false when not called', function () { - var commandCalled = false - yargs('hi') - .command('hi', 'The hi command', function (innerYargs) { - commandCalled = true - innerYargs.getStrict().should.be.false - }) - yargs.getStrict().should.be.false - yargs.argv // parse and run command - commandCalled.should.be.true - }) - - it('can be enabled just for a command', function () { - var commandCalled = false - yargs('hi') - .command('hi', 'The hi command', function (innerYargs) { - commandCalled = true - innerYargs.strict().getStrict().should.be.true - }) - yargs.getStrict().should.be.false - yargs.argv // parse and run command - commandCalled.should.be.true - }) - - it('is true but non-global when called without arguments', function () { - var commandCalled = false - yargs('hi') - .strict() - .command('hi', 'The hi command', function (innerYargs) { - commandCalled = true - innerYargs.getStrict().should.be.false - }) - yargs.getStrict().should.be.true - yargs.argv // parse and run command - commandCalled.should.be.true - }) - - it('is false when passed a value of `false`', function () { - var commandCalled = false - yargs('hi') - .strict(false) - .command('hi', 'The hi command', function (innerYargs) { - commandCalled = true - innerYargs.getStrict().should.be.false - }) - yargs.getStrict().should.be.false - yargs.argv // parse and run command - commandCalled.should.be.true - }) - - it('is true and global when passed a value of `true`', function () { - var commandCalled = false - yargs('hi') - .strict(true) - .command('hi', 'The hi command', function (innerYargs) { - commandCalled = true - innerYargs.getStrict().should.be.true - }) - yargs.getStrict().should.be.true - yargs.argv // parse and run command - commandCalled.should.be.true - }) - - it('allows a command to override global with a value of `false`', function () { - var commandCalled = false - yargs('hi') - .strict(true) - .command('hi', 'The hi command', function (innerYargs) { - commandCalled = true - innerYargs.strict(false).getStrict().should.be.false - }) - yargs.getStrict().should.be.true - yargs.argv // parse and run command - commandCalled.should.be.true - }) - }) }) describe('yargs context', function () { From 0dd09eff57277f860797baffdbab6f97b07c3d36 Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Sat, 21 Jan 2017 19:37:15 -0800 Subject: [PATCH 05/13] fix: fix a couple more tests broken by strict defaulting to global --- test/command.js | 4 ++-- test/usage.js | 21 +-------------------- test/yargs.js | 2 +- yargs.js | 6 +++--- 4 files changed, 7 insertions(+), 26 deletions(-) diff --git a/test/command.js b/test/command.js index 5df5a4539..4dc6f7d31 100644 --- a/test/command.js +++ b/test/command.js @@ -938,13 +938,13 @@ describe('Command', function () { commandCalled.should.be.true }) - it('is true but non-global when called without arguments', function () { + it('is true and global when called without arguments', function () { var commandCalled = false yargs('hi') .strict() .command('hi', 'The hi command', function (innerYargs) { commandCalled = true - innerYargs.getStrict().should.be.false + innerYargs.getStrict().should.be.true }) yargs.getStrict().should.be.true yargs.argv // parse and run command diff --git a/test/usage.js b/test/usage.js index 96d098768..1d9642b61 100644 --- a/test/usage.js +++ b/test/usage.js @@ -84,13 +84,12 @@ describe('usage tests', function () { r.exit.should.be.ok }) - it('no failure occurs if the required arguments and the required number of commands are provided.', function () { + it('no failure occurs if the required arguments and the required number of commands are provided', function () { var r = checkUsage(function () { return yargs('wombat -w 10 -m 10') .usage('Usage: $0 -w NUM -m NUM') .command('wombat', 'wombat handlers') .demand(1, ['w', 'm']) - .strict() .wrap(null) .argv }) @@ -170,24 +169,6 @@ describe('usage tests', function () { r.logs.should.have.length(0) r.exit.should.be.ok }) - - it('no failure occurs if the required arguments and the required number of commands are provided.', function () { - var r = checkUsage(function () { - return yargs('wombat -w 10 -m 10') - .usage('Usage: $0 -w NUM -m NUM') - .command('wombat', 'wombat handlers') - .require(1, ['w', 'm']) - .strict() - .wrap(null) - .argv - }) - r.result.should.have.property('w', 10) - r.result.should.have.property('m', 10) - r.result.should.have.property('_').with.length(1) - r.should.have.property('errors').with.length(0) - r.should.have.property('logs').with.length(0) - r.should.have.property('exit', false) - }) }) it('should show an error along with a custom message on demand fail', function () { diff --git a/test/yargs.js b/test/yargs.js index be1817f31..1f092cefc 100644 --- a/test/yargs.js +++ b/test/yargs.js @@ -220,7 +220,7 @@ describe('yargs dsl tests', function () { .implies('foo', 'snuh') .conflicts('qux', 'xyzzy') .group('foo', 'Group:') - .strict() + .strict(true, false) .exitProcess(false) // defaults to true. .global('foo', false) .global('qux', false) diff --git a/yargs.js b/yargs.js index d0f9433f2..712721551 100644 --- a/yargs.js +++ b/yargs.js @@ -665,9 +665,9 @@ function Yargs (processArgs, cwd, parentRequire) { var strict = false var strictGlobal = false - self.strict = function (_global) { - strict = _global !== false - strictGlobal = !!_global + self.strict = function (_strict, _global) { + strict = _strict !== false + strictGlobal = _global !== false return self } self.getStrict = function () { From db25fb8d0938c6608b65675e04624647e22f4ac1 Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Sat, 21 Jan 2017 19:47:12 -0800 Subject: [PATCH 06/13] fix: cleanup a few more tests, start working on docs --- README.md | 23 ++++++++++------------- test/command.js | 19 +++---------------- test/yargs.js | 2 +- yargs.js | 4 ++-- 4 files changed, 16 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index c14cb2928..d5e6d778a 100644 --- a/README.md +++ b/README.md @@ -551,14 +551,6 @@ yargs .argv ``` -Note that commands will not automatically inherit configuration _or_ options -of their parent context. This means you'll have to re-apply configuration -if necessary, and make options global manually using the [global](#global) method. - -Additionally, the [`help`](#help) and [`version`](#version) -options (if used) **always** apply globally, just like the -[`.wrap()`](#wrap) configuration. - `builder` can also be a function. This function is executed with a `yargs` instance, and can be used to provide _advanced_ command specific help: @@ -1293,7 +1285,7 @@ require('yargs') Outputs the same completion choices as `./test.js --foo`TAB: `--foobar` and `--foobaz` -.global(globals) +.global(globals, [isGlobal=true]) ------------ Indicate that an option (or group of options) should not be reset when a command @@ -1303,11 +1295,13 @@ is executed, as an example: var argv = require('yargs') .option('a', { alias: 'all', - default: true + default: true, + global: false }) .option('n', { alias: 'none', - default: true + default: true, + global: false }) .command('foo', 'foo command', function (yargs) { return yargs.option('b', { @@ -1322,7 +1316,7 @@ var argv = require('yargs') If the `foo` command is executed the `all` option will remain, but the `none` option will have been eliminated. -`help`, `version`, and `completion` options default to being global. +Options default to being global. .group(key(s), groupName) -------------------- @@ -1778,12 +1772,15 @@ Specify --help for available options Specifies either a single option key (string), or an array of options. If any of the options is present, yargs validation is skipped. -.strict() +.strict([global=true]) --------- Any command-line argument given that is not demanded, or does not have a corresponding description, will be reported as an error. +`global` indicates whether or not `strict()` should be applied both +at the top-level and to sub-commands. + .string(key) ------------ diff --git a/test/command.js b/test/command.js index 4dc6f7d31..65670657e 100644 --- a/test/command.js +++ b/test/command.js @@ -951,7 +951,7 @@ describe('Command', function () { commandCalled.should.be.true }) - it('is false when passed a value of `false`', function () { + it('does not apply strict globally when passed value of `false`', function () { var commandCalled = false yargs('hi') .strict(false) @@ -959,31 +959,18 @@ describe('Command', function () { commandCalled = true innerYargs.getStrict().should.be.false }) - yargs.getStrict().should.be.false - yargs.argv // parse and run command - commandCalled.should.be.true - }) - - it('is true and global when passed a value of `true`', function () { - var commandCalled = false - yargs('hi') - .strict(true) - .command('hi', 'The hi command', function (innerYargs) { - commandCalled = true - innerYargs.getStrict().should.be.true - }) yargs.getStrict().should.be.true yargs.argv // parse and run command commandCalled.should.be.true }) - it('allows a command to override global with a value of `false`', function () { + it('applies strict globally passed a value of `true`', function () { var commandCalled = false yargs('hi') .strict(true) .command('hi', 'The hi command', function (innerYargs) { commandCalled = true - innerYargs.strict(false).getStrict().should.be.false + innerYargs.getStrict().should.be.true }) yargs.getStrict().should.be.true yargs.argv // parse and run command diff --git a/test/yargs.js b/test/yargs.js index 1f092cefc..757743c0e 100644 --- a/test/yargs.js +++ b/test/yargs.js @@ -220,7 +220,7 @@ describe('yargs dsl tests', function () { .implies('foo', 'snuh') .conflicts('qux', 'xyzzy') .group('foo', 'Group:') - .strict(true, false) + .strict(false) .exitProcess(false) // defaults to true. .global('foo', false) .global('qux', false) diff --git a/yargs.js b/yargs.js index 712721551..a79e9df37 100644 --- a/yargs.js +++ b/yargs.js @@ -665,8 +665,8 @@ function Yargs (processArgs, cwd, parentRequire) { var strict = false var strictGlobal = false - self.strict = function (_strict, _global) { - strict = _strict !== false + self.strict = function (_global) { + strict = true strictGlobal = _global !== false return self } From 0c6a1ecc03c35ddf9d5f0579011ba091df8321ba Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Sun, 22 Jan 2017 00:49:42 -0800 Subject: [PATCH 07/13] chore: moving more and more default functionality over to global --- README.md | 11 ++- lib/command.js | 33 +++++-- lib/validation.js | 16 ++-- test/command.js | 58 ++++++++++++- test/yargs.js | 1 + yargs.js | 214 +++++++++++++++++++++++----------------------- 6 files changed, 206 insertions(+), 127 deletions(-) diff --git a/README.md b/README.md index d5e6d778a..5cdcf5fa1 100644 --- a/README.md +++ b/README.md @@ -408,7 +408,7 @@ explicitly set. If `key` is an array, interpret all the elements as booleans. -.check(fn) +.check(fn, [global=true]) ---------- Check that certain conditions are met in the provided arguments. @@ -418,6 +418,9 @@ Check that certain conditions are met in the provided arguments. If `fn` throws or returns a non-truthy value, show the thrown error, usage information, and exit. +`global` indicates whether or not `check()` should be enabled both +at the top-level and for each sub-command. + .choices(key, choices) ---------------------- @@ -1285,7 +1288,7 @@ require('yargs') Outputs the same completion choices as `./test.js --foo`TAB: `--foobar` and `--foobaz` -.global(globals, [isGlobal=true]) +.global(globals, [global=true]) ------------ Indicate that an option (or group of options) should not be reset when a command @@ -1778,8 +1781,8 @@ If any of the options is present, yargs validation is skipped. Any command-line argument given that is not demanded, or does not have a corresponding description, will be reported as an error. -`global` indicates whether or not `strict()` should be applied both -at the top-level and to sub-commands. +`global` indicates whether or not `strict()` should be enabled both +at the top-level and for each sub-command. .string(key) ------------ diff --git a/lib/command.js b/lib/command.js index c1afc1c55..0a9b037e3 100644 --- a/lib/command.js +++ b/lib/command.js @@ -131,47 +131,62 @@ module.exports = function (yargs, usage, validation) { } self.runCommand = function (command, yargs, parsed) { - var argv = parsed.argv + var aliases = parsed.aliases var commandHandler = handlers[command] || handlers[aliasMap[command]] - var innerArgv = argv var currentContext = yargs.getContext() var numFiles = currentContext.files.length var parentCommands = currentContext.commands.slice() + + // what does yargs look like after the buidler is run? + var innerArgv = parsed.argv + var innerYargs = null + currentContext.commands.push(command) if (typeof commandHandler.builder === 'function') { // a function can be provided, which builds // up a yargs chain and possibly returns it. - innerArgv = commandHandler.builder(yargs.reset(parsed.aliases)) + innerYargs = commandHandler.builder(yargs.reset(parsed.aliases)) // if the builder function did not yet parse argv with reset yargs // and did not explicitly set a usage() string, then apply the // original command string as usage() for consistent behavior with - // options object below + // options object below. if (yargs.parsed === false) { if (typeof yargs.getUsageInstance().getUsage() === 'undefined') { yargs.usage('$0 ' + (parentCommands.length ? parentCommands.join(' ') + ' ' : '') + commandHandler.original) } - innerArgv = innerArgv ? innerArgv.argv : yargs.argv + innerArgv = innerYargs ? innerYargs._parseArgs(null, null, true) : yargs._parseArgs(null, null, true) } else { innerArgv = yargs.parsed.argv } + + if (innerYargs && yargs.parsed === false) aliases = innerYargs.parsed.aliases + else aliases = yargs.parsed.aliases } else if (typeof commandHandler.builder === 'object') { // as a short hand, an object can instead be provided, specifying // the options that a command takes. - innerArgv = yargs.reset(parsed.aliases) - innerArgv.usage('$0 ' + (parentCommands.length ? parentCommands.join(' ') + ' ' : '') + commandHandler.original) + innerYargs = yargs.reset(parsed.aliases) + innerYargs.usage('$0 ' + (parentCommands.length ? parentCommands.join(' ') + ' ' : '') + commandHandler.original) Object.keys(commandHandler.builder).forEach(function (key) { - innerArgv.option(key, commandHandler.builder[key]) + innerYargs.option(key, commandHandler.builder[key]) }) - innerArgv = innerArgv.argv + innerArgv = innerYargs._parseArgs(null, null, true) + aliases = innerYargs.parsed.aliases } + if (!yargs._hasOutput()) populatePositionals(commandHandler, innerArgv, currentContext, yargs) if (commandHandler.handler && !yargs._hasOutput()) { commandHandler.handler(innerArgv) } + + // we apply validation post-hoc, to so that custom + // checks get passed populated positional arguments. + yargs._runValidation(innerArgv, aliases) + currentContext.commands.pop() numFiles = currentContext.files.length - numFiles if (numFiles > 0) currentContext.files.splice(numFiles * -1, numFiles) + return innerArgv } diff --git a/lib/validation.js b/lib/validation.js index 4413037d1..2536d3e64 100644 --- a/lib/validation.js +++ b/lib/validation.js @@ -194,22 +194,26 @@ module.exports = function (yargs, usage, y18n) { // custom checks, added using the `check` option on yargs. var checks = [] - self.check = function (f) { - checks.push(f) + self.check = function (f, global) { + checks.push({ + func: f, + global: global + }) } self.customChecks = function (argv, aliases) { for (var i = 0, f; (f = checks[i]) !== undefined; i++) { + var func = f.func var result = null try { - result = f(argv, aliases) + result = func(argv, aliases) } catch (err) { usage.fail(err.message ? err.message : err, err) continue } if (!result) { - usage.fail(__('Argument check failed: %s', f.toString())) + usage.fail(__('Argument check failed: %s', func.toString())) } else if (typeof result === 'string' || result instanceof Error) { usage.fail(result.toString(), result) } @@ -333,7 +337,9 @@ module.exports = function (yargs, usage, y18n) { conflicting = objFilter(conflicting, function (k, v) { return globalLookup[k] }) - checks = [] + checks = checks.filter(function (c) { + return c.global + }) return self } diff --git a/test/command.js b/test/command.js index 65670657e..61114f2e1 100644 --- a/test/command.js +++ b/test/command.js @@ -911,6 +911,31 @@ describe('Command', function () { .conflicts('foo', 'bar') .argv }) + + it('applies custom checks globally by default', function (done) { + yargs('command blerg --foo') + .command('command ', 'a command') + .check(function (argv) { + argv.snuh.should.equal('blerg') + argv.foo.should.equal(true) + argv._.should.include('command') + done() + return true + }) + .argv + }) + + it('resets custom check if global is false', function () { + var checkCalled = false + yargs('command blerg --foo') + .command('command ', 'a command') + .check(function (argv) { + checkCalled = true + return true + }, false) + .argv + checkCalled.should.equal(false) + }) }) describe('strict', function () { @@ -977,9 +1002,38 @@ describe('Command', function () { commandCalled.should.be.true }) }) + + describe('types', function () { + it('applies array type globally', function () { + const argv = yargs('command --foo 1 2') + .command('command', 'a command') + .array('foo') + .argv + argv.foo.should.eql([1, 2]) + }) + + it('allows global setting to be disabled for array type', function () { + const argv = yargs('command --foo 1 2') + .command('command', 'a command') + .array('foo') + .global('foo', false) + .argv + argv.foo.should.eql(1) + }) + + it('applies choices type globally', function (done) { + yargs('command --foo 99') + .command('command', 'a command') + .choices('foo', [33, 88]) + .fail(function (msg) { + msg.should.match(/Choices: 33, 88/) + return done() + }) + .argv + }) + }) + // TODO: tests for: - // strict mode. - // custom checks. // aliases. // types (array, boolean, number, choices, count, nargs, normalize) // coerce. diff --git a/test/yargs.js b/test/yargs.js index 757743c0e..1f2a41845 100644 --- a/test/yargs.js +++ b/test/yargs.js @@ -1280,6 +1280,7 @@ describe('yargs dsl tests', function () { .describe('bar', 'my awesome bar option') .describe('foo', 'my awesome foo option') .global('foo') + .global('bar', false) .reset() var descriptions = y.getUsageInstance().getDescriptions() Object.keys(descriptions).should.include('foo') diff --git a/yargs.js b/yargs.js index a79e9df37..0892cfcbb 100644 --- a/yargs.js +++ b/yargs.js @@ -172,46 +172,103 @@ function Yargs (processArgs, cwd, parentRequire) { frozen = undefined } - self.boolean = function (bools) { - options.boolean.push.apply(options.boolean, [].concat(bools)) + self.boolean = function (keys) { + populateParserHintArray('boolean', keys) return self } - self.array = function (arrays) { - options.array.push.apply(options.array, [].concat(arrays)) + self.array = function (keys) { + populateParserHintArray('array', keys) return self } - self.nargs = function (key, n) { - if (typeof key === 'object') { - Object.keys(key).forEach(function (k) { - self.nargs(k, key[k]) - }) - } else { - options.narg[key] = n + self.number = function (keys) { + populateParserHintArray('number', keys) + return self + } + + self.normalize = function (keys) { + populateParserHintArray('normalize', keys) + return self + } + + self.count = function (keys) { + populateParserHintArray('count', keys) + return self + } + + self.string = function (keys) { + populateParserHintArray('string', keys) + return self + } + + self.requiresArg = function (keys) { + populateParserHintArray('requiresArg', keys) + return self + } + + self.skipValidation = function (keys) { + populateParserHintArray('skipValidation', keys) + return self + } + + function populateParserHintArray (type, keys) { + keys = [].concat(keys) + keys.forEach(function (key) { + self.global(key) + options[type].push(key) + }) + } + + /* + TODO: make sure everything goes through our setter helper. + 'config', 'demandedOptions', 'demandedCommands', 'coerce' + */ + self.nargs = function (key, value) { + populateParserHintObject(self.nargs, false, 'narg', key, value) + return self + } + + self.choices = function (key, value) { + populateParserHintObject(self.choices, true, 'choices', key, value) + return self + } + + self.alias = function (key, value) { + populateParserHintObject(self.alias, true, 'alias', key, value) + return self + } + + // The 'defaults' alias is deprecated. It will be removed in the next major version. + self.default = self.defaults = function (key, value, defaultDescription) { + if (defaultDescription) options.defaultDescription[key] = defaultDescription + if (typeof value === 'function') { + if (!options.defaultDescription[key]) options.defaultDescription[key] = usage.functionDescription(value) + value = value.call() } + populateParserHintObject(self.default, false, 'default', key, value) return self } - self.number = function (numbers) { - options.number.push.apply(options.number, [].concat(numbers)) + self.describe = function (key, desc) { + populateParserHintObject(self.describe, false, 'key', key, true) + usage.describe(key, desc) return self } - self.choices = function (key, values) { + function populateParserHintObject (builder, isArray, type, key, value) { if (typeof key === 'object') { Object.keys(key).forEach(function (k) { - self.choices(k, key[k]) + builder(k, key[k]) }) } else { - options.choices[key] = (options.choices[key] || []).concat(values) + self.global(key) + if (isArray) { + options[type][key] = (options[type][key] || []).concat(value) + } else { + options[type][key] = value + } } - return self - } - - self.normalize = function (strings) { - options.normalize.push.apply(options.normalize, [].concat(strings)) - return self } self.config = function (key, msg, parseFn) { @@ -252,39 +309,6 @@ function Yargs (processArgs, cwd, parentRequire) { return self } - self.string = function (strings) { - options.string.push.apply(options.string, [].concat(strings)) - return self - } - - // The 'defaults' alias is deprecated. It will be removed in the next major version. - self.default = self.defaults = function (key, value, defaultDescription) { - if (typeof key === 'object') { - Object.keys(key).forEach(function (k) { - self.default(k, key[k]) - }) - } else { - if (defaultDescription) options.defaultDescription[key] = defaultDescription - if (typeof value === 'function') { - if (!options.defaultDescription[key]) options.defaultDescription[key] = usage.functionDescription(value) - value = value.call() - } - options.default[key] = value - } - return self - } - - self.alias = function (x, y) { - if (typeof x === 'object') { - Object.keys(x).forEach(function (key) { - self.alias(key, x[key]) - }) - } else { - options.alias[x] = (options.alias[x] || []).concat(y) - } - return self - } - self.coerce = function (key, fn) { if (typeof key === 'object' && !Array.isArray(key)) { Object.keys(key).forEach(function (k) { @@ -298,11 +322,6 @@ function Yargs (processArgs, cwd, parentRequire) { return self } - self.count = function (counts) { - options.count.push.apply(options.count, [].concat(counts)) - return self - } - // deprecated: the demand API is too overloaded, and is being // deprecated in favor of .demandCommand() .demandOption(). self.demand = self.required = self.require = function (keys, max, msg) { @@ -377,16 +396,6 @@ function Yargs (processArgs, cwd, parentRequire) { return options.demandedCommands } - self.requiresArg = function (requiresArgs) { - options.requiresArg.push.apply(options.requiresArg, [].concat(requiresArgs)) - return self - } - - self.skipValidation = function (skipValidations) { - options.skipValidation.push.apply(options.skipValidation, [].concat(skipValidations)) - return self - } - self.implies = function (key, value) { validation.implies(key, value) return self @@ -420,27 +429,15 @@ function Yargs (processArgs, cwd, parentRequire) { return self } - self.check = function (f) { - validation.check(f) - return self - } - - self.describe = function (key, desc) { - if (typeof key === 'object') { - Object.keys(key).forEach(function (k) { - options.key[k] = true - }) - } else { - options.key[key] = true - } - usage.describe(key, desc) + self.check = function (f, _global) { + validation.check(f, _global !== false) return self } - self.global = function (globals, isGlobal) { + self.global = function (globals, global) { globals = [].concat(globals) // if isGlobal isn't provided, assume true. - if (isGlobal || typeof isGlobal === 'undefined') { + if (global !== false) { globals.forEach(function (g) { if (options.global.indexOf(g) === -1) options.global.push(g) }) @@ -507,7 +504,7 @@ function Yargs (processArgs, cwd, parentRequire) { freeze() if (parseFn) exitProcess = false - var parsed = parseArgs(args, shortCircuit) + var parsed = self._parseArgs(args, shortCircuit) if (parseFn) parseFn(exitError, parsed, output) unfreeze() @@ -578,10 +575,6 @@ function Yargs (processArgs, cwd, parentRequire) { self.group(key, opt.group) } - // options default to global, this behavior can be disabled with global: false. - const isGlobal = typeof opt.global === 'undefined' ? true : opt.global - self.global(key, isGlobal) - if (opt.boolean || opt.type === 'boolean') { self.boolean(key) if (opt.alias) self.boolean(opt.alias) @@ -606,6 +599,8 @@ function Yargs (processArgs, cwd, parentRequire) { self.count(key) } + self.global(key, opt.global !== false) + if (opt.defaultDescription) { options.defaultDescription[key] = opt.defaultDescription } @@ -665,9 +660,9 @@ function Yargs (processArgs, cwd, parentRequire) { var strict = false var strictGlobal = false - self.strict = function (_global) { + self.strict = function (global) { strict = true - strictGlobal = _global !== false + strictGlobal = global !== false return self } self.getStrict = function () { @@ -675,7 +670,7 @@ function Yargs (processArgs, cwd, parentRequire) { } self.showHelp = function (level) { - if (!self.parsed) parseArgs(processArgs) // run parser, if it has not already been executed. + if (!self.parsed) self._parseArgs(processArgs) // run parser, if it has not already been executed. usage.showHelp(level) return self } @@ -878,7 +873,7 @@ function Yargs (processArgs, cwd, parentRequire) { var args = null try { - args = parseArgs(processArgs) + args = self._parseArgs(processArgs) } catch (err) { if (err instanceof YError) usage.fail(err.message, err) else throw err @@ -889,7 +884,10 @@ function Yargs (processArgs, cwd, parentRequire) { enumerable: true }) - function parseArgs (args, shortCircuit) { + self._parseArgs = function (args, shortCircuit, _skipValidation) { + var skipValidation = !!_skipValidation + args = args || processArgs + options.__ = y18n.__ options.configuration = pkgUp()['yargs'] || {} const parsed = Parser.detailed(args, options) @@ -978,8 +976,6 @@ function Yargs (processArgs, cwd, parentRequire) { return setPlaceholderKeys(argv) } - var skipValidation = false - // Handle 'help' and 'version' options Object.keys(argv).forEach(function (key) { if (key === helpOpt && argv[key]) { @@ -1005,27 +1001,31 @@ function Yargs (processArgs, cwd, parentRequire) { } // If the help or version options where used and exitProcess is false, - // or if explicitly skipped, we won't run validations + // or if explicitly skipped, we won't run validations. if (!skipValidation) { if (parsed.error) throw new YError(parsed.error.message) // if we're executed via bash completion, don't // bother with validation. if (!argv[completion.completionKey]) { - validation.nonOptionCount(argv) - validation.missingArgumentValue(argv) - validation.requiredArguments(argv) - if (strict) validation.unknownArguments(argv, aliases) - validation.customChecks(argv, aliases) - validation.limitedChoices(argv) - validation.implications(argv) - validation.conflicting(argv) + self._runValidation(argv, aliases) } } return setPlaceholderKeys(argv) } + self._runValidation = function (argv, aliases) { + validation.nonOptionCount(argv) + validation.missingArgumentValue(argv) + validation.requiredArguments(argv) + if (strict) validation.unknownArguments(argv, aliases) + validation.customChecks(argv, aliases) + validation.limitedChoices(argv) + validation.implications(argv) + validation.conflicting(argv) + } + function guessLocale () { if (!detectLocale) return From 8de98a8a6e6ee30b793851aa18e1b93628f325cc Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Sun, 22 Jan 2017 12:23:28 -0800 Subject: [PATCH 08/13] chore: continuing to port more features over to being applied globally --- README.md | 10 +++-- test/command.js | 95 +++++++++++++++++++++++++++++++++++++++++----- test/validation.js | 10 +++++ test/yargs.js | 3 +- yargs.js | 71 ++++++++++++++-------------------- 5 files changed, 132 insertions(+), 57 deletions(-) diff --git a/README.md b/README.md index 5cdcf5fa1..40e9d4a1b 100644 --- a/README.md +++ b/README.md @@ -673,7 +673,7 @@ require('yargs') console.log(`setting ${argv.key} to ${argv.value}`) } }) - .demandCommand(1) + .demandCommand() .help() .wrap(72) .argv @@ -819,7 +819,7 @@ cli.js: #!/usr/bin/env node require('yargs') .commandDir('cmds') - .demandCommand(1) + .demandCommand() .help() .argv ``` @@ -1107,9 +1107,9 @@ Options: Missing required arguments: run, path ``` -.demandCommand(min, [minMsg]) +.demandCommand([min=1], [minMsg]) ------------------------------ -.demandCommand(min, [max], [minMsg], [maxMsg]) +.demandCommand([min=1], [max], [minMsg], [maxMsg]) ------------------------------ Demand in context of commands. You can demand a minimum and a maximum number a user can have within your program, as well as provide corresponding error messages if either of the demands is not met. @@ -1129,7 +1129,9 @@ require('yargs') .help() .argv ``` + which will provide the following output: + ```bash Commands: configure [value] Set a config variable [aliases: config, cfg] diff --git a/test/command.js b/test/command.js index 61114f2e1..f9b69c1ed 100644 --- a/test/command.js +++ b/test/command.js @@ -840,9 +840,7 @@ describe('Command', function () { }) }) - // make sure yargs parsing features configured on - // the top-level apply appropriately to commands. - describe('global', function () { + describe('global parsing hints', function () { describe('config', function () { it('does not load config for command if global is false', function (done) { yargs('command --foo ./package.json') @@ -936,6 +934,17 @@ describe('Command', function () { .argv checkCalled.should.equal(false) }) + + it('applies demandOption globally', function (done) { + yargs('command blerg --foo') + .command('command ', 'a command') + .fail(function (msg) { + msg.should.match(/Missing required argument: bar/) + return done() + }) + .demandOption('bar') + .argv + }) }) describe('strict', function () { @@ -1033,14 +1042,80 @@ describe('Command', function () { }) }) + describe('aliases', function () { + it('defaults to applying aliases globally', function (done) { + yargs('command blerg --foo 22') + .command('command ', 'a command', {}, function (argv) { + argv.foo.should.equal(22) + argv.bar.should.equal(22) + argv.snuh.should.equal('blerg') + return done() + }) + .alias('foo', 'bar') + .argv + }) + + it('allows global application of alias to be disabled', function (done) { + yargs('command blerg --foo 22') + .command('command ', 'a command', {}, function (argv) { + argv.foo.should.equal(22) + expect(argv.bar).to.equal(undefined) + argv.snuh.should.equal('blerg') + return done() + }) + .option('foo', { + alias: 'bar', + global: false + }) + .argv + }) + }) + + describe('coerce', function () { + it('defaults to applying coerce rules globally', function (done) { + yargs('command blerg --foo 22') + .command('command ', 'a command', {}, function (argv) { + argv.foo.should.equal(44) + argv.snuh.should.equal('blerg') + return done() + }) + .coerce('foo', function (arg) { + return arg * 2 + }) + .argv + }) + }) + + describe('defaults', function () { + it('defaults to applying defaults globally', function (done) { + yargs('command --foo 22') + .command('command [snuh]', 'a command', {}, function (argv) { + argv.foo.should.equal(22) + argv.snuh.should.equal(55) + return done() + }) + .default('snuh', 55) + .argv + }) + }) + + describe('describe', function () { + it('sets description for command if described at top-level', function (done) { + yargs() + .command('command [snuh]', 'a command') + .describe('foo', 'an awesome argument') + .help() + .parse('command --help', function (err, argv, output) { + if (err) return done(err) + output.should.not.match(/Commands:/) + output.should.match(/an awesome argument/) + return done() + }) + }) + }) + // TODO: tests for: - // aliases. - // types (array, boolean, number, choices, count, nargs, normalize) - // coerce. - // defaults. - // demand (demandOption, demandCommand). - // describe. - // groups. // help, version. + // groups. }) }) diff --git a/test/validation.js b/test/validation.js index f08080a23..a831dea71 100644 --- a/test/validation.js +++ b/test/validation.js @@ -589,5 +589,15 @@ describe('validation tests', function () { }) .argv }) + + it('defaults to demanding 1 command', function (done) { + yargs('-a 10') + .demandCommand() + .fail(function (msg) { + msg.should.equal('Not enough non-option arguments: got 0, need at least 1') + return done() + }) + .argv + }) }) }) diff --git a/test/yargs.js b/test/yargs.js index 1f2a41845..e38a28ca7 100644 --- a/test/yargs.js +++ b/test/yargs.js @@ -263,9 +263,10 @@ describe('yargs dsl tests', function () { expect(y.getGroups()).to.deep.equal({}) }) - it('does not invoke parse with an error if reset has been called', function (done) { + it('does not invoke parse with an error if reset has been called and option is not global', function (done) { var y = yargs() .demand('cake') + .global('cake', false) y.parse('hello', function (err) { err.message.should.match(/Missing required argument/) diff --git a/yargs.js b/yargs.js index 0892cfcbb..b41f99b0c 100644 --- a/yargs.js +++ b/yargs.js @@ -212,7 +212,7 @@ function Yargs (processArgs, cwd, parentRequire) { return self } - function populateParserHintArray (type, keys) { + function populateParserHintArray (type, keys, value) { keys = [].concat(keys) keys.forEach(function (key) { self.global(key) @@ -220,10 +220,6 @@ function Yargs (processArgs, cwd, parentRequire) { }) } - /* - TODO: make sure everything goes through our setter helper. - 'config', 'demandedOptions', 'demandedCommands', 'coerce' - */ self.nargs = function (key, value) { populateParserHintObject(self.nargs, false, 'narg', key, value) return self @@ -239,7 +235,7 @@ function Yargs (processArgs, cwd, parentRequire) { return self } - // The 'defaults' alias is deprecated. It will be removed in the next major version. + // TODO: actually deprecate self.defaults. self.default = self.defaults = function (key, value, defaultDescription) { if (defaultDescription) options.defaultDescription[key] = defaultDescription if (typeof value === 'function') { @@ -256,12 +252,32 @@ function Yargs (processArgs, cwd, parentRequire) { return self } + self.demandOption = function (keys, msg) { + var value = { msg: typeof msg === 'string' ? msg : undefined } + populateParserHintObject(self.demandOption, false, 'demandedOptions', keys, value) + return self + } + + self.coerce = function (keys, value) { + populateParserHintObject(self.coerce, false, 'coerce', keys, value) + return self + } + function populateParserHintObject (builder, isArray, type, key, value) { - if (typeof key === 'object') { + if (Array.isArray(key)) { + // an array of keys with one value ['x', 'y', 'z'], function parse () {} + var temp = {} + key.forEach(function (k) { + temp[k] = value + }) + builder(temp) + } else if (typeof key === 'object') { + // an object of key value pairs: {'x': parse () {}, 'y': parse() {}} Object.keys(key).forEach(function (k) { builder(k, key[k]) }) } else { + // a single key value pair 'x', parse() {} self.global(key) if (isArray) { options[type][key] = (options[type][key] || []).concat(value) @@ -272,20 +288,19 @@ function Yargs (processArgs, cwd, parentRequire) { } self.config = function (key, msg, parseFn) { - // allow to pass a configuration object + // allow a config object to be provided directly. if (typeof key === 'object') { options.configObjects = (options.configObjects || []).concat(key) return self } - // allow to provide a parsing function + // allow for a custom parsing function. if (typeof msg === 'function') { parseFn = msg msg = null } key = key || 'config' - self.global(key) self.describe(key, msg || usage.deferY18nLookup('Path to JSON config file')) ;(Array.isArray(key) ? key : [key]).forEach(function (k) { options.config[k] = parseFn || true @@ -309,21 +324,8 @@ function Yargs (processArgs, cwd, parentRequire) { return self } - self.coerce = function (key, fn) { - if (typeof key === 'object' && !Array.isArray(key)) { - Object.keys(key).forEach(function (k) { - self.coerce(k, key[k]) - }) - } else { - [].concat(key).forEach(function (k) { - options.coerce[k] = fn - }) - } - return self - } - - // deprecated: the demand API is too overloaded, and is being - // deprecated in favor of .demandCommand() .demandOption(). + // TODO: deprecate self.demand in favor of + // .demandCommand() .demandOption(). self.demand = self.required = self.require = function (keys, max, msg) { // you can optionally provide a 'max' key, // which will raise an exception if too many '_' @@ -355,24 +357,9 @@ function Yargs (processArgs, cwd, parentRequire) { return self } - self.demandOption = function (key, msg) { - if (Array.isArray(key)) { - key.forEach(function (key) { - self.demandOption(key, msg) - }) - } else { - if (typeof msg === 'string') { - options.demandedOptions[key] = { msg: msg } - // allow edge-case of options: {a: {demand: true}, b: {demand: false}} - } else if (msg === true || typeof msg === 'undefined') { - options.demandedOptions[key] = { msg: undefined } - } - } - - return self - } - self.demandCommand = function (min, max, minMsg, maxMsg) { + if (typeof min === 'undefined') min = 1 + if (typeof max !== 'number') { minMsg = max max = Infinity From 3d7759a6ea1fb6551085af5180ee283ea809236f Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Sun, 22 Jan 2017 21:18:34 -0800 Subject: [PATCH 09/13] chore: added more tests for global behavior --- test/command.js | 85 +++++++++++++++++++++++++++++-------------------- yargs.js | 3 +- 2 files changed, 52 insertions(+), 36 deletions(-) diff --git a/test/command.js b/test/command.js index f9b69c1ed..ebdf881bc 100644 --- a/test/command.js +++ b/test/command.js @@ -876,14 +876,14 @@ describe('Command', function () { .argv }) - it('applies implies logic for command by default', function (done) { - yargs('command --foo 99') + it('applies conflicts logic for command by default', function (done) { + yargs('command --foo --bar') .command('command', 'a command', {}, function (argv) {}) .fail(function (msg) { - msg.should.match(/foo -> bar/) + msg.should.match(/mutually exclusive/) return done() }) - .implies('foo', 'bar') + .conflicts('foo', 'bar') .argv }) @@ -899,17 +899,6 @@ describe('Command', function () { .argv }) - it('applies conflicts logic for command by default', function (done) { - yargs('command --foo --bar') - .command('command', 'a command', {}, function (argv) {}) - .fail(function (msg) { - msg.should.match(/mutually exclusive/) - return done() - }) - .conflicts('foo', 'bar') - .argv - }) - it('applies custom checks globally by default', function (done) { yargs('command blerg --foo') .command('command ', 'a command') @@ -972,7 +961,7 @@ describe('Command', function () { commandCalled.should.be.true }) - it('is true and global when called without arguments', function () { + it('applies strict globally by default', function () { var commandCalled = false yargs('hi') .strict() @@ -997,19 +986,6 @@ describe('Command', function () { yargs.argv // parse and run command commandCalled.should.be.true }) - - it('applies strict globally passed a value of `true`', function () { - var commandCalled = false - yargs('hi') - .strict(true) - .command('hi', 'The hi command', function (innerYargs) { - commandCalled = true - innerYargs.getStrict().should.be.true - }) - yargs.getStrict().should.be.true - yargs.argv // parse and run command - commandCalled.should.be.true - }) }) describe('types', function () { @@ -1087,7 +1063,7 @@ describe('Command', function () { }) describe('defaults', function () { - it('defaults to applying defaults globally', function (done) { + it('applies defaults globally', function (done) { yargs('command --foo 22') .command('command [snuh]', 'a command', {}, function (argv) { argv.foo.should.equal(22) @@ -1100,7 +1076,7 @@ describe('Command', function () { }) describe('describe', function () { - it('sets description for command if described at top-level', function (done) { + it('flags an option as global if a description is set', function (done) { yargs() .command('command [snuh]', 'a command') .describe('foo', 'an awesome argument') @@ -1114,8 +1090,49 @@ describe('Command', function () { }) }) - // TODO: tests for: - // help, version. - // groups. + describe('help', function () { + it('applies help globally', function (done) { + yargs() + .command('command [snuh]', 'a command') + .describe('foo', 'an awesome argument') + .help('hellllllp') + .parse('command --hellllllp', function (err, argv, output) { + if (err) return done(err) + output.should.match(/--hellllllp {2}Show help/) + return done() + }) + }) + }) + + describe('version', function () { + it('applies version globally', function (done) { + yargs() + .command('command [snuh]', 'a command') + .describe('foo', 'an awesome argument') + .version('ver', 'show version', '9.9.9') + .parse('command --ver', function (err, argv, output) { + if (err) return done(err) + output.should.equal('9.9.9') + return done() + }) + }) + }) + + describe('groups', function () { + it('should apply custom option groups globally', function (done) { + yargs() + .command('command [snuh]', 'a command') + .group('foo', 'Bad Variable Names:') + .group('snuh', 'Bad Variable Names:') + .describe('foo', 'foo option') + .describe('snuh', 'snuh positional') + .help() + .parse('command --help', function (err, argv, output) { + if (err) return done(err) + output.should.match(/Bad Variable Names:\W*--foo/) + return done() + }) + }) + }) }) }) diff --git a/yargs.js b/yargs.js index b41f99b0c..c01e53d6e 100644 --- a/yargs.js +++ b/yargs.js @@ -615,8 +615,7 @@ function Yargs (processArgs, cwd, parentRequire) { self.group = function (opts, groupName) { var existing = preservedGroups[groupName] || groups[groupName] if (preservedGroups[groupName]) { - // the preserved group will be moved to the set of explicitly declared - // groups + // we now only need to track this group name in groups. delete preservedGroups[groupName] } From bae1d2d39d22fae86d843db97f2fa743ec87eff9 Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Sun, 22 Jan 2017 21:26:54 -0800 Subject: [PATCH 10/13] nit: a parenthesis somehow snuck in --- test/yargs.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/yargs.js b/test/yargs.js index e38a28ca7..2a16b397f 100644 --- a/test/yargs.js +++ b/test/yargs.js @@ -766,7 +766,7 @@ describe('yargs dsl tests', function () { }) }) - // yargs.parse(['foo', '--bar'], function (err, argv, output) {}) + // yargs.parse(['foo', '--bar'], function (err, argv, output) {} context('function passed as second argument to parse', function () { it('does not print to stdout', function () { var r = checkOutput(function () { From 4cff044f8e15dc6d9b08b772d0401327b4a058a9 Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Sun, 22 Jan 2017 21:30:42 -0800 Subject: [PATCH 11/13] nit: comment didn't add much --- yargs.js | 1 - 1 file changed, 1 deletion(-) diff --git a/yargs.js b/yargs.js index c01e53d6e..6db4d8ebd 100644 --- a/yargs.js +++ b/yargs.js @@ -423,7 +423,6 @@ function Yargs (processArgs, cwd, parentRequire) { self.global = function (globals, global) { globals = [].concat(globals) - // if isGlobal isn't provided, assume true. if (global !== false) { globals.forEach(function (g) { if (options.global.indexOf(g) === -1) options.global.push(g) From 5e6a042b95d02640b3ebc9bc1c31b4f69ab4231b Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Sat, 28 Jan 2017 15:34:07 -0800 Subject: [PATCH 12/13] chore: address @nexdrew's code-review, switching to tracking locals rather than globals --- lib/usage.js | 4 ++-- lib/validation.js | 6 +++--- test/usage.js | 1 + test/yargs.js | 12 ++++++++---- yargs.js | 46 +++++++++++++++++++++++----------------------- 5 files changed, 37 insertions(+), 32 deletions(-) diff --git a/lib/usage.js b/lib/usage.js index 2e503acfa..0efabe868 100644 --- a/lib/usage.js +++ b/lib/usage.js @@ -436,7 +436,7 @@ module.exports = function (yargs, y18n) { else logger.log(version) } - self.reset = function (globalLookup) { + self.reset = function (localLookup) { // do not reset wrap here // do not reset fails here failMessage = null @@ -446,7 +446,7 @@ module.exports = function (yargs, y18n) { examples = [] commands = [] descriptions = objFilter(descriptions, function (k, v) { - return globalLookup[k] + return !localLookup[k] }) return self } diff --git a/lib/validation.js b/lib/validation.js index 2536d3e64..475ee3180 100644 --- a/lib/validation.js +++ b/lib/validation.js @@ -330,12 +330,12 @@ module.exports = function (yargs, usage, y18n) { if (recommended) usage.fail(__('Did you mean %s?', recommended)) } - self.reset = function (globalLookup) { + self.reset = function (localLookup) { implied = objFilter(implied, function (k, v) { - return globalLookup[k] + return !localLookup[k] }) conflicting = objFilter(conflicting, function (k, v) { - return globalLookup[k] + return !localLookup[k] }) checks = checks.filter(function (c) { return c.global diff --git a/test/usage.js b/test/usage.js index 1d9642b61..87635aa21 100644 --- a/test/usage.js +++ b/test/usage.js @@ -93,6 +93,7 @@ describe('usage tests', function () { .wrap(null) .argv }) + r.result.should.have.property('w', 10) r.result.should.have.property('m', 10) r.result.should.have.property('_').with.length(1) diff --git a/test/yargs.js b/test/yargs.js index 2a16b397f..5d5771df8 100644 --- a/test/yargs.js +++ b/test/yargs.js @@ -246,9 +246,13 @@ describe('yargs dsl tests', function () { config: {}, configObjects: [], envPrefix: 'YARGS', // preserved as global - global: ['help'], demandedCommands: {}, - demandedOptions: {} + demandedOptions: {}, + local: [ + '_', + 'foo', + 'qux' + ] } expect(y.getOptions()).to.deep.equal(emptyOptions) @@ -1266,14 +1270,14 @@ describe('yargs dsl tests', function () { var y = yargs('--foo') .help('help') var options = y.getOptions() - options.global.should.include('help') + options.local.should.not.include('help') }) it('should set version to global option by default', function () { var y = yargs('--foo') .version() var options = y.getOptions() - options.global.should.include('version') + options.local.should.not.include('version') }) it('should not reset usage descriptions of global options', function () { diff --git a/yargs.js b/yargs.js index 6db4d8ebd..6e9ea9093 100644 --- a/yargs.js +++ b/yargs.js @@ -66,23 +66,23 @@ function Yargs (processArgs, cwd, parentRequire) { // logic is used to build a nested command // hierarchy. var tmpOptions = {} - tmpOptions.global = options.global ? options.global : [] + tmpOptions.local = options.local ? options.local : [] tmpOptions.configObjects = options.configObjects ? options.configObjects : [] - // if a key has been set as a global, we - // do not want to reset it or its aliases. - var globalLookup = {} - tmpOptions.global.forEach(function (g) { - globalLookup[g] = true - ;(aliases[g] || []).forEach(function (a) { - globalLookup[a] = true + // if a key has been explicitly set as local, + // we should reset it before passing options to command. + var localLookup = {} + tmpOptions.local.forEach(function (l) { + localLookup[l] = true + ;(aliases[l] || []).forEach(function (a) { + localLookup[a] = true }) }) - // preserve groups containing global keys + // preserve all groups not set to local. preservedGroups = Object.keys(groups).reduce(function (acc, groupName) { var keys = groups[groupName].filter(function (key) { - return key in globalLookup + return !(key in localLookup) }) if (keys.length > 0) { acc[groupName] = keys @@ -104,13 +104,13 @@ function Yargs (processArgs, cwd, parentRequire) { arrayOptions.forEach(function (k) { tmpOptions[k] = (options[k] || []).filter(function (k) { - return globalLookup[k] + return !localLookup[k] }) }) objectOptions.forEach(function (k) { tmpOptions[k] = objFilter(options[k], function (k, v) { - return globalLookup[k] + return !localLookup[k] }) }) @@ -119,8 +119,8 @@ function Yargs (processArgs, cwd, parentRequire) { // if this is the first time being executed, create // instances of all our helpers -- otherwise just reset. - usage = usage ? usage.reset(globalLookup) : Usage(self, y18n) - validation = validation ? validation.reset(globalLookup) : Validation(self, usage, y18n) + usage = usage ? usage.reset(localLookup) : Usage(self, y18n) + validation = validation ? validation.reset(localLookup) : Validation(self, usage, y18n) command = command ? command.reset() : Command(self, usage, validation) if (!completion) completion = Completion(self, usage, command) @@ -215,7 +215,6 @@ function Yargs (processArgs, cwd, parentRequire) { function populateParserHintArray (type, keys, value) { keys = [].concat(keys) keys.forEach(function (key) { - self.global(key) options[type].push(key) }) } @@ -278,7 +277,6 @@ function Yargs (processArgs, cwd, parentRequire) { }) } else { // a single key value pair 'x', parse() {} - self.global(key) if (isArray) { options[type][key] = (options[type][key] || []).concat(value) } else { @@ -365,6 +363,8 @@ function Yargs (processArgs, cwd, parentRequire) { max = Infinity } + self.global('_', false) + options.demandedCommands._ = { min: min, max: max, @@ -424,12 +424,12 @@ function Yargs (processArgs, cwd, parentRequire) { self.global = function (globals, global) { globals = [].concat(globals) if (global !== false) { - globals.forEach(function (g) { - if (options.global.indexOf(g) === -1) options.global.push(g) + options.local = options.local.filter(function (l) { + return globals.indexOf(l) === -1 }) } else { - options.global = options.global.filter(function (g) { - return globals.indexOf(g) === -1 + globals.forEach(function (g) { + if (options.local.indexOf(g) === -1) options.local.push(g) }) } return self @@ -585,7 +585,9 @@ function Yargs (processArgs, cwd, parentRequire) { self.count(key) } - self.global(key, opt.global !== false) + if (typeof opt.global === 'boolean') { + self.global(key, opt.global) + } if (opt.defaultDescription) { options.defaultDescription[key] = opt.defaultDescription @@ -677,7 +679,6 @@ function Yargs (processArgs, cwd, parentRequire) { usage.version(ver || undefined) self.boolean(versionOpt) - self.global(versionOpt) self.describe(versionOpt, msg) return self } @@ -714,7 +715,6 @@ function Yargs (processArgs, cwd, parentRequire) { // use arguments, fallback to defaults for opt and msg helpOpt = opt || 'help' self.boolean(helpOpt) - self.global(helpOpt) self.describe(helpOpt, msg || usage.deferY18nLookup('Show help')) return self } From dadd3008e5234678d4a0edc1780fbafcd8dacf7e Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Sat, 28 Jan 2017 15:46:15 -0800 Subject: [PATCH 13/13] docs: fix up a couple more doc nits based on code-review --- README.md | 4 ++-- lib/command.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 40e9d4a1b..c4c006c9c 100644 --- a/README.md +++ b/README.md @@ -418,7 +418,7 @@ Check that certain conditions are met in the provided arguments. If `fn` throws or returns a non-truthy value, show the thrown error, usage information, and exit. -`global` indicates whether or not `check()` should be enabled both +`global` indicates whether `check()` should be enabled both at the top-level and for each sub-command. .choices(key, choices) @@ -1783,7 +1783,7 @@ If any of the options is present, yargs validation is skipped. Any command-line argument given that is not demanded, or does not have a corresponding description, will be reported as an error. -`global` indicates whether or not `strict()` should be enabled both +`global` indicates whether `strict()` should be enabled both at the top-level and for each sub-command. .string(key) diff --git a/lib/command.js b/lib/command.js index 0a9b037e3..4b4cbeaaa 100644 --- a/lib/command.js +++ b/lib/command.js @@ -179,7 +179,7 @@ module.exports = function (yargs, usage, validation) { commandHandler.handler(innerArgv) } - // we apply validation post-hoc, to so that custom + // we apply validation post-hoc, so that custom // checks get passed populated positional arguments. yargs._runValidation(innerArgv, aliases)