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

Added support for using the keys of an array as XML tag. #129

Merged
merged 1 commit into from
Jul 2, 2012

Conversation

Spea
Copy link
Contributor

@Spea Spea commented Jun 13, 2012

As discussed in #59 this PR adds the possibility to use the keys of an array as XML tags.

Example:

Currently I only check for numeric keys to avoid problems with naming convention. But when a key looks like "1_foo" an exception is thrown. Possible solutions could be:

  • Catch the DOMException and set the tag name back to "entry"
  • Use PCRE to check if the key is a valid tag name

What do you think?

@schmittjoh
Copy link
Owner

What does the Symfony Serializer do here? Can we mirror its behavior for this case? // cc @Seldaek

@Spea
Copy link
Contributor Author

Spea commented Jun 29, 2012

The symfony seralizer basically uses PCRE to check if an element name is valid -> https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Serializer/Encoder/XmlEncoder.php#L196

@Seldaek
Copy link
Contributor

Seldaek commented Jun 30, 2012

Great stuff, thanks

@Spea
Copy link
Contributor Author

Spea commented Jun 30, 2012

@schmittjoh I guess this is a yes from @Seldaek ;) Will adjust it sometime today.

@Spea
Copy link
Contributor Author

Spea commented Jul 2, 2012

Updated

*
* @return Boolean
*/
final protected function isElementNameValid($name)
Copy link
Owner

Choose a reason for hiding this comment

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

I think we can use private here as the class is not designed for extending.

@schmittjoh
Copy link
Owner

How about renaming @XmlArray to @XmlKeyValuePairs?

@Spea
Copy link
Contributor Author

Spea commented Jul 2, 2012

Yeah sure, @XmlKeyValuePairs makes it more clear what the annoation is used to do. I'll refactor this

@Spea
Copy link
Contributor Author

Spea commented Jul 2, 2012

Before I forget it. Right now the tag name entry will be used when the tag name is invalid, should I add a key attribute in this case so the key can still be accessed? Another option would be, to make this choosable by the user via an option in the annoataion

@schmittjoh
Copy link
Owner

Wouldn't an exception be more appropriate as this seems like something that a developer would want to fix, no?

@Spea
Copy link
Contributor Author

Spea commented Jul 2, 2012

I'm not sure about this, an Exception seems a little bit too "hard" for me, but on the other hand the developer could also be confused when he used an invalid tag name and he can't find it in the generated output.

@Seldaek
Copy link
Contributor

Seldaek commented Jul 2, 2012

I don't think an exception is appropriate at all. As I said on #59 (comment) - if it is user data that you can't control, it just might break everything in production, which is much worse than just "losing" one key IMO.

@Spea
Copy link
Contributor Author

Spea commented Jul 2, 2012

Good point, I will keep it as it is right now.

@Spea
Copy link
Contributor Author

Spea commented Jul 2, 2012

Refactored annotation name and slightly updated doc.

~~~~~~~
This allows you to use the keys of an array as xml tags.
**Note:** When a key is an invalid xml tag name (e.g. 1_foo) the tag name *entry* will be used instead of the key.
Copy link
Owner

Choose a reason for hiding this comment

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

This should be

.. note ::

    When a key ...

to be formatted correctly on the website.

@Spea
Copy link
Contributor Author

Spea commented Jul 2, 2012

Fixed. Do you want me to squash the commits?

~~~~~~~~~~~~~~~~~
This allows you to use the keys of an array as xml tags.
.. note ::
Copy link
Owner

Choose a reason for hiding this comment

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

There must be a blank line before and after the .. note :: line.

@schmittjoh
Copy link
Owner

Yeah, please squash them.

@Spea
Copy link
Contributor Author

Spea commented Jul 2, 2012

Squashed and fixed.

@schmittjoh schmittjoh merged commit 61f3965 into schmittjoh:master Jul 2, 2012
@schmittjoh
Copy link
Owner

Thanks, merged!

@schmittjoh
Copy link
Owner

btw, can you add support for this to the XML and YAML metadata drivers as well?

@Spea
Copy link
Contributor Author

Spea commented Jul 2, 2012

Sure, will add them tomorrow.

@naroga
Copy link

naroga commented Sep 30, 2015

This PR is merged, but using jms/serializer-bundle still gives me <entry>...</entry> instead of the array keys.

Am I doing something wrong? My array is also dinamically generated from user data, and I cannot use objects.

@MConrotte
Copy link

+1 Idem to naroga, in xml I have always ... instead of the array keys.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants