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: .usage() can now be used to configure a default command #975

Merged
merged 13 commits into from Oct 18, 2017

Conversation

bcoe
Copy link
Member

@bcoe bcoe commented Oct 14, 2017

This pull allows you to configure the default command (your application's entry point) in the .usage() method -- combining the usage and command APIs in a way that I feel is more intuitive.

const argv = require('./')
  .usage('$0 <port> [batman]', 'start the server', (yargs) => {
    yargs.positional('port', {
      describe: 'provide a port for your application',
      type: 'number'
    })
  }).argv

Along with introducing this standardization, there have been a few cosmetic changes in an effort to make default commands (and commands in general) more intuitive:

  • we now print the positional options accepted by the default command in the top-level help output.
  • we now display information about the default command in help output, regardless of whether it has aliases.
  • we now default to displaying usage instructions for the entry-point to your application, which demonstrate how to invoke commands.

BREAKING CHANGE: .usage() no longer accepts an options object as the second argument

would love feedback, CC: @matatk, @jeffijoe

@bcoe bcoe requested a review from Morishiri October 14, 2017 22:53
@bcoe
Copy link
Member Author

bcoe commented Oct 14, 2017

CC: @laggingreflex

@bcoe
Copy link
Member Author

bcoe commented Oct 14, 2017

CC: @iarna, @zkat, I think the command/positional argument overhaul is getting pretty close to where I was thinking it should be; would love your feedback.

@matatk
Copy link

matatk commented Oct 15, 2017

I haven't tried this code (yet), though in API it seems to match the example you gave yesterday, without the requirement to include the word 'command'. I think that, for new users, who are just wanting to make a simple single command that accepts positional arguments, this looks pretty-much ideal :-).

@lostintangent
Copy link
Contributor

Yep, this looks like exactly what I’d love for one of my single command CLIs :)

@bcoe
Copy link
Member Author

bcoe commented Oct 15, 2017

@lostintangent @matatk if you want to take it for a spin, and give me some feedback before it goes live:

npm i yargs@next

@lostintangent
Copy link
Contributor

lostintangent commented Oct 15, 2017

@bcoe Just pulled down the next bits, and it works great! Thanks so much. That said, I had been previously using the usage command to override the "generic" CLI-level description text (e.g. kyte [filePath] [options]), and now that I'm using usage to define my default command (which requires the $0 syntax, I notice the following help text output:

screen shot 2017-10-15 at 1 42 25 pm

This is probably a dumb question, but it there a way to replace the highlighted text with kyte (the name of my CLI in this case)? I tried using alias and calling the usage command again, but neither seemed to work.

It would be awesome if the $0 variable were automatically replaced at display time with the name of the CLI itself.

@lostintangent
Copy link
Contributor

lostintangent commented Oct 15, 2017

Also, I noticed that if I define some additional global settings (e.g. aliases for the help arg), within the command builder (as opposed to chaining them off of the result of calling usage, then the help output is subtlety different (e.g. the Commands section is no longer displayed, and the usage text displays the CLI script's file path twice).

screen shot 2017-10-15 at 1 59 33 pm

For optimizing the syntax for single-command CLIs, it would be awesome to be able to write the following:

const { filePath } = require("yargs")
  .usage(
    "$0 [filePath] [options]",
    "Start a new co-editing editing session",
    yargs => {
      yargs
        .positional("filePath", {
          describe: "The file path to begin collaborating on"
        })
        .alias({ h: "help", v: "version" })
        .strict()
        .fail(exit);
    }
  )
  .parse();

Functionally, this seems to actually work beautifully. It's just that the help output isn't the same as when writing it this way:

const { filePath } = require("yargs")
  .usage(
    "$0 [filePath] [options]",
    "Start a new co-editing editing session",
    yargs => {
      yargs.positional("filePath", {
        describe: "The file path to begin collaborating on"
      });
    }
  )
  .fail(exit)
  .strict()
  .alias({ h: "help", v: "version" });

I'm not entirely sure whether this method is actually "good" or not, and I'm happy with writing the code either way. I just thought I'd mention it since the CLI seems to be functionally equivalent in both scenarios, but the help text is different (and therefore may be a bug?).

@lostintangent
Copy link
Contributor

lostintangent commented Oct 16, 2017

@bcoe I noticed that the file path behavior is completely unrelated to this PR, though I wasn't experiencing it previously since my usage of usage included explicitly setting the displayed string (e.g. kyte [filePath] [options]), whereas this PR requires the use of the $0 prefix.

I opened up #983 as one potential "solution" to allowing the use of $0 in usage, without sacrificing the "cleanliness" of your help text displaying the CLI's name instead of the file path. That may or may not be the right way to address this scenario though, so I just wanted to convey my scenario and open the discussion. Thanks so much!

That said, the fact that the string literal $0 is displayed in the Commands: section does seem like a bug with this change? It seems like that token needs to be replaced just like it already being done for the usage, epilog and example methods.

@bcoe
Copy link
Member Author

bcoe commented Oct 16, 2017

regarding $0 behavior:

@lostintangent my home was that the string literal $0 might be a good indicator to people that this is the behavior that will run if you run the application with no commands. Have any thoughts about what might be more intuitive? I don't want to replace it with the bin name, because this would read as though there's a command of this name.

regarding difference in output top-level vs. nested configuration:

Testing, I think the problem might just be that you're missing .argv or .parse() on the second example?

const { filePath } = require("./")
  .usage(
    "$0 [filePath] [options]",
    "Start a new co-editing editing session",
    yargs => {
      yargs.positional("filePath", {
        describe: "The file path to begin collaborating on"
      });
    }
  )
  .fail(exit)
  .strict()
  .alias({ h: "help", v: "version" })
  .parse()

I seem to have the same output (mind confirming?).

@lostintangent
Copy link
Contributor

@bcoe Yeah that's a good point regarding displaying $0 in the Commands: section. My initial thinking was that it would be more intuitive to display the bin name instead, but I also agree that might be strange. I'm just not sure how immediately obvious it would be to end-users when they saw $0 displayed in the help text. I'll give it some more thought, but in the meantime, the current behavior seems cool to me.

Regarding the other issue, I actually was calling parse. What do you see if you run this?

const { filePath } = require("./")
  .usage(
    "$0 [filePath] [options]",
    "Start a new co-editing editing session",
    yargs => {
      yargs.positional("filePath", {
        describe: "The file path to begin collaborating on"
      })
     .fail(exit)
     .strict()
     .alias({ h: "help", v: "version" })
    }
  )
  .parse()

In my case, the Commands: section disappears, and the value of $0 is displayed twice in the "usage" text at the top of the help output. Functionally, everything else works as expected though.

@iarna
Copy link
Contributor

iarna commented Oct 16, 2017

So, if I were a consumer of a binary written with yargs and I saw $0 I would assume it was a bug. Giving the full path to the script isn't great either ideally one would give the command they typed (but I'm not sure that's actually possible in Node.js)

@bcoe
Copy link
Member Author

bcoe commented Oct 16, 2017

@iarna @lostintangent does it feel less weird to replace it with . -- I'd like to allow people to provide a description for the default command that runs, even when there's no alias provided ... I hear that $0 feels a bit weird:

Commands:
  . [filePath] [options] start a new co-editing session/

@bcoe
Copy link
Member Author

bcoe commented Oct 16, 2017

@lostintangent regarding the behavior around .alias(), I see what's happening now. Because there's no top-level -h defined, help isn't fired until it reaches the context of the default command. so, you're seeing the help for the default command itself, rather than the top-level help which shows a listing of all the available commands, you're seeing this?

test.js test.js [filePath] [options]

Positionals:
  filePath  The file path to begin collaborating on

Options:
  -h, --help     Show help                                             [boolean]
  -v, --version  Show version number                                   [boolean]

rather than this?

test.js [filePath] [options]

Commands:
  $0 [filePath] [options]  Start a new co-editing editing session      [default]

Positionals:
  filePath  The file path to begin collaborating on

Options:
  -h, --help     Show help                                             [boolean]
  -v, --version  Show version number                                   [boolean]

I don't think this behavior is a bug per-se (or it's at least expected). Basically, we needed to make some usability decisions around how to display help for all commands, vs. help for a default command ($0). It's hard to get this perfect but it's my hope we've reached an okay compromise with the behavior:

  • if help is configured on the top-level yargs builder:
  • the positional and option arguments for the default command ($0) will be shown.
  • all other available commands will also be shown.
  • if help is run for a specific command (a command other than $0):
    • the positional and option arguments will be shown.
    • any nested commands that are available will also be shown.

I wonder, is there anywhere in the docs we could call this out?

@lostintangent
Copy link
Contributor

lostintangent commented Oct 16, 2017

@bcoe Ah OK, the behavior makes sense now that I think about it. Thanks for explaining that! I was a little confused initially since I have a CLI that is literally just a single command. Therefore, I wasn’t thinking of there being a difference between the top-level help and the command-level help. In fact, in the single-command CLI, maybe the “buggy” behavior is in fact my expected behavior, since you don’t neccesssarily need the top-level help (which then coincidentally gets around the “$0” conversation as well).

Maybe we could just add a quick mention to the new “usage” method docs, that clarifies that the builder function is configuring the default command’s help output, not the top-level CLI’s help output? That would have been helpful in my case. Additionally, there may be some bugs in the default command’s help output, such as the script/bin name being displayed twice in the usage text (see screenshot above)?

Regarding the “.” vs. “$0” display: I definitely think “.” is less strange looking, but I’m still not 100% sure. Is there any prior art we could take inspiration from here? When I look at Zeit’s now CLI (for example), their help output doesn’t display a special token for the default command since they also alias it as “deploy” and can just annotate it as also being the default (that’s nice and clean). Though their docs display the default command as “[]”, which I think I like better than “.” (since the brackets represent optional arcs, and we’re not trying to overload the dot)?

@bcoe
Copy link
Member Author

bcoe commented Oct 16, 2017

@lostintangent if $0 was aliased to server you would get the same output you described:

server run the server [default]

I'm wondering if perhaps:

  1. we should switch to the . from $0, if more than one command is configured.
  2. if only one command is configured, and it's default, always show help for that command specifically rather than help for the top-level (i.e., don't show help for the top-level unless more than one command is configured).

