Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor!: rework results to have parsedOptions property and store true in values for boolean options #80

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 30 additions & 27 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@

[![Coverage][coverage-image]][coverage-url]

>
>
> 🚨 THIS REPO IS AN EARLY WIP -- DO NOT USE ... yet 🚨
>
>

Polyfill of future proposal to the [nodejs/tooling](https://github.com/nodejs/tooling) repo for `util.parseArgs()`

Expand Down Expand Up @@ -86,8 +86,8 @@ process.mainArgs = process.argv.slice(process._exec ? 1 : 2)
* `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
* `values` {Object}, have properties and `String` values corresponding to parsed options passed
* `foundOptions` {Object}, key:true for each option found in the input `args`
* `values` {Object}, key:value for each option found. Value is a string for string options, or `true` for boolean options, or an array for options configured as `multiple:true`.
shadowspawn marked this conversation as resolved.
Show resolved Hide resolved
* `positionals` {string[]}, containing [Positionals][]

----
Expand All @@ -103,40 +103,40 @@ const { parseArgs } = require('@pkgjs/parseargs');
const { parseArgs } = require('@pkgjs/parseargs');
const args = ['-f', '--foo=a', '--bar', 'b'];
const options = {};
const { flags, values, positionals } = parseArgs({ args, options });
// flags = { f: true, bar: true }
// values = { foo: 'a' }
const { foundOptions, values, positionals } = parseArgs({ args, options });
// foundOptions = { f: true, foo: true, bar: true }
// values = { f: true, foo: 'a', bar: true }
// positionals = ['b']
```

```js
const { parseArgs } = require('@pkgjs/parseargs');
// withValue
// type:string
const args = ['-f', '--foo=a', '--bar', 'b'];
const options = {
foo: {
bar: {
type: 'string',
},
};
const { flags, values, positionals } = parseArgs({ args, options });
// flags = { f: true }
// values = { foo: 'a', bar: 'b' }
const { foundOptions, values, positionals } = parseArgs({ args, options });
// foundOptions = { f: true, foo: true, bar: true }
// values = { f: true, foo: 'a', bar: 'b' }
// positionals = []
```

```js
const { parseArgs } = require('@pkgjs/parseargs');
// withValue & multiple
// type:string & multiple
const args = ['-f', '--foo=a', '--foo', 'b'];
const options = {
foo: {
type: 'string',
multiple: true,
},
};
const { flags, values, positionals } = parseArgs({ args, options });
// flags = { f: true }
// values = { foo: ['a', 'b'] }
const { foundOptions, values, positionals } = parseArgs({ args, options });
// foundOptions = { f: true, foo: true }
// values = { f: true, foo: [ 'a', 'b' ] }
// positionals = []
```

Expand All @@ -149,9 +149,9 @@ const options = {
short: 'f',
},
};
const { flags, values, positionals } = parseArgs({ args, options });
// flags = { foo: true }
// values = {}
const { foundOptions, values, positionals } = parseArgs({ args, options });
// foundOptions = { foo: true }
// values = { foo: true }
// positionals = ['b']
```

Expand Down Expand Up @@ -189,26 +189,29 @@ const { flags, values, positionals } = parseArgs({ args, options });
- `"0o22"`
- Does it coerce types?
- no
- Does `--no-foo` coerce to `--foo=false`? For all flags? Only boolean flags?
- no, it sets `{args:{'no-foo': true}}`
- Does `--no-foo` coerce to `--foo=false`? For all options? Only boolean options?
- no, it sets `{values:{'no-foo': true}}`
- Is `--foo` the same as `--foo=true`? Only for known booleans? Only at the end?
- no, `--foo` is the same as `--foo=`
- no, `--foo` is a boolean option and `--foo=true` is a string option
- Does it read environment variables? Ie, is `FOO=1 cmd` the same as `cmd --foo=1`?
- no
- Do unknown arguments raise an error? Are they parsed? Are they treated as positional arguments?
- no, they are parsed, not treated as positionals
- Does `--` signal the end of flags/options?
- **open question**
- If `--` signals the end, is `--` included as a positional? is `program -- foo` the same as `program foo`? Are both `{positionals:['foo']}`, or is the first one `{positionals:['--', 'foo']}`?
- Does `--` signal the end of options?
- yes
- Is `--` included as a positional?
- no
- Is `program -- foo` the same as `program foo`?
- yes, both store `{positionals:['foo']}`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems wrong; — means “stop parsing options”; everything after it should be untouched.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean.

The QA don't include an example storing an (otherwise) option as a positional, is that what you were expecting?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What i mean is, program a b c should have three positionals (a, b, and c) - but program -- a b c should only have one positional, the string "a b c", so it can be passed verbatim to something else.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, combining the strings like that would lose the information about the separate arguments and require another interpretation step to split them up again.

This might be too opaque, but imagine implementing subcommands:

// utility --option1 -- dispatch --option2
const result1 = parseArgs({ options: mainOptions );
if (result1.positionals[0] === 'dispatch') {
   const result2 = parseArgs({ args: result1.positionals.slice(1), options: dispatchOptions });
}

A different example. The -- turns off option processing and the remaining arguments end up untouched and treated as positionals, so these three examples all have the same result. There isn't anything special about the handling of arguments after the -- than before the -- other than no option processing.

utility foo bar -- --unprocessed-option
utility foo -- bar --unprocessed-option
utility -- foo bar --unprocessed-option

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's the idea. They're going to be passed to another program, so THAT program will be interpreting them on its own - perhaps with different rules than the current one.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally that other program will be interpreting an array of arguments, not a single string?

The -- stops option detection. It is not a marker for separate collection of arguments into another structure.

(There are some programs that treat -- as a marker for separate processing, but in my experience that is unusual. It is easy to form the impression programs work that way though, since for easy reading the -- is often put before the passthrough arguments!)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One way of thinking about -- is that it is another user-level intervention. No author action required.

  • suppress shell interpretation of special characters in arguments: quote argument
  • suppress utility interpretation of arguments as options: --

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example behaviours of reference implementations added to: #58 (comment)

- Does the API specify whether a `--` was present/relevant?
- no
- Is `-bar` the same as `--bar`?
- no, `-bar` is a short option or options, with expansion logic that follows the
[Utility Syntax Guidelines in POSIX.1-2017](https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html). `-bar` expands to `-b`, `-a`, `-r`.
- Is `---foo` the same as `--foo`?
- no
- the first flag would be parsed as `'-foo'`
- the second flag would be parsed as `'foo'`
- the first is a long option named `'-foo'`
- the second is a long option named `'foo'`
- Is `-` a positional? ie, `bash some-test.sh | tap -`
- yes

Expand Down
12 changes: 6 additions & 6 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,23 +69,23 @@ function getMainArgs() {
function storeOptionValue(options, longOption, value, result) {
const optionConfig = options[longOption] || {};

// Flags
result.flags[longOption] = true;
// foundOptions
result.foundOptions[longOption] = true;

// Values
const usedAsFlag = value === undefined;
const newValue = usedAsFlag ? true : value;
if (optionConfig.multiple) {
// Always store value in array, including for flags.
// 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[longOption] !== undefined)
ArrayPrototypePush(result.values[longOption], newValue);
else
result.values[longOption] = [newValue];
} else {
result.values[longOption] = value;
result.values[longOption] = newValue;
}
}

Expand Down Expand Up @@ -119,7 +119,7 @@ const parseArgs = ({
);

const result = {
flags: {},
foundOptions: {},
values: {},
positionals: []
};
Expand Down
4 changes: 2 additions & 2 deletions test/dash.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const { parseArgs } = require('../index.js');

test("dash: when args include '-' used as positional then result has '-' in positionals", (t) => {
const passedArgs = ['-'];
const expected = { flags: {}, values: {}, positionals: ['-'] };
const expected = { foundOptions: {}, values: {}, positionals: ['-'] };

const result = parseArgs({ args: passedArgs });

Expand All @@ -25,7 +25,7 @@ test("dash: when args include '-' used as positional then result has '-' in posi
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 expected = { foundOptions: { v: true }, values: { v: '-' }, positionals: [] };

const result = parseArgs({ args: passedArgs, options: passedOptions });

Expand Down
48 changes: 48 additions & 0 deletions test/examples.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
'use strict';
/* eslint max-len: 0 */
const { parseArgs } = require('../index.js');

// Examples from README for checking got them right!

function show(args, options) {
console.log('args = %O', args);
console.log('options = %O', options);
console.log('---');
const result = parseArgs({ args, options });
console.log('foundOptions = %O', result.foundOptions);
console.log('values = %O', result.values);
console.log('positionals = %O', result.positionals);
console.log('-------------------------------------------\n');
}

let args;
let options;

args = ['-f', '--foo=a', '--bar', 'b'];
options = {};
show(args, options);

args = ['-f', '--foo=a', '--bar', 'b'];
options = {
bar: {
type: 'string',
},
};
show(args, options);

args = ['-f', '--foo=a', '--foo', 'b'];
options = {
foo: {
type: 'string',
multiple: true,
},
};
show(args, options);

args = ['-f', 'b'];
options = {
foo: {
short: 'f',
},
};
show(args, options);
Loading