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

Enable custom help description messages for -V and -h #870

Closed
wants to merge 1 commit into from
Closed

Enable custom help description messages for -V and -h #870

wants to merge 1 commit into from

Conversation

zentus
Copy link

@zentus zentus commented Sep 28, 2018

Enable custom help description messages for -V and -h

  • Add description argument to .version()

  • Add command .helpDescription()

For overriding default -h and -V flag help descriptions.

  • Capitalize default help descriptions
    -V: 'Output version number'
    -h: 'Output usage information'

  • Add example (help-description.js)

  • Add test (test.command.helpDescription)

  • Update Readme

* Add description argument to .version()

* Add command .helpDescription()

For overriding default -h and -V flag help descriptions.

* Capitalize default help descriptions
  -V: 'Output version number'
  -h: 'Output usage information'

* Add example (help-description.js)

* Add test (test.command.helpDescription)

* Update Readme
@haochuan9421
Copy link

Can't change -v and -h description is bad practice for localization. Waiting for this PR merged!

@shadowspawn
Copy link
Collaborator

Just making some notes.

There are requests to be able to change the flags for version and help, conflicts with user defined flags, change the description (like this PR), or the behaviour, or disable the help, et al. I am wondering if the best approach is to allow turning off either of the default version and help and leave it up to the user to handle it entirely themselves and get whatever behaviour they need. I have not worked through what that would look like to see if it is simple enough to expect people to do it.

@abetomo abetomo requested a review from shadowspawn April 26, 2019 06:29
Copy link
Collaborator

@shadowspawn shadowspawn left a comment

Choose a reason for hiding this comment

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

Thanks @zentus

  1. I like adding the description to the .version call in consistent way with .option itself and with optional parameters so no code changes required for existing behaviour. You have updated the JSDoc description. Are you able to update the TypeScript definition too, or would you like a hand with that?

  2. I do not like changing output ... to Output ..., simple as it might seem, and I expect you suggested it to match the other descriptions in the README! However, it has been that way for 7 years and IMHO the change is not compelling enough to justify the churn. In my own program, I used lowercase for all my option descriptions to match the built-in options provided by Commander and would be a little annoyed if they changed. And if people really want upper case, with your change they will be able to change some of it themselves...

  3. .helpDescription is a reasonable suggestion, but I do not want to add a routine just for the description. People want to be able to change the help flags or the description or suppress the behaviour entirely, so I think we need a more general solution than just the description. (For comparison, after your change .version will be optional, and allow custom flags, and allow custom description.)

@shadowspawn
Copy link
Collaborator

There is a new Pull Request covering same ground in #963.

Would you like to continue working on this @zentus, or shall I work with the new Pull Request? I'll wait a week for a reply.

@shadowspawn
Copy link
Collaborator

Adding reference to related issue: #47

@zentus
Copy link
Author

zentus commented May 17, 2019

@shadowspawn

I am not sure I understand what changes you wish me to make.

  1. No, unfortunately I have no knowledge of TypeScript. Should I keep the description argument to program.version() the way it is?

  2. So, change default descriptions back to lowercase?

  3. So I should remove the implementation of program.helpDescription() all together? Could you give an example of what a better way of changing the help description for the -h flag would look like?

@shadowspawn
Copy link
Collaborator

Thanks for reply @zentus.

The other Pull Request has leaped ahead, so I'll switch to working with it for now. So no action necessary here unless #963 does not work out.

FYI:

  1. Description argument is fine. The three forms of documentation we have for the API are the README (+ example), JSDoc, and TypeScript definition. Arguably a matching test is a fourth. Credit to you for adding README + example + test.
  2. In general with Pull Requests it is helpful to do just what is needed for what is being fixed. Avoid making other indentation changes or spelling changes etc, just fix the issue.
  3. 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.

Closing in meantime. Will reopen if needed.

@shadowspawn
Copy link
Collaborator

Thank you for your contributions.

@crystalfp
Copy link

Almost there.
Using .helpOption() I can localize the help text for "-h, --help". Now if only it was available a .versionOption() I could localize also the -V help.
This done, only the strings Usage: and Options: are missing for localization.
Now I'll take a look at the PR.
Thanks!
mario

@shadowspawn
Copy link
Collaborator

See .version() which supports modifying flags and description, like .helpOption() does.

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.

5 participants