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

Corner cases with #text #90

Closed
jzelinskie opened this issue Mar 6, 2021 · 4 comments
Closed

Corner cases with #text #90

jzelinskie opened this issue Mar 6, 2021 · 4 comments

Comments

@jzelinskie
Copy link

Hey there! I'm using this library for my multi-format jq tool faq. Thanks again for such a great library.
faq converts XML into JSON, processes it with libjq, and converts it back into XML.

A user reported an issue jzelinskie/faq#85 which highlights two bits of surprising behavior that can be reproduced as follows:

xmap, err := mxj.NewMapXml([]byte(`<p>test<span>hello</span></p>`), true)
if err != nil { panic(err) }

jsonBytes, err := xmap.Json()
if err != nil { panic(err) }

// expected: string(jsonBytes) == `{"p": {"#text": "test", "span": "hello"}}`
// got: string(jsonBytes) == `{"p": "test"}`
jmap, err := mxj.NewMapJson(`{"p": {"#text": "test", "span": "hello" }}`)
if err != nil { panic(err) }

xmlBytes, err := jmap.Xml()
if err != nil { panic(err) }

// expected: string(xmlBytes) == `<p>text<span>hello</span></p>`
// got: panic: invalid attribute key label: #text - due to attributes not being prefixed

Do you think these are bugs in your parser?
I could see an argument against supporting the last example since #text is being synthesized.

@clbanning
Copy link
Owner

I see what might be reasonably doable.

As noted in the README, this package was developed to manage data streams, not HTML. So it's really not aware of style element tags, etc.

clbanning added a commit that referenced this issue Mar 8, 2021
@clbanning
Copy link
Owner

clbanning commented Mar 8, 2021

v2.5.4 (at tip) addresses part 1. Will get to part 2 later today or tomorrow.

@jzelinskie
Copy link
Author

Thanks for the extremely quick updates!

As noted in the README, this package was developed to manage data streams, not HTML. So it's really not aware of style element tags, etc.

No problem! I only used HTML as an example here -- the issue on my repo is for parsing XML documents.

clbanning added a commit that referenced this issue Mar 9, 2021
@clbanning
Copy link
Owner

v2.5.5 (at tip) address part 2.

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

No branches or pull requests

2 participants