-
Notifications
You must be signed in to change notification settings - Fork 237
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Do not write excess indents arount text fields when serialize with serde #820
Conversation
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 #820 +/- ##
==========================================
+ Coverage 60.08% 60.19% +0.10%
==========================================
Files 41 41
Lines 15975 15999 +24
==========================================
+ Hits 9599 9630 +31
+ Misses 6376 6369 -7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Updated: even better solution. Now serialization returns the classification result (instead of just boolean) and you can decide what to do with it. |
|
||
// Note that sequences of primitives serialized without delimiters! | ||
value!(seq: vec![1, 2, 3] => "1\n 2\n 3"); | ||
value!(seq_empty: Vec::<usize>::new()); | ||
value!(seq: vec![1, 2, 3] => "123"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is vec![1, 2, 3]
distinguished from vec![123]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, in any way. This is what happens now, if you do not use indents.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In #823 I changed the return value in that case to error
3"); | ||
value!(tuple_struct: Tuple("first", 42) => "first\n 42"); | ||
value!(tuple_struct: Tuple("first", 42) => "first42"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm definitely missing something here.
How would Tuple("first42", 0)
be distinguished from Tuple("first", 420)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same answer as above. This is restriction of current implementation. I have a TODO to forbid serialization consequent text fields:
Line 75 in 6eea6bb
//TODO: add settings to disallow consequent serialization of primitives |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an issue written up for that already, and if not can you write one and reference it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, there is no issue yet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In #823 I changed the return value in that case to error
That means, never indent "$text" fields and do not indent "$value" fields that produces string without surrounding tags failures (208): se::content::tests::with_indent::enum_with_text_field::char_amp se::content::tests::with_indent::enum_with_text_field::char_apos se::content::tests::with_indent::enum_with_text_field::char_gt se::content::tests::with_indent::enum_with_text_field::char_lt se::content::tests::with_indent::enum_with_text_field::char_non_escaped se::content::tests::with_indent::enum_with_text_field::char_quot se::content::tests::with_indent::enum_with_text_field::char_space se::content::tests::with_indent::enum_with_text_field::enum_unit se::content::tests::with_indent::enum_with_text_field::enum_unit_escaped se::content::tests::with_indent::enum_with_text_field::f32_ se::content::tests::with_indent::enum_with_text_field::f64_ se::content::tests::with_indent::enum_with_text_field::false_ se::content::tests::with_indent::enum_with_text_field::i128_ se::content::tests::with_indent::enum_with_text_field::i16_ se::content::tests::with_indent::enum_with_text_field::i32_ se::content::tests::with_indent::enum_with_text_field::i64_ se::content::tests::with_indent::enum_with_text_field::i8_ se::content::tests::with_indent::enum_with_text_field::isize_ se::content::tests::with_indent::enum_with_text_field::newtype se::content::tests::with_indent::enum_with_text_field::option_none se::content::tests::with_indent::enum_with_text_field::option_some se::content::tests::with_indent::enum_with_text_field::option_some_empty_str se::content::tests::with_indent::enum_with_text_field::seq se::content::tests::with_indent::enum_with_text_field::seq_empty se::content::tests::with_indent::enum_with_text_field::str_escaped se::content::tests::with_indent::enum_with_text_field::str_non_escaped se::content::tests::with_indent::enum_with_text_field::true_ se::content::tests::with_indent::enum_with_text_field::tuple se::content::tests::with_indent::enum_with_text_field::tuple_struct se::content::tests::with_indent::enum_with_text_field::u128_ se::content::tests::with_indent::enum_with_text_field::u16_ se::content::tests::with_indent::enum_with_text_field::u32_ se::content::tests::with_indent::enum_with_text_field::u64_ se::content::tests::with_indent::enum_with_text_field::u8_ se::content::tests::with_indent::enum_with_text_field::unit se::content::tests::with_indent::enum_with_text_field::unit_struct se::content::tests::with_indent::enum_with_text_field::unit_struct_escaped se::content::tests::with_indent::enum_with_text_field::usize_ se::content::tests::with_indent::enum_with_value_field::char_amp se::content::tests::with_indent::enum_with_value_field::char_apos se::content::tests::with_indent::enum_with_value_field::char_gt se::content::tests::with_indent::enum_with_value_field::char_lt se::content::tests::with_indent::enum_with_value_field::char_non_escaped se::content::tests::with_indent::enum_with_value_field::char_quot se::content::tests::with_indent::enum_with_value_field::char_space se::content::tests::with_indent::enum_with_value_field::f32_ se::content::tests::with_indent::enum_with_value_field::f64_ se::content::tests::with_indent::enum_with_value_field::false_ se::content::tests::with_indent::enum_with_value_field::i128_ se::content::tests::with_indent::enum_with_value_field::i16_ se::content::tests::with_indent::enum_with_value_field::i32_ se::content::tests::with_indent::enum_with_value_field::i64_ se::content::tests::with_indent::enum_with_value_field::i8_ se::content::tests::with_indent::enum_with_value_field::isize_ se::content::tests::with_indent::enum_with_value_field::newtype se::content::tests::with_indent::enum_with_value_field::option_none se::content::tests::with_indent::enum_with_value_field::option_some se::content::tests::with_indent::enum_with_value_field::option_some_empty_str se::content::tests::with_indent::enum_with_value_field::seq se::content::tests::with_indent::enum_with_value_field::seq_empty se::content::tests::with_indent::enum_with_value_field::str_escaped se::content::tests::with_indent::enum_with_value_field::str_non_escaped se::content::tests::with_indent::enum_with_value_field::true_ se::content::tests::with_indent::enum_with_value_field::tuple se::content::tests::with_indent::enum_with_value_field::tuple_struct se::content::tests::with_indent::enum_with_value_field::u128_ se::content::tests::with_indent::enum_with_value_field::u16_ se::content::tests::with_indent::enum_with_value_field::u32_ se::content::tests::with_indent::enum_with_value_field::u64_ se::content::tests::with_indent::enum_with_value_field::u8_ se::content::tests::with_indent::enum_with_value_field::usize_ se::content::tests::with_indent::seq se::content::tests::with_indent::text_field::enum_struct se::content::tests::with_indent::tuple se::content::tests::with_indent::tuple_struct se::element::tests::with_indent::text_field::map::char_amp se::element::tests::with_indent::text_field::map::char_apos se::element::tests::with_indent::text_field::map::char_gt se::element::tests::with_indent::text_field::map::char_lt se::element::tests::with_indent::text_field::map::char_non_escaped se::element::tests::with_indent::text_field::map::char_quot se::element::tests::with_indent::text_field::map::char_space se::element::tests::with_indent::text_field::map::enum_unit se::element::tests::with_indent::text_field::map::enum_unit_escaped se::element::tests::with_indent::text_field::map::f32_ se::element::tests::with_indent::text_field::map::f64_ se::element::tests::with_indent::text_field::map::false_ se::element::tests::with_indent::text_field::map::i128_ se::element::tests::with_indent::text_field::map::i16_ se::element::tests::with_indent::text_field::map::i32_ se::element::tests::with_indent::text_field::map::i64_ se::element::tests::with_indent::text_field::map::i8_ se::element::tests::with_indent::text_field::map::isize_ se::element::tests::with_indent::text_field::map::newtype se::element::tests::with_indent::text_field::map::option_some se::element::tests::with_indent::text_field::map::seq se::element::tests::with_indent::text_field::map::str_escaped se::element::tests::with_indent::text_field::map::str_non_escaped se::element::tests::with_indent::text_field::map::true_ se::element::tests::with_indent::text_field::map::tuple se::element::tests::with_indent::text_field::map::tuple_struct se::element::tests::with_indent::text_field::map::u128_ se::element::tests::with_indent::text_field::map::u16_ se::element::tests::with_indent::text_field::map::u32_ se::element::tests::with_indent::text_field::map::u64_ se::element::tests::with_indent::text_field::map::u8_ se::element::tests::with_indent::text_field::map::usize_ se::element::tests::with_indent::text_field::struct_::char_amp se::element::tests::with_indent::text_field::struct_::char_apos se::element::tests::with_indent::text_field::struct_::char_gt se::element::tests::with_indent::text_field::struct_::char_lt se::element::tests::with_indent::text_field::struct_::char_non_escaped se::element::tests::with_indent::text_field::struct_::char_quot se::element::tests::with_indent::text_field::struct_::char_space se::element::tests::with_indent::text_field::struct_::enum_unit se::element::tests::with_indent::text_field::struct_::enum_unit_escaped se::element::tests::with_indent::text_field::struct_::f32_ se::element::tests::with_indent::text_field::struct_::f64_ se::element::tests::with_indent::text_field::struct_::false_ se::element::tests::with_indent::text_field::struct_::i128_ se::element::tests::with_indent::text_field::struct_::i16_ se::element::tests::with_indent::text_field::struct_::i32_ se::element::tests::with_indent::text_field::struct_::i64_ se::element::tests::with_indent::text_field::struct_::i8_ se::element::tests::with_indent::text_field::struct_::isize_ se::element::tests::with_indent::text_field::struct_::newtype se::element::tests::with_indent::text_field::struct_::option_none se::element::tests::with_indent::text_field::struct_::option_some se::element::tests::with_indent::text_field::struct_::option_some_empty_str se::element::tests::with_indent::text_field::struct_::seq se::element::tests::with_indent::text_field::struct_::seq_empty se::element::tests::with_indent::text_field::struct_::str_escaped se::element::tests::with_indent::text_field::struct_::str_non_escaped se::element::tests::with_indent::text_field::struct_::true_ se::element::tests::with_indent::text_field::struct_::tuple se::element::tests::with_indent::text_field::struct_::tuple_struct se::element::tests::with_indent::text_field::struct_::u128_ se::element::tests::with_indent::text_field::struct_::u16_ se::element::tests::with_indent::text_field::struct_::u32_ se::element::tests::with_indent::text_field::struct_::u64_ se::element::tests::with_indent::text_field::struct_::u8_ se::element::tests::with_indent::text_field::struct_::unit se::element::tests::with_indent::text_field::struct_::unit_struct se::element::tests::with_indent::text_field::struct_::unit_struct_escaped se::element::tests::with_indent::text_field::struct_::usize_ se::element::tests::with_indent::value_field::map::char_amp se::element::tests::with_indent::value_field::map::char_apos se::element::tests::with_indent::value_field::map::char_gt se::element::tests::with_indent::value_field::map::char_lt se::element::tests::with_indent::value_field::map::char_non_escaped se::element::tests::with_indent::value_field::map::char_quot se::element::tests::with_indent::value_field::map::char_space se::element::tests::with_indent::value_field::map::f32_ se::element::tests::with_indent::value_field::map::f64_ se::element::tests::with_indent::value_field::map::false_ se::element::tests::with_indent::value_field::map::i128_ se::element::tests::with_indent::value_field::map::i16_ se::element::tests::with_indent::value_field::map::i32_ se::element::tests::with_indent::value_field::map::i64_ se::element::tests::with_indent::value_field::map::i8_ se::element::tests::with_indent::value_field::map::isize_ se::element::tests::with_indent::value_field::map::newtype se::element::tests::with_indent::value_field::map::option_some se::element::tests::with_indent::value_field::map::seq se::element::tests::with_indent::value_field::map::str_escaped se::element::tests::with_indent::value_field::map::str_non_escaped se::element::tests::with_indent::value_field::map::true_ se::element::tests::with_indent::value_field::map::tuple se::element::tests::with_indent::value_field::map::tuple_struct se::element::tests::with_indent::value_field::map::u128_ se::element::tests::with_indent::value_field::map::u16_ se::element::tests::with_indent::value_field::map::u32_ se::element::tests::with_indent::value_field::map::u64_ se::element::tests::with_indent::value_field::map::u8_ se::element::tests::with_indent::value_field::map::usize_ se::element::tests::with_indent::value_field::struct_::char_amp se::element::tests::with_indent::value_field::struct_::char_apos se::element::tests::with_indent::value_field::struct_::char_gt se::element::tests::with_indent::value_field::struct_::char_lt se::element::tests::with_indent::value_field::struct_::char_non_escaped se::element::tests::with_indent::value_field::struct_::char_quot se::element::tests::with_indent::value_field::struct_::char_space se::element::tests::with_indent::value_field::struct_::f32_ se::element::tests::with_indent::value_field::struct_::f64_ se::element::tests::with_indent::value_field::struct_::false_ se::element::tests::with_indent::value_field::struct_::i128_ se::element::tests::with_indent::value_field::struct_::i16_ se::element::tests::with_indent::value_field::struct_::i32_ se::element::tests::with_indent::value_field::struct_::i64_ se::element::tests::with_indent::value_field::struct_::i8_ se::element::tests::with_indent::value_field::struct_::isize_ se::element::tests::with_indent::value_field::struct_::newtype se::element::tests::with_indent::value_field::struct_::option_none se::element::tests::with_indent::value_field::struct_::option_some se::element::tests::with_indent::value_field::struct_::option_some_empty_str se::element::tests::with_indent::value_field::struct_::seq se::element::tests::with_indent::value_field::struct_::seq_empty se::element::tests::with_indent::value_field::struct_::str_escaped se::element::tests::with_indent::value_field::struct_::str_non_escaped se::element::tests::with_indent::value_field::struct_::true_ se::element::tests::with_indent::value_field::struct_::tuple se::element::tests::with_indent::value_field::struct_::tuple_struct se::element::tests::with_indent::value_field::struct_::u128_ se::element::tests::with_indent::value_field::struct_::u16_ se::element::tests::with_indent::value_field::struct_::u32_ se::element::tests::with_indent::value_field::struct_::u64_ se::element::tests::with_indent::value_field::struct_::u8_ se::element::tests::with_indent::value_field::struct_::usize_
Fixes all 208 tests
This PR alternative and more full implementation of the fix for excess indents around text fields. It implements the Python scheme of writing indents:
before
,$text
andafter
$text
andafter
before
and$text
$text
onlyThe implementation never write indent after
Option
fields, becauseOption
can contain a type that is sensible to the surrounding text content, for exampleOption<String>
. Because serde does not provide the type ofNone
when serialize it, it is impossible to be smart here and write indent in one cases but not other, so I choosed the safe variant.As a side effect,
to_writer
,to_writer_with_root
andSerializer
not returnsbool
on success. Thetrue
means that is will be safe to write text (in particular, indent) after the serialized object, it will not be interpreted as a part of content during deserialization. Iffalse
will be returned, then writing additional text content may change data what deserializer will see.Closes #655, closes #814