First pass at using the NodeSet class.

The documentation still leaves a lot to be desired and so does the API. There
also appears to be a problem where NodeSet#remove doesn't properly remove all
nodes from a set. Outside of that we're making slow progress towards a proper
DOM API.

lib/oga/xml/document.rb

@@ -4,10 +4,6 @@ module Oga
     # Class used for storing information about an entire XML document. This
     # includes the doctype, XML declaration, child nodes and more.
     #
-    # @!attribute [rw] children
-    #  The child nodes of the document.
-    #  @return [Array]
-    #
     # @!attribute [rw] doctype
     #  The doctype of the document.
     #  @return [Oga::XML::Doctype]
@@ -17,19 +13,40 @@ module Oga
     #  @return [Oga::XML::XmlDeclaration]
     #
     class Document
-      attr_accessor :children, :doctype, :xml_declaration
+      attr_accessor :doctype, :xml_declaration
 
       ##
       # @param [Hash] options
       #
-      # @option options [Array] :children
+      # @option options [Oga::XML::NodeSet] :children
       # @option options [Oga::XML::Doctype] :doctype
       # @option options [Oga::XML::XmlDeclaration] :xml_declaration
       #
       def initialize(options = {})
-        @children        = options[:children] || []
         @doctype         = options[:doctype]
         @xml_declaration = options[:xml_declaration]
+
+        self.children = options[:children] if options[:children]
+      end
+
+      ##
+      # @return [Oga::XML::NodeSet]
+      #
+      def children
+        return @children ||= NodeSet.new([], self)
+      end
+
+      ##
+      # Sets the child nodes of the document.
+      #
+      # @param [Oga::XML::NodeSet|Array] nodes
+      #
+      def children=(nodes)
+        if nodes.is_a?(NodeSet)
+          @children = nodes
+        else
+          @children = NodeSet.new(nodes, self)
+        end
       end
 
       ##

lib/oga/xml/node.rb

@@ -4,39 +4,79 @@ module Oga
     # A single, generic XML node that can have a parent, next, previous and
     # child nodes.
     #
-    # @!attribute [rw] parent
-    #  @return [Oga::XML::Node]
-    #
-    # @!attribute [rw] children
-    #  @return [Array<Oga::XML::Node>]
-    #
-    # @!attribute [rw] next
-    #  @return [Oga::XML::Node]
-    #
-    # @!attribute [rw] previous
-    #  @return [Oga::XML::Node]
-    #
     # @!attribute [rw] node_set
     #  @return [Oga::XML::NodeSet]
     #
     class Node
-      attr_accessor :parent, :children, :next, :previous, :node_set
+      attr_accessor :node_set
 
       ##
       # @param [Hash] options
       #
-      # @option options [Array] :children The child nodes of the current
+      # @option options [Oga::XML::NodeSet] :node_set The node set that this
-      #  element.
+      #  node belongs to.
       #
-      # @option options [Oga::XML::Node] :parent The parent node.
+      # @option options [Oga::XML::NodeSet|Array] :children The child nodes of
-      # @option options [Oga::XML::Node] :next The following node.
+      #  the current node.
-      # @option options [Oga::XML::Node] :previous The previous node.
       #
       def initialize(options = {})
-        @parent   = options[:parent]
+        @node_set = options[:node_set]
-        @children = options[:children] || []
+
-        @next     = options[:next]
+        self.children = options[:children] if options[:children]
-        @previous = options[:previous]
+      end
+
+      ##
+      # Returns the child nodes of the current node.
+      #
+      # @return [Oga::XML::NodeSet]
+      #
+      def children
+        return @children ||= NodeSet.new([], self)
+      end
+
+      ##
+      # Sets the child nodes of the element.
+      #
+      # @param [Oga::XML::NodeSet|Array] nodes
+      #
+      def children=(nodes)
+        if nodes.is_a?(NodeSet)
+          @children = nodes
+        else
+          @children = NodeSet.new(nodes, self)
+        end
+      end
+
+      ##
+      # Returns the parent node of the current node.
+      #
+      # @return [Oga::XML::Node]
+      #
+      def parent
+        return node_set.owner
+      end
+
+      ##
+      # Returns the preceding node, or nil if there is none.
+      #
+      # @return [Oga::XML::Node]
+      #
+      def previous
+        index = node_set.index(self) - 1
+
+        return index >= 0 ? node_set[index] : nil
+      end
+
+      ##
+      # Returns the following node, or nil if there is none.
+      #
+      # @return [Oga::XML::Node]
+      #
+      def next
+        index  = node_set.index(self) + 1
+        length = node_set.length
+
+        return index <= length ? node_set[index] : nil
       end
 
       ##

lib/oga/xml/node_set.rb

@@ -6,14 +6,48 @@ module Oga
     # of a node (besides just containing it). This allows the nodes to query
     # their previous and next elements.
     #
+    # There are two types of sets:
+    #
+    # 1. Regular node sets
+    # 2. Owned node sets
+    #
+    # Both behave similar to Ruby's Array class. The difference between an
+    # owned and regular node set is that an owned set modifies nodes that are
+    # added or removed by certain operations. For example, when a node is added
+    # to an owned set the `node_set` attribute of said node points to the set
+    # it was just added to.
+    #
+    # Owned node sets are used when building a DOM tree with
+    # {Oga::XML::Parser}. By taking ownership of nodes in a set Oga makes it
+    # possible to use these sets as following:
+    #
+    #     document = Oga::XML::Document.new
+    #     element  = Oga::XML::Element.new
+    #
+    #     document.children << element
+    #
+    #     element.node_set == document.children # => true
+    #
+    # If ownership was not handled then you'd have to manually set the
+    # `element` variable's `node_set` attribute after pushing it into a set.
+    #
+    # @!attribute [rw] owner
+    #  @return [Oga::XML::Node]
+    #
     class NodeSet
       include Enumerable
 
+      attr_accessor :owner
+
       ##
       # @param [Array] nodes The nodes to add to the set.
+      # @param [Oga::XML::NodeSet] owner The owner of the set.
       #
-      def initialize(nodes = [])
+      def initialize(nodes = [], owner = nil)
         @nodes = nodes
+        @owner = owner
+
+        @nodes.each { |node| take_ownership(node) }
       end
 
       ##
@@ -72,6 +106,8 @@ module Oga
       #
       def push(node)
         @nodes << node
+
+        take_ownership(node)
       end
 
       alias_method :<<, :push
@@ -83,6 +119,8 @@ module Oga
       #
       def unshift(node)
         @nodes.unshift(node)
+
+        take_ownership(node)
       end
 
       ##
@@ -91,7 +129,11 @@ module Oga
       # @return [Oga::XML::Node]
       #
       def shift
-        return @nodes.shift
+        node = @nodes.shift
+
+        remove_ownership(node)
+
+        return node
       end
 
       ##
@@ -100,7 +142,11 @@ module Oga
       # @return [Oga::XML::Node]
       #
       def pop
-        return @nodes.pop
+        node = @nodes.pop
+
+        remove_ownership(node)
+
+        return node
       end
 
       ##
@@ -170,13 +216,25 @@ module Oga
         return text
       end
 
+      private
+
       ##
-      # Takes ownership of all the nodes in the current set.
+      # Takes ownership of the given node. This only occurs when the current
+      # set has an owner.
       #
-      def associate_nodes!
+      # @param [Oga::XML::Node] node
-        @nodes.each do |node|
+      #
-          node.node_set = self
+      def take_ownership(node)
+        node.node_set = self if owner
       end
+
+      ##
+      # Removes ownership of the node if it belongs to the current set.
+      #
+      # @param [Oga::XML::Node] node
+      #
+      def remove_ownership(node)
+        node.node_set = nil if node.node_set == self
       end
     end # NodeSet
   end # XML

lib/oga/xml/parser.y

@@ -266,8 +266,6 @@ Unexpected #{name} with value #{value.inspect} on line #{@line}:
       end
     end
 
-    link_children(document)
-
     return document
   end
 
@@ -335,8 +333,6 @@ Unexpected #{name} with value #{value.inspect} on line #{@line}:
   def on_element_children(element, children = [])
     element.children = children
 
-    link_children(element)
-
     return element
   end
 
@@ -362,31 +358,4 @@ Unexpected #{name} with value #{value.inspect} on line #{@line}:
     return attrs
   end
 
-  private
-
-  ##
-  # Links the child nodes together by setting attributes such as the
-  # previous, next and parent node.
-  #
-  # @param [Oga::XML::Node] node
-  #
-  def link_children(node)
-    amount = node.children.length
-
-    node.children.each_with_index do |child, index|
-      prev_index = index - 1
-      next_index = index + 1
-
-      if index > 0
-        child.previous = node.children[prev_index]
-      end
-
-      if next_index <= amount
-        child.next = node.children[next_index]
-      end
-
-      child.parent = node
-    end
-  end
-
 # vim: set ft=racc:

spec/oga/xml/document_spec.rb

@@ -3,19 +3,19 @@ require 'spec_helper'
 describe Oga::XML::Document do
   context 'setting attributes' do
     example 'set the child nodes via the constructor' do
-      children = [Oga::XML::Comment.new(:text => 'foo')]
+      child    = Oga::XML::Comment.new(:text => 'foo')
-      document = described_class.new(:children => children)
+      document = described_class.new(:children => [child])
 
