-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Serializer] XmlEncoder: Make load flags configurable #18036
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
Conversation
dunglas
commented
Mar 6, 2016
Q | A |
---|---|
Branch | master |
Bug fix? | no |
New feature? | yes |
BC breaks? | no |
Deprecations? | no |
Tests pass? | yes |
Fixed tickets | #17987 |
License | MIT |
Doc PR | todo |
{ | ||
$this->rootNodeName = $rootNodeName; | ||
$this->loadOptions = $loadOptions ?: LIBXML_NONET | LIBXML_NOBLANKS; |
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.
$this->loadOptions = null !== $loadOptions ?: LIBXML_NONET | LIBXML_NOBLANKS;
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
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.
oh wait, the suggestion was bad, it must now be $this->loadOptions = null !== $loadOptions ? $loadOptions : LIBXML_NONET | LIBXML_NOBLANKS;
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 copy/pasted without care, sorry. It's fixed now.
@@ -80,8 +83,10 @@ public function decode($data, $format, array $context = array()) | |||
$disableEntities = libxml_disable_entity_loader(true); | |||
libxml_clear_errors(); | |||
|
|||
$loadOptions = isset($context['xml_load_options']) ? $context['xml_load_options'] : $this->loadOptions; |
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 it really a good idea to make this overridable through the context?
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 think you're right, I'll update the PR.
ping @symfony/deciders |
|
||
/** | ||
* Construct new XmlEncoder and allow to change the root node element name. | ||
* | ||
* @param string $rootNodeName | ||
* @param string $rootNodeName | ||
* @param int|null $loadOptions |
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.
We should add a short explanation what $loadOptions
means (something like "a bit field of LIBXML_ constants").
👍 (apart from my last minor comment) |
@xabbuh comment added |
Thank you @dunglas. |
…(dunglas) This PR was squashed before being merged into the 3.1-dev branch (closes #18036). Discussion ---------- [Serializer] XmlEncoder: Make load flags configurable | Q | A | ------------- | --- | Branch | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #17987 | License | MIT | Doc PR | todo Commits ------- 0826068 [Serializer] XmlEncoder: Make load flags configurable
Great 👍 Can someone point me in the direction how to override this settings for |
Add this class with the new options as a new encoder: http://symfony.com/doc/current/cookbook/serializer.html#adding-normalizers-and-encoders |