Fix conflict between "ls -l" and "init -l" #391
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Set up the `.option` on `list` and not at the root to avoid conflict. * Update "Getting Started" to use "ls -l" * Use command aliases so that yargs will recognize "ls" as a command (also allows "cdk ls --help" to work). * Upgrade to yargs 12.0.1 * Use command aliases for "synth" as well
RomainMuller
approved these changes
Jul 23, 2018
packages/aws-cdk/bin/cdk.ts
Outdated
| @@ -51,10 +51,10 @@ async function parseCommandLineArguments() { | |||
| .option('json', { type: 'boolean', alias: 'j', desc: 'Use JSON output instead of YAML' }) | |||
| .option('verbose', { type: 'boolean', alias: 'v', desc: 'Show debug logs' }) | |||
| .demandCommand(1) | |||
| .command('list', 'Lists all stacks in the cloud executable (alias: ls)') | |||
| .option('long', { type: 'boolean', default: false, alias: 'l', desc: 'display environment information for each stack' }) | |||
| .command([ 'ls', 'list' ], 'Lists all stacks in the app (alias: ls)', yargs => yargs | |||
There was a problem hiding this comment.
Is this not going to cause "alias: ls" to be output twice? (once because it's literally here, and once by yargs)
There was a problem hiding this comment.
Output where? In the help? No.
This is how cdk --help looks like now:
Usage: cdk -a <cdk-app> COMMAND
Commands:
cdk list Lists all stacks in the app [aliases: ls]
cdk synthesize [STACKS..] Synthesizes and prints the CloudFormation
template for this stack [aliases: synth]
cdk bootstrap [ENVIRONMENTS..] Deploys the CDK toolkit stack into an AWS
environment
cdk deploy [STACKS...] Deploys the stack(s) named STACKS into your
AWS account
cdk destroy [STACKS...] Destroy the stack(s) named STACKS
cdk diff [STACK] Compares the specified stack with the deployed
stack or a local template file
cdk metadata [STACK] Returns all metadata associated with this
stack
cdk init [TEMPLATE] Create a new, empty CDK project from a
template. Invoked without TEMPLATE, the app
template will be used.
cdk docs Opens the documentation in a browser
[aliases: doc]
cdk doctor Check your set-up for potential problems
Options:
--help Show help [boolean]
--app, -a REQUIRED: Command-line for executing your CDK app (e.g. "node
bin/my-app.js") [string]
--context, -c Add contextual string parameter. [array]
--plugin, -p Name or path of a node package that extend the CDK features.
Can be specified multiple times [array]
--rename Rename stack name if different then the one defined in the
cloud executable [string]
--trace Print trace for stack warnings [boolean]
--strict Do not construct stacks with warnings [boolean]
--ignore-errors Ignores synthesis errors, which will likely produce an
invalid output [boolean] [default: false]
--json, -j Use JSON output instead of YAML [boolean]
--verbose, -v Show debug logs [boolean]
--version Show version number [boolean]
If your app has a single stack, there is no need to specify the stack name
If one of cdk.json or ~/.cdk.json exists, options specified there will be used
as defaults. Settings in cdk.json take precedence.
Not enough non-option arguments: got 0, need at least 1
There was a problem hiding this comment.
Well the code in the PR has the description for list as: "Lists all stacks in the app (alias: ls)", however your command output has "Lists all stacks in the app [aliases: ls]".
Given the code, I would have expected it to look as:
cdk ls Lists all stacks in the app (alias: ls) [aliases: list]
There was a problem hiding this comment.
Fixed in a subsequent commit
packages/aws-cdk/bin/cdk.ts
Outdated
| // tslint:disable-next-line:max-line-length | ||
| .command('synth [STACKS..]', 'Synthesizes and prints the cloud formation template for this stack (alias: synthesize, construct, cons)', yargs => yargs | ||
| .command(['synth [STACKS..]', 'synthesize [STACKS...]', 'construct [STACKS...]', 'cons [STACKS...]'], 'Synthesizes and prints the CloudFormation template for this stack', yargs => yargs |
There was a problem hiding this comment.
I vote against cons & construct here...
added 2 commits
July 24, 2018 04:28
Probably changed in new yargs version
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
.optiononlistand not at the root to avoid conflict."ls" as a command (also allows "cdk ls --help" to work).
Fixes #389
Fixes #388
Related to #380