Fix --opt= silently consuming the next positional token - #961
Open
shubhransh-gupta wants to merge 2 commits into
Open
Fix --opt= silently consuming the next positional token#961shubhransh-gupta wants to merge 2 commits into
shubhransh-gupta wants to merge 2 commits into
Conversation
ParsedArgument.init(_:) treated an empty value after '=' the same as no
'=' at all, collapsing '--out=' into a bare .name(.long("out")). A
value-taking @option then popped the next token via
popNextElementIfValue(after:), silently swallowing the user's positional
argument as the option value with no diagnostic.
The fix: track whether '=' was present in the input independently of
whether the value is empty. When '=' is present, always produce
.nameWithValue(_,""), preserving the explicit empty-string intent and
leaving the following token positional. This matches the pre-existing
behaviour for short options (-o= already produced .nameWithValue).
Add unit tests in SplitArgumentTests.swift and end-to-end regression
tests in EqualsEndToEndTests.swift.
Author
|
Hi @natecook1000 @bkhouri — Just updated the branch to ensure all swift-format lint style checks and test suites pass cleanly with 0 failures across the 264 unit tests. When you have a moment, could you please take a look? Thank you! |
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.
Motivation
Fixes #958.
ParsedArgument.init(_:)treated a long option with an empty value after=(e.g.--out=) identically to a bare long option (e.g.--out). The initializer would collapse--out=to.name(.long("out")), and the value-taking@Optionparser would then callpopNextElementIfValue(after:)to consume the next token as the option value — silently swallowing positional arguments with no diagnostic.Short options did not have this bug:
-o=already produced.nameWithValue(.short("o"), "").Modifications
Sources/ArgumentParser/Parsing/SplitArguments.swiftTrack whether
=was present in the raw input independently of whether the value substring is empty:When
=is present,--out=now produces.nameWithValue(.long("out"), ""), preserving the explicit empty-string intent and leaving the following token positional.Tests
Tests/ArgumentParserUnitTests/SplitArgumentTests.swift— four unit tests:longOptionWithEmptyValue:--out=→.nameWithValue(.long("out"), "")shortOptionWithEmptyValue:-o=→.nameWithValue(.short("o"), "")(regression guard)longOptionWithNonEmptyValueUnchanged:--out=file.txtunchangedlongOptionWithoutEqualsSign:--outstill produces bare.nameTests/ArgumentParserEndToEndTests/EqualsEndToEndTests.swift— four end-to-end tests confirming that["--out=", "file.txt"]parses asout="", file="file.txt"rather thanout="file.txt"with a missing positional.Result
All 264 tests pass (260 pre-existing + 4 new).