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

Improve roundtrip for some combinations of parser options #461

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

j-santander
Copy link

The problem I was trying to solve was to parse some XML data, modify it and encode it back, preserving the order of the XML elements.

The parser have options for that (preserveChildrenOrder), but the builder is not able to follow the new structure.

The PR addresses the object structure, addresses the cases where the explicitChildren or preserveChildrenOrder options are set. Two new test cases have been added for them.

This is done not relying in additional builder options. This way the parser can accept objects produced by parsers with different options.

I've also refactored slightly the builder loop making these metadata keys ($, $$, _) addressed before going into the key loop (allowing to take the decision of not looking into the remaining redundant keys)

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 97.479% when pulling f16cad0 on j-santander:master into 049242d on Leonidas-from-XIV:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 97.479% when pulling f16cad0 on j-santander:master into 049242d on Leonidas-from-XIV:master.

@coveralls
Copy link

coveralls commented Jun 7, 2018

Coverage Status

Coverage increased (+0.7%) to 98.333% when pulling b0c51f8 on j-santander:master into 049242d on Leonidas-from-XIV:master.

@j-santander
Copy link
Author

I'll try to add some more test later....

@brospars
Copy link

brospars commented Aug 6, 2020

This works great and should be merged !

if (typeof entry === "object") {
if (Object.keys(entry).length === 1) {
name = Object.keys(entry)[0];
element = render(element.ele(name), entry[name]).up();
Copy link

Choose a reason for hiding this comment

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

This throws an error for self-closing elements without attributes because entry for <foo /> looks like:

{
  "#name": "foo"
}

I think if ('#name' in entry) should be checked first.

Can you add round-trip test cases for self-closing elements, both with and without attributes?

@samatcolumn
Copy link

I know this PR is super old but I would still love to see it merged!

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.

6 participants