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

Handling arrays of aliases #131

Closed
benfletcher opened this issue Dec 10, 2019 · 3 comments
Closed

Handling arrays of aliases #131

benfletcher opened this issue Dec 10, 2019 · 3 comments

Comments

@benfletcher
Copy link

When the minimist-options package parses aliases, it was apparently designed to handle both a single string as well as an array of strings (it arrifys the alias and turns a single string into an array before processing). (This package was also written by @sindresorhus, but the original minimist handled aliases similarly.)

That is quite convenient if you want multiple aliases for a single option.

However, it creates some inconsistent behavior in the latest version with unnormalizedFlags. If you have more than one alias for a flag, those items (the flag and all aliases) get included in cli.flags as well as unnormalizedFlags. If there's only one alias (either a single element array or a plain string), the normal behavior of the alias being excluded from flags is observed.

The ability to pass in an array into aliases is admittedly an undocumented, if not unsupported, feature, but for me it's quite helpful. In particular it helps with backwards compatibility and gradual deprecation of flags that have been renamed. I can support both the old and new flag names for 1-2 versions while warning of the deprecation, without having to rewrite the underlying code.

The current behavior is not difficult to deal with, once known. I am seeking to raise awareness of the handling of alias arrays and propose that it become a documented and supported feature.

@benfletcher
Copy link
Author

I created a PR #132, in case arrays of aliases is desired functionality.

@sindresorhus
Copy link
Owner

For anyone that wants to work on this, see initial attempt and feedback here: #131

@tommy-mitchell
Copy link
Contributor

@sindresorhus I think this was handled with #226.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants