From 930dae70d82e0c7b2c18eb01e303239e225d6fa7 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Wed, 26 Sep 2018 15:39:44 -0400 Subject: [PATCH 1/3] fix(@angular/cli): allow empty string arguments --- packages/angular/cli/models/parser.ts | 4 ---- packages/angular/cli/models/parser_spec.ts | 7 ++++++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/angular/cli/models/parser.ts b/packages/angular/cli/models/parser.ts index 91082430d3b6..99d31f93c3dd 100644 --- a/packages/angular/cli/models/parser.ts +++ b/packages/angular/cli/models/parser.ts @@ -269,10 +269,6 @@ export function parseArguments(args: string[], options: Option[] | null): Argume const errors: string[] = []; for (let arg = args.shift(); arg !== undefined; arg = args.shift()) { - if (!arg) { - break; - } - if (arg == '--') { // If we find a --, we're done. leftovers.push(...args); diff --git a/packages/angular/cli/models/parser_spec.ts b/packages/angular/cli/models/parser_spec.ts index 88de630a2ddd..73513c235c44 100644 --- a/packages/angular/cli/models/parser_spec.ts +++ b/packages/angular/cli/models/parser_spec.ts @@ -35,6 +35,7 @@ describe('parseArguments', () => { const tests: { [test: string]: Partial | ['!!!', Partial, string[]] } = { '--bool': { bool: true }, '--bool=1': ['!!!', {}, ['--bool=1']], + '--bool ': { bool: true, p1: '' }, '-- --bool=1': { '--': ['--bool=1'] }, '--bool=yellow': ['!!!', {}, ['--bool=yellow']], '--bool=true': { bool: true }, @@ -68,6 +69,10 @@ describe('parseArguments', () => { '--arr=1 --arr --arr c d': { arr: ['1', '', 'c'], p1: 'd' }, '--arr=1 --arr --arr c d e': { arr: ['1', '', 'c'], p1: 'd', p2: 'e' }, '--str=1': { str: '1' }, + '--str=': { str: '' }, + '--str ': { str: '' }, + '--str ': { str: '', p1: '' }, + '--str ': { str: '', p1: '', p2: '', '--': [''] }, '--hello-world=1': { helloWorld: '1' }, '--hello-bool': { helloBool: true }, '--helloBool': { helloBool: true }, @@ -119,7 +124,7 @@ describe('parseArguments', () => { Object.entries(tests).forEach(([str, expected]) => { it(`works for ${str}`, () => { try { - const actual = parseArguments(str.split(/\s+/), options); + const actual = parseArguments(str.split(' '), options); expect(Array.isArray(expected)).toBe(false); expect(actual).toEqual(expected as Arguments); From c3e2ab74ffa668d905b26d5e154e0eca5cb4c03c Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Wed, 26 Sep 2018 15:47:33 -0400 Subject: [PATCH 2/3] fix(@angular/cli): allow -a=value arguments If a flag is followed by an equal sign, just treat it as a long name. So the example above would translate to --a=value, while -abc=123 would be "-a -b -c=123". Fixes #12308 --- packages/angular/cli/models/parser.ts | 6 ++++++ packages/angular/cli/models/parser_spec.ts | 2 ++ 2 files changed, 8 insertions(+) diff --git a/packages/angular/cli/models/parser.ts b/packages/angular/cli/models/parser.ts index 99d31f93c3dd..60344f416d57 100644 --- a/packages/angular/cli/models/parser.ts +++ b/packages/angular/cli/models/parser.ts @@ -281,6 +281,12 @@ export function parseArguments(args: string[], options: Option[] | null): Argume // Argument is of form -abcdef. Starts at 1 because we skip the `-`. for (let i = 1; i < arg.length; i++) { const flag = arg[i]; + // If the next character is an '=', treat it as a long flag. + if (arg[i + 1] == '=') { + const f = '--' + flag + arg.slice(i + 1); + _assignOption(f, args, options, parsedOptions, positionals, leftovers, ignored, errors); + break; + } // Treat the last flag as `--a` (as if full flag but just one letter). We do this in // the loop because it saves us a check to see if the arg is just `-`. if (i == arg.length - 1) { diff --git a/packages/angular/cli/models/parser_spec.ts b/packages/angular/cli/models/parser_spec.ts index 73513c235c44..972c5b6b25f4 100644 --- a/packages/angular/cli/models/parser_spec.ts +++ b/packages/angular/cli/models/parser_spec.ts @@ -80,7 +80,9 @@ describe('parseArguments', () => { '--noHelloBool': { helloBool: false }, '--noBool': { bool: false }, '-b': { bool: true }, + '-b=true': { bool: true }, '-sb': { bool: true, str: '' }, + '-s=b': { str: 'b' }, '-bs': { bool: true, str: '' }, '--t1=true': { t1: true }, '--t1': { t1: true }, From 2ffde25c846085b53b13b04378d7b5e0340dc3e0 Mon Sep 17 00:00:00 2001 From: Hans Larsen Date: Wed, 26 Sep 2018 15:58:47 -0400 Subject: [PATCH 3/3] fix(@angular/cli): numerical flags should not give 0 if empty And numerical positional flags will be ignored. If the value is an empty string, a number conversion would give 0. It is unexpected from the user standpoint ("--num=" has the user expect a string value). --- packages/angular/cli/models/parser.ts | 11 ++++++++--- packages/angular/cli/models/parser_spec.ts | 11 +++++++++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/packages/angular/cli/models/parser.ts b/packages/angular/cli/models/parser.ts index 60344f416d57..9cba5e6355de 100644 --- a/packages/angular/cli/models/parser.ts +++ b/packages/angular/cli/models/parser.ts @@ -54,6 +54,8 @@ function _coerceType(str: string | undefined, type: OptionType, v?: Value): Valu case OptionType.Number: if (str === undefined) { return 0; + } else if (str === '') { + return undefined; } else if (Number.isFinite(+str)) { return +str; } else { @@ -308,6 +310,8 @@ export function parseArguments(args: string[], options: Option[] | null): Argume } // Deal with positionals. + // TODO(hansl): this is by far the most complex piece of code in this file. Try to refactor it + // simpler. if (positionals.length > 0) { let pos = 0; for (let i = 0; i < positionals.length;) { @@ -317,10 +321,11 @@ export function parseArguments(args: string[], options: Option[] | null): Argume // We do this with a found flag because more than 1 option could have the same positional. for (const option of options) { - // If any option has this positional and no value, we need to remove it. + // If any option has this positional and no value, AND fit the type, we need to remove it. if (option.positional === pos) { - if (parsedOptions[option.name] === undefined) { - parsedOptions[option.name] = positionals[i]; + const coercedValue = _coerce(positionals[i], option, parsedOptions[option.name]); + if (parsedOptions[option.name] === undefined && coercedValue !== undefined) { + parsedOptions[option.name] = coercedValue; found = true; } else { incrementI = false; diff --git a/packages/angular/cli/models/parser_spec.ts b/packages/angular/cli/models/parser_spec.ts index 972c5b6b25f4..dbec3d2712d6 100644 --- a/packages/angular/cli/models/parser_spec.ts +++ b/packages/angular/cli/models/parser_spec.ts @@ -19,6 +19,7 @@ describe('parseArguments', () => { { name: 'arr', aliases: [ 'a' ], type: OptionType.Array, description: '' }, { name: 'p1', positional: 0, aliases: [], type: OptionType.String, description: '' }, { name: 'p2', positional: 1, aliases: [], type: OptionType.String, description: '' }, + { name: 'p3', positional: 2, aliases: [], type: OptionType.Number, description: '' }, { name: 't1', aliases: [], type: OptionType.Boolean, types: [OptionType.Boolean, OptionType.String], description: '' }, { name: 't2', aliases: [], type: OptionType.Boolean, @@ -63,8 +64,13 @@ describe('parseArguments', () => { 'val1 --num=1 val2': { num: 1, p1: 'val1', p2: 'val2' }, '--p1=val1 --num=1 val2': { num: 1, p1: 'val1', p2: 'val2' }, '--p1=val1 --num=1 --p2=val2 val3': { num: 1, p1: 'val1', p2: 'val2', '--': ['val3'] }, - '--bool val1 --etc --num val2 --v': { bool: true, num: 0, p1: 'val1', p2: 'val2', - '--': ['--etc', '--v'] }, + '--bool val1 --etc --num val2 --v': [ + '!!!', + { bool: true, p1: 'val1', p2: 'val2', '--': ['--etc', '--v'] }, + ['--num' ], + ], + '--bool val1 --etc --num=1 val2 --v': { bool: true, num: 1, p1: 'val1', p2: 'val2', + '--': ['--etc', '--v'] }, '--arr=a --arr=b --arr c d': { arr: ['a', 'b', 'c'], p1: 'd' }, '--arr=1 --arr --arr c d': { arr: ['1', '', 'c'], p1: 'd' }, '--arr=1 --arr --arr c d e': { arr: ['1', '', 'c'], p1: 'd', p2: 'e' }, @@ -121,6 +127,7 @@ describe('parseArguments', () => { '--e3': { e3: true }, '--e3 true': { e3: true }, '--e3=true': { e3: true }, + 'a b c 1': { p1: 'a', p2: 'b', '--': ['c', '1'] }, }; Object.entries(tests).forEach(([str, expected]) => {