Skip to content

Allow to normalize \Traversable when serializing xml #17984

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

Merged
merged 1 commit into from
Mar 4, 2016

Conversation

GuilhemN
Copy link
Contributor

@GuilhemN GuilhemN commented Mar 2, 2016

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? no
Fixed tickets
License MIT

It's impossible to normalize an object implementing \Traversable when using the XMLEncoder. For example we can't customize the serializer output when serializing a FormInterface instance.

So my proposition is to fix this by using the default XML encoder output only when the serializer can't normalize the data.

@fabpot
Copy link
Member

fabpot commented Mar 2, 2016

ping @dunglas

$serializer = new Serializer(array(new CustomNormalizer()), array('xml' => new XmlEncoder()));
$this->encoder->setSerializer($serializer);

$expected = '<?xml version="1.0"?>'."\n".
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can use the heredoc syntax for clarity.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@dunglas
Copy link
Member

dunglas commented Mar 2, 2016

👍

@GuilhemN
Copy link
Contributor Author

GuilhemN commented Mar 2, 2016

@dunglas should I change something here too ?

@dunglas
Copy link
Member

dunglas commented Mar 3, 2016

It should be tested but it looks like a good idea.

@GuilhemN
Copy link
Contributor Author

GuilhemN commented Mar 3, 2016

@dunglas I already added a test here.

And I didn't completely understand what does selectNodeType so I can't tell if this patch has to be added in it and what would it change.

@fabpot
Copy link
Member

fabpot commented Mar 4, 2016

Thank you @Ener-Getick.

@fabpot fabpot merged commit 97c5d27 into symfony:2.3 Mar 4, 2016
fabpot added a commit that referenced this pull request Mar 4, 2016
…-Getick)

This PR was merged into the 2.3 branch.

Discussion
----------

Allow to normalize \Traversable when serializing xml

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | no
| Fixed tickets |
| License       | MIT

It's impossible to normalize an object implementing ``\Traversable`` when using the ``XMLEncoder``. For example we can't customize the serializer output when serializing a ``FormInterface`` instance.

So my proposition is to fix this by using the default XML encoder output only when the serializer can't normalize the data.

Commits
-------

97c5d27 Allow to normalize \Traversable
@fabpot
Copy link
Member

fabpot commented Mar 4, 2016

@Ener-Getick I've merged this one as this is a bug fix. Feel free to open a new PR for the other change if it makes sense.

@GuilhemN GuilhemN deleted the XMLENCODER branch March 4, 2016 07:30
@fabpot fabpot mentioned this pull request Mar 13, 2016
xabbuh added a commit to FriendsOfSymfony/FOSRestBundle that referenced this pull request Apr 25, 2016
This PR was merged into the 2.0 branch.

Discussion
----------

Removes a hack

Removes a hack as symfony/symfony#17984 has been merged.

Commits
-------

7bcd70b Remove a hack
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants