-
Notifications
You must be signed in to change notification settings - Fork 113
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
Decoding containers with (potentially)-empty elements #123
Comments
Thank you for reporting this. I previously also had an attempt at this, or a similar case related to optionals and empty elements, and I wasn't able to fix and at the same time to keep all existing tests passing. If you see a way to make this work well, please go ahead! |
@MaxDesiatov Any word on this? I have the same issue decoding the following xml (see the part with
If there is no official answer to this as of yet, I'm curious to know what you're doing @jsbean to handle this case in the interim (both for decoding and encoding)? Thanks, and kudos on a great library! |
Hi @JUSTINMKAUFMAN, We made a patch for this in this PR on our fork. We haven't submitted it here yet because it changes an expectation in one test (@bwetherfield mentions it in the PR), and because we have been focused on other things in the meantime. Our patch is working great for us, though! What kinda value is Perhaps give our fork on |
Thanks @jsbean that works perfectly! |
Thanks folks! My main reason for being cautious with this is the effort to maintain widest possible compatibility in CoreXLSX. But I need to double check, maybe the fork works fine in most important cases and slight breakage in backward compatibility would be ok. I'll keep the issue open until that's verified, and as usual I'm open to any possible improvements. |
## Overview Fixes #123 and extends #145 to accommodate decoding of arrays of empty strings or mixed arrays of non-empty and empty strings. ## Example We may now decode ```xml <container> <empty/> <empty/> <empty/> </container> ``` into the following type ```swift struct EmptyArray: Equatable, Codable { enum CodingKeys: String, CodingKey { case empties = "empty" } let empties: [Empty] } ``` where ```swift struct Empty: Equatable, Codable { } ``` We can also decode a value of the following type ```swift struct EmptyWrapper { let empty: Empty } ``` from ```xml <wrapper> <empty/> </wrapper> ``` Further, following from #145 we can decode arrays of empty strings ```xml <container> <string-type/> <string-type>My neighbors are empty<string-type> <string-type/> </container> ``` as ```swift struct EmptyArray: Equatable, Codable { enum CodingKeys: String, CodingKey { case stringType = "string-type" } let stringType: [String] } ``` and variants. ## Source Compatibility In cases where we decode an array of type `[String?]`, an empty element is now decoded as `""` rather than `nil`, the justification being that `String` can itself take empty values. We use the convention that `nil` signifies the absence of an element (if 0 or 1 of the element are allowed) as in the updated [BreakfastTest](https://github.com/bwetherfield/XMLCoder/blob/0d20614e47df98d1a10174e992c585edf629c9b9/Tests/XMLCoderTests/BreakfastTest.swift) and in the following error-throwing [test case](https://github.com/MaxDesiatov/XMLCoder/blob/2855777ff868e8a4c1d944c7da0ddb49a8b3893e/Tests/XMLCoderTests/Minimal/NullTests.swift#L56-L68). * Add nested choice unkeyed container decoding test * Fix nested unkeyed container implementation for nested keyed box * Clean up unwrapping syntax * Treat corner case of empty string value intrinsic * Remove xcodeproj junk * Add some logging to assess where we're at * Add cases for empty string as null element decoding * Swiftformat * Transform precondition to where clause in switch statement * Remove print statements * Add failing test for a nested array of empty-string value intrinsic elements * Do a little cleanup of single keyed box passing around * Refactor XMLKeyedDecodingContainer.decodeConcrete elements massaging * Remove xcscheme junk * Add fix for empty arrays, wrapped empties etc * Clean up * Revert singleKeyed dive change * Accommodate singleKeyed reading options * Alter Border Test * Add test case that returns [nil] (exists a non-optional property) * Eliminate possibly empty Int from Breakfast test * Fix formatting * Fix forcecast * Fix formatting * Update LinuxMain * Fix tests such that null elements read as empty strings * Fix linux main * Add nested array of empty strings decoding in the explicit style * Add mixed case empty and non-empty string cases * Reinstate missing test * Add test for decoding a null element into an optional type
## Overview Fixes CoreOffice#123 and extends CoreOffice#145 to accommodate decoding of arrays of empty strings or mixed arrays of non-empty and empty strings. ## Example We may now decode ```xml <container> <empty/> <empty/> <empty/> </container> ``` into the following type ```swift struct EmptyArray: Equatable, Codable { enum CodingKeys: String, CodingKey { case empties = "empty" } let empties: [Empty] } ``` where ```swift struct Empty: Equatable, Codable { } ``` We can also decode a value of the following type ```swift struct EmptyWrapper { let empty: Empty } ``` from ```xml <wrapper> <empty/> </wrapper> ``` Further, following from CoreOffice#145 we can decode arrays of empty strings ```xml <container> <string-type/> <string-type>My neighbors are empty<string-type> <string-type/> </container> ``` as ```swift struct EmptyArray: Equatable, Codable { enum CodingKeys: String, CodingKey { case stringType = "string-type" } let stringType: [String] } ``` and variants. ## Source Compatibility In cases where we decode an array of type `[String?]`, an empty element is now decoded as `""` rather than `nil`, the justification being that `String` can itself take empty values. We use the convention that `nil` signifies the absence of an element (if 0 or 1 of the element are allowed) as in the updated [BreakfastTest](https://github.com/bwetherfield/XMLCoder/blob/0d20614e47df98d1a10174e992c585edf629c9b9/Tests/XMLCoderTests/BreakfastTest.swift) and in the following error-throwing [test case](https://github.com/MaxDesiatov/XMLCoder/blob/2855777ff868e8a4c1d944c7da0ddb49a8b3893e/Tests/XMLCoderTests/Minimal/NullTests.swift#L56-L68). * Add nested choice unkeyed container decoding test * Fix nested unkeyed container implementation for nested keyed box * Clean up unwrapping syntax * Treat corner case of empty string value intrinsic * Remove xcodeproj junk * Add some logging to assess where we're at * Add cases for empty string as null element decoding * Swiftformat * Transform precondition to where clause in switch statement * Remove print statements * Add failing test for a nested array of empty-string value intrinsic elements * Do a little cleanup of single keyed box passing around * Refactor XMLKeyedDecodingContainer.decodeConcrete elements massaging * Remove xcscheme junk * Add fix for empty arrays, wrapped empties etc * Clean up * Revert singleKeyed dive change * Accommodate singleKeyed reading options * Alter Border Test * Add test case that returns [nil] (exists a non-optional property) * Eliminate possibly empty Int from Breakfast test * Fix formatting * Fix forcecast * Fix formatting * Update LinuxMain * Fix tests such that null elements read as empty strings * Fix linux main * Add nested array of empty strings decoding in the explicit style * Add mixed case empty and non-empty string cases * Reinstate missing test * Add test for decoding a null element into an optional type
We have a use case where we need to decode an array of potentially empty elements.
Currently, unkeyed and keyed containers holding onto empty (e.g., a struct with no fields) or a potentially empty (e.g., a struct with all optional fields) elements does not decode.
Say we've got a struct like this:
If we have some XML like this:
It would be nice to decode like this:
The poetics are strong.
Wrapping it up a bit, consider this:
Given the
xml
from above, we can try to decode it like this:Lastly, given this container:
And we have some XML like this:
And we try to decode it like this:
We get the same outcomes if you substitute the
Empty
type with something like this:If this seems like something that should theoretically be supported, I can give a go at it. It smells like it might touch similar neighborhoods of the codebase as #122.
The text was updated successfully, but these errors were encountered: