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

Support streams of multiple documents #16

Open
nichtich opened this issue Jul 17, 2019 · 6 comments
Open

Support streams of multiple documents #16

nichtich opened this issue Jul 17, 2019 · 6 comments
Labels
format:xml kind:feature Request for new functionality status:help-wanted Accepting PRs

Comments

@nichtich
Copy link
Contributor

$ echo '{"x":1}{"x":2}' | oq . -o json
{
  "x": 1
}
{
  "x": 2
}
$ echo '{"x":1}{"x":2}' | oq . -o xml
oq error: unexpected token '{' at 4:1

Expected:

$ echo '{"x":1}{"x":2}' | oq . -o xml
<?xml version="1.0" encoding="UTF-8"?>
<root>
  <x>1</x>
</root>
<?xml version="1.0" encoding="UTF-8"?>
<root>
  <x>2</x>
</root>

Sure this makes more sense with #14. YAML on the other hand supports multiple documents concatenated.

sorry for coming up with all of these edge cases, I'm actually looking for them 🙈

@Blacksmoke16 Blacksmoke16 added format:xml kind:feature Request for new functionality status:todo labels Jul 17, 2019
@Blacksmoke16
Copy link
Owner

Blacksmoke16 commented Jul 18, 2019

@nichtich You seem to be well versed in XML. What would you expect the output of

{
  "type": "Polygon",
  "coordinates": [
    [
      [
        -122.422003528252475,
        37.808480096967251,
        0.0
      ],
      [
        -122.422003528252475,
        37.808480096967251,
        0.0
      ]
    ]
  ]
}

to be in XML?

@nichtich
Copy link
Contributor Author

I created another issue for this and related examples. I first thought about not supporting nested arrays at all but the current solution with <item> as default XML element name also makes sense, it only needs to be configurable (--xml-item NAME)?

@Blacksmoke16
Copy link
Owner

Cool thanks. The issue with being able to specify that is it would be global, which is probably fine? I.e. in a 3D array like that, there wouldn't be a way to target a specific level of the nested array. The way around this is using a transformation to give keys to those values, as it'll use the key name if there is one.

@Blacksmoke16 Blacksmoke16 added this to the 0.2.0 milestone Jul 18, 2019
@nichtich
Copy link
Contributor Author

The issue with being able to specify that is it would be global, which is probably fine?

Yes. Alternatives I can think of is to ignore nested array elements, flatten nested arrays or to raise an error, but your approach looks more useful. The only argument against is more difficult XML reading to ensure round-tripping conversion (XML->JSON->XML->JSON should be equal to XML->JSON).

I worked a lot with XML and created a similar JSON/XML mapping tool (see Catmandu::XML and other Catmandu modules) but there is no silver bullet.

@Blacksmoke16
Copy link
Owner

Yea I can't imagine being able to convert formats and have everything be 1:1 conversions. Are always going to be some edge cases IMO. It helps that you can use jq to transform the JSON to get it into a better structure for serialization to XML.

@Blacksmoke16 Blacksmoke16 added status:help-wanted Accepting PRs and removed status:wip labels Jul 20, 2019
@Blacksmoke16 Blacksmoke16 removed this from the 0.2.0 milestone Jul 20, 2019
@Blacksmoke16
Copy link
Owner

The current implementation of Crystal's JSON::PullParser allows the consumption of only 1 JSON document at a time. I don't think this would be trivial to do. I'll keep this open and will accept PRs if someone wants to tackle it.

Possibly can revisit it in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
format:xml kind:feature Request for new functionality status:help-wanted Accepting PRs
Projects
None yet
Development

No branches or pull requests

2 participants