From ad9fcacb001a7eb842924408f3a06865a7c7a3b6 Mon Sep 17 00:00:00 2001 From: Landon Yarrington <33426811+jly36963@users.noreply.github.com> Date: Thu, 31 Mar 2022 09:39:16 -0600 Subject: [PATCH] fix: show message when showHelpOnFail is chained globally (#2154) --- lib/typings/common-types.ts | 5 ++ lib/typings/yargs-parser-types.ts | 4 +- lib/usage.ts | 29 +++++++---- lib/yargs-factory.ts | 22 +++++--- test/usage.cjs | 83 +++++++++++++++++++++++++++++++ 5 files changed, 122 insertions(+), 21 deletions(-) diff --git a/lib/typings/common-types.ts b/lib/typings/common-types.ts index 10e44d5ec..2c1c15cb8 100644 --- a/lib/typings/common-types.ts +++ b/lib/typings/common-types.ts @@ -1,5 +1,10 @@ import {Parser} from './yargs-parser-types.js'; +/** + * A type that represents undefined or null + */ +export type nil = undefined | null; + /** * An object whose all properties have the same type. */ diff --git a/lib/typings/yargs-parser-types.ts b/lib/typings/yargs-parser-types.ts index 4a25fa260..a001d5847 100644 --- a/lib/typings/yargs-parser-types.ts +++ b/lib/typings/yargs-parser-types.ts @@ -1,6 +1,6 @@ // Taken from yargs-parser@19.0.1 // TODO: update this file periodically. -import type {Dictionary, ValueOf} from './common-types.js'; +import type {Dictionary, ValueOf, nil} from './common-types.js'; type KeyOf = { [K in keyof T]: string extends K ? never : number extends K ? never : K; @@ -142,7 +142,7 @@ export interface Parser { detailed(args: ArgsInput, opts?: Partial): DetailedArguments; camelCase(str: string): string; decamelize(str: string, joinString?: string): string; - looksLikeNumber(x: null | undefined | number | string): boolean; + looksLikeNumber(x: nil | number | string): boolean; } export declare type StringFlag = Dictionary; export declare type BooleanFlag = Dictionary; diff --git a/lib/usage.ts b/lib/usage.ts index cea2f7c22..ba54cbcde 100644 --- a/lib/usage.ts +++ b/lib/usage.ts @@ -1,6 +1,6 @@ // this file handles outputting usage instructions, // failures, etc. keeps logging in one place. -import {Dictionary, PlatformShim} from './typings/common-types.js'; +import {Dictionary, PlatformShim, nil} from './typings/common-types.js'; import {objFilter} from './utils/obj-filter.js'; import {YargsInstance} from './yargs-factory.js'; import {YError} from './yerror.js'; @@ -20,16 +20,22 @@ export function usage(yargs: YargsInstance, shim: PlatformShim) { self.failFn = function failFn(f) { fails.push(f); }; - let failMessage: string | undefined | null = null; + let failMessage: string | nil = null; + let globalFailMessage: string | nil = null; let showHelpOnFail = true; self.showHelpOnFail = function showHelpOnFailFn( arg1: boolean | string = true, arg2?: string ) { - function parseFunctionArgs(): [boolean, string?] { - return typeof arg1 === 'string' ? [true, arg1] : [arg1, arg2]; + const [enabled, message] = + typeof arg1 === 'string' ? [true, arg1] : [arg1, arg2]; + + // If global context, set globalFailMessage + // Addresses: https://github.com/yargs/yargs/issues/2085 + if (yargs.getInternalMethods().isGlobalContext()) { + globalFailMessage = message; } - const [enabled, message] = parseFunctionArgs(); + failMessage = message; showHelpOnFail = enabled; return self; @@ -60,9 +66,10 @@ export function usage(yargs: YargsInstance, shim: PlatformShim) { logger.error(); } if (msg || err) logger.error(msg || err); - if (failMessage) { + const globalOrCommandFailMessage = failMessage || globalFailMessage; + if (globalOrCommandFailMessage) { if (msg || err) logger.error(''); - logger.error(failMessage); + logger.error(globalOrCommandFailMessage); } } @@ -150,7 +157,7 @@ export function usage(yargs: YargsInstance, shim: PlatformShim) { }; let wrapSet = false; - let wrap: number | null | undefined; + let wrap: number | nil; self.wrap = cols => { wrapSet = true; wrap = cols; @@ -771,19 +778,19 @@ export interface UsageInstance { unfreeze(defaultCommand?: boolean): void; usage(msg: string | null, description?: string | false): UsageInstance; version(ver: any): void; - wrap(cols: number | null | undefined): void; + wrap(cols: number | nil): void; } export interface FailureFunction { ( - msg: string | undefined | null, + msg: string | nil, err: YError | string | undefined, usage: UsageInstance ): void; } export interface FrozenUsageInstance { - failMessage: string | undefined | null; + failMessage: string | nil; failureOutput: boolean; usages: [string, string][]; usageDisabled: boolean; diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index 2dff7e395..69d882072 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -22,6 +22,7 @@ import type { RequireDirectoryOptions, PlatformShim, RequireType, + nil, } from './typings/common-types.js'; import { assertNotStrictEqual, @@ -116,6 +117,7 @@ const kGetParseContext = Symbol('getParseContext'); const kGetUsageInstance = Symbol('getUsageInstance'); const kGetValidationInstance = Symbol('getValidationInstance'); const kHasParseCallback = Symbol('hasParseCallback'); +const kIsGlobalContext = Symbol('isGlobalContext'); const kPostProcess = Symbol('postProcess'); const kRebase = Symbol('rebase'); const kReset = Symbol('reset'); @@ -135,6 +137,7 @@ export interface YargsInternalMethods { getUsageInstance(): UsageInstance; getValidationInstance(): ValidationInstance; hasParseCallback(): boolean; + isGlobalContext(): boolean; postProcess>( argv: Arguments | Promise, populateDoubleDash: boolean, @@ -172,7 +175,7 @@ export class YargsInstance { #completion: CompletionInstance | null = null; #completionCommand: string | null = null; #defaultShowHiddenOpt = 'show-hidden'; - #exitError: YError | string | undefined | null = null; + #exitError: YError | string | nil = null; #detectLocale = true; #emittedWarnings: Dictionary = {}; #exitProcess = true; @@ -181,6 +184,7 @@ export class YargsInstance { #groups: Dictionary = {}; #hasOutput = false; #helpOpt: string | null = null; + #isGlobalContext = true; #logger: LoggerInstance; #output = ''; #options: Options; @@ -1423,7 +1427,7 @@ export class YargsInstance { this.describe(this.#versionOpt, msg); return this; } - wrap(cols: number | null | undefined): YargsInstance { + wrap(cols: number | nil): YargsInstance { argsert('', [cols], arguments.length); this.#usage.wrap(cols); return this; @@ -1760,6 +1764,7 @@ export class YargsInstance { getUsageInstance: this[kGetUsageInstance].bind(this), getValidationInstance: this[kGetValidationInstance].bind(this), hasParseCallback: this[kHasParseCallback].bind(this), + isGlobalContext: this[kIsGlobalContext].bind(this), postProcess: this[kPostProcess].bind(this), reset: this[kReset].bind(this), runValidation: this[kRunValidation].bind(this), @@ -1792,6 +1797,9 @@ export class YargsInstance { [kHasParseCallback](): boolean { return !!this.#parseFn; } + [kIsGlobalContext](): boolean { + return this.#isGlobalContext; + } [kPostProcess]>( argv: Arguments | Promise, populateDoubleDash: boolean, @@ -2012,6 +2020,8 @@ export class YargsInstance { } } + this.#isGlobalContext = false; + const handlerKeys = this.#command.getCommands(); const requestCompletions = this.#completion!.completionKey in argv; const skipRecommendation = helpOptSet || requestCompletions || helpOnly; @@ -2344,7 +2354,7 @@ interface FrozenYargsInstance { strictOptions: boolean; completionCommand: string | null; output: string; - exitError: YError | string | undefined | null; + exitError: YError | string | nil; hasOutput: boolean; parsed: DetailedArguments | false; parseFn: ParseCallback | null; @@ -2352,11 +2362,7 @@ interface FrozenYargsInstance { } interface ParseCallback { - ( - err: YError | string | undefined | null, - argv: Arguments, - output: string - ): void; + (err: YError | string | nil, argv: Arguments, output: string): void; } interface Aliases { diff --git a/test/usage.cjs b/test/usage.cjs index 1697364a6..becd9b125 100644 --- a/test/usage.cjs +++ b/test/usage.cjs @@ -1711,6 +1711,89 @@ describe('usage tests', () => { 'Specify --help for available options', ]); }); + + describe('should handle being chained both globally and on a command ', () => { + const options = { + opt1: { + alias: 'o', + demandOption: true, + }, + }; + const cmdMessage = 'What have you done (command)????'; + const globalMessage = 'What have you done (global)????'; + const errorMessage = 'Unknown argument: extraOpt'; + + it('chained on to command', () => { + const r = checkUsage(() => + yargs('cmd1 --opt1 hello --extraOpt oops') + .command( + 'cmd1', + 'cmd1 desc', + yargs => yargs.options(options).showHelpOnFail(false, cmdMessage), + argv => console.log(argv) + ) + .strict() + .parse() + ); + r.should.have.property('result'); + r.result.should.have.property('_').with.length(1); + r.should.have.property('errors'); + r.should.have.property('logs').with.length(0); + r.should.have.property('exit').and.equal(true); + r.errors + .join('\n') + .split(/\n/) + .should.deep.equal([errorMessage, '', cmdMessage]); + }); + + it('chained on globally', () => { + const r = checkUsage(() => + yargs('cmd1 --opt1 hello --extraOpt oops') + .command( + 'cmd1', + 'cmd1 desc', + yargs => yargs.option(options), + argv => console.log(argv) + ) + .showHelpOnFail(false, globalMessage) + .strict() + .parse() + ); + r.should.have.property('result'); + r.result.should.have.property('_').with.length(1); + r.should.have.property('errors'); + r.should.have.property('logs').with.length(0); + r.should.have.property('exit').and.equal(true); + r.errors + .join('\n') + .split(/\n/) + .should.deep.equal([errorMessage, '', globalMessage]); + }); + + it('chained on command and globally (priority given to command message)', () => { + const r = checkUsage(() => + yargs('cmd1 --opt1 hello --extraOpt oops') + .command( + 'cmd1', + 'cmd1 desc', + yargs => yargs.option(options).showHelpOnFail(false, cmdMessage), + argv => console.log(argv) + ) + .showHelpOnFail(false, globalMessage) + .strict() + .parse() + ); + r.should.have.property('result'); + r.result.should.have.property('_').with.length(1); + r.should.have.property('errors'); + r.should.have.property('logs').with.length(0); + r.should.have.property('exit').and.equal(true); + r.errors + .join('\n') + .split(/\n/) + .should.deep.equal([errorMessage, '', cmdMessage]); + }); + }); }); describe('exitProcess', () => {