-
Notifications
You must be signed in to change notification settings - Fork 20
Update EnvironmentValueArrayDecoder to emit empty values when present #133
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
base: main
Are you sure you want to change the base?
Conversation
|
What I found while working on the issue is that Swift’s native API is String.split(separator:), whereas components(separatedBy:) is an NSString-bridged method backed by UTF-16 storage. Swift’s String uses UTF-8-backed storage. The preferred API is split(separator:), with omittingEmptySubsequences when needed. In the project, CLIArgumentParser.swift:125 uses the NSString API. It would be worth converting this to String.split(separator:) and disallowing future use of NSString APIs. Due to bridging between the two types, it is easy to accidentally rely on NSString when working with String. If needed, I will revert the commit: ae5cf01 |
czechboy0
left a comment
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.
One ask about the test, otherwise looks great - thank you, @Adobels! 🙏
| } | ||
|
|
||
| @available(Configuration 1.0, *) | ||
| @Test func valueForKeyOfIntArrayTypes() throws { |
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.
Please add some success cases as well here for the ints and doubles
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.
Done.
0757dc1 to
2116aa3
Compare
Motivation
Closes #122
Modifications
omittingEmptySubsequencestofalseinString.split(separator:omittingEmptySubsequences:)when decoding a string that represents Bool, Int, or Double arrays separated by commas inEnvironmentValuesProvider.CLIArgumentParser.swift:125to usesplitinstead ofcomponents(separatedBy:), ensuring homogeneous usage of Swift’s splitting APIs.Result
A string representing an array of values separated by
","will now emit empty values when two separators appear together while parsingEnvironmentValuesProviderdictionaries.This results in treating an empty value as an empty string and, consequently, throwing an error when parsed by individual value decoders for Int, Double, and Bool.
The additional update enforces homogeneous API usage and favors Swift’s native
split(separator:)method.Test Plan
Add tests covering empty values in Int, Double, and Bool arrays.