fix(XMLSerializer): Preserve whitespace character references#310
Conversation
|
I have a couple of questions:
I am also wondering if xmltest can eventually be considered a total overlap with the tests we have added and fixed, or if we will just keep it as an extra test in the long term. |
brody2consult
left a comment
There was a problem hiding this comment.
LGTM, thanks.
It would be nice to get my questions answered but I would consider them to be non-blocking.
My perspective on xmltest is that it provides an orientation regarding going into the "right"/expected direction (actual getting closer to or same as expected) and I would prefer to keep this, even if it's treated as a separate test run (e.g. only after the other tests passed). The more I work with all the tests, the clearer the redundancies get, despite all the chaos. I think at some point not to far in the future we will be able to unclutter this without loosing any coverage/mutation score. Regarding comments in lib, I usually put them to make things more obvious. |
karfau
left a comment
There was a problem hiding this comment.
Your suggestions for additional comments make total sense
5217b37 to
6840849
Compare
brody2consult
left a comment
There was a problem hiding this comment.
LGTM with a couple more nits. Yeah your comments make total sense.
ff7210e to
63538b3
Compare
f204e98 to
84ef8c7
Compare
according to XML1.1 spec. Fixes #49 Since #307 only implemented XML 1.0 spec https://www.w3.org/TR/xml/#sec-line-ends https://www.w3.org/TR/xml11/#sec-line-ends
as specified in https://www.w3.org/TR/xml/#AVNormalize and document the parts that are not implemented
Co-authored-by: Chris Brody <[email protected]>
Co-authored-by: Chris Brody <[email protected]>
Co-authored-by: Chris Brody <[email protected]>
and update links to XML 1.1
brody2consult
left a comment
There was a problem hiding this comment.
Still LGTM with a minor comment / question
| * > The replacement text of any entity referred to directly or indirectly | ||
| * > in an attribute value must not contain a <. |
There was a problem hiding this comment.
+1 for starting quoted lines with > here 👍
…tespace-literals-to-character-references
add semgrep support uodate xmldom breaking changes in xmldom: xmldom/xmldom#310 xmldom/xmldom#314 xmldom/xmldom#307
add semgrep support uodate xmldom breaking changes in xmldom: xmldom/xmldom#310 xmldom/xmldom#314 xmldom/xmldom#307
BREAKING CHANGE: If you relied on the not spec compliant preservation of
\t,\nor\rin attribute values. To preserve those you will have to create XML that instead contains the correct numerical (or hexadecimal) equivalent (	,
,
).