Skip to content

Eliminate XPathNode#325

Merged
naitoh merged 1 commit into
ruby:masterfrom
tompng:eliminate_xpathnode
Jun 9, 2026
Merged

Eliminate XPathNode#325
naitoh merged 1 commit into
ruby:masterfrom
tompng:eliminate_xpathnode

Conversation

@tompng

@tompng tompng commented Jun 5, 2026

Copy link
Copy Markdown
Member

XPathNode requires :position, but in most case, this :position is not used at all, and just doing useless packing/unpacking.

Extracted from #315

Copilot AI review requested due to automatic review settings June 5, 2026 15:50
@tompng tompng mentioned this pull request Jun 5, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@tompng tompng force-pushed the eliminate_xpathnode branch from 959e98a to 136147c Compare June 7, 2026 14:29
XPathNode requires :position, but in most case, this :position is not used at all, and just doing useless packing/unpacking.
@tompng tompng force-pushed the eliminate_xpathnode branch from 136147c to 767d092 Compare June 7, 2026 16:17
Copilot AI review requested due to automatic review settings June 7, 2026 16:17

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

Comment thread lib/rexml/xpath_parser.rb
Comment on lines 377 to 382
target_context = context
else
target_context = {:size => nodeset.size}
if node.is_a?(XPathNode)
target_context[:node] = node.raw_node
target_context[:index] = node.position
else
target_context[:node] = node
target_context[:index] = 1
end
target_context[:node] = node
target_context[:position] = 1
end
Comment thread lib/rexml/xpath_parser.rb
else
seen = {}.compare_by_identity
raw_nodes = []
nodes = Set.new.compare_by_identity
Comment thread lib/rexml/xpath_parser.rb
true
end
end
new_nodesets = nodesets

@naitoh naitoh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it’s much easier to read now.
Thank you!

@naitoh naitoh merged commit 27e4b54 into ruby:master Jun 9, 2026
72 of 73 checks passed
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.

3 participants