Fix string option parsing with nopt v9 - #11018
Closed
2005rishabh wants to merge 1 commit into
Closed
Conversation
Contributor
|
what's the regression? I'm not familiar |
Member
|
@NullVoxPopuli I think regression is the wrong word here. I tried updating to the latest version and it was failing for some unknown reason #11010 @2005rishabh would you not be better to target the branch that updated the nopt version? 🤔 I don't see how making this change on main would help |
Author
|
You're right — this compatibility fix only makes sense alongside the existing I'll retarget the changes onto the |
Author
|
Closing this in favor of a PR targeting the existing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I investigated the
nopt@9regression and narrowed the issue down to parsing string option values that begin with--.nopt@9interprets those values as additional flags instead of string values, which breaks cases like:--options "--split 2 --random"The fix adds a small compatibility layer in
parseArgs()that temporarily masks string option values beginning with--during parsing and restores them afterward.Verified locally with:
tests/unit/models/command-test.jsember new uses yarn when blueprint has yarn.lock)Current local result: