From f3f955006019a4269bebfb52d3e51f282cc43438 Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Sat, 25 Feb 2017 12:22:15 -0800 Subject: [PATCH 1/4] chore: add failing test for #794 --- test/command.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/command.js b/test/command.js index 8c6df70bb..1e7fd6bc5 100644 --- a/test/command.js +++ b/test/command.js @@ -1100,6 +1100,20 @@ describe('Command', function () { }) .argv }) + + // addresses https://github.com/yargs/yargs/issues/794 + it('should bubble errors thrown by coerce function inside commands', function (done) { + yargs + .command('foo', 'the foo command', (yargs) => { + yargs.coerce('x', function (arg) { + throw Error('yikes an error') + }) + }) + .parse('foo -x 99', function (err) { + err.message.should.match(/yikes an error/) + return done() + }) + }) }) describe('defaults', function () { From e47f4f092e8dff32ad9db6f5ad51096bc9451bbb Mon Sep 17 00:00:00 2001 From: Daniel Zou Date: Sat, 25 Feb 2017 20:36:12 -0600 Subject: [PATCH 2/4] fix: coerce function errors are no longer suppressed and YErrors are now caught --- lib/command.js | 2 +- yargs.js | 205 ++++++++++++++++++++++++------------------------- 2 files changed, 100 insertions(+), 107 deletions(-) diff --git a/lib/command.js b/lib/command.js index 89dadc1f9..abc92c29e 100644 --- a/lib/command.js +++ b/lib/command.js @@ -180,7 +180,7 @@ module.exports = function (yargs, usage, validation) { // we apply validation post-hoc, so that custom // checks get passed populated positional arguments. - yargs._runValidation(innerArgv, aliases, positionalMap) + yargs._runValidation(innerArgv, aliases, positionalMap, yargs.parsed.error) if (commandHandler.handler && !yargs._hasOutput()) { commandHandler.handler(innerArgv) diff --git a/yargs.js b/yargs.js index a1e351f76..1427c9e2e 100644 --- a/yargs.js +++ b/yargs.js @@ -537,10 +537,6 @@ function Yargs (processArgs, cwd, parentRequire) { return parsed } - self._getParseContext = function () { - return parseContext || {} - } - self._hasParseCallback = function () { return !!parseFn } @@ -923,16 +919,7 @@ function Yargs (processArgs, cwd, parentRequire) { Object.defineProperty(self, 'argv', { get: function () { - var args = null - - try { - args = self._parseArgs(processArgs) - } catch (err) { - if (err instanceof YError) usage.fail(err.message, err) - else throw err - } - - return args + return self._parseArgs(processArgs) }, enumerable: true }) @@ -951,124 +938,130 @@ function Yargs (processArgs, cwd, parentRequire) { argv.$0 = self.$0 self.parsed = parsed - guessLocale() // guess locale lazily, so that it can be turned off in chain. + try { + guessLocale() // guess locale lazily, so that it can be turned off in chain. - // while building up the argv object, there - // are two passes through the parser. If completion - // is being performed short-circuit on the first pass. - if (shortCircuit) { - return argv - } + // while building up the argv object, there + // are two passes through the parser. If completion + // is being performed short-circuit on the first pass. + if (shortCircuit) { + return argv + } - if (argv._.length) { - // check for helpOpt in argv._ before running commands - // assumes helpOpt must be valid if useHelpOptAsCommand is true - if (useHelpOptAsCommand) { - // consider any multi-char helpOpt alias as a valid help command - // unless all helpOpt aliases are single-char - // note that parsed.aliases is a normalized bidirectional map :) - var helpCmds = [helpOpt].concat(aliases[helpOpt] || []) - var multiCharHelpCmds = helpCmds.filter(function (k) { - return k.length > 1 - }) - if (multiCharHelpCmds.length) helpCmds = multiCharHelpCmds - // look for and strip any helpCmds from argv._ - argv._ = argv._.filter(function (cmd) { - if (~helpCmds.indexOf(cmd)) { - argv[helpOpt] = true - return false + if (argv._.length) { + // check for helpOpt in argv._ before running commands + // assumes helpOpt must be valid if useHelpOptAsCommand is true + if (useHelpOptAsCommand) { + // consider any multi-char helpOpt alias as a valid help command + // unless all helpOpt aliases are single-char + // note that parsed.aliases is a normalized bidirectional map :) + var helpCmds = [helpOpt].concat(aliases[helpOpt] || []) + var multiCharHelpCmds = helpCmds.filter(function (k) { + return k.length > 1 + }) + if (multiCharHelpCmds.length) helpCmds = multiCharHelpCmds + // look for and strip any helpCmds from argv._ + argv._ = argv._.filter(function (cmd) { + if (~helpCmds.indexOf(cmd)) { + argv[helpOpt] = true + return false + } + return true + }) + } + + // if there's a handler associated with a + // command defer processing to it. + var handlerKeys = command.getCommands() + if (handlerKeys.length) { + var firstUnknownCommand + for (var i = 0, cmd; (cmd = argv._[i]) !== undefined; i++) { + if (~handlerKeys.indexOf(cmd) && cmd !== completionCommand) { + setPlaceholderKeys(argv) + return command.runCommand(cmd, self, parsed) + } else if (!firstUnknownCommand && cmd !== completionCommand) { + firstUnknownCommand = cmd + } } - return true - }) - } - // if there's a handler associated with a - // command defer processing to it. - var handlerKeys = command.getCommands() - if (handlerKeys.length) { - var firstUnknownCommand - for (var i = 0, cmd; (cmd = argv._[i]) !== undefined; i++) { - if (~handlerKeys.indexOf(cmd) && cmd !== completionCommand) { - setPlaceholderKeys(argv) - return command.runCommand(cmd, self, parsed) - } else if (!firstUnknownCommand && cmd !== completionCommand) { - firstUnknownCommand = cmd + // recommend a command if recommendCommands() has + // been enabled, and no commands were found to execute + if (recommendCommands && firstUnknownCommand) { + validation.recommendCommands(firstUnknownCommand, handlerKeys) } } - // recommend a command if recommendCommands() has - // been enabled, and no commands were found to execute - if (recommendCommands && firstUnknownCommand) { - validation.recommendCommands(firstUnknownCommand, handlerKeys) + // generate a completion script for adding to ~/.bashrc. + if (completionCommand && ~argv._.indexOf(completionCommand) && !argv[completion.completionKey]) { + if (exitProcess) setBlocking(true) + self.showCompletionScript() + self.exit(0) } } - // generate a completion script for adding to ~/.bashrc. - if (completionCommand && ~argv._.indexOf(completionCommand) && !argv[completion.completionKey]) { + // we must run completions first, a user might + // want to complete the --help or --version option. + if (completion.completionKey in argv) { if (exitProcess) setBlocking(true) - self.showCompletionScript() - self.exit(0) - } - } - // we must run completions first, a user might - // want to complete the --help or --version option. - if (completion.completionKey in argv) { - if (exitProcess) setBlocking(true) - - // we allow for asynchronous completions, - // e.g., loading in a list of commands from an API. - var completionArgs = args.slice(args.indexOf('--' + completion.completionKey) + 1) - completion.getCompletion(completionArgs, function (completions) { - ;(completions || []).forEach(function (completion) { - _logger.log(completion) + // we allow for asynchronous completions, + // e.g., loading in a list of commands from an API. + var completionArgs = args.slice(args.indexOf('--' + completion.completionKey) + 1) + completion.getCompletion(completionArgs, function (completions) { + ;(completions || []).forEach(function (completion) { + _logger.log(completion) + }) + + self.exit(0) }) + return setPlaceholderKeys(argv) + } - self.exit(0) - }) - return setPlaceholderKeys(argv) - } + // Handle 'help' and 'version' options + Object.keys(argv).forEach(function (key) { + if (key === helpOpt && argv[key]) { + if (exitProcess) setBlocking(true) - // Handle 'help' and 'version' options - Object.keys(argv).forEach(function (key) { - if (key === helpOpt && argv[key]) { - if (exitProcess) setBlocking(true) + skipValidation = true + self.showHelp('log') + self.exit(0) + } else if (key === versionOpt && argv[key]) { + if (exitProcess) setBlocking(true) - skipValidation = true - self.showHelp('log') - self.exit(0) - } else if (key === versionOpt && argv[key]) { - if (exitProcess) setBlocking(true) + skipValidation = true + usage.showVersion() + self.exit(0) + } + }) - skipValidation = true - usage.showVersion() - self.exit(0) + // Check if any of the options to skip validation were provided + if (!skipValidation && options.skipValidation.length > 0) { + skipValidation = Object.keys(argv).some(function (key) { + return options.skipValidation.indexOf(key) >= 0 && argv[key] === true + }) } - }) - // Check if any of the options to skip validation were provided - if (!skipValidation && options.skipValidation.length > 0) { - skipValidation = Object.keys(argv).some(function (key) { - return options.skipValidation.indexOf(key) >= 0 && argv[key] === true - }) - } + // If the help or version options where used and exitProcess is false, + // or if explicitly skipped, we won't run validations. + if (!skipValidation) { + if (parsed.error) throw new YError(parsed.error.message) - // If the help or version options where used and exitProcess is false, - // 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]) { - self._runValidation(argv, aliases, {}) + // if we're executed via bash completion, don't + // bother with validation. + if (!argv[completion.completionKey]) { + self._runValidation(argv, aliases, {}, parsed.error) + } } + } catch (err) { + if (err instanceof YError) usage.fail(err.message, err) + else throw err } return setPlaceholderKeys(argv) } - self._runValidation = function (argv, aliases, positionalMap) { + self._runValidation = function (argv, aliases, positionalMap, parseErrors) { + if (parseErrors) throw new YError(parseErrors.message) validation.nonOptionCount(argv) validation.missingArgumentValue(argv) validation.requiredArguments(argv) From e8e931dc3bdb3e09d0d7901530dc1bbe156e523b Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Sat, 25 Feb 2017 22:39:20 -0800 Subject: [PATCH 3/4] fix: address merge conflict --- yargs.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/yargs.js b/yargs.js index 1427c9e2e..f3cad8d7d 100644 --- a/yargs.js +++ b/yargs.js @@ -537,6 +537,10 @@ function Yargs (processArgs, cwd, parentRequire) { return parsed } + self._getParseContext = function () { + return parseContext || {} + } + self._hasParseCallback = function () { return !!parseFn } From 429ae2d403683cab563e40fdf646631886bdb138 Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Sat, 25 Feb 2017 22:59:46 -0800 Subject: [PATCH 4/4] fix: whoops, not quite ready to drop 0.10 yet --- test/command.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/command.js b/test/command.js index 1e7fd6bc5..058e2a1cb 100644 --- a/test/command.js +++ b/test/command.js @@ -1104,7 +1104,7 @@ describe('Command', function () { // addresses https://github.com/yargs/yargs/issues/794 it('should bubble errors thrown by coerce function inside commands', function (done) { yargs - .command('foo', 'the foo command', (yargs) => { + .command('foo', 'the foo command', function (yargs) { yargs.coerce('x', function (arg) { throw Error('yikes an error') })