Skip to content

Commit

Permalink
fix: show message when showHelpOnFail is chained globally (#2154)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jly36963
jly36963 committed Mar 31, 2022
1 parent b58b5bc commit ad9fcac
Show file tree
Hide file tree
Showing 5 changed files with 122 additions and 21 deletions.
5 changes: 5 additions & 0 deletions lib/typings/common-types.ts
View file
@@ -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.
*/
Expand Down
4 changes: 2 additions & 2 deletions lib/typings/yargs-parser-types.ts
View file
@@ -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<T> = {
[K in keyof T]: string extends K ? never : number extends K ? never : K;
Expand Down Expand Up @@ -142,7 +142,7 @@ export interface Parser {
detailed(args: ArgsInput, opts?: Partial<Options>): 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<string[]>;
export declare type BooleanFlag = Dictionary<boolean>;
Expand Down
29 changes: 18 additions & 11 deletions lib/usage.ts
View file
@@ -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';
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
22 changes: 14 additions & 8 deletions lib/yargs-factory.ts
View file
Expand Up @@ -22,6 +22,7 @@ import type {
RequireDirectoryOptions,
PlatformShim,
RequireType,
nil,
} from './typings/common-types.js';
import {
assertNotStrictEqual,
Expand Down Expand Up @@ -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');
Expand All @@ -135,6 +137,7 @@ export interface YargsInternalMethods {
getUsageInstance(): UsageInstance;
getValidationInstance(): ValidationInstance;
hasParseCallback(): boolean;
isGlobalContext(): boolean;
postProcess<T extends Arguments | Promise<Arguments>>(
argv: Arguments | Promise<Arguments>,
populateDoubleDash: boolean,
Expand Down Expand Up @@ -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<boolean> = {};
#exitProcess = true;
Expand All @@ -181,6 +184,7 @@ export class YargsInstance {
#groups: Dictionary<string[]> = {};
#hasOutput = false;
#helpOpt: string | null = null;
#isGlobalContext = true;
#logger: LoggerInstance;
#output = '';
#options: Options;
Expand Down Expand Up @@ -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('<number|null|undefined>', [cols], arguments.length);
this.#usage.wrap(cols);
return this;
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -1792,6 +1797,9 @@ export class YargsInstance {
[kHasParseCallback](): boolean {
return !!this.#parseFn;
}
[kIsGlobalContext](): boolean {
return this.#isGlobalContext;
}
[kPostProcess]<T extends Arguments | Promise<Arguments>>(
argv: Arguments | Promise<Arguments>,
populateDoubleDash: boolean,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -2344,19 +2354,15 @@ 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;
parseContext: object | null;
}

interface ParseCallback {
(
err: YError | string | undefined | null,
argv: Arguments,
output: string
): void;
(err: YError | string | nil, argv: Arguments, output: string): void;
}

interface Aliases {
Expand Down
83 changes: 83 additions & 0 deletions test/usage.cjs
View file
Expand Up @@ -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', () => {
Expand Down

0 comments on commit ad9fcac

Please sign in to comment.