-      document.children.should == children
+      document.children[0].should == child
     end
 
     example 'set the child nodes via a setter' do
-      children = [Oga::XML::Comment.new(:text => 'foo')]
+      child    = Oga::XML::Comment.new(:text => 'foo')
       document = described_class.new
 
-      document.children = children
+      document.children = [child]
 
-      document.children.should == children
+      document.children[0].should == child
     end
   end
 

spec/oga/xml/node_set_spec.rb

@@ -11,6 +11,20 @@ describe Oga::XML::NodeSet do
 
       described_class.new([node]).length.should == 1
     end
+
+    example 'set the owner of a set' do
+      node = Oga::XML::Element.new
+      set  = described_class.new([], node)
+
+      set.owner.should == node
+    end
+
+    example 'take ownership of the nodes when the set has an owner' do
+      node = Oga::XML::Element.new
+      set  = described_class.new([node], node)
+
+      node.node_set.should == set
+    end
   end
 
   context '#each' do
@@ -79,6 +93,15 @@ describe Oga::XML::NodeSet do
 
       @set.length.should == 1
     end
+
+    example 'take ownership of a node if the set has an owner' do
+      child      = Oga::XML::Element.new
+      @set.owner = Oga::XML::Element.new
+
+      @set.push(child)
+
+      child.node_set.should == @set
+    end
   end
 
   context '#unshift' do
@@ -94,12 +117,22 @@ describe Oga::XML::NodeSet do
 
       @set.first.should == n2
     end
+
+    example 'take ownership of a node if the set has an owner' do
+      child      = Oga::XML::Element.new
+      @set.owner = Oga::XML::Element.new
+
+      @set.unshift(child)
+
+      child.node_set.should == @set
+    end
   end
 
   context '#shift' do
     before do
-      @n1  = Oga::XML::Element.new(:name => 'a')
+      owner = Oga::XML::Element.new
-      @set = described_class.new([@n1])
+      @n1   = Oga::XML::Element.new
+      @set  = described_class.new([@n1], owner)
     end
 
     example 'remove the node from the set' do
@@ -110,12 +143,19 @@ describe Oga::XML::NodeSet do
     example 'return the node when shifting it' do
       @set.shift.should == @n1
     end
+
+    example 'remove ownership if the node belongs to a node set' do
+      @set.shift
+
+      @n1.node_set.nil?.should == true
+    end
   end
 
   context '#pop' do
     before do
-      @n1  = Oga::XML::Element.new(:name => 'a')
+      owner = Oga::XML::Element.new
-      @set = described_class.new([@n1])
+      @n1   = Oga::XML::Element.new
+      @set  = described_class.new([@n1], owner)
     end
 
     example 'remove the node from the set' do
@@ -126,6 +166,12 @@ describe Oga::XML::NodeSet do
     example 'return the node when popping it' do
       @set.pop.should == @n1
     end
+
+    example 'remove ownership if the node belongs to a node set' do
+      @set.pop
+
+      @n1.node_set.nil?.should == true
+    end
   end
 
   context '#[]' do
@@ -141,13 +187,12 @@ describe Oga::XML::NodeSet do
 
   context '#remove' do
     before do
-      @n1  = Oga::XML::Element.new(:name => 'a')
+      owner = Oga::XML::Element.new
-      @n2  = Oga::XML::Element.new(:name => 'b')
+      @n1   = Oga::XML::Element.new
+      @n2   = Oga::XML::Element.new
 
-      @doc_set   = described_class.new([@n1, @n2])
+      @doc_set   = described_class.new([@n1, @n2], owner)
       @query_set = described_class.new([@n1, @n2])
-
-      @doc_set.associate_nodes!
     end
 
     example 'do not remove the nodes from the current set' do
@@ -216,17 +261,4 @@ describe Oga::XML::NodeSet do
       @set.text.should == "foo\nbar"
     end
   end
-
-  context '#associate_nodes!' do
-    before do
-      @n1  = Oga::XML::Element.new(:name => 'a')
-      @set = described_class.new([@n1])
-    end
-
-    example 'associate a node with a set' do
-      @set.associate_nodes!
-
-      @n1.node_set.should == @set
-    end
-  end
 end

spec/oga/xml/node_spec.rb

@@ -2,15 +2,11 @@ require 'spec_helper'
 
 describe Oga::XML::Node do
   context '#initialize' do
-    example 'set the parent node' do
+    example 'set the node set' do
-      parent = described_class.new
+      set  = Oga::XML::NodeSet.new
-      child  = described_class.new(:parent => parent)
+      node = described_class.new(:node_set => set)
 
-      child.parent.should == parent
+      node.node_set.should == set
-    end
-
-    example 'set the default child nodes' do
-      described_class.new.children.should == []
     end
   end