Skip to content
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][XmlEncoder] Allow removing empty tags in generated XML #20524

Closed

Conversation

amoiraud
Copy link
Contributor

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #20398
License MIT
Doc PR ~

Allow a new option in $context of XmlEncoder.php to remove empty tags if $context['remove_empty_tags'] setted to true, changing this :

    <node>
         <subnode>Value</subnode>
         <emptysubnode/>
    </node>

To this :

    <node>
         <subnode>Value</subnode>
    </node>

@@ -393,7 +393,9 @@ private function buildXml(\DOMNode $parentNode, $data, $xmlRootNodeName = null)
} elseif (is_numeric($key) || !$this->isElementNameValid($key)) {
$append = $this->appendNode($parentNode, $data, 'item', $key);
} else {
$append = $this->appendNode($parentNode, $data, $key);
if ($data !== null || !isset($this->context['remove_empty_tags']) || $this->context['remove_empty_tags'] === false) {
Copy link
Member

Choose a reason for hiding this comment

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

should be elseif

Copy link
Member

Choose a reason for hiding this comment

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

and please also use a Yoda condition here: null !== $data

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's done :

} elseif (null !== $data || !isset($this->context['remove_empty_tags']) || false === $this->context['remove_empty_tags']) {
    $append = $this->appendNode($parentNode, $data, $key);
}

$expected = '<?xml version="1.0"?>'."\n".
'<response><person><firstname>Peter</firstname></person></response>'."\n";

$context = array(
Copy link
Member

Choose a reason for hiding this comment

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

Can be inline

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right i have made the modification

@dunglas
Copy link
Member

dunglas commented Nov 15, 2016

👍

@dunglas
Copy link
Member

dunglas commented Dec 6, 2016

Can you open a doc PR please @amoiraud

@nicolas-grekas nicolas-grekas added this to the 3.x milestone Dec 6, 2016
@dunglas
Copy link
Member

dunglas commented Dec 6, 2016

Thank you @amoiraud.

@dunglas dunglas closed this Dec 6, 2016
dunglas added a commit that referenced this pull request Dec 6, 2016
…generated XML (amoiraud)

This PR was squashed before being merged into the 3.3-dev branch (closes #20524).

Discussion
----------

[Serializer][XmlEncoder] Allow removing empty tags in generated XML

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #20398
| License       | MIT
| Doc PR        | ~

Allow a new option in $context of XmlEncoder.php to remove empty tags if $context['remove_empty_tags'] setted to true, changing this :

```xml
    <node>
         <subnode>Value</subnode>
         <emptysubnode/>
    </node>
```

To this :

```xml
    <node>
         <subnode>Value</subnode>
    </node>
```

Commits
-------

0cb4d8e [Serializer][XmlEncoder] Allow removing empty tags in generated XML
@amoiraud
Copy link
Contributor Author

amoiraud commented Dec 6, 2016

Hi @dunglas
I'm OK to open a doc PR but can you tell me where should I write this documentation ?
Because in the actual documentation I haven't found any mention of the $context param
Thanks

@dunglas
Copy link
Member

dunglas commented Dec 7, 2016

@amoiraud, I suggest to add a new section about the XmlEncoder in https://github.com/symfony/symfony-docs/blob/master/components/serializer.rst and list its available options.

@nicolas-grekas nicolas-grekas modified the milestones: 3.x, 3.3 Mar 24, 2017
@fabpot fabpot mentioned this pull request May 1, 2017
javiereguiluz added a commit to symfony/symfony-docs that referenced this pull request Jan 5, 2018
… javiereguiluz)

This PR was submitted for the master branch but it was merged into the 3.3 branch instead (closes #7231).

Discussion
----------

Add documentation for XmlEncoder context param

Add documentation for XmlEncoder context param

Related to : symfony/symfony#20524
This fixes #7227

Commits
-------

5e23045 Minor tweaks
adbc7b2 Minor rewords
2dc6ed3 Minor syntax issue
6050370 Minor syntax issues and some rewordings
2f3cf02 Correcting my bad english with a bilingual friend
60c7657 Add XmlEncoder documentation with $context available options
04af434 erratum
c83e59d Add XmlEncoder documentation with $context available options
sandergo90 pushed a commit to sandergo90/symfony that referenced this pull request Jul 4, 2019
…ags in generated XML (amoiraud)

This PR was squashed before being merged into the 3.3-dev branch (closes symfony#20524).

Discussion
----------

[Serializer][XmlEncoder] Allow removing empty tags in generated XML

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | symfony#20398
| License       | MIT
| Doc PR        | ~

Allow a new option in $context of XmlEncoder.php to remove empty tags if $context['remove_empty_tags'] setted to true, changing this :

```xml
    <node>
         <subnode>Value</subnode>
         <emptysubnode/>
    </node>
```

To this :

```xml
    <node>
         <subnode>Value</subnode>
    </node>
```

Commits
-------

0cb4d8e [Serializer][XmlEncoder] Allow removing empty tags in generated XML
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.

None yet

5 participants