Add a way to override help and version help message

[idmontie]

Close #47.

Add override for the help message by calling program.help(message).

Add override for the version message by calling
program.version(version, flag, message);

[shadowspawn]

Did you see #870, which does the same thing? I'll give them a week to make contact, otherwise I'll work with you.

Some of my feedback will be same as in #870 as it happens! I like the version changes, but not the help changes. In the longer term we might want something like .help(helpFlags, helpDescription, helpOptions) and I don't want partial solutions that will get in way of a more complete solution.

edit: pull request was updated to match this approach

If we go ahead, we'll want JSDoc and TypeScript updates for the .version function.

Thank you for your contributions.

[idmontie]

@shadowspawn I changed my code to support .help(helpFlags, helpDescription). I didn't add helpOptions since I don't know what those options would be or what they would do. But it should be easy to add them later.

I changed the hardcoded help and h references throughout the index and added some examples and a test to check that changing the flags works as intended.

I added typings as well.

Let me know if there are any other changes I should make.

[shadowspawn]

Oh, exciting! I was not expecting you to run with the full help customisation.

I gave .help as example not paying attention to existing call. I'll need to think if tidy enough to overload that, but does avoid adding a new routine which could conflict with existing client use, so there is an advantage to overload too.

I'll let you know if I have any more requests or questions.

[idmontie]

@shadowspawn Did you have time to think about whether overriding the .help function was the best way forward?

[shadowspawn]

I'm still ok with .help, so don't change code @idmontie.

The other names I came up with were .autoHelp and .helpOption, but I currently prefer the simplicity of .help. Comments welcome from any readers.

[shadowspawn]

(In top two of my PR queue. Hopefully take another look this week.)

[shadowspawn]

Sorry, lots of feedback! But looking pretty good.

I added a bunch of smaller minor suggested changes on lines.

The bigger one is ideally want subcommand to get the custom flags too. (I have not looked into how to sensibly do that, possibly checking parent when created?)

i.e. currently get mixture of old and new help flags when have a sub-command

program
   .help("-i, --ihelp", "foo foo") ;

program
  .command("child")
  .option("--gender", "specific gender of child")
  .action((cmd) => {
    console.log("Childsubcommand...");
  });

Get the new flags for global help (as expected), but default flags for subcommand help:

$ node index.js -i
Usage: index [options] [command]

Options:
  --global         A global option
  -i, --ihelp      foo foo

Commands:
  child [options]
Bagheera:963 john$ node index.js child
Childsubcommand...
Bagheera:963 john$ node index.js child -i
error: unknown option `-i'
Bagheera:963 john$ node index.js child -h
Usage: child [options]

Options:
  --gender    choose sex of child
  -h, --help  output usage information

[shadowspawn]

I looked briefly at the addImplicitHelpCommand and help foo behaviour but willing to leave that for future!

[shadowspawn]

(Feel free to ask if you have any questions about my comments, or what I was thinking.)

[shadowspawn]

I should say, I am planning to investigate pattern for inheritance of settings from "program" down into commands myself anyway for #951. So don't feel you have to solve that yourself, feel free to wait for a pattern for that part.

[idmontie]

Sorry for disappearing for a bit. I'm hoping to have have time this week to tackle your comments.

[shadowspawn]

No worries, thanks for update. I usually give people a month before taking other action. :-)

[shadowspawn]

Thanks for updates, I am excited to check them out. Might be a few days before I have enough time.

[shadowspawn]

LGTM

[shadowspawn]

Adds custom version description, custom help flags, custom help description. (Nice!)

This is technically a minor version as new features that should be backwards compatible, but changes enough that we might be better to be cautious and roll it into v3 @abetomo ?

[abetomo]

I think that it is all right with the v2 release.

[abetomo]

👍

[shadowspawn]

I am wondering again whether overloading .help might not be best approach and cause confusion in future, because the IDE autocompletion will be complicated with three ways of calling .help. In TypeScript we could improve this by having separate function overloads, but with JSDoc I don't think that is an option.

I came back to this because of a fresh report over confusion due to command description triggering git-style executable commands, reminding me that compact and "clever" can be too subtle and cause confusion for years!

The two names I had come up with previously were .autoHelp and .helpOption. Comments and alternatives welcome from any readers. .helpOption aligns with .option and flags and description, while autoHelp emphasizes that it is a bit special.

No code changes wanted at this stage @idmontie, but I am thinking again about what will be best in longterm.

[shadowspawn]

This is a strong candidate for v3 as has passed two code reviews. I am leaning towards naming the custom help routine something different rather than overloading though. If I decide I want to go with that, would you like to do the work @idmontie ? (I am willing to make the changes, in which case I would invite you to review it.)

[idmontie]

@shadowspawn I can do that change if you provide the function name 👍

[shadowspawn]

Thanks @idmontie. Settled on .helpOption. e.g.

program
   .helpOption('-c, --HELP', 'custom help message')

[shadowspawn]

Thanks @idmontie. I am going to start merging to release/v3 branch.

There are a couple of left-over bits in typings/index.d.ts from the earlier overload approach, and I will fix those.

Thank you for your contributions.

[shadowspawn]

Merged into v3 and will be included in that release. Closing to make it clear that should not be merged into master.

FYI Ivan, I did a squash during manual merging, fixed the typings, and made you the author of the commit.

Thank you for your contributions.

[shadowspawn]

Available now as a prerelease. See #1001

[shadowspawn]

Shipped in v3: https://github.com/tj/commander.js/releases/tag/v3.0.0