Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(builder): apply default builder for showHelp/getHelp #1913

Merged
merged 2 commits into from Apr 12, 2021
Merged

fix(builder): apply default builder for showHelp/getHelp #1913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5f8c144
fix(builder): apply default builder for showHelp/getHelp
bcoe Apr 12, 2021
09924a6
build: we are up to candidate 12
bcoe Apr 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 20 additions & 14 deletions lib/command.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
@@ -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
Open in desktop
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
Open in desktop
Expand Up @@ -2192,7 +2192,6 @@ describe('yargs dsl tests', () => {
})
.coerce('option2', () => undefined)
.getHelp();
console.info(help);
help.should.match(/option2 description/);
});
});
Expand Down