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

feat: add parsed meta-data to returned properties #129

Merged
merged 69 commits into from
Jul 20, 2022

Conversation

shadowspawn
Copy link
Collaborator

@shadowspawn shadowspawn commented May 29, 2022

Problem

We don't wish to offer extensive configuration knobs and dials for modifying parseArgs behaviour, but we don't currently return rich information for authors to implement features themselves.

See #84.

Solution

Return a property which contains an array of the parsed tokens detected in the args. This allows the author to inspect the meta-data to add additional checks to throw for repeated options (say), or reprocess the tokens entirely to add auto-arrays for repeated options (say).

The internal implementation uses the tokens to do the error checking and generate the values and positionals. "Eating our own dog food."

The extra details are opt-in and not returned by default.

Closes: #84

This includes some minor lint in the existing (upstream) documentation, including one which was independently fixed:

Example

const { parseArgs } = require('@pkgjs/parseargs');
console.log(parseArgs(
    { tokens: true, strict: false, 
      options: { port: { short: 'p', type: 'string'} } }
));
% node index.js -p80 --foo run
{
  values: [Object: null prototype] { port: '80', foo: true },
  positionals: [ 'run' ],
  tokens: [
    {
      kind: 'option',
      name: 'port',
      rawName: '-p',
      index: 0,
      value: '80',
      inlineValue: true
    },
    {
      kind: 'option',
      name: 'foo',
      rawName: '--foo',
      index: 1,
      value: undefined,
      inlineValue: undefined
    },
    { kind: 'positional', index: 2, value: 'run' }
  ]
}

Examples Folder Experimental Only

I am not planning to commit the examples other than tokens.js in this PR. I'll submit them in separate PRs. I'll leave them in the PR until last minute as examples of using the tokens for custom processing.

index.js Outdated Show resolved Hide resolved
@bakkot

This comment was marked as outdated.

@shadowspawn

This comment was marked as outdated.

@shadowspawn

This comment was marked as outdated.

@shadowspawn

This comment was marked as outdated.

@shadowspawn shadowspawn changed the title Add parseElements to returned properties Add parsed meta-data to returned properties May 30, 2022
index.js Outdated Show resolved Hide resolved
@bcoe

This comment was marked as outdated.

@shadowspawn

This comment was marked as outdated.

@bcoe

This comment was marked as resolved.

@bakkot bakkot mentioned this pull request Jul 4, 2022
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
examples/negate.js Outdated Show resolved Hide resolved
* `kind` {string} One of 'option', 'positional', or 'option-terminator'.
* `index` {number} Index of element in `args` containing token. So the
source argument for a token is `args[token.index]`.
* option tokens
Copy link
Collaborator

Choose a reason for hiding this comment

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

To improve the readability I would explain that we mean the kind=option token

Suggested change
* option tokens
* 'option' tokens

Copy link
Member

Choose a reason for hiding this comment

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

rather than quotes, which i find confusing, how about linking the term to the appropriate section?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, that works too.

I use the same syntax in the kind description to be homogeneous

Copy link
Collaborator Author

@shadowspawn shadowspawn Jul 16, 2022

Choose a reason for hiding this comment

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

The compact POJO description is a bit subtle to read. How about an expanded version with all the properties listed?

Proposed expanded

A returned token has two properties which are always defined,
and some other properties which vary depending on the kind:

  • kind {string} One of 'option', 'positional', or 'option-terminator'.
  • index {number} Index of element in args containing token. So the
    source argument for a token is args[token.index].

An option token has additional parse details
for an option detected in the input args:

  • kind = 'option'
  • index {number} Index of element in args containing token.
  • name {string} Long name of option.
  • rawName {string} How option used in args, like -f of --foo.
  • value {string | undefined} Option value specified in args.
    Undefined for boolean options.
  • inlineValue {boolean | undefined} Whether option value specified inline,
    like --foo=bar.

A positional token has just one additional property with the positional value:

  • kind = 'positional'
  • index {number} Index of element in args containing token.
  • value {string} The value of the positional argument in args (i.e. args[index]).

An option-terminator token has only the base properties:

  • kind = 'option-terminator'
  • index {number} Index of element in args containing token.

Old compact

The returned tokens have properties describing:

  • all tokens
    • kind {string} One of 'option', 'positional', or 'option-terminator'.
    • index {number} Index of element in args containing token. So the
      source argument for a token is args[token.index].
  • option tokens
    • name {string} Long name of option.
    • rawName {string} How option used in args, like -f of --foo.
    • value {string | undefined} Option value specified in args.
      Undefined for boolean options.
    • inlineValue {boolean | undefined} Whether option value specified inline,
      like --foo=bar.
  • positional tokens
    • value {string} The value of the positional argument in args (i.e. args[index]).
  • option-terminator token

(Also asked in: nodejs/node#43459 (comment))

Undefined for boolean options.
* `inlineValue` {boolean | undefined} Whether option value specified inline,
like `--foo=bar`.
* positional tokens
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* positional tokens
* 'positional' tokens

like `--foo=bar`.
* positional tokens
* `value` {string} The value of the positional argument in args (i.e. `args[index]`).
* option-terminator token
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* option-terminator token
* 'option-terminator' token

Copy link
Collaborator

@bcoe bcoe left a comment

Choose a reason for hiding this comment

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

👍 assuming this has been synced with the upstream Node.js PR.

Congrats on the contribution to Node 🥳

@shadowspawn shadowspawn changed the title Add parsed meta-data to returned properties feat: add parsed meta-data to returned properties Jul 19, 2022
@bcoe
Copy link
Collaborator

bcoe commented Jul 19, 2022

@shadowspawn may I merge this?

@shadowspawn
Copy link
Collaborator Author

Yes, it is up to date with upstream. (I'll get to it this weekend otherwise.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reporting indexes
6 participants