From 2065a95b297bd2168b43cf8d193e7d73082c6270 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 26 Feb 2022 13:13:37 +1300 Subject: [PATCH 01/13] Refactor parsing for improved clarity, robustness, and future expansion. --- index.js | 210 ++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 146 insertions(+), 64 deletions(-) diff --git a/index.js b/index.js index 1b5eb3b..a0eabde 100644 --- a/index.js +++ b/index.js @@ -53,7 +53,7 @@ function getMainArgs() { return ArrayPrototypeSlice(process.argv, 2); } -function storeOptionValue(parseOptions, option, value, result) { +function storeOptionValue(option, value, parseOptions, result) { const multiple = parseOptions.multiples && ArrayPrototypeIncludes(parseOptions.multiples, option); @@ -97,84 +97,166 @@ const parseArgs = ( let pos = 0; while (pos < argv.length) { - let arg = argv[pos]; - - if (StringPrototypeStartsWith(arg, '-')) { - if (arg === '-') { - // '-' commonly used to represent stdin/stdout, treat as positional - result.positionals = ArrayPrototypeConcat(result.positionals, '-'); - ++pos; - continue; - } else if (arg === '--') { - // Everything after a bare '--' is considered a positional argument - // and is returned verbatim - result.positionals = ArrayPrototypeConcat( - result.positionals, - ArrayPrototypeSlice(argv, ++pos) - ); - return result; - } else if (StringPrototypeCharAt(arg, 1) !== '-') { - // Look for shortcodes: -fXzy and expand them to -f -X -z -y: - if (arg.length > 2) { - for (let i = 2; i < arg.length; i++) { - const short = StringPrototypeCharAt(arg, i); - // Add 'i' to 'pos' such that short options are parsed in order - // of definition: - ArrayPrototypeSplice(argv, pos + (i - 1), 0, `-${short}`); - } - } + const arg = argv[pos]; + const nextArg = argv[pos + 1]; - arg = StringPrototypeCharAt(arg, 1); // short - if (options.short && options.short[arg]) - arg = options.short[arg]; // now long! - // ToDo: later code tests for `=` in arg and wrong for shorts - } else { - arg = StringPrototypeSlice(arg, 2); // remove leading -- + // Check if `arg` is an options terminator. + // Guideline 10 in https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html + if (arg === '--') { + // Everything after a bare '--' is considered a positional argument. + result.positionals = ArrayPrototypeConcat( + result.positionals, + ArrayPrototypeSlice(argv, pos + 1) + ); + break; // Finished processing argv, leave while loop. + } + + if (isLoneShortOption(arg)) { + // e.g. '-f' + const optionKey = getOptionKey(StringPrototypeCharAt(arg, 1), options); + let optionValue; + if (isExpectingValue(optionKey, options) && isOptionValue(nextArg)) { + // e.g. '-f' 'bar' + optionValue = nextArg; + pos++; + } + storeOptionValue(optionKey, optionValue, options, result); + pos++; + continue; + } + + if (isShortOptionGroup(arg)) { + // Expand -fXzy to -f -X -z -y + const expanded = []; + for (let index = 1; index < arg.length; index++) { + ArrayPrototypePush(expanded, `-${StringPrototypeCharAt(arg, index)}`); } + // Replace group with expansion. + ArrayPrototypeSplice(argv, pos, 1, ...expanded); + continue; + } + if (isLongOption(arg)) { + let optionKey; + let optionValue; if (StringPrototypeIncludes(arg, '=')) { - // Store option=value same way independent of `withValue` as: - // - looks like a value, store as a value - // - match the intention of the user - // - preserve information for author to process further + // e.g. '--foo=bar' const index = StringPrototypeIndexOf(arg, '='); - storeOptionValue( - options, - StringPrototypeSlice(arg, 0, index), - StringPrototypeSlice(arg, index + 1), - result); - } else if (pos + 1 < argv.length && - !StringPrototypeStartsWith(argv[pos + 1], '-') - ) { - // withValue option should also support setting values when '= - // isn't used ie. both --foo=b and --foo b should work - - // If withValue option is specified, take next position argument as - // value and then increment pos so that we don't re-evaluate that - // arg, else set value as undefined ie. --foo b --bar c, after setting - // b as the value for foo, evaluate --bar next and skip 'b' - const val = options.withValue && - ArrayPrototypeIncludes(options.withValue, arg) ? argv[++pos] : - undefined; - storeOptionValue(options, arg, val, result); + optionKey = StringPrototypeSlice(arg, 2, index); + optionValue = StringPrototypeSlice(arg, index + 1); } else { - // Cases when an arg is specified without a value, example - // '--foo --bar' <- 'foo' and 'bar' flags should be set to true and - // save value as undefined - storeOptionValue(options, arg, undefined, result); + // e.g. '--foo' + optionKey = StringPrototypeSlice(arg, 2); + if (isExpectingValue(optionKey, options) && isOptionValue(nextArg)) { + // e.g. '--foo' 'bar' + optionValue = nextArg; + pos++; + } } - - } else { - // Arguments without a dash prefix are considered "positional" - ArrayPrototypePush(result.positionals, arg); + storeOptionValue(optionKey, optionValue, options, result); + pos++; + continue; } + // Anything that did not get handled above is a positional. + ArrayPrototypePush(result.positionals, arg); pos++; } return result; }; +/** + * Determines if `arg` is a just a short option. + * @example '-f' + */ +function isLoneShortOption(arg) { + return arg.length === 2 && + StringPrototypeCharAt(arg, 0) === '-' && + StringPrototypeCharAt(arg, 1) !== '-'; +} + +/** + * Determines if `arg` is a short option group. + * + * See Guideline 5 of the [Open Group Utility Conventions](https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html). + * One or more options without option-arguments, followed by at most one + * option that takes an option-argument, should be accepted when grouped + * behind one '-' delimiter. + * @example + * isShortOptionGroup('-a', {}) // returns false + * isShortOptionGroup('-ab', {}) // returns true + * isShortOptionGroup('-fb', { withValues: ['f'] }) // returns false + * isShortOptionGroup('-bf', { withValues: ['f'] }) // returns true + */ +function isShortOptionGroup(arg, options) { + if (arg.length <= 2) return false; + if (StringPrototypeCharAt(arg, 0) !== '-') return false; + if (StringPrototypeCharAt(arg, 1) === '-') return false; + + const onlyFlags = arg.slice(1, -1); + for (let index = 0; index < onlyFlags.length; index++) { + const optionKey = getOptionKey(StringPrototypeCharAt(onlyFlags, index)); + if (isExpectingValue(optionKey)) { + return false; + } + } + return true; +} + +/** + * Determines if `arg` is a long option, which may have a trailing value. + * @example + * isLongOption('-a) // returns false + * isLongOption('--foo) // returns true + * isLongOption('--foo=bar) // returns true + */ +function isLongOption(arg) { + return arg.length > 2 && StringPrototypeStartsWith(arg, '--'); +} + +/** + * Expand known short options into long, otherwise return original. + * @example + * getOptionKey('f', { short: { f: 'file'}}) // returns 'file' + * getOptionKey('b', {}) // returns 'b' + * getOptionKey('long-option', {}) // returns 'long-option' + */ +function getOptionKey(option, options) { + if (option.length === 1 && options?.short?.[option]) { + return options.short[option]; // long option + } + return option; +} + +/** + * Determines if the option is expecting a value. + */ +function isExpectingValue(optionKey, options) { + return options && options.withValue && + ArrayPrototypeIncludes(options.withValue, optionKey); +} + +/** + * Determines if the argument can be used as an option value. + * NB: We are choosing not to accept option-looking arguments. + * @example + * isOptionValue(['-v', 'V'], 1) // returns true + * isOptionValue(['-v'], 1) // returns false + * isOptionValue(['-v', '-b'], 1) // returns false + */ +function isOptionValue(value) { + if (value === undefined) return false; + if (value === '-') return true; // e.g. representing stdin/stdout for file + + // Open Group Utility Conventions are that an option-argument may start + // with a dash, but we are currentlly rejecting these and prioritising the + // option-like appearance of the argument. Rejection allows error detection + // if strict:true, but comes at the cost of rejecting intended values starting + // with a dash, especially negative numbers. + return !StringPrototypeStartsWith(value, '-'); +} + module.exports = { parseArgs }; From bdd8530884277f00f534191ba9a97230a974c492 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 26 Feb 2022 16:24:19 +1300 Subject: [PATCH 02/13] Support multiple test files, and extend dash tests --- index.js | 4 ++-- package.json | 4 ++-- test/dash.js | 31 +++++++++++++++++++++++++++++++ test/index.js | 10 ---------- 4 files changed, 35 insertions(+), 14 deletions(-) create mode 100644 test/dash.js diff --git a/index.js b/index.js index a0eabde..7f222ac 100644 --- a/index.js +++ b/index.js @@ -186,8 +186,8 @@ function isLoneShortOption(arg) { * @example * isShortOptionGroup('-a', {}) // returns false * isShortOptionGroup('-ab', {}) // returns true - * isShortOptionGroup('-fb', { withValues: ['f'] }) // returns false - * isShortOptionGroup('-bf', { withValues: ['f'] }) // returns true + * isShortOptionGroup('-fb', { withValue: ['f'] }) // returns false + * isShortOptionGroup('-bf', { withValue: ['f'] }) // returns true */ function isShortOptionGroup(arg, options) { if (arg.length <= 2) return false; diff --git a/package.json b/package.json index fbef77d..2488d50 100644 --- a/package.json +++ b/package.json @@ -4,8 +4,8 @@ "description": "Polyfill of future proposal for `util.parseArgs()`", "main": "index.js", "scripts": { - "coverage": "c8 --check-coverage node test/index.js", - "test": "c8 node test/index.js", + "coverage": "c8 --check-coverage tape 'test/*.js'", + "test": "c8 tape 'test/*.js'", "posttest": "eslint .", "fix": "npm run posttest -- --fix" }, diff --git a/test/dash.js b/test/dash.js new file mode 100644 index 0000000..e0a384c --- /dev/null +++ b/test/dash.js @@ -0,0 +1,31 @@ +'use strict'; +/* eslint max-len: 0 */ + +const test = require('tape'); +const { parseArgs } = require('../index.js'); + +// The use of `-` as a positional is specifically mentioned in the Open Group Utility Conventions. +// The interpretation is up to the utility, and for a file positional (operand) the examples are +// '-' may stand for standard input (or standard output), or for a file named -. +// https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html +test("when args include '-' used as positional then result has '-' in positionals", function(t) { + const passedArgs = ['-']; + + const args = parseArgs(passedArgs); + const expected = { flags: {}, values: {}, positionals: ['-'] }; + t.deepEqual(args, expected); + + t.end(); +}); + +// If '-' is a valid positional, it is symmetrical to allow it as an option value too. +test("when args include '-' used as option value then result has '-' in option value", function(t) { + const passedArgs = ['-v', '-']; + const options = { withValue: ['v'] }; + + const args = parseArgs(passedArgs, options); + const expected = { flags: { v: true }, values: { v: '-' }, positionals: [] }; + t.deepEqual(args, expected); + + t.end(); +}); diff --git a/test/index.js b/test/index.js index 8487923..4571518 100644 --- a/test/index.js +++ b/test/index.js @@ -163,16 +163,6 @@ test('args equals are passed "withValue"', function(t) { t.end(); }); -test('when args include single dash then result stores dash as positional', function(t) { - const passedArgs = ['-']; - const expected = { flags: { }, values: { }, positionals: ['-'] }; - const args = parseArgs(passedArgs); - - t.deepEqual(args, expected); - - t.end(); -}); - test('zero config args equals are parsed as if "withValue"', function(t) { const passedArgs = ['--so=wat']; const passedOptions = { }; From 0cfbe0f29e6021956c266e8c42f097cf9de5a849 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 26 Feb 2022 16:25:30 +1300 Subject: [PATCH 03/13] Clarify test title context --- test/dash.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/dash.js b/test/dash.js index e0a384c..c6c8c96 100644 --- a/test/dash.js +++ b/test/dash.js @@ -19,7 +19,7 @@ test("when args include '-' used as positional then result has '-' in positional }); // If '-' is a valid positional, it is symmetrical to allow it as an option value too. -test("when args include '-' used as option value then result has '-' in option value", function(t) { +test("when args include '-' used as space-separated option value then result has '-' in option value", function(t) { const passedArgs = ['-v', '-']; const options = { withValue: ['v'] }; From 6b918bf906f20f37d7a25cdf4024e1380346f6f7 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 26 Feb 2022 16:48:07 +1300 Subject: [PATCH 04/13] Move and extend short option group tests. Fix bug uncovered by new test. --- index.js | 4 +-- test/index.js | 52 --------------------------- test/short-option-group.js | 72 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 54 deletions(-) create mode 100644 test/short-option-group.js diff --git a/index.js b/index.js index 7f222ac..83a2c50 100644 --- a/index.js +++ b/index.js @@ -125,7 +125,7 @@ const parseArgs = ( continue; } - if (isShortOptionGroup(arg)) { + if (isShortOptionGroup(arg, options)) { // Expand -fXzy to -f -X -z -y const expanded = []; for (let index = 1; index < arg.length; index++) { @@ -197,7 +197,7 @@ function isShortOptionGroup(arg, options) { const onlyFlags = arg.slice(1, -1); for (let index = 0; index < onlyFlags.length; index++) { const optionKey = getOptionKey(StringPrototypeCharAt(onlyFlags, index)); - if (isExpectingValue(optionKey)) { + if (isExpectingValue(optionKey, options)) { return false; } } diff --git a/test/index.js b/test/index.js index 4571518..38fa8f9 100644 --- a/test/index.js +++ b/test/index.js @@ -70,58 +70,6 @@ test('when short option withValue used without value then stored as flag', funct t.end(); }); -test('short option group behaves like multiple short options', function(t) { - const passedArgs = ['-rf']; - const passedOptions = { }; - const expected = { flags: { r: true, f: true }, values: { r: undefined, f: undefined }, positionals: [] }; - const args = parseArgs(passedArgs, passedOptions); - - t.deepEqual(args, expected); - - t.end(); -}); - -test('short option group does not consume subsequent positional', function(t) { - const passedArgs = ['-rf', 'foo']; - const passedOptions = { }; - const expected = { flags: { r: true, f: true }, values: { r: undefined, f: undefined }, positionals: ['foo'] }; - const args = parseArgs(passedArgs, passedOptions); - t.deepEqual(args, expected); - - t.end(); -}); - -// See: Guideline 5 https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html -test('if terminal of short-option group configured withValue, subsequent positional is stored', function(t) { - const passedArgs = ['-rvf', 'foo']; - const passedOptions = { withValue: ['f'] }; - const expected = { flags: { r: true, f: true, v: true }, values: { r: undefined, v: undefined, f: 'foo' }, positionals: [] }; - const args = parseArgs(passedArgs, passedOptions); - t.deepEqual(args, expected); - - t.end(); -}); - -test('handles short-option groups in conjunction with long-options', function(t) { - const passedArgs = ['-rf', '--foo', 'foo']; - const passedOptions = { withValue: ['foo'] }; - const expected = { flags: { r: true, f: true, foo: true }, values: { r: undefined, f: undefined, foo: 'foo' }, positionals: [] }; - const args = parseArgs(passedArgs, passedOptions); - t.deepEqual(args, expected); - - t.end(); -}); - -test('handles short-option groups with "short" alias configured', function(t) { - const passedArgs = ['-rf']; - const passedOptions = { short: { r: 'remove' } }; - const expected = { flags: { remove: true, f: true }, values: { remove: undefined, f: undefined }, positionals: [] }; - const args = parseArgs(passedArgs, passedOptions); - t.deepEqual(args, expected); - - t.end(); -}); - test('Everything after a bare `--` is considered a positional argument', function(t) { const passedArgs = ['--', 'barepositionals', 'mopositionals']; const expected = { flags: {}, values: {}, positionals: ['barepositionals', 'mopositionals'] }; diff --git a/test/short-option-group.js b/test/short-option-group.js new file mode 100644 index 0000000..3a0edd2 --- /dev/null +++ b/test/short-option-group.js @@ -0,0 +1,72 @@ +'use strict'; +/* eslint max-len: 0 */ + +const test = require('tape'); +const { parseArgs } = require('../index.js'); + +test('when short option group of zero-config flags then result same as expanded options', function(t) { + const passedArgs = ['-rf']; + const passedOptions = { }; + + const args = parseArgs(passedArgs, passedOptions); + const expected = { flags: { r: true, f: true }, values: { r: undefined, f: undefined }, positionals: [] }; + t.deepEqual(args, expected); + + t.end(); +}); + +test('short option group of flags does not consume subsequent positional', function(t) { + const passedArgs = ['-rf', 'foo']; + const passedOptions = { }; + + const args = parseArgs(passedArgs, passedOptions); + const expected = { flags: { r: true, f: true }, values: { r: undefined, f: undefined }, positionals: ['foo'] }; + t.deepEqual(args, expected); + + t.end(); +}); + +// See: Guideline 5 https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html +test('when terminal of short-option group expects value then subsequent argument is stored as value', function(t) { + const passedArgs = ['-rvf', 'foo']; + const passedOptions = { withValue: ['f'] }; + + const args = parseArgs(passedArgs, passedOptions); + const expected = { flags: { r: true, f: true, v: true }, values: { r: undefined, v: undefined, f: 'foo' }, positionals: [] }; + t.deepEqual(args, expected); + + t.end(); +}); + +test('when middle of short-option group expects value then arg returned as positional (as not a valid group)', function(t) { + const passedArgs = ['-afb']; + const passedOptions = { withValue: ['f'] }; + + const args = parseArgs(passedArgs, passedOptions); + const expected = { flags: {}, values: {}, positionals: ['-afb'] }; + t.deepEqual(args, expected); + + t.end(); +}); + +test('handles short-option groups in conjunction with long-options', function(t) { + const passedArgs = ['-rf', '--foo', 'foo']; + const passedOptions = { withValue: ['foo'] }; + + const args = parseArgs(passedArgs, passedOptions); + const expected = { flags: { r: true, f: true, foo: true }, values: { r: undefined, f: undefined, foo: 'foo' }, positionals: [] }; + t.deepEqual(args, expected); + + t.end(); +}); + +test('handles short-option groups with "short" alias configured', function(t) { + const passedArgs = ['-rf']; + const passedOptions = { short: { r: 'remove' } }; + + const args = parseArgs(passedArgs, passedOptions); + const expected = { flags: { remove: true, f: true }, values: { remove: undefined, f: undefined }, positionals: [] }; + t.deepEqual(args, expected); + + t.end(); +}); From 91646b95122a307c73b51b4457d34587d690ee89 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 26 Feb 2022 16:52:29 +1300 Subject: [PATCH 05/13] Add explicit strict for case that may fail in future --- test/short-option-group.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/short-option-group.js b/test/short-option-group.js index 3a0edd2..d82579a 100644 --- a/test/short-option-group.js +++ b/test/short-option-group.js @@ -38,9 +38,9 @@ test('when terminal of short-option group expects value then subsequent argument t.end(); }); -test('when middle of short-option group expects value then arg returned as positional (as not a valid group)', function(t) { +test('when middle of short-option group expects value and strict:false then arg returned as positional (as not a valid group)', function(t) { const passedArgs = ['-afb']; - const passedOptions = { withValue: ['f'] }; + const passedOptions = { withValue: ['f'], strict: false }; const args = parseArgs(passedArgs, passedOptions); const expected = { flags: {}, values: {}, positionals: ['-afb'] }; From 52507d3dd0ca43643c71328b02a67724259a1b7f Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 26 Feb 2022 17:11:45 +1300 Subject: [PATCH 06/13] Update comments for current calling signature --- index.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index 83a2c50..2a1e3ac 100644 --- a/index.js +++ b/index.js @@ -239,11 +239,12 @@ function isExpectingValue(optionKey, options) { /** * Determines if the argument can be used as an option value. - * NB: We are choosing not to accept option-looking arguments. + * NB: We are choosing not to accept option-ish arguments. * @example - * isOptionValue(['-v', 'V'], 1) // returns true - * isOptionValue(['-v'], 1) // returns false - * isOptionValue(['-v', '-b'], 1) // returns false + * isOptionValue('V']) // returns true + * isOptionValue('-v') // returns false + * isOptionValue('--foo') // returns false + * isOptionValue(undefined) // returns false */ function isOptionValue(value) { if (value === undefined) return false; From 3aa6ddbd322a5072413b28f214d130b3807efebb Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 26 Feb 2022 19:14:33 +1300 Subject: [PATCH 07/13] Prefer map to loop for style and lines of code --- index.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index 2a1e3ac..81b885f 100644 --- a/index.js +++ b/index.js @@ -3,6 +3,7 @@ const { ArrayPrototypeConcat, ArrayPrototypeIncludes, + ArrayPrototypeMap, ArrayPrototypeSlice, ArrayPrototypeSplice, ArrayPrototypePush, @@ -127,10 +128,7 @@ const parseArgs = ( if (isShortOptionGroup(arg, options)) { // Expand -fXzy to -f -X -z -y - const expanded = []; - for (let index = 1; index < arg.length; index++) { - ArrayPrototypePush(expanded, `-${StringPrototypeCharAt(arg, index)}`); - } + const expanded = ArrayPrototypeMap(StringPrototypeSlice(arg, 1), (char) => `-${char}`); // Replace group with expansion. ArrayPrototypeSplice(argv, pos, 1, ...expanded); continue; From b0e0e924eeaf3b362f253b463435080536131f07 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 26 Feb 2022 19:18:46 +1300 Subject: [PATCH 08/13] Use primordial slice Co-authored-by: Jordan Harband --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index 81b885f..645c19f 100644 --- a/index.js +++ b/index.js @@ -192,7 +192,7 @@ function isShortOptionGroup(arg, options) { if (StringPrototypeCharAt(arg, 0) !== '-') return false; if (StringPrototypeCharAt(arg, 1) === '-') return false; - const onlyFlags = arg.slice(1, -1); + const onlyFlags = StringPrototypeSlice(arg, 1, -1); for (let index = 0; index < onlyFlags.length; index++) { const optionKey = getOptionKey(StringPrototypeCharAt(onlyFlags, index)); if (isExpectingValue(optionKey, options)) { From 42103c614cd4344f606161661647d5febfd67940 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 26 Feb 2022 21:17:48 +1300 Subject: [PATCH 09/13] Address two review suggestions --- index.js | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/index.js b/index.js index 645c19f..6123582 100644 --- a/index.js +++ b/index.js @@ -2,6 +2,7 @@ const { ArrayPrototypeConcat, + ArrayPrototypeEvery, ArrayPrototypeIncludes, ArrayPrototypeMap, ArrayPrototypeSlice, @@ -192,14 +193,12 @@ function isShortOptionGroup(arg, options) { if (StringPrototypeCharAt(arg, 0) !== '-') return false; if (StringPrototypeCharAt(arg, 1) === '-') return false; - const onlyFlags = StringPrototypeSlice(arg, 1, -1); - for (let index = 0; index < onlyFlags.length; index++) { - const optionKey = getOptionKey(StringPrototypeCharAt(onlyFlags, index)); - if (isExpectingValue(optionKey, options)) { - return false; - } - } - return true; + const leadingShorts = StringPrototypeSlice(arg, 1, -1); + const allLeadingAreBoolean = ArrayPrototypeEvery(leadingShorts, (short) => { + const optionKey = getOptionKey(short); + return !isExpectingValue(optionKey, options); + }); + return allLeadingAreBoolean; } /** From 381e356827a8c2e9fc47ac11c8296b28c928b94f Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 26 Feb 2022 21:35:01 +1300 Subject: [PATCH 10/13] Replace splice and spread with concat and slice --- index.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index 6123582..311026c 100644 --- a/index.js +++ b/index.js @@ -6,7 +6,6 @@ const { ArrayPrototypeIncludes, ArrayPrototypeMap, ArrayPrototypeSlice, - ArrayPrototypeSplice, ArrayPrototypePush, ObjectHasOwn, StringPrototypeCharAt, @@ -131,7 +130,11 @@ const parseArgs = ( // Expand -fXzy to -f -X -z -y const expanded = ArrayPrototypeMap(StringPrototypeSlice(arg, 1), (char) => `-${char}`); // Replace group with expansion. - ArrayPrototypeSplice(argv, pos, 1, ...expanded); + argv = ArrayPrototypeConcat( + ArrayPrototypeSlice(argv, 0, pos), + expanded, + ArrayPrototypeSlice(argv, pos + 1), + ); continue; } From 9fd16fe1c666df080527fb1c68699b36229342dd Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 27 Feb 2022 07:57:52 +1300 Subject: [PATCH 11/13] Add a paranoid tests since short groups "expand" into shorts --- test/short-option-group.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/short-option-group.js b/test/short-option-group.js index d82579a..74711bc 100644 --- a/test/short-option-group.js +++ b/test/short-option-group.js @@ -70,3 +70,13 @@ test('handles short-option groups with "short" alias configured', function(t) { t.end(); }); + +test('when parse explicit args containing short group then callers args not modified', function(t) { + const passedArgs = ['-rf']; + const originalArgs = passedArgs.slice(); + + parseArgs(passedArgs); + t.deepEqual(passedArgs, originalArgs); + + t.end(); +}); From 80eae79affa5b0163e44ee9cd42c44d95941c23d Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 27 Feb 2022 08:54:16 +1300 Subject: [PATCH 12/13] Fix bug with alias due to missing parameter --- index.js | 2 +- test/short-option-group.js | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index 311026c..742b3ee 100644 --- a/index.js +++ b/index.js @@ -198,7 +198,7 @@ function isShortOptionGroup(arg, options) { const leadingShorts = StringPrototypeSlice(arg, 1, -1); const allLeadingAreBoolean = ArrayPrototypeEvery(leadingShorts, (short) => { - const optionKey = getOptionKey(short); + const optionKey = getOptionKey(short, options); return !isExpectingValue(optionKey, options); }); return allLeadingAreBoolean; diff --git a/test/short-option-group.js b/test/short-option-group.js index 74711bc..357b54f 100644 --- a/test/short-option-group.js +++ b/test/short-option-group.js @@ -49,6 +49,17 @@ test('when middle of short-option group expects value and strict:false then arg t.end(); }); +test('when middle of short-option group expects value via alias and strict:false then arg returned as positional (as not a valid group)', function(t) { + const passedArgs = ['-afb']; + const passedOptions = { short: { f: 'file' }, withValue: ['file'], strict: false }; + + const args = parseArgs(passedArgs, passedOptions); + const expected = { flags: {}, values: {}, positionals: ['-afb'] }; + t.deepEqual(args, expected); + + t.end(); +}); + test('handles short-option groups in conjunction with long-options', function(t) { const passedArgs = ['-rf', '--foo', 'foo']; const passedOptions = { withValue: ['foo'] }; From 99e6718d581c3d3d1f2ff710b7458f3cd65ea50a Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 27 Feb 2022 09:05:11 +1300 Subject: [PATCH 13/13] feat: add support for short option and value in same arg --- index.js | 23 ++++++++++++++++++++ test/short-option-with-value.js | 38 +++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) create mode 100644 test/short-option-with-value.js diff --git a/index.js b/index.js index 742b3ee..fd7229f 100644 --- a/index.js +++ b/index.js @@ -138,6 +138,15 @@ const parseArgs = ( continue; } + if (isShortAndValue(arg, options)) { + // e.g. '-fBAR' + const optionKey = getOptionKey(StringPrototypeCharAt(arg, 1), options); + const optionValue = StringPrototypeSlice(arg, 2); + storeOptionValue(optionKey, optionValue, options, result); + pos++; + continue; + } + if (isLongOption(arg)) { let optionKey; let optionValue; @@ -204,6 +213,20 @@ function isShortOptionGroup(arg, options) { return allLeadingAreBoolean; } +/** + * Determines if `arg` is a combined short option and its value. + * @example '-fBAR' + */ +function isShortAndValue(arg, options) { + if (arg.length <= 2) return false; + if (StringPrototypeCharAt(arg, 0) !== '-') return false; + if (StringPrototypeCharAt(arg, 1) === '-') return false; + + const optionKey = getOptionKey(StringPrototypeCharAt(arg, 1), options); + return isExpectingValue(optionKey, options); +} + + /** * Determines if `arg` is a long option, which may have a trailing value. * @example diff --git a/test/short-option-with-value.js b/test/short-option-with-value.js new file mode 100644 index 0000000..da4437b --- /dev/null +++ b/test/short-option-with-value.js @@ -0,0 +1,38 @@ +'use strict'; +/* eslint max-len: 0 */ + +const test = require('tape'); +const { parseArgs } = require('../index.js'); + +test('when combine short and value then result has option with value', function(t) { + const passedArgs = ['-fBAR']; + const passedOptions = { withValue: ['f'] }; + + const args = parseArgs(passedArgs, passedOptions); + const expected = { flags: { f: true }, values: { f: 'BAR' }, positionals: [] }; + t.deepEqual(args, expected); + + t.end(); +}); + +test('when combine short and value followed by positional then result has positional', function(t) { + const passedArgs = ['-fBAR', 'positional']; + const passedOptions = { withValue: ['f'] }; + + const args = parseArgs(passedArgs, passedOptions); + const expected = { flags: { f: true }, values: { f: 'BAR' }, positionals: ['positional'] }; + t.deepEqual(args, expected); + + t.end(); +}); + +test('when combine short with alias and value then result has long option with value', function(t) { + const passedArgs = ['-fBAR']; + const passedOptions = { short: { f: 'foo' }, withValue: ['foo'] }; + + const args = parseArgs(passedArgs, passedOptions); + const expected = { flags: { foo: true }, values: { foo: 'BAR' }, positionals: [] }; + t.deepEqual(args, expected); + + t.end(); +});