Yet another approach to disable trim by ggodlewski · Pull Request #865 · tafia/quick-xml

ggodlewski · 2025-05-29T15:07:33Z

I implemented disabling trim by modifying Text struct.
See: #285 (comment)

I need this for parsing .odt files.
Can anybody give me some feedback? This issue is 4 years old and I'm willing to fix it if possible.

Mingun

Good job! Because you anyway will add changes I would like to see that PR force pushed and to have at least 2 commits:

the first one introduces new fields and API for Text (with mention in changelog), adapting code to creating Texts and nothing more
the second uses this API, changes tests and adds changelog about fixed issue

src/de/mod.rs

codecov-commenter · 2025-05-29T18:16:56Z

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

Codecov Report

Attention: Patch coverage is 91.35802% with 14 lines in your changes missing coverage. Please review.

Project coverage is 60.42%. Comparing base (254fbd2) to head (0adae15).
Report is 17 commits behind head on master.

Files with missing lines	Patch %	Lines
src/de/mod.rs	91.02%	14 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #865      +/-   ##
==========================================
- Coverage   60.74%   60.42%   -0.32%     
==========================================
  Files          41       42       +1     
  Lines       16044    16488     +444     
==========================================
+ Hits         9746     9963     +217     
- Misses       6298     6525     +227

Flag	Coverage Δ
unittests	`60.42% <91.35%> (-0.32%)`	⬇️

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

☔ View full report in Codecov by Sentry.
📢 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.

…t pointed to the test

The Reader will constantly return Eof after reaching it so if we peek that event in end we wrongly will think that not all events consumed

This allow us to emit more informative errors which contains the event which was not consumed

failures (177): --lib (33): de::tests::borrowing_reader_events de::tests::merge_text::comment_between::cdata_and_text de::tests::merge_text::comment_between::empty_cdata_and_text de::tests::merge_text::pi_between::cdata_and_text de::tests::merge_text::pi_between::empty_cdata_and_text de::tests::read_to_end::complex de::tests::skip::limit de::tests::skip::partial_replay de::tests::skip::read_and_peek de::tests::skip::read_to_end de::tests::triples::cdata::cdata::text de::tests::triples::cdata::start::text de::tests::triples::cdata::text::end de::tests::triples::cdata::text::eof de::tests::triples::cdata::text::start de::tests::triples::start::cdata::text de::tests::triples::start::end::text de::tests::triples::start::start::text de::tests::triples::start::text::cdata de::tests::triples::start::text::end de::tests::triples::start::text::eof de::tests::triples::start::text::start de::tests::triples::text::cdata::cdata de::tests::triples::text::cdata::end de::tests::triples::text::cdata::eof de::tests::triples::text::cdata::start de::tests::triples::text::cdata::text de::tests::triples::text::end de::tests::triples::text::start::cdata de::tests::triples::text::start::end de::tests::triples::text::start::eof de::tests::triples::text::start::start de::tests::triples::text::start::text serde-de (13): borrow::element_name borrow::escaped::element borrow::non_escaped::element from_str_should_ignore_encoding map::attribute_and_element resolve::resolve_custom_entity struct_::attribute_and_element struct_::excess_data_before::text_non_spaces struct_::excess_data_before::text_spaces_only struct_::excess_elements xml_prolog::comments xml_prolog::pi xml_prolog::spaces serde-de-enum (4): externally_tagged::text::newtype::mixed externally_tagged::text::newtype::text externally_tagged::text::struct_::mixed externally_tagged::text::struct_::text serde-de-seq (115): fixed_name::fixed_size::excess_elements fixed_name::fixed_size::fever_elements fixed_name::fixed_size::field_after_list::after fixed_name::fixed_size::field_after_list::before fixed_name::fixed_size::field_after_list::overlapped fixed_name::fixed_size::field_after_list::overlapped_with_nested_list fixed_name::fixed_size::field_before_list::after fixed_name::fixed_size::field_before_list::before fixed_name::fixed_size::field_before_list::overlapped fixed_name::fixed_size::field_before_list::overlapped_with_nested_list fixed_name::fixed_size::list_of_list fixed_name::fixed_size::list_of_struct fixed_name::fixed_size::mixed_content fixed_name::fixed_size::one_element fixed_name::fixed_size::primitives fixed_name::fixed_size::simple fixed_name::fixed_size::two_lists::overlapped fixed_name::fixed_size::two_lists::overlapped_with_nested_list fixed_name::fixed_size::two_lists::splitted fixed_name::fixed_size::unknown_items::after fixed_name::fixed_size::unknown_items::before fixed_name::fixed_size::unknown_items::overlapped fixed_name::fixed_size::unknown_items::overlapped_with_nested_list fixed_name::fixed_size::xs_list::element fixed_name::fixed_size::xs_list::empty fixed_name::fixed_size::xs_list::many fixed_name::fixed_size::xs_list::one fixed_name::fixed_size::xs_list::zero fixed_name::variable_size::field_after_list::after fixed_name::variable_size::field_after_list::before fixed_name::variable_size::field_after_list::overlapped fixed_name::variable_size::field_after_list::overlapped_with_nested_list fixed_name::variable_size::field_before_list::after fixed_name::variable_size::field_before_list::before fixed_name::variable_size::field_before_list::overlapped fixed_name::variable_size::field_before_list::overlapped_with_nested_list fixed_name::variable_size::list_of_struct fixed_name::variable_size::mixed_content fixed_name::variable_size::one_element fixed_name::variable_size::primitives fixed_name::variable_size::simple fixed_name::variable_size::two_lists::overlapped fixed_name::variable_size::two_lists::overlapped_with_nested_list fixed_name::variable_size::two_lists::splitted fixed_name::variable_size::unknown_items::after fixed_name::variable_size::unknown_items::before fixed_name::variable_size::unknown_items::overlapped fixed_name::variable_size::unknown_items::overlapped_with_nested_list fixed_name::variable_size::xs_list::element fixed_name::variable_size::xs_list::empty fixed_name::variable_size::xs_list::many fixed_name::variable_size::xs_list::one fixed_name::variable_size::xs_list::zero top_level::list_of_enum top_level::list_of_struct top_level::mixed_content variable_name::fixed_size::field_after_list::after variable_name::fixed_size::field_after_list::before variable_name::fixed_size::field_after_list::overlapped variable_name::fixed_size::field_after_list::overlapped_with_nested_list variable_name::fixed_size::field_before_list::after variable_name::fixed_size::field_before_list::before variable_name::fixed_size::field_before_list::overlapped variable_name::fixed_size::field_before_list::overlapped_with_nested_list variable_name::fixed_size::list_of_enum variable_name::fixed_size::mixed_content variable_name::fixed_size::one_element variable_name::fixed_size::primitives variable_name::fixed_size::simple variable_name::fixed_size::two_lists::choice_and_fixed::fixed_after variable_name::fixed_size::two_lists::choice_and_fixed::fixed_before variable_name::fixed_size::two_lists::choice_and_fixed::overlapped::fixed_after variable_name::fixed_size::two_lists::choice_and_fixed::overlapped::fixed_before variable_name::fixed_size::two_lists::choice_and_fixed::overlapped::with_nested_list_fixed_after variable_name::fixed_size::two_lists::choice_and_fixed::overlapped::with_nested_list_fixed_before variable_name::fixed_size::two_lists::fixed_and_choice::fixed_after variable_name::fixed_size::two_lists::fixed_and_choice::fixed_before variable_name::fixed_size::two_lists::fixed_and_choice::overlapped::fixed_after variable_name::fixed_size::two_lists::fixed_and_choice::overlapped::fixed_before variable_name::fixed_size::two_lists::fixed_and_choice::overlapped::with_nested_list_fixed_after variable_name::fixed_size::two_lists::fixed_and_choice::overlapped::with_nested_list_fixed_before variable_name::fixed_size::xs_list::element variable_name::fixed_size::xs_list::empty variable_name::fixed_size::xs_list::many variable_name::fixed_size::xs_list::one variable_name::fixed_size::xs_list::zero variable_name::variable_size::field_after_list::after variable_name::variable_size::field_after_list::before variable_name::variable_size::field_after_list::overlapped variable_name::variable_size::field_after_list::overlapped_with_nested_list variable_name::variable_size::field_before_list::after variable_name::variable_size::field_before_list::before variable_name::variable_size::field_before_list::overlapped variable_name::variable_size::field_before_list::overlapped_with_nested_list variable_name::variable_size::mixed_content variable_name::variable_size::one_element variable_name::variable_size::primitives variable_name::variable_size::simple variable_name::variable_size::two_lists::choice_and_fixed::fixed_after variable_name::variable_size::two_lists::choice_and_fixed::fixed_before variable_name::variable_size::two_lists::choice_and_fixed::overlapped::fixed_after variable_name::variable_size::two_lists::choice_and_fixed::overlapped::fixed_before variable_name::variable_size::two_lists::choice_and_fixed::overlapped::with_nested_list_fixed_after variable_name::variable_size::two_lists::choice_and_fixed::overlapped::with_nested_list_fixed_before variable_name::variable_size::two_lists::fixed_and_choice::fixed_after variable_name::variable_size::two_lists::fixed_and_choice::fixed_before variable_name::variable_size::two_lists::fixed_and_choice::overlapped::fixed_after variable_name::variable_size::two_lists::fixed_and_choice::overlapped::fixed_before variable_name::variable_size::two_lists::fixed_and_choice::overlapped::with_nested_list_fixed_after variable_name::variable_size::two_lists::fixed_and_choice::overlapped::with_nested_list_fixed_before variable_name::variable_size::xs_list::element variable_name::variable_size::xs_list::empty variable_name::variable_size::xs_list::many variable_name::variable_size::xs_list::one variable_name::variable_size::xs_list::zero serde-issues (2): issue580 issue683 serde-migrated (3): test_namespaces test_option test_parse_string --doc (7): src\de\mod.rs - de (line 1339) src\de\mod.rs - de (line 1361) src\de\mod.rs - de (line 1810) src\de\mod.rs - de (line 347) src\de\resolver.rs - de::resolver::EntityResolver (line 13) src\serde_helpers.rs - serde_helpers::impl_deserialize_for_internally_tagged_enum (line 104) src\serde_helpers.rs - serde_helpers::impl_deserialize_for_internally_tagged_enum (line 174)

