From 43f368bbdb018cb793125522b9757e3ea1c2083c Mon Sep 17 00:00:00 2001 From: Landon Yarrington Date: Sat, 26 Mar 2022 09:18:38 -0600 Subject: [PATCH 1/4] Show message when showHelpOnFail is chained globally --- lib/usage.ts | 25 ++++++++----- lib/yargs-factory.ts | 12 +++---- test/usage.cjs | 85 +++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 105 insertions(+), 17 deletions(-) diff --git a/lib/usage.ts b/lib/usage.ts index cea2f7c22..ec2013baf 100644 --- a/lib/usage.ts +++ b/lib/usage.ts @@ -2,7 +2,7 @@ // failures, etc. keeps logging in one place. import {Dictionary, PlatformShim} from './typings/common-types.js'; import {objFilter} from './utils/obj-filter.js'; -import {YargsInstance} from './yargs-factory.js'; +import {YargsInstance, nil} from './yargs-factory.js'; import {YError} from './yerror.js'; import {DetailedArguments} from './typings/yargs-parser-types.js'; import setBlocking from './utils/set-blocking.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().getContext().commands.length === 0) { + 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); } } @@ -776,14 +783,14 @@ export interface UsageInstance { 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..23cd5c799 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -172,7 +172,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; @@ -2228,6 +2228,8 @@ export class YargsInstance { } } +export type nil = undefined | null; + export function isYargsInstance(y: YargsInstance | void): y is YargsInstance { return !!y && typeof y.getInternalMethods === 'function'; } @@ -2344,7 +2346,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 +2354,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..453d1e865 100644 --- a/test/usage.cjs +++ b/test/usage.cjs @@ -1682,7 +1682,7 @@ describe('usage tests', () => { }); describe('showHelpOnFail', () => { - it('should display user supplied message', () => { + it('should display user supplied message (command usage)', () => { const opts = { foo: {desc: 'foo option', alias: 'f'}, bar: {desc: 'bar option', alias: 'b'}, @@ -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', () => { From 016d575a8eaefd061739faafe24c65c976423ad9 Mon Sep 17 00:00:00 2001 From: Landon Yarrington Date: Sat, 26 Mar 2022 09:20:02 -0600 Subject: [PATCH 2/4] oops --- test/usage.cjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/usage.cjs b/test/usage.cjs index 453d1e865..becd9b125 100644 --- a/test/usage.cjs +++ b/test/usage.cjs @@ -1682,7 +1682,7 @@ describe('usage tests', () => { }); describe('showHelpOnFail', () => { - it('should display user supplied message (command usage)', () => { + it('should display user supplied message', () => { const opts = { foo: {desc: 'foo option', alias: 'f'}, bar: {desc: 'bar option', alias: 'b'}, From 43aa30947f0cfc71efb9eede470cbcca0b67077e Mon Sep 17 00:00:00 2001 From: Landon Yarrington Date: Sat, 26 Mar 2022 09:29:07 -0600 Subject: [PATCH 3/4] nil --- lib/typings/common-types.ts | 5 +++++ lib/typings/yargs-parser-types.ts | 4 ++-- lib/usage.ts | 8 ++++---- lib/yargs-factory.ts | 5 ++--- 4 files changed, 13 insertions(+), 9 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 ec2013baf..4e49cbe08 100644 --- a/lib/usage.ts +++ b/lib/usage.ts @@ -1,8 +1,8 @@ // 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, nil} from './yargs-factory.js'; +import {YargsInstance} from './yargs-factory.js'; import {YError} from './yerror.js'; import {DetailedArguments} from './typings/yargs-parser-types.js'; import setBlocking from './utils/set-blocking.js'; @@ -157,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; @@ -778,7 +778,7 @@ 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 { diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index 23cd5c799..e5bf30dcf 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, @@ -1423,7 +1424,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; @@ -2228,8 +2229,6 @@ export class YargsInstance { } } -export type nil = undefined | null; - export function isYargsInstance(y: YargsInstance | void): y is YargsInstance { return !!y && typeof y.getInternalMethods === 'function'; } From 22609b321f36fc9dcd078be35f1a25475818dc3d Mon Sep 17 00:00:00 2001 From: Landon Yarrington Date: Wed, 30 Mar 2022 16:29:37 -0600 Subject: [PATCH 4/4] fixes --- lib/usage.ts | 2 +- lib/yargs-factory.ts | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/usage.ts b/lib/usage.ts index 4e49cbe08..ba54cbcde 100644 --- a/lib/usage.ts +++ b/lib/usage.ts @@ -32,7 +32,7 @@ export function usage(yargs: YargsInstance, shim: PlatformShim) { // If global context, set globalFailMessage // Addresses: https://github.com/yargs/yargs/issues/2085 - if (yargs.getInternalMethods().getContext().commands.length === 0) { + if (yargs.getInternalMethods().isGlobalContext()) { globalFailMessage = message; } diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index e5bf30dcf..69d882072 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -117,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'); @@ -136,6 +137,7 @@ export interface YargsInternalMethods { getUsageInstance(): UsageInstance; getValidationInstance(): ValidationInstance; hasParseCallback(): boolean; + isGlobalContext(): boolean; postProcess>( argv: Arguments | Promise, populateDoubleDash: boolean, @@ -182,6 +184,7 @@ export class YargsInstance { #groups: Dictionary = {}; #hasOutput = false; #helpOpt: string | null = null; + #isGlobalContext = true; #logger: LoggerInstance; #output = ''; #options: Options; @@ -1761,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), @@ -1793,6 +1797,9 @@ export class YargsInstance { [kHasParseCallback](): boolean { return !!this.#parseFn; } + [kIsGlobalContext](): boolean { + return this.#isGlobalContext; + } [kPostProcess]>( argv: Arguments | Promise, populateDoubleDash: boolean, @@ -2013,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;