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 remove redundant flags property and store value true for boolean options #83

Merged
Merged
Show file tree
Hide file tree
Changes from 10 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
52 changes: 25 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,7 @@ 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
* `values` {Object}, key:value for each option found. Value is a string for string options, or `true` for boolean options, or an array (of strings or booleans) for options configured as `multiple:true`.
* `positionals` {string[]}, containing [Positionals][]

----
Expand All @@ -103,40 +102,37 @@ 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 { values, positionals } = parseArgs({ args, options });
// 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 { values, positionals } = parseArgs({ args, options });
// 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 { values, positionals } = parseArgs({ args, options });
// values = { f: true, foo: [ 'a', 'b' ] }
// positionals = []
```

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

Expand Down Expand Up @@ -189,26 +184,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}}`
Comment on lines +188 to +189
Copy link
Member

Choose a reason for hiding this comment

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

unrelated to this PR, but it would be nice to have parseArgs handle "what if i do --foo --no-foo" for me

Copy link
Collaborator

Choose a reason for hiding this comment

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

I note that's something you could trivially build on top of parseArgs if it reports indexes of arguments (#84).

- Is `--foo` the same as `--foo=true`? Only for known booleans? Only at the end?
- no, `--foo` is the same as `--foo=`
- no, they are not the same. There is no special handling of `true` as a value so it is just another string.
- 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']}`
- 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
10 changes: 3 additions & 7 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,23 +69,20 @@ function getMainArgs() {
function storeOptionValue(options, longOption, value, result) {
const optionConfig = options[longOption] || {};

// Flags
result.flags[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 +116,6 @@ const parseArgs = ({
);

const result = {
flags: {},
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 = { 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 = { values: { v: '-' }, positionals: [] };

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

Expand Down
Loading