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

fix help option text for user-defined -h short flags #658

Closed
wants to merge 1 commit into from

Conversation

TDress
Copy link

@TDress TDress commented Jul 10, 2017

This fixes issue #645
if there is a user-defined -h option, the text displayed when running --help should not list a -h short flag for both the help and user-defined options.

index.js Outdated
* @api private
*/
Command.prototype.optionHelp = function() {
// check if there is an overriding user-defined `-h` short flag
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you modify the tab width to match other codes?

Copy link
Author

Choose a reason for hiding this comment

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

my bad!
See amended commit.

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@abetomo abetomo requested a review from roman-vanesyan July 11, 2017 03:05
@roman-vanesyan
Copy link
Collaborator

As far as I understand the pr, it introduce only overrides of short option for help (e.g. -h), but --help is still non overredable?

@TDress
Copy link
Author

TDress commented Jul 11, 2017

@vanesyan, correct. On master, before PR, a custom short option -h overrides the help short option, but the help information text was still showing -h as an option for the help command. The PR doesn't touch the functionality of the options, and --help is still not overridable.

@roman-vanesyan
Copy link
Collaborator

BTW, I think it's breaking change, and we should come back here, when the next major release happen.

@TDress
Copy link
Author

TDress commented Jul 12, 2017 via email

@roman-vanesyan
Copy link
Collaborator

Yes, I will addressing this issue when we come to 3.0. Thank you :)

@HRK44
Copy link

HRK44 commented Feb 21, 2018

How is this a breaking change?
It just modifies an output to remove '-h' from the built-in help option when it is already used by an user-defined option.

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.

Are you still interested in more work on this @TDress?

I have some changes I would like made. Checking whether you are still engaged before I go further.

(One reminder to self, user can actually override --help as well as -h.)

No problem if you are no longer interested, I'll find somewhere to reference this work for future use by someone else.

Thank you for your contributions.

@shadowspawn
Copy link
Collaborator

I had a look into this and don't think it is much risk of it being a breaking change @vanesyan since it just affects the display of the built-in help line in the built-in help and does not change the runtime behaviour otherwise. Add a comment if you have a usage case in mind which would break. Almost every change in a widely used program may break someone @HRK44, but I am trying not to be too paranoid! 😨

@TDress
Copy link
Author

TDress commented Apr 26, 2019 via email

@shadowspawn
Copy link
Collaborator

Thanks @TDress
I have explicitly referenced this pull request in #645 and closing this Pull Request for now.

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.

5 participants