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

Add support for multiple aliases #1236

Merged
merged 5 commits into from
Apr 21, 2020

Conversation

shadowspawn
Copy link
Collaborator

@shadowspawn shadowspawn commented Apr 2, 2020

Pull Request

See #531

Problem

Some people would like to add more than one alias for a command, but not have them all visible in the help.

Solution

Allow adding multiple aliases. Only display the first one in the built-in help, and leave any mention of the rest up to the caller.

This is intended to be backwards compatible. There is a small change from v5.0 behaviour, calling .alias() when there is no alias will now return undefined instead of null, but seems unlikely to affect much code and can be made fully backwards compatible if necessary.

To provide public access to the possibly multiple aliases, added .aliases() which works with array rather than string.

ChangeLog

  • add support for multiple aliases

@shadowspawn shadowspawn changed the base branch from master to develop April 2, 2020 06:23
@kodie
Copy link

kodie commented Apr 3, 2020

This is awesome!

typings/index.d.ts Outdated Show resolved Hide resolved
@shadowspawn
Copy link
Collaborator Author

Changes: added .aliases() mainly to give public access to full list of aliases (e.g. for code adding command-line completion).

index.js Show resolved Hide resolved
@shadowspawn
Copy link
Collaborator Author

I'll leave PR in draft for a few more days for any other feedback.

Thanks @kodie and @victornpb and @abetomo

@shadowspawn shadowspawn changed the title WIP: Add support for multiple aliases Add support for multiple aliases Apr 18, 2020
@shadowspawn shadowspawn marked this pull request as ready for review April 18, 2020 06:51
@shadowspawn
Copy link
Collaborator Author

No further feedback. Moving out of draft.

I have deliberately not added to README for now, hopefully anyone who wants the feature will be able to find it easily enough, and the JSDoc and TSDoc cover the support for multiple aliases.

Copy link
Collaborator

@abetomo abetomo left a comment

Choose a reason for hiding this comment

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

Awesome!

@shadowspawn shadowspawn added the pending release Merged into a branch for a future release, but not released yet label Apr 21, 2020
@shadowspawn shadowspawn merged commit 28e8d3f into tj:develop Apr 21, 2020
@shadowspawn shadowspawn deleted the feature/multiple-aliases branch April 21, 2020 06:27
@shadowspawn shadowspawn removed the pending release Merged into a branch for a future release, but not released yet label Apr 25, 2020
@shadowspawn
Copy link
Collaborator Author

Included in Commander v5.1 which has been released.

https://github.com/tj/commander.js/releases/tag/v5.1.0

@viceice
Copy link

viceice commented Apr 28, 2020

This is blocking us from upgrading, since we use aliasses as an option.

Wha'ts the best compatible way to solve this.

@shadowspawn
Copy link
Collaborator Author

Sorry for the clash @viceice

See the section in the README about avoiding name clashes, and have a look at storeOptionsAsProperties and passCommandToAction:

@viceice
Copy link

viceice commented Apr 28, 2020

@shadowspawn Ok, thanks for the info. Works with the suggested changes.

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