Fix empty Text event emitted with trim_text_end (#984) by dralley · Pull Request #997 · tafia/quick-xml

dralley · 2026-07-31T13:57:50Z

When trim_text_end is enabled, whitespace-only text before markup or a reference is now suppressed instead of producing an empty Text("") event.

closes #984

dralley · 2026-07-31T13:59:40Z

-                    $self.config_mut().trim_text_start = trim;
+                    let config = $self.config_mut();
+                    config.trim_text_start = trim_start;
+                    config.trim_text_end = trim_end;


I don't really like the clear->restore pattern, but it was already like this. Would be nice to refactor this away

Yes, I also does not like it here... I was thinking about removing auto-trim altogether and making it an explicit method call on the Event

I probably don't agree with that, at least in this specific case it structurally changes what comes out of the parser. If someone doesn't want to deal with whitespace-only text events at all (and I understand that completely, it's really quite annoying, I use trim for precisely that reason) manual trim wouldn't help.

I have an old partial commit from 2022 which changes the configuration options slightly. I think this would make a lot more sense (but I think might be more difficult to implement now, will need to think about how to make it work)

- /// Trims leading whitespace in Text events, skip the element if text is empty - pub trim_text_start: bool, - /// Trims trailing whitespace in Text events. - pub trim_text_end: bool, + /// Preserves whitespace-only text elements between events (e.g. indented "pretty" formatting). + pub preserve_indentation: bool, + /// Trims leading and trailing whitespace in Text events. + pub trim_text: bool,

I can file an issue for that if you agree.

I doubt that we can reliable detect indentation and I think that is not the work for parser at all.

dralley · 2026-07-31T14:02:02Z

replaces #993 and #986

codecov-commenter · 2026-07-31T14:12:51Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.21%. Comparing base (e00ae5c) to head (fd372a8).
⚠️ Report is 40 commits behind head on master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #997      +/-   ##
==========================================
- Coverage   57.31%   56.21%   -1.10%     
==========================================
  Files          46       47       +1     
  Lines       18197    18446     +249     
==========================================
- Hits        10429    10370      -59     
- Misses       7768     8076     +308

Flag	Coverage Δ
unittests	`56.21% <100.00%> (-1.10%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

dralley · 2026-08-15T02:59:28Z

Will resume this PR as well as 996 after 1001 merges.

Would be nice to include those, 1002, and maybe 897 (#897 (comment)) before cutting a new release.

When trim_text_end is enabled, whitespace-only text before markup or a reference is now suppressed instead of producing an empty Text("") event. closes tafia#984

The comment claimed `<` was consumed, but `read_text` never consumes it, `<` is consumed later by `read_with` inside `read_until_close`.

The tests (and Reader) do not currently exhibit proper handling of XML end-of-line normalization.

Mingun

Unfortunately, this cannot be fix at low-level parser level. We cannot strip whitespaces before all entity references -- it is possible only after them will be expanded. & should preserve spaces before it, but &xml; where it is expanded into <a>...</a> must not.

We also probably cannot blindly trim everything before <, because of CDATA sections, comments and processing instructions. Comments and PIs are stripped out from the output, so when them inside the text, spaces around them would be trimmed. I think, this would be unexpected for users.

Mingun · 2026-08-16T10:57:02Z

+                        if trim_text_end && buf[start..].iter().all(|&b| is_whitespace(b)) {
+                            return ReadTextResult::Markup(buf);
+                        }


I think that this is not the correct place for trim. I would trim spaces before comments and CDATA sections. It seems totally wrong to me.

Mingun · 2026-08-16T10:59:27Z

+                        if trim_text_end && buf[start..].iter().all(|&b| is_whitespace(b)) {
+                            return ReadTextResult::Ref(buf);
+                        }


This change is wrong. We do not want to trim spaces before entity references, otherwise " & text" becomes "& text" instead of " & text". Compare the result with " x text" -- this will be unexpected for the user if the replacement in the text x with & suddenly eats up the spaces before this character.

Good point, but given the existing tests pass, we probably need some additional test cases around all of this.

Mingun · 2026-08-16T11:01:39Z

-                    $self.config_mut().trim_text_start = trim;
+                    let config = $self.config_mut();
+                    config.trim_text_start = trim_start;
+                    config.trim_text_end = trim_end;


I doubt that we can reliable detect indentation and I think that is not the work for parser at all.

dralley · 2026-08-17T04:37:26Z

I will split off the documentation fixes and new tests (which I'm adding) to a separate PR, then experiment with how to avoid having this code in the parser.

dralley commented Jul 31, 2026

View reviewed changes

dralley force-pushed the empty-text2 branch from 95ccb87 to f40347a Compare August 15, 2026 16:33

dralley commented Aug 15, 2026

View reviewed changes

Comment thread tests/reader-config.rs

dralley added 2 commits August 15, 2026 18:17

Fix empty Text event emitted with trim_text_end (tafia#984)

8171d08

When trim_text_end is enabled, whitespace-only text before markup or a reference is now suppressed instead of producing an empty Text("") event. closes tafia#984

Fix incorrect doc comment on ReadTextResult::UpToMarkup

8563595

The comment claimed `<` was consumed, but `read_text` never consumes it, `<` is consumed later by `read_with` inside `read_until_close`.

dralley force-pushed the empty-text2 branch from f40347a to 8563595 Compare August 15, 2026 22:17

Leave note about incorrect tests

fd372a8

The tests (and Reader) do not currently exhibit proper handling of XML end-of-line normalization.

dralley requested a review from Mingun August 15, 2026 22:32

dralley marked this pull request as ready for review August 15, 2026 22:32

Mingun requested changes Aug 16, 2026

View reviewed changes

dralley marked this pull request as draft August 17, 2026 02:51

Conversation

dralley commented Jul 31, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dralley Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dralley commented Jul 31, 2026

Uh oh!

codecov-commenter commented Jul 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dralley commented Aug 15, 2026

Uh oh!

Uh oh!

Mingun left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dralley commented Aug 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dralley Aug 15, 2026 •

edited

Loading

codecov-commenter commented Jul 31, 2026 •

edited

Loading