-
Notifications
You must be signed in to change notification settings - Fork 88
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
fix: Normalize white space characters and preserve references in attributes #291
Conversation
I don't see support or testing of the referenced "end-of-line handling". Should we add this or am I missing something here? |
@brodybits yes, this is something that xmldom currently doesn't do and I consider this as a change outside of the scope of this fix. |
Thanks for raising ... and proposing a solution for #303. But TBH I felt a bit surprised to see the solution for #303 kept in a separate PR due to multiple factors:
I got a couple of test snapshot errors when I merged these updates together with the updates proposed in PR #305, which I would consider to be a little bit of rapid snapshot churn. Thoughts? Am I too pedantic here? |
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.
I think we need to remove- now fixed.only
from a test in one place, guessing it was just an oversight- suggesting to add some spaces & blank lines
Code changes look good overall, +1 (+100) for cleaning up & removing what looks like repeated regex replacements.
Regarding Regarding the split of issues:
For me sounds more like like a prerequisite than a part of the algorithm. So I propose the I land the PR for line endinge normalization first. And then deal with the issues in this PR. Or even rebase this one on top of the other to see the impact right away. Thx for going as as trying to merge them both. |
Yeah I would favor landing PR #305 first. +1 for adding eslint rules to keep things like |
6b3cdef
to
aa06c95
Compare
Didn't mean to close the PR |
509085e
to
d5405e4
Compare
03b9fd1
to
4801a70
Compare
aa06c95
to
5d32b43
Compare
95ffbab
to
acf094b
Compare
02de7af
to
d2fb058
Compare
acf094b
to
ae9c1ea
Compare
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]>
Such a mess... the subsequent PR landing seems to only work when the branches are all in the same repository. |
BREAKING CHANGE: If you relied on the not spec compliant preservation of
\t
,\n
or\r
in attribute values. To preserve those you will have to create XML that instead contains the correct numerical (or hexadecimal) equivalent (	
,

,
).