Skip to content

Commit b3de7e6

Browse files
mrgrainmrgrain
authored
refactor(cli): fix various minor codegen issues (#32024)
### Reason for this change Fixes some small issues that got missed before #31850 was unintentionally merged too early. ### Description of changes * re-add support for `CDK_` env variables * remove square bracktes when commands don't have an alias * remove extra space in command args * fixed `demandCommand` call ### Description of how you validated changes Run via tests, compared manually with cli options before we started to generate them. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 4f8ecae commit b3de7e6

File tree

3 files changed

+74
-57
lines changed

3 files changed

+74
-57
lines changed

‎packages/aws-cdk/lib/parse-command-line-arguments.ts

+54-50
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ export function parseCommandLineArguments(
1515
yargsNegativeAlias: any
1616
): any {
1717
return yargs
18+
.env('CDK')
1819
.usage('Usage: cdk -a <cdk-app> COMMAND')
1920
.option('app', {
2021
type: 'string',
@@ -148,7 +149,7 @@ export function parseCommandLineArguments(
148149
desc: 'Opt in to unstable features. The flag indicates that the scope and API of a feature might still change. Otherwise the feature is generally production ready and fully supported. Can be specified multiple times.',
149150
default: [],
150151
})
151-
.command(['list [STACKS..]', 'ls [STACKS..]'], 'Lists all stacks in the app', (yargs: Argv) =>
152+
.command(['list [STACKS..]', 'ls [STACKS..]'], 'Lists all stacks in the app', (yargs: Argv) =>
152153
yargs
153154
.option('long', {
154155
type: 'boolean',
@@ -163,7 +164,7 @@ export function parseCommandLineArguments(
163164
desc: 'Display stack dependency information for each stack',
164165
})
165166
)
166-
.command(['synthesize [STACKS..]', 'synth [STACKS..]'], 'Synthesizes and prints the CloudFormation template for this stack', (yargs: Argv) =>
167+
.command(['synthesize [STACKS..]', 'synth [STACKS..]'], 'Synthesizes and prints the CloudFormation template for this stack', (yargs: Argv) =>
167168
yargs
168169
.option('exclusively', {
169170
type: 'boolean',
@@ -182,7 +183,7 @@ export function parseCommandLineArguments(
182183
default: false,
183184
})
184185
)
185-
.command(['bootstrap [ENVIRONMENTS..]'], 'Deploys the CDK toolkit stack into an AWS environment', (yargs: Argv) =>
186+
.command('bootstrap [ENVIRONMENTS..]', 'Deploys the CDK toolkit stack into an AWS environment', (yargs: Argv) =>
186187
yargs
187188
.option('bootstrap-bucket-name', {
188189
type: 'string',
@@ -292,40 +293,43 @@ export function parseCommandLineArguments(
292293
desc: 'Use previous values for existing parameters (you must specify all parameters on every deployment if this is disabled)',
293294
})
294295
)
295-
.command(['gc [ENVIRONMENTS..]'], 'Garbage collect assets', (yargs: Argv) =>
296-
yargs
297-
.option('action', {
298-
type: 'string',
299-
desc: 'The action (or sub-action) you want to perform. Valid entires are "print", "tag", "delete-tagged", "full".',
300-
default: 'full',
301-
})
302-
.option('type', {
303-
type: 'string',
304-
desc: 'Specify either ecr, s3, or all',
305-
default: 'all',
306-
})
307-
.option('rollback-buffer-days', {
308-
type: 'number',
309-
desc: 'Delete assets that have been marked as isolated for this many days',
310-
default: 0,
311-
})
312-
.option('created-buffer-days', {
313-
type: 'number',
314-
desc: 'Never delete assets younger than this (in days)',
315-
default: 1,
316-
})
317-
.option('confirm', {
318-
type: 'boolean',
319-
desc: 'Confirm via manual prompt before deletion',
320-
default: true,
321-
})
322-
.option('bootstrap-stack-name', {
323-
type: 'string',
324-
desc: 'The name of the CDK toolkit stack, if different from the default "CDKToolkit"',
325-
requiresArg: true,
326-
})
296+
.command(
297+
'gc [ENVIRONMENTS..]',
298+
'Garbage collect assets. Options detailed here: https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk/README.md#cdk-gc',
299+
(yargs: Argv) =>
300+
yargs
301+
.option('action', {
302+
type: 'string',
303+
desc: 'The action (or sub-action) you want to perform. Valid entires are "print", "tag", "delete-tagged", "full".',
304+
default: 'full',
305+
})
306+
.option('type', {
307+
type: 'string',
308+
desc: 'Specify either ecr, s3, or all',
309+
default: 'all',
310+
})
311+
.option('rollback-buffer-days', {
312+
type: 'number',
313+
desc: 'Delete assets that have been marked as isolated for this many days',
314+
default: 0,
315+
})
316+
.option('created-buffer-days', {
317+
type: 'number',
318+
desc: 'Never delete assets younger than this (in days)',
319+
default: 1,
320+
})
321+
.option('confirm', {
322+
type: 'boolean',
323+
desc: 'Confirm via manual prompt before deletion',
324+
default: true,
325+
})
326+
.option('bootstrap-stack-name', {
327+
type: 'string',
328+
desc: 'The name of the CDK toolkit stack, if different from the default "CDKToolkit"',
329+
requiresArg: true,
330+
})
327331
)
328-
.command(['deploy [STACKS..]'], 'Deploys the stack(s) named STACKS into your AWS account', (yargs: Argv) =>
332+
.command('deploy [STACKS..]', 'Deploys the stack(s) named STACKS into your AWS account', (yargs: Argv) =>
329333
yargs
330334
.option('all', {
331335
type: 'boolean',
@@ -459,7 +463,7 @@ export function parseCommandLineArguments(
459463
default: false,
460464
})
461465
)
462-
.command(['rollback [STACKS..]'], 'Rolls back the stack(s) named STACKS to their last stable state', (yargs: Argv) =>
466+
.command('rollback [STACKS..]', 'Rolls back the stack(s) named STACKS to their last stable state', (yargs: Argv) =>
463467
yargs
464468
.option('all', {
465469
type: 'boolean',
@@ -488,7 +492,7 @@ export function parseCommandLineArguments(
488492
default: [],
489493
})
490494
)
491-
.command(['import [STACK]'], 'Import existing resource(s) into the given STACK', (yargs: Argv) =>
495+
.command('import [STACK]', 'Import existing resource(s) into the given STACK', (yargs: Argv) =>
492496
yargs
493497
.option('execute', {
494498
type: 'boolean',
@@ -526,7 +530,7 @@ export function parseCommandLineArguments(
526530
desc: 'If specified, CDK will use the given file to map physical resources to CDK resources for import, instead of interactively asking the user. Can be run from scripts',
527531
})
528532
)
529-
.command(['watch [STACKS..]'], "Shortcut for 'deploy --watch'", (yargs: Argv) =>
533+
.command('watch [STACKS..]', "Shortcut for 'deploy --watch'", (yargs: Argv) =>
530534
yargs
531535
.option('build-exclude', {
532536
type: 'array',
@@ -589,7 +593,7 @@ export function parseCommandLineArguments(
589593
requiresArg: true,
590594
})
591595
)
592-
.command(['destroy [STACKS..]'], 'Destroy the stack(s) named STACKS', (yargs: Argv) =>
596+
.command('destroy [STACKS..]', 'Destroy the stack(s) named STACKS', (yargs: Argv) =>
593597
yargs
594598
.option('all', {
595599
type: 'boolean',
@@ -608,7 +612,7 @@ export function parseCommandLineArguments(
608612
})
609613
)
610614
.command(
611-
['diff [STACKS..]'],
615+
'diff [STACKS..]',
612616
'Compares the specified stack with the deployed stack or a local template file, and returns with status 1 if any difference is found',
613617
(yargs: Argv) =>
614618
yargs
@@ -660,17 +664,17 @@ export function parseCommandLineArguments(
660664
default: true,
661665
})
662666
)
663-
.command(['metadata [STACK]'], 'Returns all metadata associated with this stack')
664-
.command(['acknowledge [ID]', 'ack [ID]'], 'Acknowledge a notice so that it does not show up anymore')
665-
.command(['notices'], 'Returns a list of relevant notices', (yargs: Argv) =>
667+
.command('metadata [STACK]', 'Returns all metadata associated with this stack')
668+
.command(['acknowledge [ID]', 'ack [ID]'], 'Acknowledge a notice so that it does not show up anymore')
669+
.command('notices', 'Returns a list of relevant notices', (yargs: Argv) =>
666670
yargs.option('unacknowledged', {
667671
type: 'boolean',
668672
alias: 'u',
669673
default: false,
670674
desc: 'Returns a list of unacknowledged notices',
671675
})
672676
)
673-
.command(['init [TEMPLATE]'], 'Create a new, empty CDK project from a template.', (yargs: Argv) =>
677+
.command('init [TEMPLATE]', 'Create a new, empty CDK project from a template.', (yargs: Argv) =>
674678
yargs
675679
.option('language', {
676680
type: 'string',
@@ -688,7 +692,7 @@ export function parseCommandLineArguments(
688692
desc: 'If true, only generates project files, without executing additional operations such as setting up a git repo, installing dependencies or compiling the project',
689693
})
690694
)
691-
.command(['migrate'], false, (yargs: Argv) =>
695+
.command('migrate', false, (yargs: Argv) =>
692696
yargs
693697
.option('stack-name', {
694698
type: 'string',
@@ -736,7 +740,7 @@ export function parseCommandLineArguments(
736740
desc: 'Use this flag to zip the generated CDK app',
737741
})
738742
)
739-
.command(['context'], 'Manage cached context values', (yargs: Argv) =>
743+
.command('context', 'Manage cached context values', (yargs: Argv) =>
740744
yargs
741745
.option('reset', {
742746
alias: 'e',
@@ -755,17 +759,17 @@ export function parseCommandLineArguments(
755759
type: 'boolean',
756760
})
757761
)
758-
.command(['docs', 'doc '], 'Opens the reference documentation in a browser', (yargs: Argv) =>
762+
.command(['docs', 'doc'], 'Opens the reference documentation in a browser', (yargs: Argv) =>
759763
yargs.option('browser', {
760764
alias: 'b',
761765
desc: 'the command to use to open the browser, using %u as a placeholder for the path of the file to open',
762766
type: 'string',
763767
default: browserDefault,
764768
})
765769
)
766-
.command(['doctor'], 'Check your set-up for potential problems')
770+
.command('doctor', 'Check your set-up for potential problems')
767771
.version(version)
768-
.demandCommand(1, "''")
772+
.demandCommand(1, '')
769773
.recommendCommands()
770774
.help()
771775
.alias('h', 'help')

‎tools/@aws-cdk/yargs-gen/lib/yargs-gen.ts

+18-6
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,9 @@ export async function renderYargs(config: CliConfig): Promise<string> {
6464
// ./prog --arg one --arg two position => will parse to { arg: ['one', 'two'], _: ['positional'] }.
6565
function makeYargs(config: CliConfig): Statement {
6666
let yargsExpr: Expression = code.expr.ident('yargs');
67-
yargsExpr = yargsExpr.callMethod('usage', lit('Usage: cdk -a <cdk-app> COMMAND'));
67+
yargsExpr = yargsExpr
68+
.callMethod('env', lit('CDK'))
69+
.callMethod('usage', lit('Usage: cdk -a <cdk-app> COMMAND'));
6870

6971
// we must compute global options first, as they are not part of an argument to a command call
7072
yargsExpr = makeOptions(yargsExpr, config.globalOptions);
@@ -75,16 +77,26 @@ function makeYargs(config: CliConfig): Statement {
7577
? ` [${commandFacts.arg?.name}${commandFacts.arg?.variadic ? '..' : ''}]`
7678
: '';
7779
const aliases = commandFacts.aliases
78-
? commandFacts.aliases.map((alias) => `, '${alias} ${commandArg}'`)
80+
? commandFacts.aliases.map((alias) => `, '${alias}${commandArg}'`)
7981
: '';
8082

8183
// must compute options before we compute the full command, because in yargs, the options are an argument to the command call.
8284
let optionsExpr: Expression = code.expr.directCode('(yargs: Argv) => yargs');
8385
optionsExpr = makeOptions(optionsExpr, commandFacts.options ?? {});
8486

85-
yargsExpr = commandFacts.options
86-
? yargsExpr.callMethod('command', code.expr.directCode(`['${command}${commandArg}'${aliases}]`), lit(commandFacts.description), optionsExpr)
87-
: yargsExpr.callMethod('command', code.expr.directCode(`['${command}${commandArg}'${aliases}]`), lit(commandFacts.description));
87+
const commandCallArgs: Array<Expression> = [];
88+
if (aliases) {
89+
commandCallArgs.push(code.expr.directCode(`['${command}${commandArg}'${aliases}]`));
90+
} else {
91+
commandCallArgs.push(code.expr.directCode(`'${command}${commandArg}'`));
92+
}
93+
commandCallArgs.push(lit(commandFacts.description));
94+
95+
if (commandFacts.options) {
96+
commandCallArgs.push(optionsExpr);
97+
}
98+
99+
yargsExpr = yargsExpr.callMethod('command', ...commandCallArgs);
88100
}
89101

90102
return code.stmt.ret(makeEpilogue(yargsExpr));
@@ -129,7 +141,7 @@ function makeOptions(prefix: Expression, options: { [optionName: string]: YargsO
129141

130142
function makeEpilogue(prefix: Expression) {
131143
let completeDefinition = prefix.callMethod('version', code.expr.ident('version'));
132-
completeDefinition = completeDefinition.callMethod('demandCommand', lit(1), lit("''")); // just print help
144+
completeDefinition = completeDefinition.callMethod('demandCommand', lit(1), lit('')); // just print help
133145
completeDefinition = completeDefinition.callMethod('recommendCommands');
134146
completeDefinition = completeDefinition.callMethod('help');
135147
completeDefinition = completeDefinition.callMethod('alias', lit('h'), lit('help'));

‎tools/@aws-cdk/yargs-gen/test/cli.test.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ describe('render', () => {
4040
yargsNegativeAlias: any
4141
): any {
4242
return yargs
43+
.env('CDK')
4344
.usage('Usage: cdk -a <cdk-app> COMMAND')
4445
.option('one', {
4546
type: 'string',
@@ -59,7 +60,7 @@ describe('render', () => {
5960
requiresArg: true,
6061
})
6162
.version(version)
62-
.demandCommand(1, "''")
63+
.demandCommand(1, '')
6364
.recommendCommands()
6465
.help()
6566
.alias('h', 'help')

0 commit comments

Comments
 (0)