Skip to content

Commit

Permalink
Merge pull request #2177 from sparklemotion/flavorjones-1223-jruby-li…
Browse files Browse the repository at this point in the history
…ne-numbers

(jruby) Fix Node#line

---

**What problem is this PR intended to solve?**

#1223 to make `Node#line` on JRuby smarter.

For background, the Java DOM interfaces don't provide any way for a Node to know or find the line number in the original document at which it appeared.

I'm unhappy with this approach, but was unable to figure out how to make the original approach at #1223 work. If you're familiar with the Java DOM interfaces and want to help, please do.


**Have you included adequate test coverage?**

Yes.


**Does this change affect the behavior of either the C or the Java implementations?**

The Java implementation's behavior should be closer to the C implementation's behavior now.
  • Loading branch information
flavorjones committed Jan 19, 2021
2 parents 614378c + b32c875 commit be33917
Show file tree
Hide file tree
Showing 5 changed files with 1,067 additions and 1,044 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Expand Up @@ -12,6 +12,11 @@ Nokogiri follows [Semantic Versioning](https://semver.org/), please see the [REA
* [JRuby] `{XML,HTML}::Document.parse` now invokes `#initialize` exactly once. Previously `#initialize` was not called, which was a problem for subclassing such as done by `Loofah`.


### Improved

* [JRuby] Update the algorithm used to calculate `Node#line` to be wrong less-often. The underlying parser, Xerces, does not track line numbers, and so we've always used a hacky solution for this method. [[#1223](https://github.com/sparklemotion/nokogiri/issues/1223)]


## v1.11.1 / 2021-01-06

### Fixed
Expand Down
20 changes: 14 additions & 6 deletions ext/java/nokogiri/XmlNode.java
Expand Up @@ -1406,14 +1406,22 @@ private boolean count(Node node, int[] counter) {
if (node == this.node) {
return true;
}

NodeList list = node.getChildNodes();
for (int i=0; i<list.getLength(); i++) {
Node n = list.item(i);
if (n instanceof Text
&& ((Text)n).getData().contains("\n")) {
counter[0] += 1;
for (int jchild=0; jchild<list.getLength(); jchild++) {
Node child = list.item(jchild);

if (child instanceof Text) {
String text = ((Text)child).getData();
int textLength = text.length();
for (int jchar = 0; jchar < textLength; jchar++) {
if (text.charAt(jchar) == '\n') {
counter[0] += 1;
}
}
}
if (count(n, counter)) return true;

if (count(child, counter)) return true;
}
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion nokogiri.gemspec
Expand Up @@ -300,7 +300,7 @@ Gem::Specification.new do |spec|

spec.add_development_dependency("bundler", "~> 2.2")
spec.add_development_dependency("concourse", "~> 0.41")
spec.add_development_dependency("hoe-markdown", "~> 1.1")
spec.add_development_dependency("hoe-markdown", "~> 1.4")
spec.add_development_dependency("minitest", "~> 5.8")
spec.add_development_dependency("minitest-reporters", "~> 1.4")
spec.add_development_dependency("rake", "~> 13.0")
Expand Down
2 changes: 1 addition & 1 deletion rakelib/markdown.rake
@@ -1,2 +1,2 @@
require "hoe/markdown"
Hoe::Markdown::Standalone.new("nokogiri").define_markdown_tasks
Hoe::Markdown::Standalone.new("nokogiri").define_markdown_tasks("CHANGELOG.md")

0 comments on commit be33917

Please sign in to comment.