wip

Author: flavorjones

ext/nokogiri/xml_node_set.c

@@ -471,21 +471,4 @@ void
 noko_init_xml_node_set(void)
 {
   cNokogiriXmlNodeSet = rb_define_class_under(mNokogiriXml, "NodeSet", rb_cArray);
-
-  /* rb_define_alloc_func(cNokogiriXmlNodeSet, allocate); */
-
-  /* rb_define_method(cNokogiriXmlNodeSet, "length", length, 0); */
-  /* rb_define_method(cNokogiriXmlNodeSet, "[]", slice, -1); */
-  /* rb_define_method(cNokogiriXmlNodeSet, "slice", slice, -1); */
-  /* rb_define_method(cNokogiriXmlNodeSet, "push", push, 1); */
-  /* rb_define_method(cNokogiriXmlNodeSet, "|", rb_xml_node_set_union, 1); */
-  /* rb_define_method(cNokogiriXmlNodeSet, "-", minus, 1); */
-  /* rb_define_method(cNokogiriXmlNodeSet, "unlink", unlink_nodeset, 0); */
-  /* rb_define_method(cNokogiriXmlNodeSet, "to_a", to_array, 0); */
-  /* rb_define_method(cNokogiriXmlNodeSet, "dup", duplicate, 0); */
-  /* rb_define_method(cNokogiriXmlNodeSet, "delete", delete, 1); */
-  /* rb_define_method(cNokogiriXmlNodeSet, "&", intersection, 1); */
-  /* rb_define_method(cNokogiriXmlNodeSet, "include?", include_eh, 1); */
-
-  /* decorate = rb_intern("decorate"); */
 }

ext/nokogiri/xml_xpath_context.c

@@ -182,6 +182,16 @@ xpath2ruby(xmlXPathObjectPtr xpath_object, xmlXPathContextPtr xpath_context)
 }
 
 
+static VALUE
+ruby2xpath_node_set_append(RB_BLOCK_CALL_FUNC_ARGLIST(rb_node, wrapped_c_node_set))
+{
+  xmlNodeSetPtr c_node_set = (xmlNodeSetPtr)wrapped_c_node_set;
+  xmlNodePtr c_node;
+  Data_Get_Struct(rb_node, xmlNode, c_node);
+  xmlXPathNodeSetAddUnique(c_node_set, c_node);
+  return Qnil;
+}
+
 /*
  *  convert a Ruby object into an XPath object of the appropriate type.
  *  raises an exception if no conversion was possible.
@@ -209,28 +219,11 @@ ruby2xpath(VALUE rb_object, xmlXPathContextPtr xpath_context)
       break;
     case T_ARRAY:
       {
-        xmlNodeSetPtr c_node_set = NULL;
-        VALUE rb_node_set;
-        VALUE args[2];
-
-        assert(xpath_context->doc);
-        assert(DOC_RUBY_OBJECT_TEST(xpath_context->doc));
-
-        args[0] = DOC_RUBY_OBJECT(xpath_context->doc);
-        args[1] = rb_object;
-        rb_node_set = rb_class_new_instance(2, args, cNokogiriXmlNodeSet);
-        Data_Get_Struct(rb_node_set, xmlNodeSet, c_node_set);
+        xmlNodeSetPtr c_node_set = xmlXPathNodeSetCreate(NULL);
+        rb_block_call(rb_object, rb_intern("each"), 0, NULL, ruby2xpath_node_set_append, (VALUE)c_node_set);
         result = xmlXPathWrapNodeSet(xmlXPathNodeSetMerge(NULL, c_node_set));
       }
       break;
-    case T_DATA:
-      if (rb_obj_is_kind_of(rb_object, cNokogiriXmlNodeSet)) {
-        xmlNodeSetPtr c_node_set = NULL;
-        Data_Get_Struct(rb_object, xmlNodeSet, c_node_set);
-        /* Copy the node set, otherwise it will get GC'd. */
-        result = xmlXPathWrapNodeSet(xmlXPathNodeSetMerge(NULL, c_node_set));
-        break;
-      }
     default:
       rb_raise(rb_eRuntimeError, "Invalid return type");
   }

lib/nokogiri/xml/namespace.rb

@@ -6,6 +6,11 @@ class Namespace
       include Nokogiri::XML::PP::Node
       attr_reader :document
 
+      # Returns true if this is a Namespace
+      def namespace?
+        true
+      end
+
       private
 
       def inspect_attributes

lib/nokogiri/xml/node_set.rb

@@ -13,6 +13,7 @@ class NodeSet < ::Array
       attr_accessor :document
 
       # Create a NodeSet with +document+ defaulting to +list+
+      # TODO: test that it can only contain Node and Namespace objects
       def initialize(document, list = [])
         super()
         @document = document
@@ -21,40 +22,6 @@ def initialize(document, list = [])
         yield self if block_given?
       end
 
-      # ###
-      # # Get the first element of the NodeSet.
-      # def first(n = nil)
-      #   return self[0] unless n
-
-      #   list = []
-      #   [n, length].min.times { |i| list << self[i] }
-      #   list
-      # end
-
-      # ###
-      # # Get the last element of the NodeSet.
-      # def last
-      #   self[-1]
-      # end
-
-      # ###
-      # # Is this NodeSet empty?
-      # def empty?
-      #   length == 0
-      # end
-
-      # ###
-      # # Returns the index of the first node in self that is == to +node+ or meets the given block. Returns nil if no match is found.
-      # def index(node = nil)
-      #   if node
-      #     warn("given block not used") if block_given?
-      #     each_with_index { |member, j| return j if member == node }
-      #   elsif block_given?
-      #     each_with_index { |member, j| return j if yield(member) }
-      #   end
-      #   nil
-      # end
-
       ###
       # Insert +datum+ before the first Node in this NodeSet
       # TODO: this method only makes sense in the context of siblings
@@ -69,8 +36,6 @@ def after(datum)
         last.after(datum)
       end
 
-      # alias_method :<<, :push
-
       #  call-seq:
       #    unlink
       #
@@ -235,17 +200,6 @@ def remove_attr(name)
       end
       alias_method :remove_attribute, :remove_attr
 
-      # ###
-      # # Iterate over each node, yielding  to +block+
-      # def each
-      #   return to_enum unless block_given?
-
-      #   0.upto(length - 1) do |x|
-      #     yield self[x]
-      #   end
-      #   self
-      # end
-
       ###
       # Get the inner text of all contained Node objects
       #
@@ -305,41 +259,6 @@ def to_xml(*args)
         map { |x| x.to_xml(*args) }.join
       end
 
-      # alias_method :size, :length
-      # alias_method :to_ary, :to_a
-
-      # ###
-      # # Removes the last element from set and returns it, or +nil+ if
-      # # the set is empty
-      # def pop
-      #   return nil if length == 0
-
-      #   delete(last)
-      # end
-
-      # ###
-      # # Returns the first element of the NodeSet and removes it.  Returns
-      # # +nil+ if the set is empty.
-      # def shift
-      #   return nil if length == 0
-
-      #   delete(first)
-      # end
-
-      # ###
-      # # Equality -- Two NodeSets are equal if the contain the same number
-      # # of elements and if each element is equal to the corresponding
-      # # element in the other NodeSet
-      # def ==(other)
-      #   return false unless other.is_a?(Nokogiri::XML::NodeSet)
-      #   return false unless length == other.length
-
-      #   each_with_index do |node, i|
-      #     return false unless node == other[i]
-      #   end
-      #   true
-      # end
-
       ###
       # Returns a new NodeSet containing all the children of all the nodes in
       # the NodeSet
@@ -351,28 +270,45 @@ def children
         node_set
       end
 
-      # ###
-      # # Returns a new NodeSet containing all the nodes in the NodeSet
-      # # in reverse order
-      # def reverse
-      #   node_set = NodeSet.new(document)
-      #   (length - 1).downto(0) do |x|
-      #     node_set.push(self[x])
-      #   end
-      #   node_set
-      # end
-
-      # ###
-      # # Return a nicely formated string representation
-      # def inspect
-      #   "[#{map(&:inspect).join(", ")}]"
-      # end
+      ###
+      # Returns a new NodeSet containing all the nodes in the NodeSet
+      # in reverse order
+      def reverse
+        NodeSet.new(document, super)
+      end
+
+      # TODO: document
+      def difference(*args)
+        NodeSet.new(document, super)
+      end
+      alias_method :-, :difference
 
       # TODO: document
       # TODO: can raise TypeError (was formerly ArgumentError)
+      def union(other)
+        NodeSet.new(document, super)
+      end
       alias_method :+, :union
       alias_method :|, :union
 
+      # TODO: document
+      def intersection(*args)
+        NodeSet.new(document, super)
+      end
+      alias_method :&, :intersection
+
+      # TODO: document
+      def slice(*args)
+        result = super
+        Array === result ? NodeSet.new(document, result) : result
+      end
+      alias_method :[], :slice
+
+      # TODO: document
+      def dup
+        NodeSet.new(document, super)
+      end
+
       IMPLIED_XPATH_CONTEXTS = [".//", "self::"].freeze # :nodoc:
     end
   end

test/xml/test_node_set.rb

@@ -306,8 +306,11 @@ def awesome(ns)
         describe "#pop" do
           it "returns the last element and mutates the set" do
             set = xml.xpath("//employee")
-            last = set.last
-            assert_equal(last, set.pop)
+            length = set.length
+            expected = set.last
+            actual = set.pop
+            assert_equal(expected, actual)
+            assert_equal(length - 1, set.length)
           end
 
           it "returns nil for an empty set" do
@@ -319,8 +322,11 @@ def awesome(ns)
         describe "#shift" do
           it "returns the first element and mutates the set" do
             set = xml.xpath("//employee")
-            first = set.first
-            assert_equal(first, set.shift)
+            length = set.length
+            expected = set.first
+            actual = set.shift
+            assert_equal(expected, actual)
+            assert_equal(length - 1, set.length)
           end
 
           it "returns nil for an empty set" do
@@ -470,6 +476,7 @@ def awesome(ns)
 
         describe "#delete" do
           it "raises ArgumentError when given an invalid argument" do
+            skip "TODO: arrays just do the right thing here"
             employees = xml.search("//employee")
             positions = xml.search("//position")
 
@@ -483,6 +490,7 @@ def awesome(ns)
             length = employees.length
 
             result = employees.delete(wally)
+            assert_instance_of(Nokogiri::XML::NodeSet, result)
             assert_equal(result, wally)
             refute_includes(employees, wally)
             assert_equal(length - 1, employees.length)
@@ -623,7 +631,7 @@ def awesome(ns)
           employees_len = employees.length
           women_len = women.length
 
-          assert_raises(ArgumentError) { employees - women.first }
+          assert_raises(TypeError) { employees - women.first }
 
           result = employees - women
           assert_equal(employees_len, employees.length)
@@ -830,6 +838,7 @@ def awesome(ns)
           assert_instance_of(Nokogiri::XML::NodeSet, children)
 
           reversed = children.reverse
+          assert_instance_of(Nokogiri::XML::NodeSet, reversed)
           assert_equal(reversed[0], children[4])
           assert_equal(reversed[1], children[3])
           assert_equal(reversed[2], children[2])
@@ -850,6 +859,17 @@ def awesome!; end
           assert_respond_to(new_set, :awesome!)
         end
 
+        it "node_set_clone_result_has_document_and_is_decorated" do
+          x = Module.new do
+            def awesome!; end
+          end
+          util_decorate(xml, x)
+          node_set = xml.css("address")
+          new_set  = node_set.clone
+          assert_equal(node_set.document, new_set.document)
+          assert_respond_to(new_set, :awesome!)
+        end
+
         it "node_set_union_result_has_document_and_is_decorated" do
           x = Module.new do
             def awesome!; end
@@ -858,7 +878,7 @@ def awesome!; end
           node_set1 = xml.css("address")
           node_set2 = xml.css("address")
           new_set = node_set1 | node_set2
-          assert_equal(node_set1.document, new_set.document)
+          # assert_equal(node_set1.document, new_set.document) # TODO: drop NodeSet#document ?
           assert_respond_to(new_set, :awesome!)
         end