How are you linking the kyte bin when you're seeing $0 printed twice? are you running npm i -g in the kyte folder? would definitely like to get this bug fixed.

@lostintangent
Copy link
Contributor

I like that proposal! For #1, would the “.” even be displayed if you added an alias for the default command? If not, then this behavior would seem to accommodate most scenarios really well.

I’m using “yarn link”, so this totally might be a dev-only issue. I’ll check an end-user install when I get home and will report back if it still repros.

@bcoe
Copy link
Member Author

bcoe commented Oct 17, 2017

@lostintangent figured out why $0 was printing out twice for you:

91620ab

now I'm working on getting fixing how we display command information; my coworkers gave me a prety good suggestion today, which was that we display the command like so:

Commands:
my-app.js [foo] [bar]
my-app.js start [port]
my-app.js restart [arg]
```

I think this works great, but we should land your changes that use `basename` rather than the full-path of the bin, so that we don't end up with stupidly long help messages.

@lostintangent
Copy link
Contributor

@bcoe Awesome! That sounds good.

@lostintangent
Copy link
Contributor

@bcoe I tried out the latest changes, and can confirm that the double script name display issue is resolved! However, I noticed that the "usage description" in the command-level help now only seems to display the bin name name (e..g kyte as opposed to kyte [filePath] [options]). Is that the expected behavior now?

screen shot 2017-10-16 at 10 24 29 pm

If I view the top-level help, the usage text is displayed correctly (note that my basename change doesn't currently apply to the Commands: section 😄):

screen shot 2017-10-16 at 10 32 09 pm

@bcoe
Copy link
Member Author

bcoe commented Oct 17, 2017

@lostintangent whoops, definitely had a somewhat screwed up release/merge with your work there.

I think I've fixed the bugs in question, I've also modified things so that if there's only a default command configured, you'll see the help for this command rather than the top-level help.

Thanks for all your help testing this new feature.

@wKovacs64
Copy link
Contributor

wKovacs64 commented Oct 17, 2017

@lostintangent Have you tried 10.0.0-alpha.3? My CLI appears to be configured like kyte and it outputs just like the example above:

my-app.js [foo] [bar]
my-app.js start [port]
my-app.js restart [arg]

Do we want the .js part? Users are typing my-app, not my-app.js right?

@bcoe
Copy link
Member Author

bcoe commented Oct 17, 2017

@wKovacs64 as long as your application is linked, rather than being run locally, I think you'll find that the .js goes away. So, in your module, add the following to your package.json:

{
  "bin": "./my-app.js"
}

Then run npm install -g to link the bin globally.

Did you like where I put the command description in the help output? I thought that having it to the left of the command seemed appropriate, and matched git's help.

@wKovacs64
Copy link
Contributor

wKovacs64 commented Oct 17, 2017

@bcoe That's what I was testing, my package.json looks just like that and I've linked it globally. The output includes the .js.

Edit: Oh. It's a Windows thing. Looks fine on macOS.

@bcoe
Copy link
Member Author

bcoe commented Oct 18, 2017

@wKovacs64 @lostintangent @iarna it took another couple days of coding, but I think I've pulled together all your suggestions into a new usage format that I'm happy with:

when multiple commands are configured:

screen shot 2017-10-17 at 9 15 44 pm

when a single root-level command is configured:

screen shot 2017-10-17 at 9 16 02 pm

You can try things out by running npm i yargs@next, unless there are any major issues I think I'm going to land this once tests pass.

@iarna
Copy link
Contributor

iarna commented Oct 18, 2017

I don't see the .js with a linked version, FWIW. That being said, there's probably no winning on Windows (where the thing the user runs is a .cmd file which then calls node on the .js file)

@wKovacs64
Copy link
Contributor

@iarna Odd you don't see it, I was able to reproduce it on multiple systems. Meh.

@bcoe :shipit:

@bcoe bcoe merged commit 7269531 into master Oct 18, 2017
@bcoe bcoe deleted the usage-command branch October 18, 2017 06:08
@lostintangent
Copy link
Contributor

Looks awesome! Thanks so much @bcoe.

@iarna
Copy link
Contributor

iarna commented Oct 18, 2017

@wKovacs64 This is the repro of it working for me:

https://gist.github.com/iarna/65a1f94660ad8240519698062e2faf88

You can npm install -g https://gist.github.com/iarna/65a1f94660ad8240519698062e2faf88 and run x or you can just do npx https://gist.github.com/iarna/65a1f94660ad8240519698062e2faf88 and it'll install it and run it.

@matatk
Copy link

matatk commented Oct 18, 2017

@bcoe the new output formats look great; thanks. It seems to me that, for a CLI with no (sub-)commands, the line "the default command that is run" seems unnecessary, and possibly confusing, as there aren no commands. Other than that, this is exactly what I was hoping for; thanks!

@wKovacs64
Copy link
Contributor

@iarna Your repro example outputs x.js on my machines when run globally, which is great because that means it isn't my program. Also a bummer as that means it's something weird on all my Windows machines. Oh well. :) Thanks for the sanity check.

@iarna
Copy link
Contributor

iarna commented Oct 19, 2017

@wKovacs64 I would expect x.js on Windows. It's everywhere else it should just be x. Windows doesn't get symlinks, it get's little .cmd scripts that run node foo.js.

@iarna
Copy link
Contributor

iarna commented Oct 19, 2017

@matatk "the default command that is run" is just the description of the command as provided by the developer. If you don't want anything there, you don't provide a description.

@wKovacs64
Copy link
Contributor

@iarna Ah, okay. When you said you didn't see the .js, I assumed you meant on Windows since you'd posted after I updated my comment to indicate it was Windows-only.

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.

None yet

6 participants