Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
refactor!: restructure configuration to take options bag #63
refactor!: restructure configuration to take options bag #63
Changes from 12 commits
697493f
9e42db0
36ef68c
67c0b03
082bec5
0909008
042480b
e44c46c
4063751
a50e940
062bdc9
28ae7b8
4436ef6
dc58a4a
14866a3
b0b71c4
92811f4
0d5a35e
ecf1fb0
e0d0c33
50d4f4d
b41fd9b
f533a98
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A quirk of the current implementation is that a one character long option will be recognised as both a long option (e.g.
--f
) and a short option (e.g.-f
). This is an existing quirk, and not introduced by this PR.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see no issue with this unless we think there are cases where authors do NOT want to allow shorts for a given option. Is this a scenario you've encountered maintaining Commander? Or foresee other problems with this quirk?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commander started out with dual short and long option names (needed to define both). Support for long-only options added in 2011. Support for short-only options added in 2020 (and more for completeness than because of demand).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for..of is never robust, because of Symbol.iterator; this should use
ArrayPrototypeForEach(ObjectEntries(options), ([option, optionConfig]) => {
instead.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! ecf1fb0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌 ecf1fb0