Fixed (40/177): --lib (33/33) serde-de (1/13): struct_::excess_data_before::text_non_spaces serde-de-enum (4/4) serde-migrated (2/3): test_option test_parse_string

- before, between and after root elements - before not-root elements Fixed all tests

…maintain the conversion state (review in all whitespace changes ignored mode)

Mingun

I fixed problems that I found:

we need to use definition of "whitespace" that is used by XML specification. That is only 4 characters: space, tab, newline and carridge return
I asked for the documentation with examples for the new methods of Text, I added them
excess count of skip_whitespaces calls. I left only necessary minimum. I suspect, that you tried to fix tests that checks that all events are consumed. I just rewrite the check (and also fixed a bug in the is_empty() method)
you forgot to add an actual link to the issue in the changelog. GitHub automatically detects some links, but the changelog should be also readable without GitHub
after changes, unused code appeared, I deleted it

@ggodlewski, could you check that the rewrited PR solves you problems? I deleted your test, because we already have similar tests and I try to not repeat.

@dralley, could you also look at this?

ggodlewski · 2025-06-07T17:00:05Z

@Mingun that solves my issue. Thanks for the changes, I'm just not familiar with this project.

Remove the same ignored test from serde-migrated That was fixed in tafia#865

Related to tafia#865

Remove the same ignored test from serde-migrated That was fixed in #865

Related to #865

Mingun requested changes May 29, 2025

View reviewed changes

src/de/mod.rs Outdated Show resolved Hide resolved

src/de/mod.rs Outdated Show resolved Hide resolved

src/de/mod.rs Show resolved Hide resolved

Mingun added enhancement serde Issues related to mapping from Rust types to XML labels May 29, 2025

ggodlewski force-pushed the untrim branch from cf6d62c to 582ea3a Compare May 30, 2025 09:59

ggodlewski requested a review from Mingun May 30, 2025 10:05

Mingun and others added 10 commits June 6, 2025 23:01

Because tafia#474 already fixed, enable corresponding tests

58c052f

Fix misprints in comments

77ea605

Add #[track_caller] attribute so the error about not consumed documen…

2e3b086

…t pointed to the test

Consider having prefetched DeEvent::Eof as absence of future events

0c82c69

The Reader will constantly return Eof after reaching it so if we peek that event in end we wrongly will think that not all events consumed

Move check for not consumed events to the Deserializer

c2cf410

This allow us to emit more informative errors which contains the event which was not consumed

Add trimming capability to Text

fe71dd9

Change tests to match updated behavior

de0668b

Fixed (40/177): --lib (33/33) serde-de (1/13): struct_::excess_data_before::text_non_spaces serde-de-enum (4/4) serde-migrated (2/3): test_option test_parse_string

Skip unsignificant whitespaces

2b85835

- before, between and after root elements - before not-root elements Fixed all tests

Replace StartTrimmer with free function because we no more need to …

0adae15

…maintain the conversion state (review in all whitespace changes ignored mode)

Mingun force-pushed the untrim branch from 582ea3a to 0adae15 Compare June 7, 2025 14:35

Mingun approved these changes Jun 7, 2025

View reviewed changes

Mingun merged commit 6017acf into tafia:master Jun 22, 2025
7 checks passed

Mingun mentioned this pull request Jul 11, 2025

Between 0.37.5 and 0.38.0 deserialized String fields stopped stripping whitespace #877

Closed

asukaminato0721 mentioned this pull request Jul 15, 2025

chore(deps): bump the http-serialization-utils group in /core with 6 updates apache/opendal#6414

Merged

Mingun mentioned this pull request Jul 25, 2025

Disable trim_text in Deserializer from_reader #285

Closed

Mingun added a commit to Mingun/quick-xml that referenced this pull request Nov 1, 2025

Check that single space not trimmed when deserialize into char

e81f464

Remove the same ignored test from serde-migrated That was fixed in tafia#865

Mingun added a commit to Mingun/quick-xml that referenced this pull request Nov 1, 2025

Replace tests for whitespace-only roundtrips

71e0f5e

Related to tafia#865

dralley pushed a commit that referenced this pull request Nov 2, 2025

Check that single space not trimmed when deserialize into char

ec1f2df

Remove the same ignored test from serde-migrated That was fixed in #865

dralley pushed a commit that referenced this pull request Nov 2, 2025

Replace tests for whitespace-only roundtrips

ea34a2a

Related to #865

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Yet another approach to disable trim#865

Yet another approach to disable trim#865
Mingun merged 10 commits intotafia:masterfrom
ggodlewski:untrim

ggodlewski commented May 29, 2025

Uh oh!

Mingun left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented May 29, 2025 •

edited

Loading

Uh oh!

Mingun left a comment

Uh oh!

ggodlewski commented Jun 7, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

Conversation

ggodlewski commented May 29, 2025

Uh oh!

Mingun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Mingun left a comment

Choose a reason for hiding this comment

Uh oh!

ggodlewski commented Jun 7, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov-commenter commented May 29, 2025 •

edited

Loading