Skip to content
This repository has been archived by the owner on Aug 26, 2023. It is now read-only.

Use Nokogiri::XML::Node#line= to set the line number on nodes #122

Closed
3 of 4 tasks
stevecheckoway opened this issue Aug 10, 2019 · 7 comments
Closed
3 of 4 tasks
Assignees

Comments

@stevecheckoway
Copy link
Collaborator

stevecheckoway commented Aug 10, 2019

If sparklemotion/nokogiri#1918 (or something like it) is accepted, then https://github.com/rubys/nokogumbo/blob/master/ext/nokogumbo/nokogumbo.c#L320 should be modified to use #line= if the node supports it.

This can be tested using rb_respond_to but apparently there's a bug on Ruby 2.3 https://bugs.ruby-lang.org/issues/12907 that causes that to always return true.

@fulldecent
Copy link
Contributor

Here is a way to do this on a branch #130 so effort can proceed.

@stevecheckoway
Copy link
Collaborator Author

I updated the issue with the list of steps that needs to happen. Looks like this should be resolved soon.

@flavorjones
Copy link
Collaborator

FWIW I'm planning to release Nokogiri v1.11.0 with #1918 by the end of December (to coincide with Ruby 2.7 release).

@stevecheckoway
Copy link
Collaborator Author

I kind of forgot about this issue. Is this worth doing pre-merge or is it better to wait?

@flavorjones
Copy link
Collaborator

🤣 I forgot to follow up, too. I think it probably makes sense to wait, since the current version of Nokogumbo will do the right thing if installed against a modern version of Nokogiri. But if you feel strongly about it, I won't object to shipping another Nokogumbo release with this change (we just won't port it into Nokogiri because we'll be compiling everything together going forward).

@stevecheckoway
Copy link
Collaborator Author

I think it makes sense to wait.

@flavorjones
Copy link
Collaborator

This has been upstreamed in Nokogiri v1.12.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants