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

Parse xml string from RAM #1

Closed
wants to merge 4 commits into from

Conversation

luc-tielen
Copy link
Contributor

Added support for parsing of an XML string from a Rust str (previously only bindings were provided to parse from a file).
Also fixed the tests (hopefully in a correct way 😄), cleaned up some warnings, probably a remainder from older Rust versions..

P.S. some tests seem to fail sometimes, may be worth looking into? (even if you hate writing them ;))

@luc-tielen
Copy link
Contributor Author

I noticed the build on travis failed, the test cases seem to fail every now and then.. if you run the code yourself though and cargo test a few times, should work..
For example, I enabled travis on my fork and it succeeded there.. tests could probably use some serious refactoring..

@dginev
Copy link
Member

dginev commented Dec 18, 2015

Hi @primordus ! I didn't get a reply to my last email to @jfschaefer , so I am unsure if he has time to maintain this wrapper. For now I have gone and added the extra features I need to a fork here:
https://github.com/dginev/rust-libxml which is still incomplete but a few steps further along.

It ended up in a significant rewrite (you may be heading in a different direction). I'll add the equivalent changes to your PR changes there by hand, while we're waiting for @jfschaefer to take a look.

@luc-tielen
Copy link
Contributor Author

Hello @dginev, that's great news!
I'll look into your fork of the code later, didn't pay much attention to what other people already did since I just needed this function for my application.
This change might not be so difficult to port over because it's only 1 new rust binding for a libxml2 function 😄!

@dginev
Copy link
Member

dginev commented Dec 18, 2015

Hi again - indeed useful API methods to add!

I am quite happy to report I have integrated them with my fork, see commit: https://github.com/dginev/rust-libxml/commit/63f11995e83166d840b6b458f45bd6393c096cc1

I have refactored the wrapper to have the XML/HTML distinction happen as a property of the Parser object, which feels a bit cleaner. And made some of the setup/cleanup magic a bit more automatic, which seems to be largely working out as intended. Feel free to take a look and use that if you prefer.

@luc-tielen
Copy link
Contributor Author

Thanks, I will probably use your fork instead then! Hope @jfschaefer some day will pull in our changes :)..

jfschaefer pushed a commit that referenced this pull request Feb 6, 2016
@dginev
Copy link
Member

dginev commented Feb 24, 2016

This PR got merged a bit indirectly, so I'll close here - the commits are already in master.

@dginev dginev closed this Feb 24, 2016
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.

2 participants