From 0b47fdbbfba506b8a809ee3feabd900e493eaa4e Mon Sep 17 00:00:00 2001 From: bcoe Date: Sun, 6 Dec 2020 13:16:13 -0800 Subject: [PATCH 01/15] feat!: improve support for async/await --- docs/advanced.md | 13 ------- docs/api.md | 17 --------- lib/command.ts | 20 ++++------- lib/yargs-factory.ts | 15 -------- test/command.cjs | 27 +++++++-------- test/middleware.cjs | 2 ++ test/yargs.cjs | 82 +++++++++++++++++++++++--------------------- 7 files changed, 63 insertions(+), 113 deletions(-) diff --git a/docs/advanced.md b/docs/advanced.md index da5c1d9ca..25792947c 100644 --- a/docs/advanced.md +++ b/docs/advanced.md @@ -484,19 +484,6 @@ yargs.parserConfiguration({ See the [yargs-parser](https://github.com/yargs/yargs-parser#configuration) module for detailed documentation of this feature. -## Command finish hook -### Example -```js -yargs(process.argv.slice(2)) - .command('cmd', 'a command', () => {}, async () => { - await this.model.find() - return Promise.resolve('result value') - }) - .onFinishCommand(async (resultValue) => { - await this.db.disconnect() - }).argv -``` - ## Middleware Sometimes you might want to transform arguments before they reach the command handler. diff --git a/docs/api.md b/docs/api.md index 1f88a0c07..a278e2e7e 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1165,23 +1165,6 @@ var argv = require('yargs/yargs')(process.argv.slice(2)) .argv ``` -.onFinishCommand([handler]) ------------- - -Called after the completion of any command. `handler` is invoked with the -result returned by the command: - -```js -yargs(process.argv.slice(2)) - .command('cmd', 'a command', () => {}, async () => { - await this.model.find() - return Promise.resolve('result value') - }) - .onFinishCommand(async (resultValue) => { - await this.db.disconnect() - }).argv -``` - .option(key, [opt]) ----------------- .options(key, [opt]) diff --git a/lib/command.ts b/lib/command.ts index 781812268..e6c3a16e8 100644 --- a/lib/command.ts +++ b/lib/command.ts @@ -307,17 +307,17 @@ export function command( handlerResult = innerArgv.then(argv => commandHandler.handler(argv)); } else { handlerResult = commandHandler.handler(innerArgv); + if (isPromise(handlerResult) && !isPromise(innerArgv)) { + const innerArgvRef = innerArgv; + innerArgv = handlerResult.then(() => { + return innerArgvRef; + }); + } } - const handlerFinishCommand = yargs.getHandlerFinishCommand(); if (isPromise(handlerResult)) { yargs.getUsageInstance().cacheHelpMessage(); handlerResult - .then(value => { - if (handlerFinishCommand) { - handlerFinishCommand(value); - } - }) .catch(error => { try { yargs.getUsageInstance().fail(null, error); @@ -328,10 +328,6 @@ export function command( .then(() => { yargs.getUsageInstance().clearCachedHelpMessage(); }); - } else { - if (handlerFinishCommand) { - handlerFinishCommand(handlerResult); - } } } @@ -680,7 +676,3 @@ type FrozenCommandInstance = { aliasMap: Dictionary; defaultCommand: CommandHandler | undefined; }; - -export interface FinishCommandHandler { - (handlerResult: any): any; -} diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index 5622dd628..d66942a23 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -12,7 +12,6 @@ import { CommandBuilder, CommandHandlerCallback, CommandHandlerDefinition, - FinishCommandHandler, DefinitionOrCommandName, } from './command.js'; import type { @@ -80,7 +79,6 @@ function Yargs( const preservedGroups: Dictionary = {}; let usage: UsageInstance; let validation: ValidationInstance; - let handlerFinishCommand: FinishCommandHandler | null = null; const y18n = shim.y18n; @@ -278,7 +276,6 @@ function Yargs( parsed: self.parsed, parseFn, parseContext, - handlerFinishCommand, }); usage.freeze(); validation.freeze(); @@ -303,7 +300,6 @@ function Yargs( completionCommand, parseFn, parseContext, - handlerFinishCommand, } = frozen); options.configObjects = configObjects; usage.unfreeze(); @@ -845,14 +841,6 @@ function Yargs( return self; }; - self.onFinishCommand = function (f) { - argsert('', [f], arguments.length); - handlerFinishCommand = f; - return self; - }; - - self.getHandlerFinishCommand = () => handlerFinishCommand; - self.check = function (f, _global) { argsert(' [boolean]', [f, _global], arguments.length); validation.check(f, _global !== false); @@ -1894,7 +1882,6 @@ export interface YargsInstance { getDetectLocale(): boolean; getExitProcess(): boolean; getGroups(): Dictionary; - getHandlerFinishCommand(): FinishCommandHandler | null; getOptions(): Options; getParserConfiguration(): Configuration; getStrict(): boolean; @@ -1924,7 +1911,6 @@ export interface YargsInstance { }; normalize(keys: string | string[]): YargsInstance; number(keys: string | string[]): YargsInstance; - onFinishCommand(f: FinishCommandHandler): YargsInstance; option: { (key: string, optionDefinition: OptionDefinition): YargsInstance; (keyOptionDefinitions: Dictionary): YargsInstance; @@ -2110,7 +2096,6 @@ interface FrozenYargsInstance { parsed: DetailedArguments | false; parseFn: ParseCallback | null; parseContext: object | null; - handlerFinishCommand: FinishCommandHandler | null; } interface ParseCallback { diff --git a/test/command.cjs b/test/command.cjs index ea89f3455..8f58e83da 100644 --- a/test/command.cjs +++ b/test/command.cjs @@ -1765,31 +1765,30 @@ describe('Command', () => { .parse(); }); - it('succeeds when the promise returned by the command handler resolves', done => { + it('returns promise that resolves arguments once handler succeeds', async () => { + let complete = false; const handler = new Promise((resolve, reject) => { setTimeout(() => { - return resolve(true); - }, 5); + complete = true; + return resolve(); + }, 10); }); - const parsed = yargs('foo hello') + const parsedPromise = yargs('foo hello') .command( 'foo ', 'foo command', () => {}, - yargs => handler + () => handler ) - .fail((msg, err) => { - return done(Error('should not have been called')); - }) .parse(); - - handler.then(called => { - called.should.equal(true); - parsed.pos.should.equal('hello'); - return done(); - }); + complete.should.equal(false); + const parsed = await parsedPromise; + complete.should.equal(true); + parsed.pos.should.equal('hello'); }); + // TODO(bcoe): add test for handler rejecting, when using parsedPromise. + // see: https://github.com/yargs/yargs/issues/1144 it('displays error and appropriate help message when handler fails', done => { const y = yargs('foo') diff --git a/test/middleware.cjs b/test/middleware.cjs index dfdcf316d..54757a773 100644 --- a/test/middleware.cjs +++ b/test/middleware.cjs @@ -138,6 +138,8 @@ describe('middleware', () => { .parse(); }); + // TODO(bcoe): add test for middleware rejecting, when using promise form. + it('calls the command handler when all middleware promises resolve', done => { const middleware = (key, value) => () => new Promise((resolve, reject) => { diff --git a/test/yargs.cjs b/test/yargs.cjs index 59ecf4974..a54501040 100644 --- a/test/yargs.cjs +++ b/test/yargs.cjs @@ -2441,46 +2441,6 @@ describe('yargs dsl tests', () => { argv['--'].should.eql(['foo']); }); - describe('onFinishCommand', () => { - it('use with promise', done => { - const result = 'noop-result'; - let calledTimes = 0; - yargs(['noop']) - .command('noop', 'a noop command', noop, async () => { - return result; - }) - .onFinishCommand(async commandResult => { - commandResult.should.eql(result); - calledTimes++; - }) - .parse('noop'); - - setTimeout(() => { - calledTimes.should.eql(1); - done(); - }, 5); - }); - - it('use without promise', done => { - const result = 'noop-result'; - let calledTimes = 0; - yargs(['noop']) - .command('noop', 'a noop command', noop, () => { - return result; - }) - .onFinishCommand(commandResult => { - commandResult.should.eql(result); - calledTimes++; - }) - .parse('noop'); - - setTimeout(() => { - calledTimes.should.eql(1); - done(); - }, 5); - }); - }); - // See: https://github.com/yargs/yargs/issues/1098 it('should allow array and requires arg to be used in conjunction', () => { const argv = yargs(['-i', 'item1', 'item2', 'item3']).option('i', { @@ -2695,3 +2655,45 @@ describe('yargs dsl tests', () => { }); }); }); + +/* + describe('asyncParse', () => { + it('use with promise', done => { + const result = 'noop-result'; + let calledTimes = 0; + yargs(['noop']) + .command('noop', 'a noop command', noop, async () => { + return result; + }) + .onFinishCommand(async commandResult => { + commandResult.should.eql(result); + calledTimes++; + }) + .parse('noop'); + + setTimeout(() => { + calledTimes.should.eql(1); + done(); + }, 5); + }); + + it('use without promise', done => { + const result = 'noop-result'; + let calledTimes = 0; + yargs(['noop']) + .command('noop', 'a noop command', noop, () => { + return result; + }) + .onFinishCommand(commandResult => { + commandResult.should.eql(result); + calledTimes++; + }) + .parse('noop'); + + setTimeout(() => { + calledTimes.should.eql(1); + done(); + }, 5); + }); + }); +*/ From 8a408732111414bf407b5c4274faf3e3470759c1 Mon Sep 17 00:00:00 2001 From: bcoe Date: Sun, 6 Dec 2020 13:39:48 -0800 Subject: [PATCH 02/15] fix: address bug with execution order --- lib/command.ts | 20 +++++++++++--------- test/middleware.cjs | 1 + 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/lib/command.ts b/lib/command.ts index e6c3a16e8..6a570d37f 100644 --- a/lib/command.ts +++ b/lib/command.ts @@ -302,22 +302,24 @@ export function command( yargs._postProcess(innerArgv, populateDoubleDash); innerArgv = applyMiddleware(innerArgv, yargs, middlewares, false); - let handlerResult; if (isPromise(innerArgv)) { - handlerResult = innerArgv.then(argv => commandHandler.handler(argv)); + const innerArgvRef = innerArgv; + innerArgv = innerArgv + .then(argv => commandHandler.handler(argv)) + .then(() => innerArgvRef); } else { - handlerResult = commandHandler.handler(innerArgv); - if (isPromise(handlerResult) && !isPromise(innerArgv)) { + const handlerResult = commandHandler.handler(innerArgv); + if (isPromise(handlerResult)) { const innerArgvRef = innerArgv; - innerArgv = handlerResult.then(() => { - return innerArgvRef; - }); + innerArgv = commandHandler + .handler(innerArgv) + .then(() => innerArgvRef); } } - if (isPromise(handlerResult)) { + if (isPromise(innerArgv)) { yargs.getUsageInstance().cacheHelpMessage(); - handlerResult + innerArgv .catch(error => { try { yargs.getUsageInstance().fail(null, error); diff --git a/test/middleware.cjs b/test/middleware.cjs index 54757a773..6212b4279 100644 --- a/test/middleware.cjs +++ b/test/middleware.cjs @@ -139,6 +139,7 @@ describe('middleware', () => { }); // TODO(bcoe): add test for middleware rejecting, when using promise form. + // TODO(bcoe): add test that ensures handler awaited after middleware. it('calls the command handler when all middleware promises resolve', done => { const middleware = (key, value) => () => From d8b10f4bef352e68f62b3bdffe5fdf9e697ec41c Mon Sep 17 00:00:00 2001 From: bcoe Date: Sun, 3 Jan 2021 16:31:48 -0800 Subject: [PATCH 03/15] feat: continuing to flesh out async behavior --- lib/command.ts | 26 ++-- lib/usage.ts | 21 ++- lib/yargs-factory.ts | 44 +++++- test/completion.cjs | 3 + test/yargs.cjs | 353 +++++++++++++++++++++++++++++++++++++------ 5 files changed, 376 insertions(+), 71 deletions(-) diff --git a/lib/command.ts b/lib/command.ts index 6a570d37f..99667c7bc 100644 --- a/lib/command.ts +++ b/lib/command.ts @@ -311,25 +311,21 @@ export function command( const handlerResult = commandHandler.handler(innerArgv); if (isPromise(handlerResult)) { const innerArgvRef = innerArgv; - innerArgv = commandHandler - .handler(innerArgv) - .then(() => innerArgvRef); + innerArgv = handlerResult.then(() => innerArgvRef); } } - if (isPromise(innerArgv)) { + if (isPromise(innerArgv) && !yargs._hasParseCallback()) { + yargs.getUsageInstance().cacheHelpMessage(); + innerArgv.catch(error => { + try { + yargs.getUsageInstance().fail(null, error); + } catch (err) { + // fail's throwing would cause an unhandled rejection. + } + }); + } else if (isPromise(innerArgv)) { yargs.getUsageInstance().cacheHelpMessage(); - innerArgv - .catch(error => { - try { - yargs.getUsageInstance().fail(null, error); - } catch (err) { - // fail's throwing would cause an unhandled rejection. - } - }) - .then(() => { - yargs.getUsageInstance().clearCachedHelpMessage(); - }); } } diff --git a/lib/usage.ts b/lib/usage.ts index c5da825d8..a120008ea 100644 --- a/lib/usage.ts +++ b/lib/usage.ts @@ -12,16 +12,19 @@ import {YError} from './yerror.js'; import {DetailedArguments} from './typings/yargs-parser-types.js'; import setBlocking from './utils/set-blocking.js'; +function isBoolean(fail: FailureFunction | boolean): fail is boolean { + return typeof fail === 'boolean'; +} + export function usage(yargs: YargsInstance, y18n: Y18N, shim: PlatformShim) { const __ = y18n.__; const self = {} as UsageInstance; // methods for ouputting/building failure message. - const fails: FailureFunction[] = []; + const fails: (FailureFunction | boolean)[] = []; self.failFn = function failFn(f) { fails.push(f); }; - let failMessage: string | undefined | null = null; let showHelpOnFail = true; self.showHelpOnFail = function showHelpOnFailFn( @@ -43,7 +46,12 @@ export function usage(yargs: YargsInstance, y18n: Y18N, shim: PlatformShim) { if (fails.length) { for (let i = fails.length - 1; i >= 0; --i) { - fails[i](msg, err, self); + const fail = fails[i]; + if (isBoolean(fail)) { + continue; + } else { + fail(msg, err, self); + } } } else { if (yargs.getExitProcess()) setBlocking(true); @@ -554,6 +562,10 @@ export function usage(yargs: YargsInstance, y18n: Y18N, shim: PlatformShim) { cachedHelpMessage = undefined; }; + self.hasCachedHelpMessage = function () { + return !!cachedHelpMessage; + }; + // given a set of keys, place any keys that are // ungrouped under the 'Options:' grouping. function addUngroupedKeys( @@ -710,6 +722,7 @@ export function usage(yargs: YargsInstance, y18n: Y18N, shim: PlatformShim) { export interface UsageInstance { cacheHelpMessage(): void; clearCachedHelpMessage(): void; + hasCachedHelpMessage(): boolean; command( cmd: string, description: string | undefined, @@ -722,7 +735,7 @@ export interface UsageInstance { epilog(msg: string): void; example(cmd: string, description?: string): void; fail(msg?: string | null, err?: YError | string): void; - failFn(f: FailureFunction): void; + failFn(f: FailureFunction | boolean): void; freeze(): void; functionDescription(fn: {name?: string}): string; getCommands(): [string, string, boolean, string[], boolean][]; diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index d66942a23..52719ede2 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -79,6 +79,7 @@ function Yargs( const preservedGroups: Dictionary = {}; let usage: UsageInstance; let validation: ValidationInstance; + let parseCompleted = false; const y18n = shim.y18n; @@ -836,7 +837,12 @@ function Yargs( }; self.fail = function (f) { - argsert('', [f], arguments.length); + argsert('', [f], arguments.length); + if (typeof f === 'boolean' && f !== false) { + throw new YError( + "Invalid first argument. Expected function or boolean 'false'" + ); + } usage.failFn(f); return self; }; @@ -1215,12 +1221,25 @@ function Yargs( }; self.getParserConfiguration = () => parserConfig; + self.getHelp = async function () { + if (!usage.hasCachedHelpMessage()) { + if (!self.parsed) await self._parseArgs(processArgs); // run parser, if it has not already been executed. + if (command.hasDefaultCommand()) { + context.resets++; // override the restriction on top-level positoinals. + command.runDefaultBuilderOn(self); + } + } + return usage.help(); + }; + self.showHelp = function (level) { argsert('[string|function]', [level], arguments.length); - if (!self.parsed) self._parseArgs(processArgs); // run parser, if it has not already been executed. - if (command.hasDefaultCommand()) { - context.resets++; // override the restriction on top-level positoinals. - command.runDefaultBuilderOn(self); + if (!usage.hasCachedHelpMessage()) { + if (!self.parsed) self._parseArgs(processArgs); // run parser, if it has not already been executed. + if (command.hasDefaultCommand()) { + context.resets++; // override the restriction on top-level positoinals. + command.runDefaultBuilderOn(self); + } } usage.showHelp(level); return self; @@ -1445,6 +1464,14 @@ function Yargs( _calledFromCommand?: boolean, commandIndex?: number ) { + // A single yargs instance may be used multiple times, e.g. + // const y = yargs(); y.parse('foo --bar'); yargs.parse('bar --foo'). + // When a prior parse has completed and a new parse is beginning, we + // need to clear the cached help message from the previous parse: + if (parseCompleted) { + parseCompleted = false; + usage.clearCachedHelpMessage(); + } let skipValidation = !!_calledFromCommand; args = args || processArgs; @@ -1615,7 +1642,7 @@ function Yargs( }; // Applies a couple post processing steps that are easier to perform - // as a final step, rather than + // as a final step. self._postProcess = function ( argv: Arguments | Promise, populateDoubleDash: boolean, @@ -1632,6 +1659,8 @@ function Yargs( if (parsePositionalNumbers) { argv = self._parsePositionalNumbers(argv); } + // Track that the most recent parse has completed: + parseCompleted = true; return argv; }; @@ -1872,7 +1901,7 @@ export interface YargsInstance { ): YargsInstance; exit(code: number, err?: YError | string): void; exitProcess(enabled: boolean): YargsInstance; - fail(f: FailureFunction): YargsInstance; + fail(f: FailureFunction | boolean): YargsInstance; getCommandInstance(): CommandInstance; getCompletion(args: string[], done: (completions: string[]) => any): void; getContext(): Context; @@ -1881,6 +1910,7 @@ export interface YargsInstance { getDeprecatedOptions(): Options['deprecatedOptions']; getDetectLocale(): boolean; getExitProcess(): boolean; + getHelp(): Promise; getGroups(): Dictionary; getOptions(): Options; getParserConfiguration(): Configuration; diff --git a/test/completion.cjs b/test/completion.cjs index 24b2e4830..7845e1404 100644 --- a/test/completion.cjs +++ b/test/completion.cjs @@ -688,4 +688,7 @@ describe('Completion', () => { r.logs.should.include('of-memory:Dream about a specific memory'); }); }); + + // TODO: add async tests for getCompletion. + // See: https://github.com/yargs/yargs/issues/1420 }); diff --git a/test/yargs.cjs b/test/yargs.cjs index a54501040..b789d0afb 100644 --- a/test/yargs.cjs +++ b/test/yargs.cjs @@ -35,12 +35,17 @@ describe('yargs dsl tests', () => { }); it('should use bin name for $0, eliminating path', () => { + const originalExec = process.execPath; process.argv[1] = '/usr/local/bin/ndm'; process.env._ = '/usr/local/bin/ndm'; process.execPath = '/usr/local/bin/ndm'; const argv = yargs([]).parse(); argv.$0.should.equal('ndm'); yargs.$0.should.equal('ndm'); + // Restore env: + process.execPath = originalExec; + process._ = originalExec; + process.argv[1] = originalExec; }); it('should not remove the 1st argument of bundled electron apps', () => { @@ -1198,7 +1203,7 @@ describe('yargs dsl tests', () => { output1 .split('\n') .should.deep.equal([ - 'ndm batman ', + 'node batman ', '', 'batman command', '', @@ -1210,7 +1215,7 @@ describe('yargs dsl tests', () => { output2 .split('\n') .should.deep.equal([ - 'ndm robin ', + 'node robin ', '', 'robin command', '', @@ -1243,7 +1248,7 @@ describe('yargs dsl tests', () => { output1 .split('\n') .should.deep.equal([ - 'ndm batman ', + 'node batman ', '', 'batman command', '', @@ -1258,7 +1263,7 @@ describe('yargs dsl tests', () => { output2 .split('\n') .should.deep.equal([ - 'ndm robin ', + 'node robin ', '', 'robin command', '', @@ -1292,10 +1297,10 @@ describe('yargs dsl tests', () => { output .split('\n') .should.deep.equal([ - 'ndm ', + 'node ', '', 'Commands:', - ' ndm one The one and only command', + ' node one The one and only command', '', 'Options:', ' --help Show help [boolean]', @@ -2654,46 +2659,304 @@ describe('yargs dsl tests', () => { argv._[1].should.equal(33); }); }); -}); -/* - describe('asyncParse', () => { - it('use with promise', done => { - const result = 'noop-result'; - let calledTimes = 0; - yargs(['noop']) - .command('noop', 'a noop command', noop, async () => { - return result; - }) - .onFinishCommand(async commandResult => { - commandResult.should.eql(result); - calledTimes++; - }) - .parse('noop'); - - setTimeout(() => { - calledTimes.should.eql(1); - done(); - }, 5); + // See: https://github.com/yargs/yargs/issues/1420 + describe('async', () => { + describe('parse', () => { + it('returns promise that resolves argv on success', done => { + let executionCount = 0; + yargs() + .command( + 'cmd [str]', + 'a command', + () => {}, + async argv => { + return new Promise(resolve => { + setTimeout(() => { + argv.addedAsync = 99; + executionCount++; + return resolve(argv); + }, 5); + }); + } + ) + .parse('cmd foo', async (_err, argv) => { + (typeof argv.then).should.equal('function'); + argv = await argv; + argv.addedAsync.should.equal(99); + argv.str.should.equal('foo'); + executionCount.should.equal(1); + return done(); + }); + }); + it('returns deeply nested promise that resolves argv on success', done => { + let executionCount = 0; + yargs() + .command( + 'cmd', + 'a command', + yargs => { + yargs.command( + 'foo [apple]', + 'foo command', + () => {}, + async argv => { + return new Promise(resolve => { + setTimeout(() => { + argv.addedAsync = 99; + executionCount++; + return resolve(argv); + }, 5); + }); + } + ); + }, + () => {} + ) + .parse('cmd foo orange', async (_err, argv) => { + (typeof argv.then).should.equal('function'); + argv = await argv; + argv.addedAsync.should.equal(99); + argv.apple.should.equal('orange'); + executionCount.should.equal(1); + return done(); + }); + }); + it('returns promise that can be caught if rejected', done => { + let executionCount = 0; + yargs() + .command( + 'cmd [str]', + 'a command', + () => {}, + async argv => { + return new Promise((resolve, reject) => { + setTimeout(() => { + executionCount++; + return reject(Error('async error')); + }, 5); + }); + } + ) + .parse('cmd foo', async (_err, argv) => { + (typeof argv.then).should.equal('function'); + try { + await argv; + } catch (err) { + err.message.should.equal('async error'); + executionCount.should.equal(1); + return done(); + } + }); + }); + it('caches nested help output, so that it can be output by showHelp()', done => { + let executionCount = 0; + const y = yargs(); + y.command( + 'cmd [str]', + 'a command', + () => {}, + async argv => { + return new Promise((resolve, reject) => { + setTimeout(() => { + executionCount++; + return reject(Error('async error')); + }, 5); + }); + } + ).parse('cmd foo', async (_err, argv) => { + (typeof argv.then).should.equal('function'); + try { + await argv; + } catch (err) { + y.showHelp(output => { + output.should.match(/a command/); + executionCount.should.equal(1); + return done(); + }); + } + }); + }); + it('caches deeply nested help output, so that it can be output by showHelp()', done => { + let executionCount = 0; + const y = yargs(); + y.command( + 'cmd', + 'a command', + yargs => { + yargs.command( + 'inner [foo]', + 'inner command', + () => {}, + async argv => { + return new Promise((resolve, reject) => { + setTimeout(() => { + executionCount++; + return reject(Error('async error')); + }, 5); + }); + } + ); + }, + () => {} + ).parse('cmd inner bar', async (_err, argv) => { + (typeof argv.then).should.equal('function'); + try { + await argv; + } catch (err) { + y.showHelp(output => { + output.should.match(/inner command/); + executionCount.should.equal(1); + return done(); + }); + } + }); + }); }); - - it('use without promise', done => { - const result = 'noop-result'; - let calledTimes = 0; - yargs(['noop']) - .command('noop', 'a noop command', noop, () => { - return result; - }) - .onFinishCommand(commandResult => { - commandResult.should.eql(result); - calledTimes++; - }) - .parse('noop'); - - setTimeout(() => { - calledTimes.should.eql(1); - done(); - }, 5); + describe('argv', () => { + it('returns promise that resolves argv on success', async () => { + let executionCount = 0; + const argvPromise = yargs('cmd foo').command( + 'cmd [str]', + 'a command', + () => {}, + async argv => { + return new Promise(resolve => { + setTimeout(() => { + argv.addedAsync = 99; + executionCount++; + return resolve(argv); + }, 5); + }); + } + ).argv; + (typeof argvPromise.then).should.equal('function'); + const argv = await argvPromise; + argv.addedAsync.should.equal(99); + argv.str.should.equal('foo'); + executionCount.should.equal(1); + }); + it('returns deeply nested promise that resolves argv on success', async () => { + let executionCount = 0; + const argvPromise = yargs('cmd foo orange') + .command( + 'cmd', + 'a command', + yargs => { + yargs.command( + 'foo [apple]', + 'foo command', + () => {}, + async argv => { + return new Promise(resolve => { + setTimeout(() => { + argv.addedAsync = 99; + executionCount++; + return resolve(argv); + }, 5); + }); + } + ); + }, + () => {} + ) + .parse(); + (typeof argvPromise.then).should.equal('function'); + const argv = await argvPromise; + argv.addedAsync.should.equal(99); + argv.apple.should.equal('orange'); + executionCount.should.equal(1); + }); + it('returns promise that can be caught if rejected', async () => { + let executionCount = 0; + const argv = yargs('cmd foo') + .fail(false) + .command( + 'cmd [str]', + 'a command', + () => {}, + async argv => { + return new Promise((resolve, reject) => { + setTimeout(() => { + executionCount++; + return reject(Error('async error')); + }, 5); + }); + } + ).argv; + (typeof argv.then).should.equal('function'); + try { + await argv; + throw Error('unreachable'); + } catch (err) { + err.message.should.equal('async error'); + executionCount.should.equal(1); + } + }); + it('caches nested help output, so that it can be output by showHelp()', async () => { + let executionCount = 0; + const y = yargs(); + const argv = y + .fail(false) + .command( + 'cmd [str]', + 'a command', + () => {}, + async argv => { + return new Promise((resolve, reject) => { + setTimeout(() => { + executionCount++; + return reject(Error('async error')); + }, 5); + }); + } + ) + .parse('cmd foo'); + (typeof argv.then).should.equal('function'); + try { + await argv; + throw Error('unreachable'); + } catch (err) { + const output = await y.getHelp(); + output.should.match(/a command/); + executionCount.should.equal(1); + } + }); + it('caches deeply nested help output, so that it can be output by showHelp()', async () => { + let executionCount = 0; + const y = yargs('cmd inner bar'); + const argv = y.fail(false).command( + 'cmd', + 'a command', + yargs => { + yargs.command( + 'inner [foo]', + 'inner command', + () => {}, + async argv => { + return new Promise((resolve, reject) => { + setTimeout(() => { + executionCount++; + return reject(Error('async error')); + }, 5); + }); + } + ); + }, + () => {} + ).argv; + (typeof argv.then).should.equal('function'); + try { + await argv; + throw Error('unreachable'); + } catch (err) { + const output = await y.getHelp(); + output.should.match(/inner command/); + executionCount.should.equal(1); + } + }); }); }); -*/ + // TODO(@bcoe): write tests for new async getHelp() method. + // TODO(@bcoe): document .fail(false) shorthand. +}); From ae3951e3803d57431a4011f960e778b70b16f846 Mon Sep 17 00:00:00 2001 From: bcoe Date: Sun, 3 Jan 2021 16:34:43 -0800 Subject: [PATCH 04/15] chore: add a couple notes for future work --- lib/yargs-factory.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index 52719ede2..297a7e2dd 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -1223,7 +1223,10 @@ function Yargs( self.getHelp = async function () { if (!usage.hasCachedHelpMessage()) { + // TODO(@bcoe): do we need this logic for getHelp()? what should the + // behavior be if you use `getHelp()` without having run the parser. if (!self.parsed) await self._parseArgs(processArgs); // run parser, if it has not already been executed. + // TODO(@bcoe): why is `hasDefaultCommand` separate from the _parseArgs step. if (command.hasDefaultCommand()) { context.resets++; // override the restriction on top-level positoinals. command.runDefaultBuilderOn(self); From 6f22e16a5cddabc686c97f8f0bbcb8f1b6648b07 Mon Sep 17 00:00:00 2001 From: bcoe Date: Sun, 3 Jan 2021 16:38:17 -0800 Subject: [PATCH 05/15] docs: make comment better --- lib/command.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/command.ts b/lib/command.ts index 99667c7bc..8c1b93a82 100644 --- a/lib/command.ts +++ b/lib/command.ts @@ -320,8 +320,9 @@ export function command( innerArgv.catch(error => { try { yargs.getUsageInstance().fail(null, error); - } catch (err) { - // fail's throwing would cause an unhandled rejection. + } catch (_err) { + // If .fail(false) is not set, and no parse cb() has been + // registered, run usage's default fail method. } }); } else if (isPromise(innerArgv)) { From 6ad2be0cb564cbcf01f5148db05e8cf999057caf Mon Sep 17 00:00:00 2001 From: bcoe Date: Mon, 4 Jan 2021 13:41:57 -0800 Subject: [PATCH 06/15] feat: add async support to getCompletions --- docs/api.md | 5 ++++- lib/completion.ts | 26 ++++++++++---------------- lib/yargs-factory.ts | 26 +++++++++++++++++++------- test/completion.cjs | 41 +++++++++++++++++++++++++++++++++++------ test/yargs.cjs | 9 ++++----- 5 files changed, 72 insertions(+), 35 deletions(-) diff --git a/docs/api.md b/docs/api.md index dc3487bc8..975ec842f 100644 --- a/docs/api.md +++ b/docs/api.md @@ -837,7 +837,10 @@ Allows to programmatically get completion choices for any line. `args`: An array of the words in the command line to complete. -`done`: The callback to be called with the resulting completions. +`done`: Optional callback which will be invoked with `err`, or the resulting completions. + +If no `done` callback is provided, `getCompletion` returns a promise that +resolves with the completions. For example: diff --git a/lib/completion.ts b/lib/completion.ts index ec2bb59ff..c249d83e7 100644 --- a/lib/completion.ts +++ b/lib/completion.ts @@ -29,12 +29,11 @@ export function completion( (shim.getEnv('ZSH_NAME') && shim.getEnv('ZSH_NAME')!.indexOf('zsh') !== -1); // get a list of completion commands. // 'args' is the array of strings from the line to be completed - self.getCompletion = function getCompletion(args, done) { + self.getCompletion = function getCompletion(args, done): any { const completions: string[] = []; const current = args.length ? args[args.length - 1] : ''; const argv = yargs.parse(args, true); const parentCommands = yargs.getContext().commands; - // a custom completion function can be provided // to completion(). function runCompletionFunction(argv: Arguments) { @@ -48,32 +47,29 @@ export function completion( return result .then(list => { shim.process.nextTick(() => { - done(list); + done(null, list); }); }) .catch(err => { shim.process.nextTick(() => { - throw err; + done(err, undefined); }); }); } - // synchronous completion function. - return done(result); + return done(null, result); } else { // asynchronous completion function return completionFunction(current, argv, completions => { - done(completions); + done(null, completions); }); } } - if (completionFunction) { return isPromise(argv) ? argv.then(runCompletionFunction) : runCompletionFunction(argv); } - const handlers = command.getCommandHandlers(); for (let i = 0, ii = args.length; i < ii; ++i) { if (handlers[args[i]] && handlers[args[i]].builder) { @@ -85,7 +81,6 @@ export function completion( } } } - if ( !current.match(/^-/) && parentCommands[parentCommands.length - 1] !== current @@ -102,7 +97,6 @@ export function completion( } }); } - if (current.match(/^-/) || (current === '' && completions.length === 0)) { const descs = usage.getDescriptions(); const options = yargs.getOptions(); @@ -116,7 +110,6 @@ export function completion( keyAndAliases = keyAndAliases.concat( keyAndAliases.map(key => `no-${key}`) ); - function completeOptionKey(key: string) { const notInArgs = keyAndAliases.every( val => args.indexOf(`--${val}`) === -1 @@ -140,13 +133,11 @@ export function completion( } } } - completeOptionKey(key); if (negable && !!options.default[key]) completeOptionKey(`no-${key}`); }); } - - done(completions); + done(null, completions); }; // generate the completion script to add to your .bashrc. @@ -179,7 +170,10 @@ export function completion( export interface CompletionInstance { completionKey: string; generateCompletionScript($0: string, cmd: string): string; - getCompletion(args: string[], done: (completions: string[]) => any): any; + getCompletion( + args: string[], + done: (err: Error | null, completions: string[] | undefined) => void + ): any; registerFunction(fn: CompletionFunction): void; setParsed(parsed: DetailedArguments): void; } diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index 297a7e2dd..ecf265493 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -1382,9 +1382,18 @@ function Yargs( return self; }; - self.getCompletion = function (args, done) { - argsert(' ', [args, done], arguments.length); - completion!.getCompletion(args, done); + self.getCompletion = async function (args, done) { + argsert(' [function]', [args, done], arguments.length); + if (!done) { + return new Promise((resolve, reject) => { + completion!.getCompletion(args, (err, completions) => { + if (err) reject(err); + else resolve(completions); + }); + }); + } else { + return completion!.getCompletion(args, done); + } }; self.locale = function (locale?: string): any { @@ -1588,11 +1597,11 @@ function Yargs( const completionArgs = args.slice( args.indexOf(`--${completion!.completionKey}`) + 1 ); - completion!.getCompletion(completionArgs, completions => { + completion!.getCompletion(completionArgs, (err, completions) => { + if (err) throw new YError(err.message); (completions || []).forEach(completion => { _logger.log(completion); }); - self.exit(0); }); return self._postProcess(argv, !populateDoubleDash, _calledFromCommand); @@ -1906,14 +1915,17 @@ export interface YargsInstance { exitProcess(enabled: boolean): YargsInstance; fail(f: FailureFunction | boolean): YargsInstance; getCommandInstance(): CommandInstance; - getCompletion(args: string[], done: (completions: string[]) => any): void; + getCompletion( + args: string[], + done?: (err: Error | null, completions: string[] | undefined) => void + ): Promise | any; + getHelp(): Promise; getContext(): Context; getDemandedCommands(): Options['demandedCommands']; getDemandedOptions(): Options['demandedOptions']; getDeprecatedOptions(): Options['deprecatedOptions']; getDetectLocale(): boolean; getExitProcess(): boolean; - getHelp(): Promise; getGroups(): Dictionary; getOptions(): Options; getParserConfiguration(): Configuration; diff --git a/test/completion.cjs b/test/completion.cjs index 7845e1404..cf52d299a 100644 --- a/test/completion.cjs +++ b/test/completion.cjs @@ -1,5 +1,5 @@ 'use strict'; -/* global describe, it, beforeEach, after */ +/* global describe, it, before, beforeEach, after */ const checkUsage = require('./helpers/utils.cjs').checkOutput; const yargs = require('../index.cjs'); @@ -522,7 +522,7 @@ describe('Completion', () => { .command('foo', 'bar') .command('apple', 'banana') .completion() - .getCompletion([''], completions => { + .getCompletion([''], (_err, completions) => { (completions || []).forEach(completion => { console.log(completion); }); @@ -538,7 +538,7 @@ describe('Completion', () => { .option('apple') .option('foo') .completion() - .getCompletion(['$0', '-'], completions => { + .getCompletion(['$0', '-'], (_err, completions) => { (completions || []).forEach(completion => { console.log(completion); }); @@ -642,7 +642,7 @@ describe('Completion', () => { .command('foo', 'bar') .command('apple', 'banana') .completion() - .getCompletion([''], completions => { + .getCompletion([''], (_err, completions) => { (completions || []).forEach(completion => { console.log(completion); }); @@ -689,6 +689,35 @@ describe('Completion', () => { }); }); - // TODO: add async tests for getCompletion. - // See: https://github.com/yargs/yargs/issues/1420 + describe('async', () => { + before(() => { + process.env.SHELL = '/bin/bash'; + }); + describe('getCompletion', () => { + it('allows completions to be awaited', async () => { + const completions = await yargs() + .command('foo', 'bar') + .command('apple', 'banana') + .completion() + .getCompletion(['']); + completions.should.eql(['foo', 'apple', 'completion']); + }); + }); + // See: https://github.com/yargs/yargs/issues/1235 + describe('completion', () => { + it('does not apply validation if async completion command provided', async () => { + const completions = await yargs(['--get-yargs-completions', 'foo']) + .command('foo ', 'foo command') + .completion('completion', false, async () => { + return new Promise(resolve => { + setTimeout(() => { + return resolve(['foo', 'bar', 'apple']); + }, 5); + }); + }) + .getCompletion(['foo']); + completions.should.eql(['foo', 'bar', 'apple']); + }); + }); + }); }); diff --git a/test/yargs.cjs b/test/yargs.cjs index b789d0afb..07dda208a 100644 --- a/test/yargs.cjs +++ b/test/yargs.cjs @@ -21,6 +21,10 @@ describe('yargs dsl tests', () => { const oldProcess = {versions: {}}; beforeEach(() => { + process.execPath = 'node'; + process._ = 'node'; + process.argv[1] = 'node'; + oldProcess.argv = process.argv; oldProcess.defaultApp = process.defaultApp; oldProcess.versions.electron = process.versions.electron; @@ -35,17 +39,12 @@ describe('yargs dsl tests', () => { }); it('should use bin name for $0, eliminating path', () => { - const originalExec = process.execPath; process.argv[1] = '/usr/local/bin/ndm'; process.env._ = '/usr/local/bin/ndm'; process.execPath = '/usr/local/bin/ndm'; const argv = yargs([]).parse(); argv.$0.should.equal('ndm'); yargs.$0.should.equal('ndm'); - // Restore env: - process.execPath = originalExec; - process._ = originalExec; - process.argv[1] = originalExec; }); it('should not remove the 1st argument of bundled electron apps', () => { From 27c536cf07caec2e1e87cfd546b52651a74db746 Mon Sep 17 00:00:00 2001 From: bcoe Date: Mon, 4 Jan 2021 17:44:01 -0800 Subject: [PATCH 07/15] feat: implement getHelp() functionality --- docs/api.md | 14 +++++- lib/command.ts | 32 ++++++++++-- lib/usage.ts | 2 +- lib/yargs-factory.ts | 57 ++++++++++++--------- test/usage.cjs | 57 +++++++++++++++++++-- test/yargs.cjs | 116 +++++++++++++++++++++++++++++++++++++++++-- 6 files changed, 240 insertions(+), 38 deletions(-) diff --git a/docs/api.md b/docs/api.md index 975ec842f..6fba124f9 100644 --- a/docs/api.md +++ b/docs/api.md @@ -809,11 +809,15 @@ error message when this promise rejects Manually indicate that the program should exit, and provide context about why we wanted to exit. Follows the behavior set by `.exitProcess()`. -.fail(fn) +.fail(fn | boolean) --------- Method to execute when a failure occurs, rather than printing the failure message. +Providing `false` as a value for `fn` can be used to prevent failure +messages from being output. _This is useful if you wish to handle failures +yourself, using `try`/`catch` and [`.getHelp()`](#get-help). + `fn` is called with the failure message that would have been printed, the `Error` instance originally thrown and yargs state when the failure occurred. @@ -856,6 +860,12 @@ require('yargs/yargs')(process.argv.slice(2)) Outputs the same completion choices as `./test.js --foo`TAB: `--foobar` and `--foobaz` +.getHelp() +--------------------------- + +Returns a promise that resolves with a `string` equivalent to what would +be output by [`.showHelp()`](#show-help), or by running yargs with `--help`. + .global(globals, [global=true]) ------------ @@ -1467,7 +1477,7 @@ Generate a bash completion script. Users of your application can install this script in their `.bashrc`, and yargs will provide completion shortcuts for commands and options. -.showHelp([consoleLevel | printCallback]) +.showHelp([consoleLevel | printCallback]) --------------------------- Print the usage data. diff --git a/lib/command.ts b/lib/command.ts index 8c1b93a82..e4958787c 100644 --- a/lib/command.ts +++ b/lib/command.ts @@ -211,7 +211,13 @@ export function command( self.hasDefaultCommand = () => !!defaultCommand; - self.runCommand = function runCommand(command, yargs, parsed, commandIndex) { + self.runCommand = function runCommand( + command, + yargs, + parsed, + commandIndex = 0, + helpOnly = false + ) { let aliases = parsed.aliases; const commandHandler = handlers[command!] || handlers[aliasMap[command!]] || defaultCommand; @@ -243,7 +249,13 @@ export function command( commandHandler.description ); } - innerArgv = innerYargs._parseArgs(null, null, true, commandIndex); + innerArgv = innerYargs._parseArgs( + null, + undefined, + true, + commandIndex, + helpOnly + ); aliases = (innerYargs.parsed as DetailedArguments).aliases; } else if (isCommandBuilderOptionDefinitions(builder)) { // as a short hand, an object can instead be provided, specifying @@ -263,7 +275,13 @@ export function command( Object.keys(commandHandler.builder).forEach(key => { innerYargs.option(key, builder[key]); }); - innerArgv = innerYargs._parseArgs(null, null, true, commandIndex); + innerArgv = innerYargs._parseArgs( + null, + undefined, + true, + commandIndex, + helpOnly + ); aliases = (innerYargs.parsed as DetailedArguments).aliases; } @@ -275,6 +293,11 @@ export function command( ); } + // If showHelp() or getHelp() is being run, we should not + // execute middleware or handlers (these may perform expensive operations + // like creating a DB connection). + if (helpOnly) return innerArgv; + const middlewares = globalMiddleware .slice(0) .concat(commandHandler.middlewares); @@ -577,7 +600,8 @@ export interface CommandInstance { command: string | null, yargs: YargsInstance, parsed: DetailedArguments, - commandIndex?: number + commandIndex?: number, + helpOnly?: boolean ): Arguments | Promise; runDefaultBuilderOn(yargs: YargsInstance): void; unfreeze(): void; diff --git a/lib/usage.ts b/lib/usage.ts index a120008ea..308e3b213 100644 --- a/lib/usage.ts +++ b/lib/usage.ts @@ -48,7 +48,7 @@ export function usage(yargs: YargsInstance, y18n: Y18N, shim: PlatformShim) { for (let i = fails.length - 1; i >= 0; --i) { const fail = fails[i]; if (isBoolean(fail)) { - continue; + throw err; } else { fail(msg, err, self); } diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index ecf265493..6d93b1fdf 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -79,7 +79,6 @@ function Yargs( const preservedGroups: Dictionary = {}; let usage: UsageInstance; let validation: ValidationInstance; - let parseCompleted = false; const y18n = shim.y18n; @@ -887,7 +886,6 @@ function Yargs( return self; }; - const pkgs: Dictionary = {}; function pkgUp(rootPath?: string) { const npath = rootPath || '*'; @@ -1222,11 +1220,13 @@ function Yargs( self.getParserConfiguration = () => parserConfig; self.getHelp = async function () { + hasOutput = true; if (!usage.hasCachedHelpMessage()) { - // TODO(@bcoe): do we need this logic for getHelp()? what should the - // behavior be if you use `getHelp()` without having run the parser. - if (!self.parsed) await self._parseArgs(processArgs); // run parser, if it has not already been executed. - // TODO(@bcoe): why is `hasDefaultCommand` separate from the _parseArgs step. + if (!self.parsed) { + // Run the parser as if --help was passed to it (this is what + // the last parameter `true` indicates). + self._parseArgs(processArgs, undefined, undefined, 0, true); + } if (command.hasDefaultCommand()) { context.resets++; // override the restriction on top-level positoinals. command.runDefaultBuilderOn(self); @@ -1237,8 +1237,13 @@ function Yargs( self.showHelp = function (level) { argsert('[string|function]', [level], arguments.length); + hasOutput = true; if (!usage.hasCachedHelpMessage()) { - if (!self.parsed) self._parseArgs(processArgs); // run parser, if it has not already been executed. + if (!self.parsed) { + // Run the parser as if --help was passed to it (this is what + // the last parameter `true` indicates). + self._parseArgs(processArgs, undefined, undefined, 0, true); + } if (command.hasDefaultCommand()) { context.resets++; // override the restriction on top-level positoinals. command.runDefaultBuilderOn(self); @@ -1474,16 +1479,9 @@ function Yargs( args: string | string[] | null, shortCircuit?: boolean | null, _calledFromCommand?: boolean, - commandIndex?: number + commandIndex = 0, + helpOnly = false ) { - // A single yargs instance may be used multiple times, e.g. - // const y = yargs(); y.parse('foo --bar'); yargs.parse('bar --foo'). - // When a prior parse has completed and a new parse is beginning, we - // need to clear the cached help message from the previous parse: - if (parseCompleted) { - parseCompleted = false; - usage.clearCachedHelpMessage(); - } let skipValidation = !!_calledFromCommand; args = args || processArgs; @@ -1508,6 +1506,14 @@ function Yargs( argv.$0 = self.$0; self.parsed = parsed; + // A single yargs instance may be used multiple times, e.g. + // const y = yargs(); y.parse('foo --bar'); yargs.parse('bar --foo'). + // When a prior parse has completed and a new parse is beginning, we + // need to clear the cached help message from the previous parse: + if (!_calledFromCommand) { + usage.clearCachedHelpMessage(); + } + try { guessLocale(); // guess locale lazily, so that it can be turned off in chain. @@ -1550,7 +1556,13 @@ function Yargs( // commands are executed using a recursive algorithm that executes // the deepest command first; we keep track of the position in the // argv._ array that is currently being executed. - const innerArgv = command.runCommand(cmd, self, parsed, i + 1); + const innerArgv = command.runCommand( + cmd, + self, + parsed, + i + 1, + helpOnly // Don't run a handler, just figure out the help string. + ); return self._postProcess(innerArgv, populateDoubleDash); } else if (!firstUnknownCommand && cmd !== completionCommand) { firstUnknownCommand = cmd; @@ -1671,8 +1683,6 @@ function Yargs( if (parsePositionalNumbers) { argv = self._parsePositionalNumbers(argv); } - // Track that the most recent parse has completed: - parseCompleted = true; return argv; }; @@ -1790,10 +1800,11 @@ export interface YargsInstance { _hasParseCallback(): boolean; _parseArgs: { ( - args: null, - shortCircuit: null, - _calledFromCommand: boolean, - commandIndex?: number + args: string | string[] | null, + shortCircuit?: boolean, + _calledFromCommand?: boolean, + commandIndex?: number, + helpOnly?: boolean ): Arguments | Promise; (args: string | string[], shortCircuit?: boolean): | Arguments diff --git a/test/usage.cjs b/test/usage.cjs index 5df51561e..5fa5b37b7 100644 --- a/test/usage.cjs +++ b/test/usage.cjs @@ -612,7 +612,6 @@ describe('usage tests', () => { .fail(message => { console.log(message); }) - .exitProcess(false) .wrap(null) .check(argv => { throw new Error('foo'); @@ -641,7 +640,6 @@ describe('usage tests', () => { .fail((message, error, yargs) => { yargs.showHelp(); }) - .exitProcess(false) .wrap(null) .parse() ); @@ -656,7 +654,6 @@ describe('usage tests', () => { .fail((message, error) => { console.log(error.message); }) - .exitProcess(false) .wrap(null) .check(() => { throw new Error('foo'); @@ -674,7 +671,6 @@ describe('usage tests', () => { .fail(() => { console.log('is triggered last'); }) - .exitProcess(false) .wrap(null) .command( 'test', @@ -699,6 +695,36 @@ describe('usage tests', () => { r.should.have.property('exit').and.equal(false); }); }); + + it('allows "false" to be provided to prevent exit/output', () => { + try { + yargs() + .fail(false) + .wrap(null) + .check(argv => { + throw new Error('sync error'); + }) + .parse(); + throw Error('unreachable'); + } catch (err) { + err.message.should.equal('sync error'); + } + }); + + it('does not allow "true" as argument', () => { + try { + yargs() + .fail(true) + .wrap(null) + .check(argv => { + throw new Error('sync error'); + }) + .parse(); + throw Error('unreachable'); + } catch (err) { + err.message.should.match(/Invalid first argument/); + } + }); }); }); @@ -3129,6 +3155,29 @@ describe('usage tests', () => { return done(); } }); + it('should not run handler or middleware', done => { + let commandRun = false; + let middlewareRun = false; + const y = yargs(['foo']) + .command( + 'foo', + 'foo command', + () => {}, + () => { + commandRun = true; + } + ) + .middleware(() => { + middlewareRun = true; + }); + y.showHelp(printCallback); + function printCallback(msg) { + commandRun.should.equal(false); + middlewareRun.should.equal(false); + msg.should.match(/foo command/); + return done(); + } + }); }); describe('$0', () => { diff --git a/test/yargs.cjs b/test/yargs.cjs index 07dda208a..a9f3844fa 100644 --- a/test/yargs.cjs +++ b/test/yargs.cjs @@ -2824,7 +2824,7 @@ describe('yargs dsl tests', () => { setTimeout(() => { argv.addedAsync = 99; executionCount++; - return resolve(argv); + return resolve(); }, 5); }); } @@ -2851,7 +2851,7 @@ describe('yargs dsl tests', () => { setTimeout(() => { argv.addedAsync = 99; executionCount++; - return resolve(argv); + return resolve(); }, 5); }); } @@ -2956,6 +2956,114 @@ describe('yargs dsl tests', () => { }); }); }); - // TODO(@bcoe): write tests for new async getHelp() method. - // TODO(@bcoe): document .fail(false) shorthand. + + describe('getHelp', () => { + it('should run parse() and return help, if parse() not yet called', async () => { + const y = yargs(['--foo']) + .options('foo', { + alias: 'f', + describe: 'foo option', + }) + .wrap(null); + const help = await y.getHelp(); + help + .split('\n') + .should.deep.equal([ + 'Options:', + ' --help Show help [boolean]', + ' --version Show version number [boolean]', + ' -f, --foo foo option', + ]); + }); + it('should display top-level help with no command given', async () => { + const y = yargs('--help') + .command( + ['list [pattern]', 'ls', '*'], + 'List key-value pairs for pattern', + {}, + noop + ) + .command('get ', 'Get value for key', {}, noop) + .command('set [value]', 'Set value for key', {}, noop); + const help = await y.getHelp(); + help + .split('\n') + .should.deep.equal([ + 'node [pattern]', + '', + 'List key-value pairs for pattern', + '', + 'Commands:', + ' node list [pattern] List key-value pairs for pattern', + ' [default] [aliases: ls]', + ' node get Get value for key', + ' node set [value] Set value for key', + '', + 'Options:', + ' --help Show help [boolean]', + ' --version Show version number [boolean]', + ]); + }); + it('should allow help to be output for failed command', async () => { + const y = yargs('foo') + .command( + 'foo', + 'foo command', + () => {}, + async () => { + return new Promise((resolve, reject) => { + setTimeout(() => { + return reject(Error('async failure')); + }); + }); + } + ) + .fail(false); + try { + await y.argv; + throw Error('unreachable'); + } catch (err) { + err.message.should.equal('async failure'); + (await y.getHelp()).should.match(/foo command/); + } + }); + it('should allow help to be output for successful command', async () => { + const y = yargs('foo').command( + 'foo', + 'foo command', + () => {}, + async argv => { + return new Promise((resolve, reject) => { + setTimeout(() => { + argv.addedAsync = 99; + return resolve(); + }); + }); + } + ); + const argv = await y.argv; + (await y.getHelp()).should.match(/foo command/); + argv.addedAsync.should.equal(99); + argv._.should.eql(['foo']); + }); + it('should not run handler or middleware', async () => { + let commandCalled = false; + let middlewareCalled = false; + const y = yargs('foo') + .command( + 'foo', + 'foo command', + () => {}, + async argv => { + commandCalled = true; + } + ) + .middleware(() => { + middlewareCalled = true; + }); + (await y.getHelp()).should.match(/foo command/); + commandCalled.should.equal(false); + middlewareCalled.should.equal(false); + }); + }); }); From 73c133665f399558e52e6ceb2d7a94d1cb0bc4cf Mon Sep 17 00:00:00 2001 From: bcoe Date: Mon, 4 Jan 2021 17:54:18 -0800 Subject: [PATCH 08/15] test: add test for 1791 --- lib/command.ts | 4 ++-- lib/yargs-factory.ts | 10 ++++++++-- test/usage.cjs | 17 +++++++++++++++++ 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/lib/command.ts b/lib/command.ts index e4958787c..c21c417e3 100644 --- a/lib/command.ts +++ b/lib/command.ts @@ -600,8 +600,8 @@ export interface CommandInstance { command: string | null, yargs: YargsInstance, parsed: DetailedArguments, - commandIndex?: number, - helpOnly?: boolean + commandIndex: number, + helpOnly: boolean ): Arguments | Promise; runDefaultBuilderOn(yargs: YargsInstance): void; unfreeze(): void; diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index 6d93b1fdf..60335e058 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -1572,7 +1572,13 @@ function Yargs( // run the default command, if defined if (command.hasDefaultCommand() && !skipDefaultCommand) { - const innerArgv = command.runCommand(null, self, parsed); + const innerArgv = command.runCommand( + null, + self, + parsed, + 0, + helpOnly + ); return self._postProcess(innerArgv, populateDoubleDash); } @@ -1594,7 +1600,7 @@ function Yargs( self.exit(0); } } else if (command.hasDefaultCommand() && !skipDefaultCommand) { - const innerArgv = command.runCommand(null, self, parsed); + const innerArgv = command.runCommand(null, self, parsed, 0, helpOnly); return self._postProcess(innerArgv, populateDoubleDash); } diff --git a/test/usage.cjs b/test/usage.cjs index 5fa5b37b7..a2ae6a688 100644 --- a/test/usage.cjs +++ b/test/usage.cjs @@ -3178,6 +3178,23 @@ describe('usage tests', () => { return done(); } }); + // See: https://github.com/yargs/yargs/issues/1791 + it('should not run default command', done => { + let executed = false; + yargs.command( + '$0', + 'a default command', + yargs => yargs, + () => { + executed = true; + } + ); + yargs.showHelp(output => { + executed.should.equal(false); + output.should.match(/a default command/); + return done(); + }); + }); }); describe('$0', () => { From b1d066c1f42361cecbd36051fe7c3a167344560c Mon Sep 17 00:00:00 2001 From: bcoe Date: Mon, 4 Jan 2021 18:17:08 -0800 Subject: [PATCH 09/15] build: run coverage on latest node --- .github/workflows/ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 19114b433..8a3edd933 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -62,7 +62,7 @@ jobs: - uses: actions/checkout@v1 - uses: actions/setup-node@v1 with: - node-version: 13 + node-version: 15 - run: npm install - run: npm test - run: npm run coverage From 7c354982431b219fa475a84ff10639c76148d079 Mon Sep 17 00:00:00 2001 From: bcoe Date: Sat, 9 Jan 2021 10:46:28 -0800 Subject: [PATCH 10/15] test: add additional tests for async middleware --- test/command.cjs | 27 ++++++++++++++++++++- test/middleware.cjs | 58 +++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 82 insertions(+), 3 deletions(-) diff --git a/test/command.cjs b/test/command.cjs index 8f58e83da..ab284f570 100644 --- a/test/command.cjs +++ b/test/command.cjs @@ -1787,7 +1787,32 @@ describe('Command', () => { parsed.pos.should.equal('hello'); }); - // TODO(bcoe): add test for handler rejecting, when using parsedPromise. + it('returns promise that can be caught, when fail(false)', async () => { + let complete = false; + const handler = new Promise((resolve, reject) => { + setTimeout(() => { + complete = true; + return reject(Error('error from handler')); + }, 10); + }); + const parsedPromise = yargs('foo hello') + .command( + 'foo ', + 'foo command', + () => {}, + () => handler + ) + .fail(false) + .parse(); + try { + complete.should.equal(false); + await parsedPromise; + throw Error('unreachable'); + } catch (err) { + err.message.should.match(/error from handler/); + complete.should.equal(true); + } + }); // see: https://github.com/yargs/yargs/issues/1144 it('displays error and appropriate help message when handler fails', done => { diff --git a/test/middleware.cjs b/test/middleware.cjs index 6212b4279..0c779220c 100644 --- a/test/middleware.cjs +++ b/test/middleware.cjs @@ -138,8 +138,62 @@ describe('middleware', () => { .parse(); }); - // TODO(bcoe): add test for middleware rejecting, when using promise form. - // TODO(bcoe): add test that ensures handler awaited after middleware. + it('it allows middleware rejection to be caught', async () => { + const argvPromise = yargs('foo') + .command( + 'foo', + 'foo command', + () => {}, + () => {} + ) + .middleware(async () => { + return new Promise((resolve, reject) => { + setTimeout(() => { + return reject(Error('error from middleware')); + }, 5); + }); + }) + .fail(false) + .parse(); + try { + await argvPromise; + throw Error('unreachable'); + } catch (err) { + err.message.should.match(/error from middleware/); + } + }); + + it('it awaits middleware before awaiting handler, when applyBeforeValidation is "false"', async () => { + let log = ''; + const argvPromise = yargs('foo --bar') + .command( + 'foo', + 'foo command', + () => {}, + async () => { + return new Promise(resolve => { + setTimeout(() => { + log += 'handler'; + return resolve(); + }, 5); + }); + } + ) + .middleware(async argv => { + return new Promise(resolve => { + setTimeout(() => { + log += 'middleware'; + argv.fromMiddleware = 99; + return resolve(); + }, 20); + }); + }, false) + .parse(); + const argv = await argvPromise; + log.should.equal('middlewarehandler'); + argv.fromMiddleware.should.equal(99); + argv.bar.should.equal(true); + }); it('calls the command handler when all middleware promises resolve', done => { const middleware = (key, value) => () => From b997ebaa6d1f3f34b9985cf80f60fe7dcc91ceaa Mon Sep 17 00:00:00 2001 From: bcoe Date: Sat, 9 Jan 2021 11:35:32 -0800 Subject: [PATCH 11/15] docs: add async examples --- docs/advanced.md | 63 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/docs/advanced.md b/docs/advanced.md index 25792947c..79fbdabb4 100644 --- a/docs/advanced.md +++ b/docs/advanced.md @@ -554,3 +554,66 @@ var argv = require('yargs/yargs')(process.argv.slice(2)) ) .argv; ``` + +## Using Yargs with Async/await + +If you use async middleware or async handlers for commands, `yargs.parse` and +`yargs.argv` will return a Promise. When you `await` this promise the final +parsed result will be returned: + +```js +import yargs from 'yargs' +import { hideBin } from 'yargs/helpers' + +async function processValue(value) { + return new Promise((resolve) => { + // Perform some async operation on value. + setTimeout(() => { + return resolve(value) + }, 1000) + }) +} + +console.info('start') +await yargs(hideBin(process.argv)) + .command('add ', 'add two eventual values', () => {}, async (argv) => { + const sum = await processValue(argv.x) + await processValue(argv.y) + console.info(`x + y = ${sum}`) + }).parse() +console.info('finish') +``` + +### Handling async errors + +By default, when an error occurs in an during an async parse, yargs will +exit with code `1` and print the help message. If you would rather +Use `try`/`catch` to perform error handling, you can do so with the setting +`.fail(false)`: + +```js +import yargs from 'yargs' +import { hideBin } from 'yargs/helpers' + +async function processValue(value) { + return new Promise((resolve, reject) => { + // Perform some async operation on value. + setTimeout(() => { + return reject(Error('something went wrong')) + }, 1000) + }) +} + +console.info('start') +const parser = yargs(hideBin(process.argv)) + .command('add ', 'add two eventual values', () => {}, async (argv) => { + const sum = await processValue(argv.x) + await processValue(argv.y) + console.info(`x + y = ${sum}`) + }) + .fail(false) +try { + const argv = await parser.parse(); +} catch (err) { + console.info(`${err.message}\n ${await parser.getHelp()}`) +} +console.info('finish') +``` From 924ce59935a7b96ea726a85c9179e91132a71373 Mon Sep 17 00:00:00 2001 From: bcoe Date: Sun, 10 Jan 2021 17:27:32 -0800 Subject: [PATCH 12/15] docs: wordsmith docs --- docs/advanced.md | 8 ++++---- package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/advanced.md b/docs/advanced.md index 79fbdabb4..436800718 100644 --- a/docs/advanced.md +++ b/docs/advanced.md @@ -558,8 +558,8 @@ var argv = require('yargs/yargs')(process.argv.slice(2)) ## Using Yargs with Async/await If you use async middleware or async handlers for commands, `yargs.parse` and -`yargs.argv` will return a Promise. When you `await` this promise the final -parsed result will be returned: +`yargs.argv` will return a `Promise`. When you `await` this promise the +parsed arguments object will be returned after the handler completes. ```js import yargs from 'yargs' @@ -585,8 +585,8 @@ console.info('finish') ### Handling async errors -By default, when an error occurs in an during an async parse, yargs will -exit with code `1` and print the help message. If you would rather +By default, when an async error occurs within a command yargs will +exit with code `1` and print a help message. If you would rather Use `try`/`catch` to perform error handling, you can do so with the setting `.fail(false)`: diff --git a/package.json b/package.json index f55e213b7..292c02905 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "yargs", - "version": "16.2.0", + "version": "17.0.0-candidate.0", "description": "yargs the modern, pirate-themed, successor to optimist.", "main": "./index.cjs", "exports": { From e955790cf4264d2d2ae514b5b129adcdcdcc6cd8 Mon Sep 17 00:00:00 2001 From: bcoe Date: Sun, 10 Jan 2021 17:34:57 -0800 Subject: [PATCH 13/15] docs: slight tweaks to wording --- docs/advanced.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/advanced.md b/docs/advanced.md index 436800718..e32f475c9 100644 --- a/docs/advanced.md +++ b/docs/advanced.md @@ -559,7 +559,7 @@ var argv = require('yargs/yargs')(process.argv.slice(2)) If you use async middleware or async handlers for commands, `yargs.parse` and `yargs.argv` will return a `Promise`. When you `await` this promise the -parsed arguments object will be returned after the handler completes. +parsed arguments object will be returned after the handler completes: ```js import yargs from 'yargs' @@ -587,7 +587,7 @@ console.info('finish') By default, when an async error occurs within a command yargs will exit with code `1` and print a help message. If you would rather -Use `try`/`catch` to perform error handling, you can do so with the setting +Use `try`/`catch` to perform error handling, you can do so by setting `.fail(false)`: ```js From f78ee0cfdaf79e3133d3fb3c93706bf699752484 Mon Sep 17 00:00:00 2001 From: bcoe Date: Sun, 10 Jan 2021 17:37:49 -0800 Subject: [PATCH 14/15] docs: continuing to work on wording --- docs/api.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/api.md b/docs/api.md index 6fba124f9..adfefc263 100644 --- a/docs/api.md +++ b/docs/api.md @@ -814,9 +814,9 @@ wanted to exit. Follows the behavior set by `.exitProcess()`. Method to execute when a failure occurs, rather than printing the failure message. -Providing `false` as a value for `fn` can be used to prevent failure -messages from being output. _This is useful if you wish to handle failures -yourself, using `try`/`catch` and [`.getHelp()`](#get-help). +Providing `false` as a value for `fn` can be used to prevent yargs from +exiting and printing a failure message. _This is useful if you wish to +handle failures yourself using `try`/`catch` and [`.getHelp()`](#get-help). `fn` is called with the failure message that would have been printed, the `Error` instance originally thrown and yargs state when the failure From 08768b15a7d7319a30b8ff995108d79d64428605 Mon Sep 17 00:00:00 2001 From: bcoe Date: Sun, 10 Jan 2021 17:40:15 -0800 Subject: [PATCH 15/15] docs: fix typo --- docs/api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api.md b/docs/api.md index adfefc263..1a9704c5c 100644 --- a/docs/api.md +++ b/docs/api.md @@ -815,7 +815,7 @@ wanted to exit. Follows the behavior set by `.exitProcess()`. Method to execute when a failure occurs, rather than printing the failure message. Providing `false` as a value for `fn` can be used to prevent yargs from -exiting and printing a failure message. _This is useful if you wish to +exiting and printing a failure message. This is useful if you wish to handle failures yourself using `try`/`catch` and [`.getHelp()`](#get-help). `fn` is called with the failure message that would have been printed, the