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 test for existing issue trying to parse the same document twice with the same parser instance #17

Merged
merged 4 commits into from
Jul 25, 2011

Conversation

c4milo
Copy link
Contributor

@c4milo c4milo commented Jul 1, 2011

No description provided.

@c4milo
Copy link
Contributor Author

c4milo commented Jul 5, 2011

after digging a little bit, I found out that libexpat has this function XML_ParserReset, it will be nice to have a binding for it or to use it, somehow, after every parse internally, so that users don't have to worry about it.

@astro
Copy link
Collaborator

astro commented Jul 5, 2011

Cool! More libexpat functionality directly accessible from JS.

Can you implement it yourself? How urgently do you need this (up on npm)? I'm on holiday until next week.

@c4milo
Copy link
Contributor Author

c4milo commented Jul 9, 2011

sadly, no :(. I've been really busy at work. I don't need this urgently though. I managed to create a new fresh instance of the parser everytime.

@astro astro merged commit 1da0cd5 into xmppo:master Jul 25, 2011
@astro
Copy link
Collaborator

astro commented Aug 16, 2011

FYI, the added test is failing for me:
corner cases ✗ parsing twice the same document with the same parser instance should be fine » expected null, got 'junk after document element' // test.js:248 ✗ Broken » 19 honored ∙ 1 broken (1.004s)

Did this ever work? Can we drop it? That error is coming from the innards of libexpat itself.

@c4milo
Copy link
Contributor Author

c4milo commented Aug 16, 2011

It hasn't worked never. I think we can create new parser instances every time for the user or drop it if you want until we have more time to fix it properly.

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

Successfully merging this pull request may close these issues.

2 participants