From f575021756cef89d1bace3068d2a04788e0afcfa Mon Sep 17 00:00:00 2001 From: John Gee Date: Mon, 7 Feb 2022 17:43:06 +1300 Subject: [PATCH 1/6] docs: ordering of positional and option does not matter (#46) * docs: ordering of positional and option does not matter * Add test to match README FAQ --- README.md | 2 +- test/index.js | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index d62a01a..a22a9b0 100644 --- a/README.md +++ b/README.md @@ -150,7 +150,7 @@ const { flags, values, positionals } = parseArgs(argv, options); ### F.A.Qs - Is `cmd --foo=bar baz` the same as `cmd baz --foo=bar`? - - Yes, if `withValue: ['foo']`, otherwise no + - yes - Does the parser execute a function? - no - Does the parser execute one of several functions, depending on input? diff --git a/test/index.js b/test/index.js index 9ce91c7..8487923 100644 --- a/test/index.js +++ b/test/index.js @@ -228,6 +228,18 @@ test('args are passed "withValue" and "multiples"', function(t) { t.end(); }); +test('order of option and positional does not matter (per README)', function(t) { + const passedArgs1 = ['--foo=bar', 'baz']; + const passedArgs2 = ['baz', '--foo=bar']; + const passedOptions = { withValue: ['foo'] }; + const expected = { flags: { foo: true }, values: { foo: 'bar' }, positionals: ['baz'] }; + + t.deepEqual(parseArgs(passedArgs1, passedOptions), expected, 'option then positional'); + t.deepEqual(parseArgs(passedArgs2, passedOptions), expected, 'positional then option'); + + t.end(); +}); + test('correct default args when use node -p', function(t) { const holdArgv = process.argv; process.argv = [process.argv0, '--foo']; From 2ab5af3cafef37d2545de212033d9955aaaef48d Mon Sep 17 00:00:00 2001 From: John Gee Date: Fri, 25 Feb 2022 21:09:36 +1300 Subject: [PATCH 2/6] docs(fix): typos in comments (#67) --- index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index 6948332..1b5eb3b 100644 --- a/index.js +++ b/index.js @@ -149,7 +149,7 @@ const parseArgs = ( // 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 arguement as + // 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' @@ -160,12 +160,12 @@ const parseArgs = ( } else { // Cases when an arg is specified without a value, example // '--foo --bar' <- 'foo' and 'bar' flags should be set to true and - // shave value as undefined + // save value as undefined storeOptionValue(options, arg, undefined, result); } } else { - // Arguements without a dash prefix are considered "positional" + // Arguments without a dash prefix are considered "positional" ArrayPrototypePush(result.positionals, arg); } From 81eca14b12157302c80405a27939fc0d0ca67a7e Mon Sep 17 00:00:00 2001 From: John Gee Date: Mon, 28 Feb 2022 03:46:10 +1300 Subject: [PATCH 3/6] docs: add Scope to give context without wading through history (#71) --- README.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index a22a9b0..e62b314 100644 --- a/README.md +++ b/README.md @@ -9,8 +9,11 @@ Polyfill of future proposal to the [nodejs/tooling](https://github.com/nodejs/tooling) repo for `util.parseArgs()` +### Scope -This package was implemented using [tape](https://www.npmjs.com/package/tape) as its test harness. +It is already possible to build great arg parsing modules on top of what Node.js provides; the prickly API is abstracted away by these modules. Thus, process.parseArgs() is not necessarily intended for library authors; it is intended for developers of simple CLI tools, ad-hoc scripts, deployed Node.js applications, and learning materials. + +It is exceedingly difficult to provide an API which would both be friendly to these Node.js users while being extensible enough for libraries to build upon. We chose to prioritize these use cases because these are currently not well-served by Node.js' API. ### Links & Resources @@ -55,6 +58,8 @@ Any person who wants to contribute to the initiative is welcome! Please first re Additionally, reading the [`Examples w/ Output`](#-examples-w-output) section of this document will be the best way to familiarize yourself with the target expected behavior for parseArgs() once it is fully implemented. +This package was implemented using [tape](https://www.npmjs.com/package/tape) as its test harness. + ---- ## 💡 `process.mainArgs` Proposal From b4120957d90e809ee8b607b06e747d3e6a6b213e Mon Sep 17 00:00:00 2001 From: Aaron Casanova <32409546+aaronccasanova@users.noreply.github.com> Date: Wed, 2 Mar 2022 15:01:37 -0800 Subject: [PATCH 4/6] refactor!: restructure configuration to take options bag (#63) Per the discussion in #45 this PR restructures the current options API where each option is configured in three separate list and instead allows options to be configured in a single object. The goal being to make the API more intuitive for configuring options (e.g. short, withValue, and multiples) while creating a path forward for introducing more configuration options in the future (e.g. default). Co-authored-by: Benjamin E. Coe Co-authored-by: John Gee Co-authored-by: Jordan Harband --- README.md | 53 ++++++++++++---------- index.js | 98 +++++++++++++++++++++++++--------------- test/index.js | 123 ++++++++++++++++++++++++++++---------------------- validators.js | 23 ++++++++++ 4 files changed, 182 insertions(+), 115 deletions(-) diff --git a/README.md b/README.md index e62b314..088297f 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ It is exceedingly difficult to provide an API which would both be friendly to th - [🙌 Contributing](#-contributing) - [💡 `process.mainArgs` Proposal](#-processmainargs-proposal) - [Implementation:](#implementation) -- [💡 `util.parseArgs(argv)` Proposal](#-utilparseargsargv-proposal) +- [💡 `util.parseArgs([config])` Proposal](#-utilparseargsconfig-proposal) - [📃 Examples](#-examples) - [F.A.Qs](#faqs) @@ -74,19 +74,16 @@ process.mainArgs = process.argv.slice(process._exec ? 1 : 2) ---- -## 💡 `util.parseArgs([argv][, options])` Proposal +## 💡 `util.parseArgs([config])` Proposal -* `argv` {string[]} (Optional) Array of argument strings; defaults - to [`process.mainArgs`](process_argv) -* `options` {Object} (Optional) The `options` parameter is an +* `config` {Object} (Optional) The `config` parameter is an object supporting the following properties: - * `withValue` {string[]} (Optional) An `Array` of argument - strings which expect a value to be defined in `argv` (see [Options][] - for details) - * `multiples` {string[]} (Optional) An `Array` of argument - strings which, when appearing multiple times in `argv`, will be concatenated -into an `Array` - * `short` {Object} (Optional) An `Object` of key, value pairs of strings which map a "short" alias to an argument; When appearing multiples times in `argv`; Respects `withValue` & `multiples` + * `args` {string[]} (Optional) Array of argument strings; defaults + to [`process.mainArgs`](process_argv) + * `options` {Object} (Optional) An object describing the known options to look for in `args`; `options` keys are the long names of the known options, and the values are objects with the following properties: + * `type` {'string'|'boolean'} (Optional) Type of known option; defaults to `'boolean'`; + * `multiple` {boolean} (Optional) If true, when appearing one or more times in `args`, results are collected in an `Array` + * `short` {string} (Optional) A single character alias for an option; When appearing one or more times in `args`; Respects the `multiple` configuration * `strict` {Boolean} (Optional) A `Boolean` on wheather or not to throw an error when unknown args are encountered * Returns: {Object} An object having properties: * `flags` {Object}, having properties and `Boolean` values corresponding to parsed options passed @@ -104,9 +101,9 @@ const { parseArgs } = require('@pkgjs/parseargs'); ```js // unconfigured const { parseArgs } = require('@pkgjs/parseargs'); -const argv = ['-f', '--foo=a', '--bar', 'b']; +const args = ['-f', '--foo=a', '--bar', 'b']; const options = {}; -const { flags, values, positionals } = parseArgs(argv, options); +const { flags, values, positionals } = parseArgs({ args, options }); // flags = { f: true, bar: true } // values = { foo: 'a' } // positionals = ['b'] @@ -115,11 +112,13 @@ const { flags, values, positionals } = parseArgs(argv, options); ```js const { parseArgs } = require('@pkgjs/parseargs'); // withValue -const argv = ['-f', '--foo=a', '--bar', 'b']; +const args = ['-f', '--foo=a', '--bar', 'b']; const options = { - withValue: ['bar'] + foo: { + type: 'string', + }, }; -const { flags, values, positionals } = parseArgs(argv, options); +const { flags, values, positionals } = parseArgs({ args, options }); // flags = { f: true } // values = { foo: 'a', bar: 'b' } // positionals = [] @@ -127,13 +126,15 @@ const { flags, values, positionals } = parseArgs(argv, options); ```js const { parseArgs } = require('@pkgjs/parseargs'); -// withValue & multiples -const argv = ['-f', '--foo=a', '--foo', 'b']; +// withValue & multiple +const args = ['-f', '--foo=a', '--foo', 'b']; const options = { - withValue: ['foo'], - multiples: ['foo'] + foo: { + type: 'string', + multiple: true, + }, }; -const { flags, values, positionals } = parseArgs(argv, options); +const { flags, values, positionals } = parseArgs({ args, options }); // flags = { f: true } // values = { foo: ['a', 'b'] } // positionals = [] @@ -142,11 +143,13 @@ const { flags, values, positionals } = parseArgs(argv, options); ```js const { parseArgs } = require('@pkgjs/parseargs'); // shorts -const argv = ['-f', 'b']; +const args = ['-f', 'b']; const options = { - short: { f: 'foo' } + foo: { + short: 'f', + }, }; -const { flags, values, positionals } = parseArgs(argv, options); +const { flags, values, positionals } = parseArgs({ args, options }); // flags = { foo: true } // values = {} // positionals = ['b'] diff --git a/index.js b/index.js index 1b5eb3b..cf73e7c 100644 --- a/index.js +++ b/index.js @@ -2,11 +2,13 @@ const { ArrayPrototypeConcat, - ArrayPrototypeIncludes, + ArrayPrototypeFind, + ArrayPrototypeForEach, ArrayPrototypeSlice, ArrayPrototypeSplice, ArrayPrototypePush, ObjectHasOwn, + ObjectEntries, StringPrototypeCharAt, StringPrototypeIncludes, StringPrototypeIndexOf, @@ -16,7 +18,10 @@ const { const { validateArray, - validateObject + validateObject, + validateString, + validateUnion, + validateBoolean, } = require('./validators'); function getMainArgs() { @@ -53,41 +58,57 @@ function getMainArgs() { return ArrayPrototypeSlice(process.argv, 2); } -function storeOptionValue(parseOptions, option, value, result) { - const multiple = parseOptions.multiples && - ArrayPrototypeIncludes(parseOptions.multiples, option); +function storeOptionValue(options, longOption, value, result) { + const optionConfig = options[longOption] || {}; // Flags - result.flags[option] = true; + result.flags[longOption] = true; // Values - if (multiple) { + if (optionConfig.multiple) { // Always store value in array, including for flags. - // result.values[option] starts out not present, + // result.values[longOption] starts out not present, // first value is added as new array [newValue], // subsequent values are pushed to existing array. const usedAsFlag = value === undefined; const newValue = usedAsFlag ? true : value; - if (result.values[option] !== undefined) - ArrayPrototypePush(result.values[option], newValue); + if (result.values[longOption] !== undefined) + ArrayPrototypePush(result.values[longOption], newValue); else - result.values[option] = [newValue]; + result.values[longOption] = [newValue]; } else { - result.values[option] = value; + result.values[longOption] = value; } } -const parseArgs = ( - argv = getMainArgs(), +const parseArgs = ({ + args = getMainArgs(), options = {} -) => { - validateArray(argv, 'argv'); +} = {}) => { + validateArray(args, 'args'); validateObject(options, 'options'); - for (const key of ['withValue', 'multiples']) { - if (ObjectHasOwn(options, key)) { - validateArray(options[key], `options.${key}`); + ArrayPrototypeForEach( + ObjectEntries(options), + ([longOption, optionConfig]) => { + validateObject(optionConfig, `options.${longOption}`); + + if (ObjectHasOwn(optionConfig, 'type')) { + validateUnion(optionConfig.type, `options.${longOption}.type`, ['string', 'boolean']); + } + + if (ObjectHasOwn(optionConfig, 'short')) { + const shortOption = optionConfig.short; + validateString(shortOption, `options.${longOption}.short`); + if (shortOption.length !== 1) { + throw new Error(`options.${longOption}.short must be a single character, got '${shortOption}'`); + } + } + + if (ObjectHasOwn(optionConfig, 'multiple')) { + validateBoolean(optionConfig.multiple, `options.${longOption}.multiple`); + } } - } + ); const result = { flags: {}, @@ -96,8 +117,8 @@ const parseArgs = ( }; let pos = 0; - while (pos < argv.length) { - let arg = argv[pos]; + while (pos < args.length) { + let arg = args[pos]; if (StringPrototypeStartsWith(arg, '-')) { if (arg === '-') { @@ -110,30 +131,36 @@ const parseArgs = ( // and is returned verbatim result.positionals = ArrayPrototypeConcat( result.positionals, - ArrayPrototypeSlice(argv, ++pos) + ArrayPrototypeSlice(args, ++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); + const shortOption = StringPrototypeCharAt(arg, i); // Add 'i' to 'pos' such that short options are parsed in order // of definition: - ArrayPrototypeSplice(argv, pos + (i - 1), 0, `-${short}`); + ArrayPrototypeSplice(args, pos + (i - 1), 0, `-${shortOption}`); } } arg = StringPrototypeCharAt(arg, 1); // short - if (options.short && options.short[arg]) - arg = options.short[arg]; // now long! + + const [longOption] = ArrayPrototypeFind( + ObjectEntries(options), + ([, optionConfig]) => optionConfig.short === arg + ) || []; + + arg = longOption ?? arg; + // ToDo: later code tests for `=` in arg and wrong for shorts } else { arg = StringPrototypeSlice(arg, 2); // remove leading -- } if (StringPrototypeIncludes(arg, '=')) { - // Store option=value same way independent of `withValue` as: + // Store option=value same way independent of `type: "string"` as: // - looks like a value, store as a value // - match the intention of the user // - preserve information for author to process further @@ -143,18 +170,18 @@ const parseArgs = ( StringPrototypeSlice(arg, 0, index), StringPrototypeSlice(arg, index + 1), result); - } else if (pos + 1 < argv.length && - !StringPrototypeStartsWith(argv[pos + 1], '-') + } else if (pos + 1 < args.length && + !StringPrototypeStartsWith(args[pos + 1], '-') ) { - // withValue option should also support setting values when '= + // `type: "string"` 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 + // If `type: "string"` 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] : + const val = options[arg] && options[arg].type === 'string' ? + args[++pos] : undefined; storeOptionValue(options, arg, val, result); } else { @@ -163,7 +190,6 @@ const parseArgs = ( // save value as undefined storeOptionValue(options, arg, undefined, result); } - } else { // Arguments without a dash prefix are considered "positional" ArrayPrototypePush(result.positionals, arg); diff --git a/test/index.js b/test/index.js index 8487923..a393bf8 100644 --- a/test/index.js +++ b/test/index.js @@ -9,7 +9,7 @@ const { parseArgs } = require('../index.js'); test('when short option used as flag then stored as flag', function(t) { const passedArgs = ['-f']; const expected = { flags: { f: true }, values: { f: undefined }, positionals: [] }; - const args = parseArgs(passedArgs); + const args = parseArgs({ args: passedArgs }); t.deepEqual(args, expected); @@ -19,18 +19,18 @@ test('when short option used as flag then stored as flag', function(t) { test('when short option used as flag before positional then stored as flag and positional (and not value)', function(t) { const passedArgs = ['-f', 'bar']; const expected = { flags: { f: true }, values: { f: undefined }, positionals: [ 'bar' ] }; - const args = parseArgs(passedArgs); + const args = parseArgs({ args: passedArgs }); t.deepEqual(args, expected); t.end(); }); -test('when short option withValue used with value then stored as value', function(t) { +test('when short option `type: "string"` used with value then stored as value', function(t) { const passedArgs = ['-f', 'bar']; - const passedOptions = { withValue: ['f'] }; + const passedOptions = { f: { type: 'string' } }; const expected = { flags: { f: true }, values: { f: 'bar' }, positionals: [] }; - const args = parseArgs(passedArgs, passedOptions); + const args = parseArgs({ args: passedArgs, options: passedOptions }); t.deepEqual(args, expected); @@ -39,31 +39,31 @@ test('when short option withValue used with value then stored as value', functio test('when short option listed in short used as flag then long option stored as flag', function(t) { const passedArgs = ['-f']; - const passedOptions = { short: { f: 'foo' } }; + const passedOptions = { foo: { short: 'f' } }; const expected = { flags: { foo: true }, values: { foo: undefined }, positionals: [] }; - const args = parseArgs(passedArgs, passedOptions); + const args = parseArgs({ args: passedArgs, options: passedOptions }); t.deepEqual(args, expected); t.end(); }); -test('when short option listed in short and long listed in withValue and used with value then long option stored as value', function(t) { +test('when short option listed in short and long listed in `type: "string"` and used with value then long option stored as value', function(t) { const passedArgs = ['-f', 'bar']; - const passedOptions = { short: { f: 'foo' }, withValue: ['foo'] }; + const passedOptions = { foo: { short: 'f', type: 'string' } }; const expected = { flags: { foo: true }, values: { foo: 'bar' }, positionals: [] }; - const args = parseArgs(passedArgs, passedOptions); + const args = parseArgs({ args: passedArgs, options: passedOptions }); t.deepEqual(args, expected); t.end(); }); -test('when short option withValue used without value then stored as flag', function(t) { +test('when short option `type: "string"` used without value then stored as flag', function(t) { const passedArgs = ['-f']; - const passedOptions = { withValue: ['f'] }; + const passedOptions = { f: { type: 'string' } }; const expected = { flags: { f: true }, values: { f: undefined }, positionals: [] }; - const args = parseArgs(passedArgs, passedOptions); + const args = parseArgs({ args: passedArgs, options: passedOptions }); t.deepEqual(args, expected); @@ -74,7 +74,7 @@ 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); + const args = parseArgs({ args: passedArgs, options: passedOptions }); t.deepEqual(args, expected); @@ -85,18 +85,18 @@ 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); + const args = parseArgs({ args: passedArgs, options: 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) { +// // See: Guideline 5 https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html +test('if terminal of short-option group configured `type: "string"`, subsequent positional is stored', function(t) { const passedArgs = ['-rvf', 'foo']; - const passedOptions = { withValue: ['f'] }; + const passedOptions = { f: { type: 'string' } }; const expected = { flags: { r: true, f: true, v: true }, values: { r: undefined, v: undefined, f: 'foo' }, positionals: [] }; - const args = parseArgs(passedArgs, passedOptions); + const args = parseArgs({ args: passedArgs, options: passedOptions }); t.deepEqual(args, expected); t.end(); @@ -104,9 +104,9 @@ test('if terminal of short-option group configured withValue, subsequent positio test('handles short-option groups in conjunction with long-options', function(t) { const passedArgs = ['-rf', '--foo', 'foo']; - const passedOptions = { withValue: ['foo'] }; + const passedOptions = { foo: { type: 'string' } }; const expected = { flags: { r: true, f: true, foo: true }, values: { r: undefined, f: undefined, foo: 'foo' }, positionals: [] }; - const args = parseArgs(passedArgs, passedOptions); + const args = parseArgs({ args: passedArgs, options: passedOptions }); t.deepEqual(args, expected); t.end(); @@ -114,9 +114,9 @@ test('handles short-option groups in conjunction with long-options', function(t) test('handles short-option groups with "short" alias configured', function(t) { const passedArgs = ['-rf']; - const passedOptions = { short: { r: 'remove' } }; + const passedOptions = { remove: { short: 'r' } }; const expected = { flags: { remove: true, f: true }, values: { remove: undefined, f: undefined }, positionals: [] }; - const args = parseArgs(passedArgs, passedOptions); + const args = parseArgs({ args: passedArgs, options: passedOptions }); t.deepEqual(args, expected); t.end(); @@ -125,7 +125,7 @@ test('handles short-option groups with "short" alias configured', function(t) { test('Everything after a bare `--` is considered a positional argument', function(t) { const passedArgs = ['--', 'barepositionals', 'mopositionals']; const expected = { flags: {}, values: {}, positionals: ['barepositionals', 'mopositionals'] }; - const args = parseArgs(passedArgs); + const args = parseArgs({ args: passedArgs }); t.deepEqual(args, expected, 'testing bare positionals'); @@ -135,7 +135,7 @@ test('Everything after a bare `--` is considered a positional argument', functio test('args are true', function(t) { const passedArgs = ['--foo', '--bar']; const expected = { flags: { foo: true, bar: true }, values: { foo: undefined, bar: undefined }, positionals: [] }; - const args = parseArgs(passedArgs); + const args = parseArgs({ args: passedArgs }); t.deepEqual(args, expected, 'args are true'); @@ -145,18 +145,18 @@ test('args are true', function(t) { test('arg is true and positional is identified', function(t) { const passedArgs = ['--foo=a', '--foo', 'b']; const expected = { flags: { foo: true }, values: { foo: undefined }, positionals: ['b'] }; - const args = parseArgs(passedArgs); + const args = parseArgs({ args: passedArgs }); t.deepEqual(args, expected, 'arg is true and positional is identified'); t.end(); }); -test('args equals are passed "withValue"', function(t) { +test('args equals are passed `type: "string"`', function(t) { const passedArgs = ['--so=wat']; - const passedOptions = { withValue: ['so'] }; + const passedOptions = { so: { type: 'string' } }; const expected = { flags: { so: true }, values: { so: 'wat' }, positionals: [] }; - const args = parseArgs(passedArgs, passedOptions); + const args = parseArgs({ args: passedArgs, options: passedOptions }); t.deepEqual(args, expected, 'arg value is passed'); @@ -166,29 +166,29 @@ test('args equals are passed "withValue"', function(t) { 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); + const args = parseArgs({ args: passedArgs }); t.deepEqual(args, expected); t.end(); }); -test('zero config args equals are parsed as if "withValue"', function(t) { +test('zero config args equals are parsed as if `type: "string"`', function(t) { const passedArgs = ['--so=wat']; const passedOptions = { }; const expected = { flags: { so: true }, values: { so: 'wat' }, positionals: [] }; - const args = parseArgs(passedArgs, passedOptions); + const args = parseArgs({ args: passedArgs, options: passedOptions }); t.deepEqual(args, expected, 'arg value is passed'); t.end(); }); -test('same arg is passed twice "withValue" and last value is recorded', function(t) { +test('same arg is passed twice `type: "string"` and last value is recorded', function(t) { const passedArgs = ['--foo=a', '--foo', 'b']; - const passedOptions = { withValue: ['foo'] }; + const passedOptions = { foo: { type: 'string' } }; const expected = { flags: { foo: true }, values: { foo: 'b' }, positionals: [] }; - const args = parseArgs(passedArgs, passedOptions); + const args = parseArgs({ args: passedArgs, options: passedOptions }); t.deepEqual(args, expected, 'last arg value is passed'); @@ -197,31 +197,36 @@ test('same arg is passed twice "withValue" and last value is recorded', function test('args equals pass string including more equals', function(t) { const passedArgs = ['--so=wat=bing']; - const passedOptions = { withValue: ['so'] }; + const passedOptions = { so: { type: 'string' } }; const expected = { flags: { so: true }, values: { so: 'wat=bing' }, positionals: [] }; - const args = parseArgs(passedArgs, passedOptions); + const args = parseArgs({ args: passedArgs, options: passedOptions }); t.deepEqual(args, expected, 'arg value is passed'); t.end(); }); -test('first arg passed for "withValue" and "multiples" is in array', function(t) { +test('first arg passed for `type: "string"` and "multiple" is in array', function(t) { const passedArgs = ['--foo=a']; - const passedOptions = { withValue: ['foo'], multiples: ['foo'] }; + const passedOptions = { foo: { type: 'string', multiple: true } }; const expected = { flags: { foo: true }, values: { foo: ['a'] }, positionals: [] }; - const args = parseArgs(passedArgs, passedOptions); + const args = parseArgs({ args: passedArgs, options: passedOptions }); t.deepEqual(args, expected, 'first multiple in array'); t.end(); }); -test('args are passed "withValue" and "multiples"', function(t) { +test('args are passed `type: "string"` and "multiple"', function(t) { const passedArgs = ['--foo=a', '--foo', 'b']; - const passedOptions = { withValue: ['foo'], multiples: ['foo'] }; + const passedOptions = { + foo: { + type: 'string', + multiple: true, + }, + }; const expected = { flags: { foo: true }, values: { foo: ['a', 'b'] }, positionals: [] }; - const args = parseArgs(passedArgs, passedOptions); + const args = parseArgs({ args: passedArgs, options: passedOptions }); t.deepEqual(args, expected, 'both arg values are passed'); @@ -231,11 +236,11 @@ test('args are passed "withValue" and "multiples"', function(t) { test('order of option and positional does not matter (per README)', function(t) { const passedArgs1 = ['--foo=bar', 'baz']; const passedArgs2 = ['baz', '--foo=bar']; - const passedOptions = { withValue: ['foo'] }; + const passedOptions = { foo: { type: 'string' } }; const expected = { flags: { foo: true }, values: { foo: 'bar' }, positionals: ['baz'] }; - t.deepEqual(parseArgs(passedArgs1, passedOptions), expected, 'option then positional'); - t.deepEqual(parseArgs(passedArgs2, passedOptions), expected, 'positional then option'); + t.deepEqual(parseArgs({ args: passedArgs1, options: passedOptions }), expected, 'option then positional'); + t.deepEqual(parseArgs({ args: passedArgs2, options: passedOptions }), expected, 'positional then option'); t.end(); }); @@ -334,7 +339,7 @@ test('excess leading dashes on options are retained', function(t) { values: { '-triple': undefined }, positionals: [] }; - const result = parseArgs(passedArgs, passedOptions); + const result = parseArgs({ args: passedArgs, options: passedOptions }); t.deepEqual(result, expected, 'excess option dashes are retained'); @@ -345,32 +350,42 @@ test('excess leading dashes on options are retained', function(t) { test('invalid argument passed for options', function(t) { const passedArgs = ['--so=wat']; + const passedOptions = 'bad value'; - t.throws(function() { parseArgs(passedArgs, 'bad value'); }, { + t.throws(function() { parseArgs({ args: passedArgs, options: passedOptions }); }, { code: 'ERR_INVALID_ARG_TYPE' }); t.end(); }); -test('boolean passed to "withValue" option', function(t) { +test('boolean passed to "type" option', function(t) { const passedArgs = ['--so=wat']; - const passedOptions = { withValue: true }; + const passedOptions = { foo: { type: true } }; - t.throws(function() { parseArgs(passedArgs, passedOptions); }, { + t.throws(function() { parseArgs({ args: passedArgs, options: passedOptions }); }, { code: 'ERR_INVALID_ARG_TYPE' }); t.end(); }); -test('string passed to "withValue" option', function(t) { +test('invalid union value passed to "type" option', function(t) { const passedArgs = ['--so=wat']; - const passedOptions = { withValue: 'so' }; + const passedOptions = { foo: { type: 'str' } }; - t.throws(function() { parseArgs(passedArgs, passedOptions); }, { + t.throws(function() { parseArgs({ args: passedArgs, options: passedOptions }); }, { code: 'ERR_INVALID_ARG_TYPE' }); t.end(); }); + +test('invalid short option length', function(t) { + const passedArgs = []; + const passedOptions = { foo: { short: 'fo' } }; + + t.throws(function() { parseArgs({ args: passedArgs, options: passedOptions }); }); + + t.end(); +}); diff --git a/validators.js b/validators.js index a8be5ff..9dae8e9 100644 --- a/validators.js +++ b/validators.js @@ -2,6 +2,8 @@ const { ArrayIsArray, + ArrayPrototypeIncludes, + ArrayPrototypeJoin, } = require('./primordials'); const { @@ -10,6 +12,24 @@ const { } } = require('./errors'); +function validateString(value, name) { + if (typeof value !== 'string') { + throw new ERR_INVALID_ARG_TYPE(name, 'String', value); + } +} + +function validateUnion(value, name, union) { + if (!ArrayPrototypeIncludes(union, value)) { + throw new ERR_INVALID_ARG_TYPE(name, `('${ArrayPrototypeJoin(union, '|')}')`, value); + } +} + +function validateBoolean(value, name) { + if (typeof value !== 'boolean') { + throw new ERR_INVALID_ARG_TYPE(name, 'Boolean', value); + } +} + function validateArray(value, name) { if (!ArrayIsArray(value)) { throw new ERR_INVALID_ARG_TYPE(name, 'Array', value); @@ -42,4 +62,7 @@ function validateObject(value, name, options) { module.exports = { validateArray, validateObject, + validateString, + validateUnion, + validateBoolean, }; From a92600fa6c214508ab1e016fa55879a314f541af Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 12 Mar 2022 20:26:02 +1300 Subject: [PATCH 5/6] refactor!: parsing, revisit short option groups, add support for combined short and value (#75) 1) Refactor parsing to use independent blocks of code, rather than nested cascading context. This makes it easier to reason about the behaviour. 2) Split out small pieces of logic to named routines to improve readability, and allow extra documentation and examples without cluttering the parsing. (Thanks to @aaronccasanova for inspiration.) 3) Existing tests untouched to make it clear that the tested functionality has not changed. 4) Be more explicit about short option group expansion, and ready to throw error in strict mode for string option in the middle of the argument. (See #11 and #74.) 5) Add support for short option combined with value (without intervening `=`). This is what Commander and Open Group Utility Conventions do, but is _not_ what Yargs does. I don't want to block PR on this and happy to comment it out for further discussion if needed. (I have found some interesting variations in the wild.) [Edit: see also #78] 6) Add support for multiple unit tests files. Expand tests from 33 to 113, but many for internal routines rather than testing exposed API. 7) Added `.editorconfig` file Co-authored-by: Jordan Harband Co-authored-by: Aaron Casanova <32409546+aaronccasanova@users.noreply.github.com> --- .editorconfig | 12 ++ index.js | 161 ++++++++++++----------- package.json | 8 +- test/dash.js | 34 +++++ test/find-long-option-for-short.js | 20 +++ test/is-lone-long-option.js | 62 +++++++++ test/is-lone-short-option.js | 45 +++++++ test/is-long-option-and-value.js | 62 +++++++++ test/is-option-value.js | 52 ++++++++ test/is-short-option-and-value.js | 60 +++++++++ test/is-short-option-group.js | 71 ++++++++++ test/short-option-combined-with-value.js | 83 ++++++++++++ test/short-option-groups.js | 71 ++++++++++ test/store-user-intent.js | 53 ++++++++ utils.js | 155 ++++++++++++++++++++++ 15 files changed, 872 insertions(+), 77 deletions(-) create mode 100644 .editorconfig create mode 100644 test/dash.js create mode 100644 test/find-long-option-for-short.js create mode 100644 test/is-lone-long-option.js create mode 100644 test/is-lone-short-option.js create mode 100644 test/is-long-option-and-value.js create mode 100644 test/is-option-value.js create mode 100644 test/is-short-option-and-value.js create mode 100644 test/is-short-option-group.js create mode 100644 test/short-option-combined-with-value.js create mode 100644 test/short-option-groups.js create mode 100644 test/store-user-intent.js create mode 100644 utils.js diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 0000000..8f7d665 --- /dev/null +++ b/.editorconfig @@ -0,0 +1,12 @@ +# EditorConfig is awesome: http://EditorConfig.org + +# top-most EditorConfig file +root = true + +[*] +end_of_line = lf +insert_final_newline = true +indent_style = space +indent_size = 2 +tab_width = 2 +# trim_trailing_whitespace = true diff --git a/index.js b/index.js index cf73e7c..56da055 100644 --- a/index.js +++ b/index.js @@ -2,10 +2,9 @@ const { ArrayPrototypeConcat, - ArrayPrototypeFind, ArrayPrototypeForEach, + ArrayPrototypeShift, ArrayPrototypeSlice, - ArrayPrototypeSplice, ArrayPrototypePush, ObjectHasOwn, ObjectEntries, @@ -13,7 +12,6 @@ const { StringPrototypeIncludes, StringPrototypeIndexOf, StringPrototypeSlice, - StringPrototypeStartsWith, } = require('./primordials'); const { @@ -24,6 +22,16 @@ const { validateBoolean, } = require('./validators'); +const { + findLongOptionForShort, + isLoneLongOption, + isLoneShortOption, + isLongOptionAndValue, + isOptionValue, + isShortOptionAndValue, + isShortOptionGroup +} = require('./utils'); + function getMainArgs() { // This function is a placeholder for proposed process.mainArgs. // Work out where to slice process.argv for user supplied arguments. @@ -116,86 +124,89 @@ const parseArgs = ({ positionals: [] }; - let pos = 0; - while (pos < args.length) { - let arg = args[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(args, ++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 shortOption = StringPrototypeCharAt(arg, i); - // Add 'i' to 'pos' such that short options are parsed in order - // of definition: - ArrayPrototypeSplice(args, pos + (i - 1), 0, `-${shortOption}`); - } - } + let remainingArgs = ArrayPrototypeSlice(args); + while (remainingArgs.length > 0) { + const arg = ArrayPrototypeShift(remainingArgs); + const nextArg = remainingArgs[0]; + + // 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, + remainingArgs + ); + break; // Finished processing args, leave while loop. + } - arg = StringPrototypeCharAt(arg, 1); // short + if (isLoneShortOption(arg)) { + // e.g. '-f' + const shortOption = StringPrototypeCharAt(arg, 1); + const longOption = findLongOptionForShort(shortOption, options); + let optionValue; + if (options[longOption]?.type === 'string' && isOptionValue(nextArg)) { + // e.g. '-f', 'bar' + optionValue = ArrayPrototypeShift(remainingArgs); + } + storeOptionValue(options, longOption, optionValue, result); + continue; + } - const [longOption] = ArrayPrototypeFind( - ObjectEntries(options), - ([, optionConfig]) => optionConfig.short === arg - ) || []; + if (isShortOptionGroup(arg, options)) { + // Expand -fXzy to -f -X -z -y + const expanded = []; + for (let index = 1; index < arg.length; index++) { + const shortOption = StringPrototypeCharAt(arg, index); + const longOption = findLongOptionForShort(shortOption, options); + if (options[longOption]?.type !== 'string' || + index === arg.length - 1) { + // Boolean option, or last short in group. Well formed. + ArrayPrototypePush(expanded, `-${shortOption}`); + } else { + // String option in middle. Yuck. + // ToDo: if strict then throw + // Expand -abfFILE to -a -b -fFILE + ArrayPrototypePush(expanded, `-${StringPrototypeSlice(arg, index)}`); + break; // finished short group + } + } + remainingArgs = ArrayPrototypeConcat(expanded, remainingArgs); + continue; + } - arg = longOption ?? arg; + if (isShortOptionAndValue(arg, options)) { + // e.g. -fFILE + const shortOption = StringPrototypeCharAt(arg, 1); + const longOption = findLongOptionForShort(shortOption, options); + const optionValue = StringPrototypeSlice(arg, 2); + storeOptionValue(options, longOption, optionValue, result); + continue; + } - // ToDo: later code tests for `=` in arg and wrong for shorts - } else { - arg = StringPrototypeSlice(arg, 2); // remove leading -- + if (isLoneLongOption(arg)) { + // e.g. '--foo' + const longOption = StringPrototypeSlice(arg, 2); + let optionValue; + if (options[longOption]?.type === 'string' && isOptionValue(nextArg)) { + // e.g. '--foo', 'bar' + optionValue = ArrayPrototypeShift(remainingArgs); } + storeOptionValue(options, longOption, optionValue, result); + continue; + } - if (StringPrototypeIncludes(arg, '=')) { - // Store option=value same way independent of `type: "string"` as: - // - looks like a value, store as a value - // - match the intention of the user - // - preserve information for author to process further - const index = StringPrototypeIndexOf(arg, '='); - storeOptionValue( - options, - StringPrototypeSlice(arg, 0, index), - StringPrototypeSlice(arg, index + 1), - result); - } else if (pos + 1 < args.length && - !StringPrototypeStartsWith(args[pos + 1], '-') - ) { - // `type: "string"` option should also support setting values when '=' - // isn't used ie. both --foo=b and --foo b should work - - // If `type: "string"` 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[arg] && options[arg].type === 'string' ? - args[++pos] : - undefined; - storeOptionValue(options, arg, val, result); - } 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); - } - } else { - // Arguments without a dash prefix are considered "positional" - ArrayPrototypePush(result.positionals, arg); + if (isLongOptionAndValue(arg)) { + // e.g. --foo=bar + const index = StringPrototypeIndexOf(arg, '='); + const longOption = StringPrototypeSlice(arg, 2, index); + const optionValue = StringPrototypeSlice(arg, index + 1); + storeOptionValue(options, longOption, optionValue, result); + continue; } - pos++; + // Anything left is a positional + ArrayPrototypePush(result.positionals, arg); } return result; diff --git a/package.json b/package.json index fbef77d..a54bb82 100644 --- a/package.json +++ b/package.json @@ -3,9 +3,13 @@ "version": "0.3.0", "description": "Polyfill of future proposal for `util.parseArgs()`", "main": "index.js", + "exports": { + ".": "./index.js", + "./package.json": "./package.json" + }, "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..c727e0a --- /dev/null +++ b/test/dash.js @@ -0,0 +1,34 @@ +'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 +// +// A different usage and example is `git switch -` to switch back to the previous branch. + +test("dash: when args include '-' used as positional then result has '-' in positionals", (t) => { + const passedArgs = ['-']; + const expected = { flags: {}, values: {}, positionals: ['-'] }; + + const result = parseArgs({ args: passedArgs }); + + t.deepEqual(result, expected); + t.end(); +}); + +// If '-' is a valid positional, it is symmetrical to allow it as an option value too. +test("dash: when args include '-' used as space-separated option value then result has '-' in option value", (t) => { + const passedArgs = ['-v', '-']; + const passedOptions = { v: { type: 'string' } }; + const expected = { flags: { v: true }, values: { v: '-' }, positionals: [] }; + + const result = parseArgs({ args: passedArgs, options: passedOptions }); + + t.deepEqual(result, expected); + t.end(); +}); diff --git a/test/find-long-option-for-short.js b/test/find-long-option-for-short.js new file mode 100644 index 0000000..97b4603 --- /dev/null +++ b/test/find-long-option-for-short.js @@ -0,0 +1,20 @@ +'use strict'; +/* eslint max-len: 0 */ + +const test = require('tape'); +const { findLongOptionForShort } = require('../utils.js'); + +test('findLongOptionForShort: when passed empty options then returns short', (t) => { + t.equal(findLongOptionForShort('a', {}), 'a'); + t.end(); +}); + +test('findLongOptionForShort: when passed short not present in options then returns short', (t) => { + t.equal(findLongOptionForShort('a', { foo: { short: 'f', type: 'string' } }), 'a'); + t.end(); +}); + +test('findLongOptionForShort: when passed short present in options then returns long', (t) => { + t.equal(findLongOptionForShort('a', { alpha: { short: 'a' } }), 'alpha'); + t.end(); +}); diff --git a/test/is-lone-long-option.js b/test/is-lone-long-option.js new file mode 100644 index 0000000..deb95e8 --- /dev/null +++ b/test/is-lone-long-option.js @@ -0,0 +1,62 @@ +'use strict'; +/* eslint max-len: 0 */ + +const test = require('tape'); +const { isLoneLongOption } = require('../utils.js'); + +test('isLoneLongOption: when passed short option then returns false', (t) => { + t.false(isLoneLongOption('-s')); + t.end(); +}); + +test('isLoneLongOption: when passed short option group then returns false', (t) => { + t.false(isLoneLongOption('-abc')); + t.end(); +}); + +test('isLoneLongOption: when passed lone long option then returns true', (t) => { + t.true(isLoneLongOption('--foo')); + t.end(); +}); + +test('isLoneLongOption: when passed single character long option then returns true', (t) => { + t.true(isLoneLongOption('--f')); + t.end(); +}); + +test('isLoneLongOption: when passed long option and value then returns false', (t) => { + t.false(isLoneLongOption('--foo=bar')); + t.end(); +}); + +test('isLoneLongOption: when passed empty string then returns false', (t) => { + t.false(isLoneLongOption('')); + t.end(); +}); + +test('isLoneLongOption: when passed plain text then returns false', (t) => { + t.false(isLoneLongOption('foo')); + t.end(); +}); + +test('isLoneLongOption: when passed single dash then returns false', (t) => { + t.false(isLoneLongOption('-')); + t.end(); +}); + +test('isLoneLongOption: when passed double dash then returns false', (t) => { + t.false(isLoneLongOption('--')); + t.end(); +}); + +// This is a bit bogus, but simple consistent behaviour: long option follows double dash. +test('isLoneLongOption: when passed arg starting with triple dash then returns true', (t) => { + t.true(isLoneLongOption('---foo')); + t.end(); +}); + +// This is a bit bogus, but simple consistent behaviour: long option follows double dash. +test("isLoneLongOption: when passed '--=' then returns true", (t) => { + t.true(isLoneLongOption('--=')); + t.end(); +}); diff --git a/test/is-lone-short-option.js b/test/is-lone-short-option.js new file mode 100644 index 0000000..baea02b --- /dev/null +++ b/test/is-lone-short-option.js @@ -0,0 +1,45 @@ +'use strict'; +/* eslint max-len: 0 */ + +const test = require('tape'); +const { isLoneShortOption } = require('../utils.js'); + +test('isLoneShortOption: when passed short option then returns true', (t) => { + t.true(isLoneShortOption('-s')); + t.end(); +}); + +test('isLoneShortOption: when passed short option group (or might be short and value) then returns false', (t) => { + t.false(isLoneShortOption('-abc')); + t.end(); +}); + +test('isLoneShortOption: when passed long option then returns false', (t) => { + t.false(isLoneShortOption('--foo')); + t.end(); +}); + +test('isLoneShortOption: when passed long option with value then returns false', (t) => { + t.false(isLoneShortOption('--foo=bar')); + t.end(); +}); + +test('isLoneShortOption: when passed empty string then returns false', (t) => { + t.false(isLoneShortOption('')); + t.end(); +}); + +test('isLoneShortOption: when passed plain text then returns false', (t) => { + t.false(isLoneShortOption('foo')); + t.end(); +}); + +test('isLoneShortOption: when passed single dash then returns false', (t) => { + t.false(isLoneShortOption('-')); + t.end(); +}); + +test('isLoneShortOption: when passed double dash then returns false', (t) => { + t.false(isLoneShortOption('--')); + t.end(); +}); diff --git a/test/is-long-option-and-value.js b/test/is-long-option-and-value.js new file mode 100644 index 0000000..a4c9e1d --- /dev/null +++ b/test/is-long-option-and-value.js @@ -0,0 +1,62 @@ +'use strict'; +/* eslint max-len: 0 */ + +const test = require('tape'); +const { isLongOptionAndValue } = require('../utils.js'); + +test('isLongOptionAndValue: when passed short option then returns false', (t) => { + t.false(isLongOptionAndValue('-s')); + t.end(); +}); + +test('isLongOptionAndValue: when passed short option group then returns false', (t) => { + t.false(isLongOptionAndValue('-abc')); + t.end(); +}); + +test('isLongOptionAndValue: when passed lone long option then returns false', (t) => { + t.false(isLongOptionAndValue('--foo')); + t.end(); +}); + +test('isLongOptionAndValue: when passed long option and value then returns true', (t) => { + t.true(isLongOptionAndValue('--foo=bar')); + t.end(); +}); + +test('isLongOptionAndValue: when passed single character long option and value then returns true', (t) => { + t.true(isLongOptionAndValue('--f=bar')); + t.end(); +}); + +test('isLongOptionAndValue: when passed empty string then returns false', (t) => { + t.false(isLongOptionAndValue('')); + t.end(); +}); + +test('isLongOptionAndValue: when passed plain text then returns false', (t) => { + t.false(isLongOptionAndValue('foo')); + t.end(); +}); + +test('isLongOptionAndValue: when passed single dash then returns false', (t) => { + t.false(isLongOptionAndValue('-')); + t.end(); +}); + +test('isLongOptionAndValue: when passed double dash then returns false', (t) => { + t.false(isLongOptionAndValue('--')); + t.end(); +}); + +// This is a bit bogus, but simple consistent behaviour: long option follows double dash. +test('isLongOptionAndValue: when passed arg starting with triple dash and value then returns true', (t) => { + t.true(isLongOptionAndValue('---foo=bar')); + t.end(); +}); + +// This is a bit bogus, but simple consistent behaviour: long option follows double dash. +test("isLongOptionAndValue: when passed '--=' then returns false", (t) => { + t.false(isLongOptionAndValue('--=')); + t.end(); +}); diff --git a/test/is-option-value.js b/test/is-option-value.js new file mode 100644 index 0000000..199bf30 --- /dev/null +++ b/test/is-option-value.js @@ -0,0 +1,52 @@ +'use strict'; +/* eslint max-len: 0 */ + +const test = require('tape'); +const { isOptionValue } = require('../utils.js'); + +test('isOptionValue: when passed plain text then returns true', (t) => { + t.true(isOptionValue('abc')); + t.end(); +}); + +test('isOptionValue: when passed digits then returns true', (t) => { + t.true(isOptionValue(123)); + t.end(); +}); + +test('isOptionValue: when passed empty string then returns true', (t) => { + t.true(isOptionValue('')); + t.end(); +}); + +// Special case, used as stdin/stdout et al and not reason to reject +test('isOptionValue: when passed dash then returns true', (t) => { + t.true(isOptionValue('-')); + t.end(); +}); + +// Supporting undefined so can pass element off end of array without checking +test('isOptionValue: when passed undefined then returns false', (t) => { + t.false(isOptionValue(undefined)); + t.end(); +}); + +test('isOptionValue: when passed short option then returns false', (t) => { + t.false(isOptionValue('-a')); + t.end(); +}); + +test('isOptionValue: when passed short option group of short option with value then returns false', (t) => { + t.false(isOptionValue('-abd')); + t.end(); +}); + +test('isOptionValue: when passed long option then returns false', (t) => { + t.false(isOptionValue('--foo')); + t.end(); +}); + +test('isOptionValue: when passed long option with value then returns false', (t) => { + t.false(isOptionValue('--foo=bar')); + t.end(); +}); diff --git a/test/is-short-option-and-value.js b/test/is-short-option-and-value.js new file mode 100644 index 0000000..9b43b20 --- /dev/null +++ b/test/is-short-option-and-value.js @@ -0,0 +1,60 @@ +'use strict'; +/* eslint max-len: 0 */ + +const test = require('tape'); +const { isShortOptionAndValue } = require('../utils.js'); + +test('isShortOptionAndValue: when passed lone short option then returns false', (t) => { + t.false(isShortOptionAndValue('-s', {})); + t.end(); +}); + +test('isShortOptionAndValue: when passed group with leading zero-config boolean then returns false', (t) => { + t.false(isShortOptionAndValue('-ab', {})); + t.end(); +}); + +test('isShortOptionAndValue: when passed group with leading configured implicit boolean then returns false', (t) => { + t.false(isShortOptionAndValue('-ab', { aaa: { short: 'a' } })); + t.end(); +}); + +test('isShortOptionAndValue: when passed group with leading configured explicit boolean then returns false', (t) => { + t.false(isShortOptionAndValue('-ab', { aaa: { short: 'a', type: 'boolean' } })); + t.end(); +}); + +test('isShortOptionAndValue: when passed group with leading configured string then returns true', (t) => { + t.true(isShortOptionAndValue('-ab', { aaa: { short: 'a', type: 'string' } })); + t.end(); +}); + +test('isShortOptionAndValue: when passed long option then returns false', (t) => { + t.false(isShortOptionAndValue('--foo', {})); + t.end(); +}); + +test('isShortOptionAndValue: when passed long option with value then returns false', (t) => { + t.false(isShortOptionAndValue('--foo=bar', {})); + t.end(); +}); + +test('isShortOptionAndValue: when passed empty string then returns false', (t) => { + t.false(isShortOptionAndValue('', {})); + t.end(); +}); + +test('isShortOptionAndValue: when passed plain text then returns false', (t) => { + t.false(isShortOptionAndValue('foo', {})); + t.end(); +}); + +test('isShortOptionAndValue: when passed single dash then returns false', (t) => { + t.false(isShortOptionAndValue('-', {})); + t.end(); +}); + +test('isShortOptionAndValue: when passed double dash then returns false', (t) => { + t.false(isShortOptionAndValue('--', {})); + t.end(); +}); diff --git a/test/is-short-option-group.js b/test/is-short-option-group.js new file mode 100644 index 0000000..56a5e00 --- /dev/null +++ b/test/is-short-option-group.js @@ -0,0 +1,71 @@ +'use strict'; +/* eslint max-len: 0 */ + +const test = require('tape'); +const { isShortOptionGroup } = require('../utils.js'); + +test('isShortOptionGroup: when passed lone short option then returns false', (t) => { + t.false(isShortOptionGroup('-s', {})); + t.end(); +}); + +test('isShortOptionGroup: when passed group with leading zero-config boolean then returns true', (t) => { + t.true(isShortOptionGroup('-ab', {})); + t.end(); +}); + +test('isShortOptionGroup: when passed group with leading configured implicit boolean then returns true', (t) => { + t.true(isShortOptionGroup('-ab', { aaa: { short: 'a' } })); + t.end(); +}); + +test('isShortOptionGroup: when passed group with leading configured explicit boolean then returns true', (t) => { + t.true(isShortOptionGroup('-ab', { aaa: { short: 'a', type: 'boolean' } })); + t.end(); +}); + +test('isShortOptionGroup: when passed group with leading configured string then returns false', (t) => { + t.false(isShortOptionGroup('-ab', { aaa: { short: 'a', type: 'string' } })); + t.end(); +}); + +test('isShortOptionGroup: when passed group with trailing configured string then returns true', (t) => { + t.true(isShortOptionGroup('-ab', { bbb: { short: 'b', type: 'string' } })); + t.end(); +}); + +// This one is dubious, but leave it to caller to handle. +test('isShortOptionGroup: when passed group with middle configured string then returns true', (t) => { + t.true(isShortOptionGroup('-abc', { bbb: { short: 'b', type: 'string' } })); + t.end(); +}); + +test('isShortOptionGroup: when passed long option then returns false', (t) => { + t.false(isShortOptionGroup('--foo', {})); + t.end(); +}); + +test('isShortOptionGroup: when passed long option with value then returns false', (t) => { + t.false(isShortOptionGroup('--foo=bar', {})); + t.end(); +}); + +test('isShortOptionGroup: when passed empty string then returns false', (t) => { + t.false(isShortOptionGroup('', {})); + t.end(); +}); + +test('isShortOptionGroup: when passed plain text then returns false', (t) => { + t.false(isShortOptionGroup('foo', {})); + t.end(); +}); + +test('isShortOptionGroup: when passed single dash then returns false', (t) => { + t.false(isShortOptionGroup('-', {})); + t.end(); +}); + +test('isShortOptionGroup: when passed double dash then returns false', (t) => { + t.false(isShortOptionGroup('--', {})); + t.end(); +}); diff --git a/test/short-option-combined-with-value.js b/test/short-option-combined-with-value.js new file mode 100644 index 0000000..66fb5d2 --- /dev/null +++ b/test/short-option-combined-with-value.js @@ -0,0 +1,83 @@ +'use strict'; +/* eslint max-len: 0 */ + +const test = require('tape'); +const { parseArgs } = require('../index.js'); + +test('when combine string short with plain text then parsed as value', (t) => { + const passedArgs = ['-aHELLO']; + const passedOptions = { alpha: { short: 'a', type: 'string' } }; + const expected = { flags: { alpha: true }, values: { alpha: 'HELLO' }, positionals: [] }; + + const result = parseArgs({ args: passedArgs, options: passedOptions }); + + t.deepEqual(result, expected); + t.end(); +}); + +test('when combine low-config string short with plain text then parsed as value', (t) => { + const passedArgs = ['-aHELLO']; + const passedOptions = { a: { type: 'string' } }; + const expected = { flags: { a: true }, values: { a: 'HELLO' }, positionals: [] }; + + const result = parseArgs({ args: passedArgs, options: passedOptions }); + + t.deepEqual(result, expected); + t.end(); +}); + +test('when combine string short with value like short option then parsed as value', (t) => { + const passedArgs = ['-a-b']; + const passedOptions = { alpha: { short: 'a', type: 'string' } }; + const expected = { flags: { alpha: true }, values: { alpha: '-b' }, positionals: [] }; + + const result = parseArgs({ args: passedArgs, options: passedOptions }); + + t.deepEqual(result, expected); + t.end(); +}); + +test('when combine string short with value like long option then parsed as value', (t) => { + const passedArgs = ['-a--bar']; + const passedOptions = { alpha: { short: 'a', type: 'string' } }; + const expected = { flags: { alpha: true }, values: { alpha: '--bar' }, positionals: [] }; + + const result = parseArgs({ args: passedArgs, options: passedOptions }); + + t.deepEqual(result, expected); + t.end(); +}); + +test('when combine string short with value like negative number then parsed as value', (t) => { + const passedArgs = ['-a-5']; + const passedOptions = { alpha: { short: 'a', type: 'string' } }; + const expected = { flags: { alpha: true }, values: { alpha: '-5' }, positionals: [] }; + + const result = parseArgs({ args: passedArgs, options: passedOptions }); + + t.deepEqual(result, expected); + t.end(); +}); + + +test('when combine string short with value which matches configured flag then parsed as value', (t) => { + const passedArgs = ['-af']; + const passedOptions = { alpha: { short: 'a', type: 'string' }, file: { short: 'f' } }; + const expected = { flags: { alpha: true }, values: { alpha: 'f' }, positionals: [] }; + + const result = parseArgs({ args: passedArgs, options: passedOptions }); + + t.deepEqual(result, expected); + t.end(); +}); + +test('when combine string short with value including equals then parsed with equals in value', (t) => { + const passedArgs = ['-a=5']; + const passedOptions = { alpha: { short: 'a', type: 'string' } }; + const expected = { flags: { alpha: true }, values: { alpha: '=5' }, positionals: [] }; + + const result = parseArgs({ args: passedArgs, options: passedOptions }); + + t.deepEqual(result, expected); + t.end(); +}); diff --git a/test/short-option-groups.js b/test/short-option-groups.js new file mode 100644 index 0000000..f849b50 --- /dev/null +++ b/test/short-option-groups.js @@ -0,0 +1,71 @@ +'use strict'; +/* eslint max-len: 0 */ + +const test = require('tape'); +const { parseArgs } = require('../index.js'); + +test('when pass zero-config group of booleans then parsed as booleans', (t) => { + const passedArgs = ['-rf', 'p']; + const passedOptions = { }; + const expected = { flags: { r: true, f: true }, values: { r: undefined, f: undefined }, positionals: ['p'] }; + + const result = parseArgs({ args: passedArgs, options: passedOptions }); + + t.deepEqual(result, expected); + t.end(); +}); + +test('when pass low-config group of booleans then parsed as booleans', (t) => { + const passedArgs = ['-rf', 'p']; + const passedOptions = { r: {}, f: {} }; + const expected = { flags: { r: true, f: true }, values: { r: undefined, f: undefined }, positionals: ['p'] }; + + const result = parseArgs({ args: passedArgs, options: passedOptions }); + + t.deepEqual(result, expected); + t.end(); +}); + +test('when pass full-config group of booleans then parsed as booleans', (t) => { + const passedArgs = ['-rf', 'p']; + const passedOptions = { r: { type: 'boolean' }, f: { type: 'boolean' } }; + const expected = { flags: { r: true, f: true }, values: { r: undefined, f: undefined }, positionals: ['p'] }; + + const result = parseArgs({ args: passedArgs, options: passedOptions }); + + t.deepEqual(result, expected); + t.end(); +}); + +test('when pass group with string option on end then parsed as booleans and string option', (t) => { + const passedArgs = ['-rf', 'p']; + const passedOptions = { r: { type: 'boolean' }, f: { type: 'string' } }; + const expected = { flags: { r: true, f: true }, values: { r: undefined, f: 'p' }, positionals: [] }; + + const result = parseArgs({ args: passedArgs, options: passedOptions }); + + t.deepEqual(result, expected); + t.end(); +}); + +test('when pass group with string option in middle and strict:false then parsed as booleans and string option with trailing value', (t) => { + const passedArgs = ['-afb', 'p']; + const passedOptions = { f: { type: 'string' } }; + const expected = { flags: { a: true, f: true }, values: { a: undefined, f: 'b' }, positionals: ['p'] }; + + const result = parseArgs({ args: passedArgs, options: passedOptions, strict: false }); + + t.deepEqual(result, expected); + t.end(); +}); + +// Hopefully coming: +// test('when pass group with string option in middle and strict:true then error', (t) => { +// const passedArgs = ['-afb', 'p']; +// const passedOptions = { f: { type: 'string' } }; +// +// t.throws(() => { +// parseArgs({ args: passedArgs, options: passedOptions, strict: true }); +// }); +// t.end(); +// }); diff --git a/test/store-user-intent.js b/test/store-user-intent.js new file mode 100644 index 0000000..d5340a9 --- /dev/null +++ b/test/store-user-intent.js @@ -0,0 +1,53 @@ +'use strict'; +/* eslint max-len: 0 */ + +const test = require('tape'); +const { parseArgs } = require('../index.js'); + + +// Rationale +// +// John Gee: +// - Looks like a boolean option, stored like a boolean option. +// - Looks like a string option, stored like a string option. +// No loss of information. No new pattern to learn in result. +// +// Jordan Harband: In other words, the way they're stored matches the intention of the user, +// not the configurer, which will ensure the configurer can most accurately respond to the +// user's intentions. + +test('when use string short option used as boolean then result as if boolean', (t) => { + const passedArgs = ['-o']; + const stringOptions = { opt: { short: 'o', type: 'string' } }; + const booleanOptions = { opt: { short: 'o', type: 'boolean' } }; + + const stringConfigResult = parseArgs({ args: passedArgs, options: stringOptions, strict: false }); + const booleanConfigResult = parseArgs({ args: passedArgs, options: booleanOptions, strict: false }); + + t.deepEqual(stringConfigResult, booleanConfigResult); + t.end(); +}); + +test('when use string long option used as boolean then result as if boolean', (t) => { + const passedArgs = ['--opt']; + const stringOptions = { opt: { short: 'o', type: 'string' } }; + const booleanOptions = { opt: { short: 'o', type: 'boolean' } }; + + const stringConfigResult = parseArgs({ args: passedArgs, options: stringOptions, strict: false }); + const booleanConfigResult = parseArgs({ args: passedArgs, options: booleanOptions, strict: false }); + + t.deepEqual(stringConfigResult, booleanConfigResult); + t.end(); +}); + +test('when use boolean long option used as string then result as if string', (t) => { + const passedArgs = ['--bool=OOPS']; + const stringOptions = { bool: { type: 'string' } }; + const booleanOptions = { bool: { type: 'boolean' } }; + + const stringConfigResult = parseArgs({ args: passedArgs, options: stringOptions, strict: false }); + const booleanConfigResult = parseArgs({ args: passedArgs, options: booleanOptions, strict: false }); + + t.deepEqual(booleanConfigResult, stringConfigResult); + t.end(); +}); diff --git a/utils.js b/utils.js new file mode 100644 index 0000000..eca8711 --- /dev/null +++ b/utils.js @@ -0,0 +1,155 @@ +'use strict'; + +const { + ArrayPrototypeFind, + ObjectEntries, + StringPrototypeCharAt, + StringPrototypeIncludes, + StringPrototypeSlice, + StringPrototypeStartsWith, +} = require('./primordials'); + +// These are internal utilities to make the parsing logic easier to read, and +// add lots of detail for the curious. They are in a separate file to allow +// unit testing, although that is not essential (this could be rolled into +// main file and just tested implicitly via API). +// +// These routines are for internal use, not for export to client. + +/** + * Determines if the argument may be used as an option value. + * NB: We are choosing not to accept option-ish arguments. + * @example + * isOptionValue('V']) // returns true + * isOptionValue('-v') // returns false + * isOptionValue('--foo') // returns false + * isOptionValue(undefined) // returns false + */ +function isOptionValue(value) { + if (value == null) return false; + if (value === '-') return true; // e.g. representing stdin/stdout for file + + // Open Group Utility Conventions are that an option-argument + // is the argument after the option, and may start with a dash. + // However, we are currently rejecting these and prioritising the + // option-like appearance of the argument. Rejection allows more error + // detection for strict:true, but comes at the cost of rejecting intended + // values starting with a dash, especially negative numbers. + return !StringPrototypeStartsWith(value, '-'); +} + +/** + * 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 lone long option. + * @example + * isLoneLongOption('a') // returns false + * isLoneLongOption('-a') // returns false + * isLoneLongOption('--foo) // returns true + * isLoneLongOption('--foo=bar) // returns false + */ +function isLoneLongOption(arg) { + return arg.length > 2 && + StringPrototypeStartsWith(arg, '--') && + !StringPrototypeIncludes(StringPrototypeSlice(arg, 3), '='); +} + +/** + * Determines if `arg` is a long option and value in same argument. + * @example + * isLongOptionAndValue('--foo) // returns false + * isLongOptionAndValue('--foo=bar) // returns true + */ +function isLongOptionAndValue(arg) { + return arg.length > 2 && + StringPrototypeStartsWith(arg, '--') && + StringPrototypeIncludes(StringPrototypeSlice(arg, 3), '='); +} + +/** + * 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 + * // -fb is an option and a value, not a short option group + * isShortOptionGroup('-fb', { + * options: { f: { type: 'string' }} + * }) // returns false + * isShortOptionGroup('-bf', { + * options: { f: { type: 'string' }} + * }) // returns true + * // -bfb is an edge case, return true and caller sorts it out + * isShortOptionGroup('-bfb', { + * options: { f: { type: 'string' }} + * }) // 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 firstShort = StringPrototypeCharAt(arg, 1); + const longOption = findLongOptionForShort(firstShort, options); + return options[longOption]?.type !== 'string'; +} + +/** + * Determine is arg is a short string option followed by its value. + * @example + * isShortOptionAndValue('-a, {}); // returns false + * isShortOptionAndValue('-ab, {}); // returns false + * isShortOptionAndValue('-fFILE', { + * options: { foo: { short: 'f', type: 'string' }} + * }) // returns true + */ +function isShortOptionAndValue(arg, options) { + if (!options) throw new Error('Internal error, missing options argument'); + if (arg.length <= 2) return false; + if (StringPrototypeCharAt(arg, 0) !== '-') return false; + if (StringPrototypeCharAt(arg, 1) === '-') return false; + + const shortOption = StringPrototypeCharAt(arg, 1); + const longOption = findLongOptionForShort(shortOption, options); + return options[longOption]?.type === 'string'; +} + +/** + * Find the long option associated with a short option. Looks for a configured + * `short` and returns the short option itself if long option not found. + * @example + * findLongOptionForShort('a', {}) // returns 'a' + * findLongOptionForShort('b', { + * options: { bar: { short: 'b' }} + * }) // returns 'bar' + */ +function findLongOptionForShort(shortOption, options) { + if (!options) throw new Error('Internal error, missing options argument'); + const [longOption] = ArrayPrototypeFind( + ObjectEntries(options), + ([, optionConfig]) => optionConfig.short === shortOption + ) || []; + return longOption || shortOption; +} + +module.exports = { + findLongOptionForShort, + isLoneLongOption, + isLoneShortOption, + isLongOptionAndValue, + isOptionValue, + isShortOptionAndValue, + isShortOptionGroup +}; From 835b17ab04159dc0836ed7c7b995e2a7f176bdec Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 27 Mar 2022 18:46:09 +1300 Subject: [PATCH 6/6] chore(main): release 0.4.0 (#73) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- CHANGELOG.md | 13 +++++++++++++ package-lock.json | 2 +- package.json | 2 +- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 81831a3..0bd64c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,18 @@ # Changelog +## [0.4.0](https://github.com/pkgjs/parseargs/compare/v0.3.0...v0.4.0) (2022-03-12) + + +### ⚠ BREAKING CHANGES + +* parsing, revisit short option groups, add support for combined short and value (#75) +* restructure configuration to take options bag (#63) + +### Code Refactoring + +* parsing, revisit short option groups, add support for combined short and value ([#75](https://github.com/pkgjs/parseargs/issues/75)) ([a92600f](https://github.com/pkgjs/parseargs/commit/a92600fa6c214508ab1e016fa55879a314f541af)) +* restructure configuration to take options bag ([#63](https://github.com/pkgjs/parseargs/issues/63)) ([b412095](https://github.com/pkgjs/parseargs/commit/b4120957d90e809ee8b607b06e747d3e6a6b213e)) + ## [0.3.0](https://github.com/pkgjs/parseargs/compare/v0.2.0...v0.3.0) (2022-02-06) diff --git a/package-lock.json b/package-lock.json index 16427ce..e37f423 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "@pkgjs/parseargs", - "version": "0.3.0", + "version": "0.4.0", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index a54bb82..52d0da9 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@pkgjs/parseargs", - "version": "0.3.0", + "version": "0.4.0", "description": "Polyfill of future proposal for `util.parseArgs()`", "main": "index.js", "exports": {