Fix CommandParser incorrectly handling multiple end-of-option delimiters #3541
Conversation
Previously, the command parser would swallow all "--" after the first one. For me, this manifested itself in not allowing corral to be used with lldb "corral exec -- lldb ponyc -- $(ponyc) -V=0 -o ./build/ ./utility" since the second "--" is required to be passed to lldb. This change is dependency for a PR being submitted to corral to allow use of corral with lldb (for debugging ponyc itself).
|
Thanks @KittyMac, Are there any tests that can be provided to verify that it works and most importantly that it won't get accidentally broken in the future? |
|
@mfelsche I think you know the cli pretty well. Can you review this? Does this could as a changelog- changed, changelog - added or what? |
|
@KittyMac Can you write up a nice "end user friendly" description of this change for the release notes and add them as a comment on this PR? |
|
I see the cli tests now, I can add some when/if @mfelsche confirms the change is correct. Guideline 10 of the standard says "The first -- argument that is not an option-argument should be accepted as a delimiter indicating the end of options. Any following arguments should be treated as operands, even if they begin with the '-' character." The parser currently swallows all "--" after the first one, not allowing the arguments to be passed along verbatim. So, end user friendly description might be: "Fixed an issue where CommandParser was incorrectly handling cases when arguments contain multiple end-of-option delimiters" |
|
So, if i understand this right, with this change, all tokens after a A test showing that a second |
Squashed commits: [9ab2aab] Stop swallowing "--" after the first one is processed by command parser Previously, the command parser would swallow all "--" after the first one. For me, this manifested itself in not allowing corral to be used with lldb "corral exec -- lldb ponyc -- $(ponyc) -V=0 -o ./build/ ./utility" since the second "--" is required to be passed to lldb. This change is dependency for a PR being submitted to corral to allow use of corral with lldb (for debugging ponyc itself).
…yMac/ponyc into commandparser_fix_for_corral
|
This shouldnt be merged til after the 0.34.1 hot fix release that is happening tomorrow. I've marked "DO NOT MERGE" accordingly. |
|
I'm removing the DO NOT MERGE tag 0.34.1 has been released. |
SeanTAllen
added
the
changelog - fixed
SeanTAllen
changed the title
SeanTAllen
changed the title
Previously, the command parser would swallow all "--" after the first
one. For me, this manifested itself in not allowing corral to be used
with lldb "corral exec -- lldb ponyc -- $(ponyc) -V=0 -o ./build/
./utility" since the second "--" is required to be passed to lldb.
This change is dependency for a PR being submitted to corral to allow
use of corral with lldb (for debugging ponyc itself).