Skip to content

Commit

Permalink
fix(builder): apply default builder for showHelp/getHelp (#1913)
Browse files Browse the repository at this point in the history 
The builder for default commands was not being properly applied when .showHelp() and .getHelp() were used.

Fixes #1912
  • Loading branch information
@bcoe
bcoe committed Apr 12, 2021
1 parent d2128cc commit 395bb67
Show file tree
Hide file tree
Showing 5 changed files with 135 additions and 23 deletions.
34 changes: 20 additions & 14 deletions lib/command.ts
View file
Expand Up @@ -209,12 +209,13 @@ export class CommandInstance {
this.defaultCommand;
const currentContext = yargs.getInternalMethods().getContext();
const parentCommands = currentContext.commands.slice();
const isDefaultCommand = !command;
if (command) {
currentContext.commands.push(command);
currentContext.fullCommands.push(commandHandler.original);
}
const builderResult = this.applyBuilderUpdateUsageAndParse(
command,
isDefaultCommand,
commandHandler,
yargs,
parsed.aliases,
Expand All @@ -226,7 +227,7 @@ export class CommandInstance {
if (isPromise(builderResult)) {
return builderResult.then(result => {
return this.applyMiddlewareAndGetResult(
command,
isDefaultCommand,
commandHandler,
result.innerArgv,
currentContext,
Expand All @@ -237,7 +238,7 @@ export class CommandInstance {
});
} else {
return this.applyMiddlewareAndGetResult(
command,
isDefaultCommand,
commandHandler,
builderResult.innerArgv,
currentContext,
Expand All @@ -248,7 +249,7 @@ export class CommandInstance {
}
}
private applyBuilderUpdateUsageAndParse(
command: string | null,
isDefaultCommand: boolean,
commandHandler: CommandHandler,
yargs: YargsInstance,
aliases: Dictionary<string[]>,
Expand All @@ -273,7 +274,7 @@ export class CommandInstance {
return builderOutput.then(output => {
innerYargs = isYargsInstance(output) ? output : yargs;
return this.parseAndUpdateUsage(
command,
isDefaultCommand,
commandHandler,
innerYargs,
parentCommands,
Expand All @@ -291,7 +292,7 @@ export class CommandInstance {
});
}
return this.parseAndUpdateUsage(
command,
isDefaultCommand,
commandHandler,
innerYargs,
parentCommands,
Expand All @@ -300,7 +301,7 @@ export class CommandInstance {
);
}
private parseAndUpdateUsage(
command: string | null,
isDefaultCommand: boolean,
commandHandler: CommandHandler,
innerYargs: YargsInstance,
parentCommands: string[],
Expand All @@ -310,7 +311,8 @@ export class CommandInstance {
// A null command indicates we are running the default command,
// if this is the case, we should show the root usage instructions
// rather than the usage instructions for the nested default command:
if (!command) innerYargs.getInternalMethods().getUsageInstance().unfreeze();
if (isDefaultCommand)
innerYargs.getInternalMethods().getUsageInstance().unfreeze();
if (this.shouldUpdateUsage(innerYargs)) {
innerYargs
.getInternalMethods()
Expand Down Expand Up @@ -357,7 +359,7 @@ export class CommandInstance {
return `$0 ${pc.join(' ')}`;
}
private applyMiddlewareAndGetResult(
command: string | null,
isDefaultCommand: boolean,
commandHandler: CommandHandler,
innerArgv: Arguments | Promise<Arguments>,
currentContext: Context,
Expand Down Expand Up @@ -393,7 +395,7 @@ export class CommandInstance {
aliases,
positionalMap,
(yargs.parsed as DetailedArguments).error,
!command
isDefaultCommand
);
innerArgv = maybeAsyncResult<Arguments>(innerArgv, result => {
validation(result);
Expand Down Expand Up @@ -422,7 +424,10 @@ export class CommandInstance {
}
});

yargs.getInternalMethods().getUsageInstance().cacheHelpMessage();
if (!isDefaultCommand) {
yargs.getInternalMethods().getUsageInstance().cacheHelpMessage();
}

if (
isPromise(innerArgv) &&
!yargs.getInternalMethods().hasParseCallback()
Expand All @@ -438,7 +443,7 @@ export class CommandInstance {
}
}

if (command) {
if (!isDefaultCommand) {
currentContext.commands.pop();
currentContext.fullCommands.pop();
}
Expand Down Expand Up @@ -595,7 +600,7 @@ export class CommandInstance {
});
}
}
runDefaultBuilderOn(yargs: YargsInstance): void {
runDefaultBuilderOn(yargs: YargsInstance): unknown | Promise<unknown> {
if (!this.defaultCommand) return;
if (this.shouldUpdateUsage(yargs)) {
// build the root-level command string from the default string.
Expand All @@ -609,12 +614,13 @@ export class CommandInstance {
}
const builder = this.defaultCommand.builder;
if (isCommandBuilderCallback(builder)) {
builder(yargs, true);
return builder(yargs, true);
} else if (!isCommandBuilderDefinition(builder)) {
Object.keys(builder).forEach(key => {
yargs.option(key, builder[key]);
});
}
return undefined;
}
// lookup module object from require()d command and derive name
// if module was not require()d and no name given, throw error
Expand Down
22 changes: 15 additions & 7 deletions lib/yargs-factory.ts
View file
Expand Up @@ -779,6 +779,13 @@ export class YargsInstance {
});
}
}
// Ensure top level options/positionals have been configured:
const builderResponse = this.#command.runDefaultBuilderOn(this);
if (isPromise(builderResponse)) {
return builderResponse.then(() => {
return this.#usage.help();
});
}
}
return Promise.resolve(this.#usage.help());
}
Expand Down Expand Up @@ -1258,6 +1265,14 @@ export class YargsInstance {
return this;
}
}
// Ensure top level options/positionals have been configured:
const builderResponse = this.#command.runDefaultBuilderOn(this);
if (isPromise(builderResponse)) {
builderResponse.then(() => {
this.#usage.showHelp(level);
});
return this;
}
}
this.#usage.showHelp(level);
return this;
Expand Down Expand Up @@ -2044,11 +2059,6 @@ export class YargsInstance {
!!calledFromCommand,
false
);
} else if (!calledFromCommand && helpOnly) {
// TODO(bcoe): what if the default builder is async?
// TODO(bcoe): add better comments for runDefaultBuilderOn, why
// are we doing this?
this.#command.runDefaultBuilderOn(this);
}

// we must run completions first, a user might
Expand Down Expand Up @@ -2083,8 +2093,6 @@ export class YargsInstance {
if (helpOptSet) {
if (this.#exitProcess) setBlocking(true);
skipValidation = true;
// TODO: add appropriate comment.
if (!calledFromCommand) this.#command.runDefaultBuilderOn(this);
this.showHelp('log');
this.exit(0);
} else if (versionOptSet) {
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
@@ -1,6 +1,6 @@
{
"name": "yargs",
"version": "17.0.0-candidate.10",
"version": "17.0.0-candidate.12",
"description": "yargs the modern, pirate-themed, successor to optimist.",
"main": "./index.cjs",
"exports": {
Expand Down
99 changes: 99 additions & 0 deletions test/usage.cjs
View file
Expand Up @@ -596,6 +596,7 @@ describe('usage tests', () => {
// ignore the error, we only test the output here
}
});
console.info(r);
r.errors
.join('\n')
.split(/\n+/)
Expand Down Expand Up @@ -4462,6 +4463,54 @@ describe('usage tests', () => {
help.split('\n').should.deep.equal(expected);
});
});
// Refs: https://github.com/yargs/yargs/issues/1912
describe('positional', () => {
const expected = [
'Hello, world!',
'',
'Commands:',
' usage [foo] Default command description [default]',
' usage foo Foo command description',
'',
'Positionals:',
' foo foo parameter',
'',
'Options:',
' --help Show help [boolean]',
' --version Show version number [boolean]',
];
it('should contain the expected output for --help', () => {
const r = checkUsage(() =>
yargs('--help')
.scriptName('usage')
.usage('Hello, world!')
.command('* [foo]', 'Default command description', yargs => {
yargs.positional('foo', {
describe: 'foo parameter',
});
})
.commands([{command: 'foo', desc: 'Foo command description'}])
.parse()
);
r.logs[0].split('\n').should.deep.equal(expected);
});
it('should contain the expected output for showHelp', () => {
const r = checkUsage(() => {
const y = yargs()
.scriptName('usage')
.usage('Hello, world!')
.command('* [foo]', 'Default command description', yargs => {
yargs.positional('foo', {
describe: 'foo parameter',
});
})
.commands([{command: 'foo', desc: 'Foo command description'}]);
y.parse();
y.showHelp('log');
});
r.logs[0].split('\n').should.deep.equal(expected);
});
});
});

describe('async builder', async () => {
Expand Down Expand Up @@ -4502,6 +4551,56 @@ describe('usage tests', () => {
logs.should.match(/a test command/);
}
});
// Refs: https://github.com/yargs/yargs/issues/1912
describe('positional', () => {
const expected = [
'Hello, world!',
'',
'Commands:',
' usage [foo] Default command description [default]',
' usage foo Foo command description',
'',
'Positionals:',
' foo foo parameter',
'',
'Options:',
' --help Show help [boolean]',
' --version Show version number [boolean]',
];
it('should contain the expected output for --help', async () => {
const r = await checkUsage(() => {
yargs('--help')
.scriptName('usage')
.usage('Hello, world!')
.command('* [foo]', 'Default command description', async yargs => {
await wait();
yargs.positional('foo', {
describe: 'foo parameter',
});
})
.commands([{command: 'foo', desc: 'Foo command description'}])
.parse();
return wait(20);
});
r.logs[0].split('\n').should.deep.equal(expected);
});
it('should contain the expected output for getHelp', async () => {
const y = yargs()
.scriptName('usage')
.usage('Hello, world!')
.command('* [foo]', 'Default command description', async yargs => {
// await wait();
yargs.positional('foo', {
describe: 'foo parameter',
});
})
.commands([{command: 'foo', desc: 'Foo command description'}]);
await y.parse('');
await wait();
const help = await y.getHelp();
help.split('\n').should.deep.equal(expected);
});
});
});

// Refs: https://github.com/yargs/yargs/issues/1820
Expand Down
1 change: 0 additions & 1 deletion test/yargs.cjs
View file
Expand Up @@ -2192,7 +2192,6 @@ describe('yargs dsl tests', () => {
})
.coerce('option2', () => undefined)
.getHelp();
console.info(help);
help.should.match(/option2 description/);
});
});
Expand Down

0 comments on commit 395bb67

Please sign in to comment.