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

fix: support Nokogiri >= 1.13.7 #14

Closed

Conversation

flavorjones
Copy link

Fixes #12

Nokogiri 1.13.7 changed the typed-data semantics around xmlNode in order to
fully support compaction

see sparklemotion/nokogiri#2579

I've tested this with older versions of Nokogiri, it's backwards-compatible.

However, reproducing Noko_Node_Get_Struct in this project is hacky. Perhaps in a future PR I can help find and compile against nokogiri.h which is findable via Nokogiri::VERSION_INFO metadata (cppflags). Let me know if that would be welcome.

which changed the typed-data semantics around xmlNode in order to
fully support compaction

see sparklemotion/nokogiri#2579
@CLAassistant
Copy link

CLAassistant commented Jul 13, 2022

CLA assistant check
All committers have signed the CLA.

@flavorjones
Copy link
Author

Bumping this in case maintainers want to pull it in.

@maths22
Copy link
Member

maths22 commented Dec 13, 2022

Well good job us not noticing it and good job me redoing this work in #16. That said I am closing this in favor of #16 because that one also adds some better testing

@maths22 maths22 closed this Dec 13, 2022
@flavorjones
Copy link
Author

👍 FWIW we'll also be integration testing against this repo going forward, see sparklemotion/nokogiri#2595

@flavorjones flavorjones deleted the flavorjones-noko-node-fix branch December 13, 2022 22:58
@maths22
Copy link
Member

maths22 commented Dec 14, 2022

@flavorjones That sounds great! Sadly what this gem does is definitely kinda gross by extending a c extension with another c extension and attempting to work with the datastructures of the base c extension, not purely the datastructures of libxml itself. I don't know if there's a better pattern to use for doing what this gem does, but if you (or anyone else) happens to know of one I'd be happy to look into it.

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.

Tests fail with Nokogiri 1.13.7
3 participants