Implement serializing CDATA

[Ninja3047]

Closes: #353

[Mingun]

Looks good, but need to check how lists will be serialized and would we able to deserialize them back.

[codecov-commenter]

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

Codecov Report

❌ Patch coverage is 93.33333% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.74%. Comparing base (38b44d4) to head (c67c861).
⚠️ Report is 36 commits behind head on master.

Files with missing lines	Patch %	Lines
src/se/mod.rs	33.33%	4 Missing ⚠️
src/se/simple_type.rs	97.05%	2 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #904      +/-   ##
==========================================
+ Coverage   55.52%   58.74%   +3.21%     
==========================================
  Files          42       42              
  Lines       15511    15256     -255     
==========================================
+ Hits         8613     8962     +349     
+ Misses       6898     6294     -604     

Flag	Coverage Δ
unittests	58.74% <93.33%> (+3.21%)	⬆️

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.

[Mingun]

It would be better, if this serialized as one CDATA section. Probably, that is possible?

[Ninja3047]

yeah agreed, need to figure out exactly how to do that nicely with serde

[Ninja3047]

alright changed it so it gets serialized to one CDATA section
one thing i'm not clear on is if there are spaces inside the items, should the spaces get escaped even when it's wrapped with a CDATA

[JeanMertz]

Great to see this being merged soon 👍

I gave this implementation a try. One thing that confused me was that the example implies that the CDATA wrapper is only applied to fields that are marked with #[serde(rename = "$text")], but I had other String fields that also got the wrapper, even though they didn't have the $text attribute applied to them. Is this a bug, or working as expected?

[JeanMertz]

Also, I think the following test case should be added:

baz ]] baz 

As I think(?) it generates incorrect XML:

<![CDATA[baz ]] baz ]]>

EDIT: Actually, I think it's correct, with ]]> in the middle it does split up the CDATA:

baz ]]> baz 
<![CDATA[baz ]]]]><![CDATA[> baz ]]>

[Mingun]

Great to see this being merged soon

I'm a bit busy last time, but I will try to find time to review this ASAP.

Is this a bug, or working as expected?

It should use CDATA as well.

EDIT: Actually, I think it's correct, with ]]> in the middle it does split up the CDATA:

Yes, having ]] which is not part of ]]> inside CDATA is totally acceptable.

[JeanMertz]

It should use CDATA as well.

Wouldn't it be preferable to only use CDATA if $text is defined? At least until there is an auto variant?

[Mingun]

I think not. Imagine, if you want to store HTML in the String field.

[JeanMertz]

I think not. Imagine, if you want to store HTML in the String field.

Well, then you are free to mark it as $text, but if you want to store a string that you know is safe, this PR does not give you a way to serialize that string without the CDATA tags (assuming there are other fields in the data that you do want to include the CDATA tags, which is why you'd set TextFormat to CData).

I'm not saying it's a blocker, I was mostly curious why it isn't only applied to $text fields.