-
Notifications
You must be signed in to change notification settings - Fork 111
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
Implement removeWhitespaceElements
on XMLDecoder
#222
Conversation
Co-authored-by: Max Desiatov <max@desiatov.com>
Co-authored-by: Max Desiatov <max@desiatov.com>
Co-authored-by: Max Desiatov <max@desiatov.com>
Co-authored-by: Max Desiatov <max@desiatov.com>
Co-authored-by: Max Desiatov <max@desiatov.com>
Co-authored-by: Max Desiatov <max@desiatov.com>
Codecov Report
@@ Coverage Diff @@
## main #222 +/- ##
==========================================
+ Coverage 73.50% 73.87% +0.37%
==========================================
Files 46 46
Lines 2404 2431 +27
==========================================
+ Hits 1767 1796 +29
+ Misses 637 635 -2
Continue to review full report at Codecov.
|
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.
LGTM, thanks!
@wooj2 one last request, I promise 🙂 Can you add a subsection to the "Advanced features" section in Just an example of your use case and a short description of the feature, similar to other subsections that describe advanced features. Thank you! |
trimValueWhitespaces
on XMLDecoder
trimValueWhitespaces
on XMLDecoder
removeWhitespaceElements
on XMLDecoder
No prob at all! Just updated it. Thanks again for all the support |
@kneekey23 @wooj2 perfect, thanks for uncovering this edge case! Your contribution is very appreciated 👏 |
This updates the `XMLStackParser` to accept a parameter called `removeWhitespaceElements`. The purpose of the `XMLStackParser` is to call the XML parser and create a tree of `XMLCoderElement` representing the structure of the parsed XML. Assuming that XMLStackParser has `trimValueWhitespaces` set to `false`, when attempting to parse a nested data structure like the following: ```xml <SomeType> <nestedStringList> <member> <member>foo</member> <member>bar</member> </member> <member> <member>baz</member> <member>qux</member> </member> </nestedStringList> </SomeType> ``` ... then there will multiple `XMLCoderElement`s in the tree which will have `elements` set to elements that are either: a. Purely whitespaced elements or b. The child elements These purely whitespaced elements are problematic for users who are implementing custom `Decoder` logic, as they are interpreted as regular child elements. Therefore, setting `removeWhitespaceElements` to `true` while `trimValueWhitespaces` is set to `false`, will remove these whitespace elements during the construction of the `XMLCoderElement` tree. An in-depth analysis of the original problem can be found [here](CoreOffice#219). For historical purposes, a separate approach was implemented. It uses a similar algorithm in a different part of the code. CoreOffice#221
This updates the
XMLStackParser
to accept a parameter calledremoveWhitespaceElements
.The purpose of the
XMLStackParser
is to call the XML parser and create a tree ofXMLCoderElement
representing the structure of the parsed XML.Assuming that XMLStackParser has
trimValueWhitespaces
set tofalse
, when attempting to parse a nested data structure like the following:... then there will multiple
XMLCoderElement
s in the tree which will haveelements
set to elements that are either:a. Purely whitespaced elements or
b. The child elements
These purely whitespaced elements are problematic for users who are implementing custom
Decoder
logic, as they are interpreted as regular child elements. Therefore, settingremoveWhitespaceElements
totrue
whiletrimValueWhitespaces
is set tofalse
, will remove these whitespace elements during the construction of theXMLCoderElement
tree.An in-depth analysis of the original problem can be found here.
For historical purposes, a separate approach was implemented. It uses a similar algorithm in a different part of the code. #221