From 749c5821bba44c635f941b81a14bc5fef5863b26 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sat, 14 Dec 2024 15:09:14 -0500 Subject: [PATCH 1/3] style: some cleanup in xml_xpath_context.c --- ext/nokogiri/xml_xpath_context.c | 41 ++++++++++---------------------- 1 file changed, 13 insertions(+), 28 deletions(-) diff --git a/ext/nokogiri/xml_xpath_context.c b/ext/nokogiri/xml_xpath_context.c index e9dc675c95c..69b6993bdb5 100644 --- a/ext/nokogiri/xml_xpath_context.c +++ b/ext/nokogiri/xml_xpath_context.c @@ -132,12 +132,7 @@ rb_xml_xpath_context_register_ns(VALUE rb_context, VALUE prefix, VALUE uri) { xmlXPathContextPtr c_context; - TypedData_Get_Struct( - rb_context, - xmlXPathContext, - &xml_xpath_context_type, - c_context - ); + TypedData_Get_Struct(rb_context, xmlXPathContext, &xml_xpath_context_type, c_context); xmlXPathRegisterNs(c_context, (const xmlChar *)StringValueCStr(prefix), @@ -160,12 +155,7 @@ rb_xml_xpath_context_register_variable(VALUE rb_context, VALUE name, VALUE value xmlXPathContextPtr c_context; xmlXPathObjectPtr xmlValue; - TypedData_Get_Struct( - rb_context, - xmlXPathContext, - &xml_xpath_context_type, - c_context - ); + TypedData_Get_Struct(rb_context, xmlXPathContext, &xml_xpath_context_type, c_context); xmlValue = xmlXPathNewCString(StringValueCStr(value)); @@ -428,34 +418,29 @@ rb_xml_xpath_context_evaluate(int argc, VALUE *argv, VALUE rb_context) static VALUE rb_xml_xpath_context_new(VALUE klass, VALUE rb_node) { - xmlNodePtr node; + xmlNodePtr c_node; xmlXPathContextPtr c_context; VALUE rb_context; - Noko_Node_Get_Struct(rb_node, xmlNode, node); + Noko_Node_Get_Struct(rb_node, xmlNode, c_node); #if LIBXML_VERSION < 21000 /* deprecated in 40483d0 */ xmlXPathInit(); #endif - c_context = xmlXPathNewContext(node->doc); - c_context->node = node; + c_context = xmlXPathNewContext(c_node->doc); + c_context->node = c_node; xmlXPathRegisterNs(c_context, NOKOGIRI_PREFIX, NOKOGIRI_URI); xmlXPathRegisterNs(c_context, NOKOGIRI_BUILTIN_PREFIX, NOKOGIRI_BUILTIN_URI); - xmlXPathRegisterFuncNS( - c_context, - (const xmlChar *)"css-class", - NOKOGIRI_BUILTIN_URI, - xpath_builtin_css_class - ); - xmlXPathRegisterFuncNS( - c_context, - (const xmlChar *)"local-name-is", - NOKOGIRI_BUILTIN_URI, - xpath_builtin_local_name_is - ); + + xmlXPathRegisterFuncNS(c_context, + (const xmlChar *)"css-class", NOKOGIRI_BUILTIN_URI, + xpath_builtin_css_class); + xmlXPathRegisterFuncNS(c_context, + (const xmlChar *)"local-name-is", NOKOGIRI_BUILTIN_URI, + xpath_builtin_local_name_is); rb_context = TypedData_Wrap_Struct( klass, From a21aad5424414f304a02648fdf77d8e1b49dd702 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sat, 14 Dec 2024 11:34:29 -0500 Subject: [PATCH 2/3] prefactor: re-order searchable private methods --- lib/nokogiri/xml/searchable.rb | 68 +++++++++++++++++----------------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/lib/nokogiri/xml/searchable.rb b/lib/nokogiri/xml/searchable.rb index 6d0717106b2..dc203d08b69 100644 --- a/lib/nokogiri/xml/searchable.rb +++ b/lib/nokogiri/xml/searchable.rb @@ -207,35 +207,26 @@ def >(selector) # rubocop:disable Naming/BinaryOperatorParameterName private - def css_internal(node, rules, handler, ns) - xpath_internal(node, css_rules_to_xpath(rules, ns), handler, ns, nil) - end - - def xpath_internal(node, paths, handler, ns, binds) - document = node.document - return NodeSet.new(document) unless document - - if paths.length == 1 - return xpath_impl(node, paths.first, handler, ns, binds) + def extract_params(params) # :nodoc: + handler = params.find do |param| + ![Hash, String, Symbol].include?(param.class) end + params -= [handler] if handler - NodeSet.new(document) do |combined| - paths.each do |path| - xpath_impl(node, path, handler, ns, binds).each { |set| combined << set } - end + hashes = [] + while Hash === params.last || params.last.nil? + hashes << params.pop + break if params.empty? end - end + ns, binds = hashes.reverse - def xpath_impl(node, path, handler, ns, binds) - ctx = XPathContext.new(node) - ctx.register_namespaces(ns) - path = path.gsub("xmlns:", " :") unless Nokogiri.uses_libxml? + ns ||= document.root&.namespaces || {} - binds&.each do |key, value| - ctx.register_variable(key.to_s, value) - end + [params, handler, ns, binds] + end - ctx.evaluate(path, handler) + def css_internal(node, rules, handler, ns) + xpath_internal(node, css_rules_to_xpath(rules, ns), handler, ns, nil) end def css_rules_to_xpath(rules, ns) @@ -254,22 +245,31 @@ def xpath_query_from_css_rule(rule, ns) end.join(" | ") end - def extract_params(params) # :nodoc: - handler = params.find do |param| - ![Hash, String, Symbol].include?(param.class) + def xpath_internal(node, paths, handler, ns, binds) + document = node.document + return NodeSet.new(document) unless document + + if paths.length == 1 + return xpath_impl(node, paths.first, handler, ns, binds) end - params -= [handler] if handler - hashes = [] - while Hash === params.last || params.last.nil? - hashes << params.pop - break if params.empty? + NodeSet.new(document) do |combined| + paths.each do |path| + xpath_impl(node, path, handler, ns, binds).each { |set| combined << set } + end end - ns, binds = hashes.reverse + end - ns ||= document.root&.namespaces || {} + def xpath_impl(node, path, handler, ns, binds) + ctx = XPathContext.new(node) + ctx.register_namespaces(ns) + path = path.gsub("xmlns:", " :") unless Nokogiri.uses_libxml? - [params, handler, ns, binds] + binds&.each do |key, value| + ctx.register_variable(key.to_s, value) + end + + ctx.evaluate(path, handler) end end end From 5822532f72244cbcd60bf71a81d701b6d7b4fc53 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sat, 14 Dec 2024 15:04:46 -0500 Subject: [PATCH 3/3] prefactor: extract XPathContext#register_variables --- lib/nokogiri/xml/searchable.rb | 6 ++---- lib/nokogiri/xml/xpath_context.rb | 17 ++++++++++++++--- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/lib/nokogiri/xml/searchable.rb b/lib/nokogiri/xml/searchable.rb index dc203d08b69..f1b720e4cf9 100644 --- a/lib/nokogiri/xml/searchable.rb +++ b/lib/nokogiri/xml/searchable.rb @@ -263,11 +263,9 @@ def xpath_internal(node, paths, handler, ns, binds) def xpath_impl(node, path, handler, ns, binds) ctx = XPathContext.new(node) ctx.register_namespaces(ns) - path = path.gsub("xmlns:", " :") unless Nokogiri.uses_libxml? + ctx.register_variables(binds) - binds&.each do |key, value| - ctx.register_variable(key.to_s, value) - end + path = path.gsub("xmlns:", " :") unless Nokogiri.uses_libxml? ctx.evaluate(path, handler) end diff --git a/lib/nokogiri/xml/xpath_context.rb b/lib/nokogiri/xml/xpath_context.rb index 5274a5d7097..033af04b024 100644 --- a/lib/nokogiri/xml/xpath_context.rb +++ b/lib/nokogiri/xml/xpath_context.rb @@ -6,9 +6,20 @@ class XPathContext ### # Register namespaces in +namespaces+ def register_namespaces(namespaces) - namespaces.each do |k, v| - k = k.to_s.gsub(/.*:/, "") # strip off 'xmlns:' or 'xml:' - register_ns(k, v) + namespaces.each do |key, value| + key = key.to_s.gsub(/.*:/, "") # strip off 'xmlns:' or 'xml:' + + register_ns(key, value) + end + end + + def register_variables(binds) + return if binds.nil? + + binds.each do |key, value| + key = key.to_s + + register_variable(key, value) end end end