Yet another approach to disable trim #865
Open
Conversation
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
Mingun
requested changes
May 29, 2025
There was a problem hiding this comment.
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 creatingTexts and nothing more - the second uses this API, changes tests and adds changelog about fixed issue
|
Codecov ReportAttention: Patch coverage is
❗ 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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…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
approved these changes
Jun 7, 2025
There was a problem hiding this comment.
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_whitespacescalls. 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 theis_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?
|
@Mingun that solves my issue. Thanks for the changes, I'm just not familiar with this project. |
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.
I implemented disabling trim by modifying Text struct.
See: #285 (comment)
I need this for parsing
.odtfiles.Can anybody give me some feedback? This issue is 4 years old and I'm willing to fix it if possible.