-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Use createOption()
in helpOption()
to support custom option flag extraction (+ various improvements)
#1929
Conversation
(cherry picked from commit 87db4ba)
createOption()
in helpOption()
to support custom option flag extractioncreateOption()
in helpOption()
to support custom option flag extraction (+ various improvements)
Eliminates the need for the check in other places. (cherry picked from commit a12f5be)
I noticed the tests now added in 7335a9c were missing because #1934 did not fail despite not handling obscured help flags correctly. I am sorry for making this PR a mess, this one should've only included the changes described in the Solution section, and there should've been a different PR based off this one with all the other improvements for #1931, #1934, #1935 to build upon. Unfortunately, it is too late to change this now because too much depends on this PR. |
fad505b
to
513a4b5
Compare
Using a help option rather than tracking help properties separately is something I am somewhat interested in, but more to see than to action for now. So may be referring back to this in future. |
Cherry-picked from #1926 for better separation of concerns.
Serves as the base for other PRs dealing with help options (#1931, #1934, #1935) and includes various improvements relevant for all of them. For reasoning behind those improvements, see the commit descriptions.
Has been merged with the already approved tiny fixes PR #1930 because a change introduced there would cause an obscure merge conflict if merged with this PR later.
Problem
Unlike
version()
using anOption
instance returned fromcreateOption()
to calculate the version option's names,helpOption()
does not callcreateOption()
to calculate the help option's short and long flag and instead simply uses thesplitOptionFlags()
helper function.Solution
Call
createOption()
fromhelpOption()
and store the returnedOption
instance in a_helpOption
property of theCommand
instance.Do not store the individual flags in the
_helpShortFlag
and_helpLongFlag
properties anymore, access them via_helpOption.short
and_helpOption.long
instead. The reasoning behind this change is the following: in the future, there could be need to access not only the individual flags, but also the originally providedflags
, the .name(), the .attributeName() or something else pertaining to the help option. It makes no sense to add a property to theCommand
instance each time such a property is needed, it is way better to simply store theOption
instance.ChangeLog
Changed
.createOption()
in.helpOption()
to support custom option flag extractionPeer PRs
…solving similar problems
_versionOption
instead of only the.attributeName()
in_versionOptionName
)