-
Notifications
You must be signed in to change notification settings - Fork 11
Respect xml:space="preserve" (#43) #45
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
Conversation
|
Hi Josh. Is there a reason you haven't merged this? Let me know if there is anything more I need to do or if it doesn't pass muster for some reason. Thx. |
|
Sorry for not responding! I was in the middle of rewriting a bit of the XML internals when you made the PR and then both the rewrite and this PR got pushed to the back burner by other work. LGTM, and again my apologies for the delay! |
|
Thank you! Will there be a new release? |
* Address speed regression in #46 and fix reamining issues in #45. * Use @views more often for slices * undo skip orphan text nodes (cf. EzXML) * Amend `XML.write` to honour `xml:space="preserve"` * Update write to respect xml:space * Correct isspace test * Fix isspace * more isspace * Normalize_newlines * Normalize_newlines * Undo normalize newlines * Undo normalize newlines
This PR fixes the issue reported in #43. I've added extra tests, too.
One caveat in testing: I had to change one test (only):
I had to change the condition from
===to only==. With this single change, tests now all pass locally.I've also checked that this PR works well with XLSX.jl downstream and all XLSX tests also pass locally.
With this PR, XLSX now properly reads a workbook like this:

which has leading and trailing whitespace in column A and, in column B, only has whitespace.
Reading this XLSX file now works correctly:
Previously, leading and trailing spaces would be trimmed, and cells containing only whitespace would become
missing.I'm afraid I can't match the simple elegance of your coding style, though!