fix(xlsx): fix whitespace trimming bug introduced in #671

[dayongxie]

Apologies — #671 introduced a subtle bug. This PR fixes it.

The mistake

#671 used quick-xml's trim_text(true) to strip whitespace from <t> elements lacking xml:space="preserve". I overlooked that trim_text operates on each Text event independently, so when character references split text across multiple events, internal whitespace got wrongly eaten.

Concretely, for this XML:

<t>&#26085;&#26399; &#26102;&#38388;</t>

the parser emits three separate Text events:

1. 日期
2. ← a single space
3. 时间

Since trim_text trims each event in isolation, event #2 was stripped entirely, yielding "日期时间" instead of the correct "日期 时间".

Truly sorry for the sloppy review — I should have tested this edge case.

The fix

• Join all raw text events into a single string first.
• Trim only ASCII whitespace (U+0020 SPACE, U+0009 TAB, U+000A LF, U+000D CR) from the joined result — Unicode spaces like U+00A0 (NBSP) are preserved.
• Decode _xHHHH_ escapes after trimming, keeping x000D (\r) literal during trim so it doesn't get stripped.

This also eliminates the trim_text(true)/trim_text(false) toggle on the shared reader config, which was brittle.

Changes

• src/xlsx/mod.rs — accumulate raw text, then ASCII-trim the joined result before unescaping.
• tests/test.rs — added new assertions for internal space between character references in both inline and shared string tests.
• tests/whitespace_trim.xlsx — updated fixture with the new test case.

⚠️ Also updated an existing test decode_xml_entities:
Changed the expected value for cell A2 from "\n" to "". A bare 
 (newline) without xml:space="preserve" is now correctly trimmed — this is the right behavior per the XML spec, just wasn't accounted for in the original test.

[jmcnamara]

There are two potential allocations here. It would be better, since this in on the hot path to do it like this:

                let value = if preserve_space {
                    unescape_xml(&value).into_owned()
                } else {
                    // Trim ASCII whitespace (space/tab/CR/LF). Unicode spaces
                    // such as U+00A0 (NBSP) are preserved.
                    unescape_xml(value.trim_matches([' ', '\t', '\r', '\n'])).into_owned()
                };

[dayongxie]

Fixed, thanks!

[Copilot]

Pull request overview

Fixes a whitespace-trimming regression in XLSX string parsing introduced by enabling per-Text-event trimming, which incorrectly removed internal whitespace when character references caused the XML parser to split text into multiple events.

Changes:

• Accumulate all Text/CData/entity content for a <t> element into one string, then trim XML whitespace at the boundaries (when xml:space!="preserve"), and finally unescape Excel _x00HH_ escapes.
• Add regression tests to ensure internal literal spaces between character references are preserved for both inline strings and shared strings.
• Update the whitespace-trim XLSX fixture to include the new edge case.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/xlsx/mod.rs	Fixes the trimming logic to operate on the joined <t> content instead of per text event, preventing loss of internal whitespace.
tests/test.rs	Adds/updates assertions to cover the regression case (internal space between character references) and adjusts an expectation for trimmed edge-newline behavior.
tests/whitespace_trim.xlsx	Updates the fixture workbook used by whitespace-trimming tests to include the new regression scenario.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jmcnamara]

Sorry. I kicked off the GitHub review by accident. It seems to be making a similar comment to mine.

In your comment to my review you said it was fixed but I don't see that commit here (which is probably why the agent is also complaining). It looks like you may have put the fix on the wrong branch.

[dayongxie]

Sorry, I got the wrong branch. It's fixed now.

[jmcnamara]

Merged. Thanks.