Skip to content

New spec#1

Merged
rauchg merged 3 commits intomasterfrom
new-spec
Sep 19, 2017
Merged

New spec#1
rauchg merged 3 commits intomasterfrom
new-spec

Conversation

@Qix-
Copy link
Contributor

@Qix- Qix- commented Sep 11, 2017

This is a refactor of the spec definitions as discussed internally.

Note the new usage: https://github.com/zeit/zarg/blob/e09f1d5b111dc55efd15d80783ca2f55f59205f2/README.md#usage

@Qix- Qix- self-assigned this Sep 11, 2017
@Qix- Qix- requested a review from rauchg September 11, 2017 09:13
@leo leo self-requested a review September 11, 2017 17:59
Copy link
Contributor

@leo leo left a comment

Choose a reason for hiding this comment

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

We don't need the dashes on the main object argument:

{
  '--version': Boolean,
  '-v': '--version'
}

Should become:

{
  version: Boolean,
  v: 'version'
}

As a result, we need to just test if the option name has a .length of 1 (short flag) or more (normal flag) - no dashes needed.

@Qix-
Copy link
Contributor Author

Qix- commented Sep 19, 2017

@leo Disagree after a bit of usage.

For example, when I needed to debug the --token problem in now-cli, searching for --token only showed the help text, and token by itself showed just about every file in the client.

While you're not necessarily wrong, a bit of explicitness is fine in this case.

@rauchg rauchg merged commit 1ec6eab into master Sep 19, 2017
@rauchg rauchg deleted the new-spec branch September 19, 2017 16:56
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.

3 participants