-
-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
[Serializer] Add documentation about a new XmlEncoder CDATA wrapping opt-out context option #18155
[Serializer] Add documentation about a new XmlEncoder CDATA wrapping opt-out context option #18155
Conversation
…option (AndoniLarz) This PR was merged into the 6.4 branch. Discussion ---------- [Serializer] Add `XmlEncoder::CDATA_WRAPPING` context option | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | Fix #26168 | License | MIT | Doc PR | symfony/symfony-docs#18155 Usage example : ```php $data = [ 'Hello' => 'Paul & Martha <or Me>', ]; return new Response( $this->serializer->serialize( $data, XmlEncoder::FORMAT, [ XmlEncoder::CDATA_WRAPPING => false ] ), headers: [ 'Content-Type' => 'application/xml' ] ); ``` will output ```xml <?xml version="1.0"?> <response><Hello>Paul & Martha <or Me></Hello></response> ``` instead of ```xml <?xml version="1.0"?> <response><Hello><![CDATA[Paul & Martha <or Me>]]></Hello></response> ``` --- As stated in [the following comment from PHP.net](https://www.php.net/manual/fr/domdocument.createtextnode.php#116351) : PHP automatically handles the escape when calling `DOMDocument::createTextNode` which is done in [::appendText](https://github.com/symfony/symfony/blob/6.3/src/Symfony/Component/Serializer/Encoder/XmlEncoder.php#L193). `::appendText` is always called if `XmlEncoder::CDATA_WRAPPING => false`. --- Questions: * Should we allow for custom XPath to be wrapped in CDATA while others are simply escaped ? * Should we allow for a callback function to be used in place of a boolean (`callable(string $val): bool`) * [W3Schools#Entity References](https://www.w3schools.com/xml/xml_syntax.asp#:~:text=/note%3E-,Entity%20References,-Some%20characters%20have) states that `'` and `"` might also be escaped. Based on this should we instead provide a list of characters to escape/wrap ? (currently it would be set to `['<', '>', '&']`) --- - [X] Create the documentation PR - [X] Check Fabbot feedback to make sure code is well written Commits ------- 049982d [Serializer] Add `XmlEncoder::CDATA_WRAPPING` context option
9437594
to
f0a98ad
Compare
Hi @xabbuh @OskarStark , I fixed the conflicts and updated the context option name following the code that has been merged. |
``cdata_wrapping`` If set to false, will not wrap any value ``true`` | ||
containing one of the following characters ( | ||
``<``, ``>``, ``&``) in `a CDATA section`_ like | ||
following : ``<![CDATA[...]]>`` | ||
============================== ================================================= ========================== | ||
|
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.
Please add a versionadded
directive, you can find some as example in other files
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.
Done
Please target 7.1 branch for now, thanks |
Even though the feature has been added in 6.4 ? |
Ah sorry this is about another option. Sorry I mixed things up. No in this car 6.4 is fine, but a versionadded would be helpful at all:
|
Alright no problem. I'll make the changes soon |
Open for finishing this PR by handling my comments? |
It should be good now 😄 |
…opt-out context option
77e3a00
to
705d5d2
Compare
Thank you Andoni and congratulations on your first contribution to the Symfony documentation 🎉 |
Add documentation of symfony/symfony#49893 which has been merged in 6.4.
TODO :
enable_cdata_wrapping
=>cdata_wrapping
)