Skip to content

Commit 2f4489d

Browse files
tompngtompng
committed
pull320
1 parent 2732516 commit 2f4489d

3 files changed

Lines changed: 67 additions & 40 deletions

File tree

‎lib/rexml/xpath_parser.rb‎

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ def expr( path_stack, nodeset, context=nil )
325325
nodesets
326326
end
327327
when :preceding_sibling
328-
nodeset = step(path_stack, order: :reverse) do
328+
nodeset = step(path_stack) do
329329
nodesets = []
330330
nodeset.each do |node|
331331
raw_node = node.raw_node
@@ -342,7 +342,7 @@ def expr( path_stack, nodeset, context=nil )
342342
nodesets
343343
end
344344
when :preceding
345-
nodeset = step(path_stack, order: :reverse) do
345+
nodeset = step(path_stack) do
346346
unnode(nodeset) do |node|
347347
preceding(node)
348348
end
@@ -460,7 +460,7 @@ def expr( path_stack, nodeset, context=nil )
460460
leave(:expr, path_stack, nodeset) if @debug
461461
end
462462

463-
def step(path_stack, any_type: :element, order: :forward)
463+
def step(path_stack, any_type: :element)
464464
nodesets = yield
465465
begin
466466
enter(:step, path_stack, nodesets) if @debug
@@ -470,21 +470,19 @@ def step(path_stack, any_type: :element, order: :forward)
470470
predicate_expression = path_stack.shift.dclone
471471
nodesets = evaluate_predicate(predicate_expression, nodesets)
472472
end
473-
if nodesets.size == 1
474-
ordered_nodeset = nodesets[0]
475-
else
476-
seen = {}.compare_by_identity
477-
raw_nodes = []
478-
nodesets.each do |nodeset|
479-
nodeset.each do |node|
480-
raw_node = node.respond_to?(:raw_node) ? node.raw_node : node
481-
next if seen.key?(raw_node)
482-
seen[raw_node] = true
483-
raw_nodes << raw_node
484-
end
473+
474+
seen = {}.compare_by_identity
475+
raw_nodes = []
476+
nodesets.each do |nodeset|
477+
nodeset.each do |node|
478+
raw_node = node.respond_to?(:raw_node) ? node.raw_node : node
479+
next if seen.key?(raw_node)
480+
seen[raw_node] = true
481+
raw_nodes << raw_node
485482
end
486-
ordered_nodeset = sort(raw_nodes, order)
487483
end
484+
ordered_nodeset = sort(raw_nodes)
485+
488486
new_nodeset = []
489487
ordered_nodeset.each do |node|
490488
new_nodeset << XPathNode.new(node, position: new_nodeset.size + 1)
@@ -667,7 +665,9 @@ def leave(tag, *args)
667665
# in and out of function calls. If I knew what the index of the nodes was,
668666
# I wouldn't have to do this. Maybe add a document IDX for each node?
669667
# Problems with mutable documents. Or, rewrite everything.
670-
def sort(array_of_nodes, order)
668+
def sort(array_of_nodes)
669+
return array_of_nodes if array_of_nodes.size <= 1
670+
671671
new_arry = []
672672
array_of_nodes.each { |node|
673673
node_idx = []
@@ -679,11 +679,7 @@ def sort(array_of_nodes, order)
679679
new_arry << [ node_idx.reverse, node ]
680680
}
681681
ordered = new_arry.sort_by do |index, node|
682-
if order == :forward
683-
index
684-
else
685-
index.map(&:-@)
686-
end
682+
index
687683
end
688684
ordered.collect do |_index, node|
689685
node

‎test/xpath/test_axis_preceding_sibling.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ def test_preceding_sibling_axis
2323
assert_equal "6", context.attributes["id"]
2424

2525
prev = XPath.first(context, "preceding-sibling::f")
26-
assert_equal "5", prev.attributes["id"]
26+
assert_equal "3", prev.attributes["id"]
2727

2828
prev = XPath.first(context, "preceding-sibling::f[1]")
2929
assert_equal "5", prev.attributes["id"]

‎test/xpath/test_base.rb‎

Lines changed: 48 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -382,32 +382,39 @@ def test_preceding
382382
start = XPath.first( d, "/a/b[@id='1']" )
383383
assert_equal 'b', start.name
384384
c = XPath.first( start, "preceding::c" )
385-
assert_equal '2', c.attributes['id']
385+
assert_equal '0', c.attributes['id']
386386

387-
c1, c0 = XPath.match( d, "/a/b/c[@id='2']/preceding::node()" )
388-
assert_equal '1', c1.attributes['id']
387+
b0, b2, c0, c1 = XPath.match( d, "/a/b/c[@id='2']/preceding::node()" )
388+
assert_equal 'b', b0.name
389+
assert_equal 'b', b2.name
390+
assert_equal 'c', c0.name
391+
assert_equal 'c', c1.name
392+
393+
assert_equal '0', b0.attributes['id']
394+
assert_equal '2', b2.attributes['id']
389395
assert_equal '0', c0.attributes['id']
396+
assert_equal '1', c1.attributes['id']
390397

391-
c2, c1, c0, b, b2, b0 = XPath.match( start, "preceding::node()" )
398+
b0, b2, b, c0, c1, c2 = XPath.match( start, "preceding::node()" )
392399

393-
assert_equal 'c', c2.name
394-
assert_equal 'c', c1.name
395400
assert_equal 'c', c0.name
396-
assert_equal 'b', b.name
397-
assert_equal 'b', b2.name
401+
assert_equal 'c', c1.name
402+
assert_equal 'c', c2.name
398403
assert_equal 'b', b0.name
404+
assert_equal 'b', b2.name
405+
assert_equal 'b', b.name
399406

400-
assert_equal '2', c2.attributes['id']
401-
assert_equal '1', c1.attributes['id']
402407
assert_equal '0', c0.attributes['id']
403-
assert b.attributes.empty?
404-
assert_equal '2', b2.attributes['id']
408+
assert_equal '1', c1.attributes['id']
409+
assert_equal '2', c2.attributes['id']
405410
assert_equal '0', b0.attributes['id']
411+
assert_equal '2', b2.attributes['id']
412+
assert b.attributes.empty?
406413

407414
d = REXML::Document.new("<a><b/><c/><d/></a>")
408415
matches = REXML::XPath.match(d, "/a/d/preceding::node()")
409-
assert_equal("c", matches[0].name)
410-
assert_equal("b", matches[1].name)
416+
assert_equal("b", matches[0].name)
417+
assert_equal("c", matches[1].name)
411418

412419
s = "<a><b><c id='1'/></b><b><b><c id='2'/><c id='3'/></b><c id='4'/></b><c id='NOMATCH'><c id='5'/></c></a>"
413420
d = REXML::Document.new(s)
@@ -425,7 +432,7 @@ def test_preceding_multiple
425432
XML
426433
doc = REXML::Document.new(source)
427434
matches = REXML::XPath.match(doc, "a/d/preceding::*")
428-
assert_equal(["d", "c", "b"], matches.map(&:name))
435+
assert_equal(["b", "c", "d"], matches.map(&:name))
429436
end
430437

431438
def test_following_multiple
@@ -498,7 +505,7 @@ def test_preceding_sibling_across_multiple_nodes
498505
XML
499506
doc = REXML::Document.new(source)
500507
matches = REXML::XPath.match(doc, "a/b/x/preceding-sibling::*")
501-
assert_equal(["e", "d", "c"], matches.map(&:name))
508+
assert_equal(["c", "d", "e"], matches.map(&:name))
502509
end
503510

504511
def test_preceding_sibling_within_single_node
@@ -511,7 +518,7 @@ def test_preceding_sibling_within_single_node
511518
XML
512519
doc = REXML::Document.new(source)
513520
matches = REXML::XPath.match(doc, "a/b/x/preceding-sibling::*")
514-
assert_equal(["e", "x", "d", "c"], matches.map(&:name))
521+
assert_equal(["c", "d", "x", "e"], matches.map(&:name))
515522
end
516523

517524
def test_following
@@ -883,6 +890,30 @@ def test_ordering
883890
r.collect {|element| element.attribute("id").value})
884891
end
885892

893+
def test_order_consistency
894+
doc = REXML::Document.new('<a><b><c><d id="1"><d id="2"/></d></c></b></a>')
895+
assert_equal('a', REXML::XPath.first(doc, '//d/ancestor::*').name)
896+
assert_equal('a', REXML::XPath.first(doc, '//d[@id="2"]/ancestor::*').name)
897+
assert_equal(%w[a b c d], REXML::XPath.match(doc, '//d/ancestor::*').map(&:name))
898+
assert_equal(%w[a b c d], REXML::XPath.match(doc, '//d[@id="2"]/ancestor::*').map(&:name))
899+
900+
assert_equal('a', REXML::XPath.first(doc, '//d/ancestor-or-self::*').name)
901+
assert_equal('a', REXML::XPath.first(doc, '//d[@id="2"]/ancestor-or-self::*').name)
902+
assert_equal(%w[a b c d d], REXML::XPath.match(doc, '//d/ancestor-or-self::*').map(&:name))
903+
assert_equal(%w[a b c d d], REXML::XPath.match(doc, '//d[@id="2"]/ancestor-or-self::*').map(&:name))
904+
905+
doc = REXML::Document.new('<a><b/><c/><d id="1"/><d id="2"/></a>')
906+
assert_equal('b', REXML::XPath.first(doc, '//d/preceding::*').name)
907+
assert_equal('b', REXML::XPath.first(doc, '//d[@id="2"]/preceding::*').name)
908+
assert_equal(%w[b c d], REXML::XPath.match(doc, '//d/preceding::*').map(&:name))
909+
assert_equal(%w[b c d], REXML::XPath.match(doc, '//d[@id="2"]/preceding::*').map(&:name))
910+
911+
assert_equal('b', REXML::XPath.first(doc, '//d/preceding-sibling::*').name)
912+
assert_equal('b', REXML::XPath.first(doc, '//d[@id="2"]/preceding-sibling::*').name)
913+
assert_equal(%w[b c d], REXML::XPath.match(doc, '//d/preceding-sibling::*').map(&:name))
914+
assert_equal(%w[b c d], REXML::XPath.match(doc, '//d[@id="2"]/preceding-sibling::*').map(&:name))
915+
end
916+
886917
def test_descendant_or_self_ordering
887918
source = "<a>
888919
<b>

0 commit comments

Comments
 (0)