Skip to content

Update args from 3.0.8 to 5.0.0#131

Merged
joshbetz merged 3 commits intomasterfrom
update/args-5.0.0
Jun 27, 2018
Merged

Update args from 3.0.8 to 5.0.0#131
joshbetz merged 3 commits intomasterfrom
update/args-5.0.0

Conversation

@joshbetz
Copy link
Copy Markdown
Contributor

@joshbetz
Copy link
Copy Markdown
Contributor Author

Needs testing

joshbetz added 3 commits June 22, 2018 09:35
There's now a `version` subcommand that's always defined in addition to
`help`. We only want to showHelp() if those are the only two defined
subcommands.
The commands name `help` and `version` are defined by the args library.
Here we get a list of commands that doesn't include those. If there are
other subcommands (and we haven't specified one), show a list of them.
@joshbetz joshbetz force-pushed the update/args-5.0.0 branch from e1dc40b to bfba901 Compare June 22, 2018 14:54
@joshbetz
Copy link
Copy Markdown
Contributor Author

The most obvious advantages for us are the updated dependencies and the more compact layout:

Before:

$ vip app

  Usage: vip app [options] [command]

  Commands:

    help  Display help
    list  List your VIP Go apps

  Options:

    -f, --format [value]  Format results (defaults to "table")
    -h, --help            Output usage information
    -v, --version         Output the version number

  Examples:

    - Pass an app name or ID to get details about that app

    $ vip app <app>


    - Get details about the app with ID 123

    $ vip app 123


    - Get details about the app named vip-test

    $ vip app vip-test

After:

$ vip app
  Usage: vip app [options] [command]

  Commands:
    help     Display help
    list     List your VIP Go apps
    version  Display version

  Options:
    -f, --format [value]  Format results (defaults to "table")
    -h, --help            Output usage information
    -v, --version         Output the version number

  Examples:
    - Pass an app name or ID to get details about that app
    $ vip app <app>

    - Get details about the app with ID 123
    $ vip app 123

    - Get details about the app named vip-test
    $ vip app vip-test

@joshbetz joshbetz merged commit 4980b41 into master Jun 27, 2018
@joshbetz joshbetz deleted the update/args-5.0.0 branch June 27, 2018 15:53
Copy link
Copy Markdown
Contributor

@chrean chrean left a comment

Choose a reason for hiding this comment

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

Looks good, I have only a minor question.


// Show help if no args passed
if ( this.details.commands.length > 1 && ! this.sub.length ) {
if ( !! customCommands.length && ! this.sub.length ) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why are there two different bang policies here, both on a .length params?

Copy link
Copy Markdown
Contributor Author

@joshbetz joshbetz Jul 5, 2018

Choose a reason for hiding this comment

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

It's a common Javascript way to cast something to a Boolean. The first casts the value to a Boolean but inverts it so the second one is needed to invert it back.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I know, I just wanted to know why !! customCommands.length has 2 bangs while ! this.sub.length has only one, thought they were similar.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We're checking to see if we have custom commands defined (!! customCommands.length; looking for true) and not passing any args (! this.sub.length; looking for false).

I think for clarity, this would be better written as:

if ( customCommands.length > 1
    && this.sub.length === 0 ) {

or take it a step further:

const haveCustomCommands = customCommands.length > 1;
const runningSubcommand = this.sub.length === 0;

if ( haveCustomCommands && ! runningSubcommand ) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants