fix: write xml:space="preserve" for inline strings - #625
Open
closset703 wants to merge 1 commit into
Open
Conversation
Excel trims leading and trailing whitespace from cells written with inlineString(int, int, String), because the <t> element is emitted without xml:space="preserve". StringCache.write() and RichText.Run.write() already emit the attribute, so the same value is preserved or trimmed depending on which string API the caller picks. The added test asserts the attribute on all three string paths (shared, inline, rich). Excel's trimming cannot be reproduced in a unit test, so it verifies the emitted XML instead; it fails without the one-line change in Cell.write() and passes with it. Fixes dhatim#624 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #624.
Cell.write()now emitsxml:space="preserve"for plain inline strings, matching whatStringCache.write()andRichText.Run.write()already do. Without it Excel trims leading and trailing whitespace from cells written throughinlineString(int, int, String), so the same value is kept or trimmed depending on which string API the caller happens to use.Behaviour change
This changes output for existing users: values that Excel used to trim are now kept. Anyone who relied on that trimming would have to trim before writing.
My reading is that this matches the direction already set in #475 — "Preserve white spaces by default. The user just needs to trim the strings themselves in case they don't want it." — and that this PR only applies that policy to the path it missed, rather than introducing a new one. Flagging it explicitly so you can confirm, since it does alter existing output. Happy to make it opt-in instead if you'd prefer that.
Test
stringCellsKeepEdgeWhitespaceasserts the attribute on all three string paths (shared, inline, rich). Excel's trimming cannot be reproduced in a unit test, so the test verifies the emitted XML instead — it fails without the one-line change inCell.write()and passes with it.The workbook part is read through
ZipFilerather thanZipInputStream, because the streaming zip records entry sizes in a data descriptor thatZipInputStreamrejects.