[gxml] * document NodeList * throw some exceptions

[Richard Hans Schwarting <rschwart src gnome org>]

commit 7625dea7af035a5173d6e534fd1580f1e08afac4
Author: Richard Schwarting <aquarichy gmail com>
Date:   Thu Jul 21 14:41:55 2011 -0400

    * document NodeList
    * throw some exceptions

 gxml/NodeList.vala |   98 ++++++++++++++++++++++++++++++++++++++++++++-------
 1 files changed, 84 insertions(+), 14 deletions(-)
---
diff --git a/gxml/NodeList.vala b/gxml/NodeList.vala
index 6b3e592..1144fb9 100644
--- a/gxml/NodeList.vala
+++ b/gxml/NodeList.vala
@@ -3,36 +3,87 @@
 using Gee;
 
 namespace GXml.Dom {
+	/**
+	 * The NodeList is a live list used to store nodes, often the
+	 * children of a node, or a list of nodes matching a tag name.
+	 */
 	public interface NodeList : Gee.Iterable<XNode> {
 		public abstract ulong length {
 			get; private set;
 		}
-		// children should define constructors like:
-		// internal NodeList (Xml.Node* head, Document owner);
+		/* NOTE:
+		 * children should define constructors like:
+		 *     internal NodeList (Xml.Node* head, Document owner);
+		 */
 
 		/** NodeList methods */
-		public abstract XNode item (ulong idx);
-		// public ulong length;   // children should implement // TODO: figure out how to require this as a property; maybe have to make it into a method
 
-		/** GNOME List conventions
-		 ** Probably don't want to keep all of them since they're not all relevant.
-		 **/
+		/**
+		 * Access the idx'th item in the list.
+		 */
+		// TODO: this should throw invalid index or something
+		public abstract XNode item (ulong idx);
+		/* NOTE: children should implement
+		 *     public ulong length;
+		 * TODO: figure out how to require this as a property; maybe have to make it into a method
+		 */
+
+		/*** GNOME List conventions ***
+		 * Probably don't want to keep all of them since they're not all relevant.
+		 */
+		/**
+		 * Call the provided func on each item of the list.
+		 */
 		public abstract void foreach (Func<XNode> func);
+		// TODO: add hints for performance below, perhaps
+		/**
+		 * Retrieve the first node in the list.
+		 */
 		public abstract XNode first ();
+		/**
+		 * Retrieve the last node in the list.
+		 */
 		public abstract XNode last ();
+		/**
+		 * Obtain the n'th item in the list. Used for compatibility with GLib.List.
+		 */
 		public abstract XNode? nth (ulong n);
+		/**
+		 * Obtain the n'th item in the list. Used for compatibility with GLib.List.
+		 */
 		public abstract XNode? nth_data (ulong n);
+		/**
+		 * Obtain the item n places before pivot in the list.
+		 */
 		public abstract XNode? nth_prev (XNode pivot, ulong n);
+		/**
+		 * Obtain index for node target in the list.
+		 */
 		public abstract int find (XNode target);
+		/**
+		 * Obtain index for node target in the list, using CompareFunc to compare.
+		 */
 		public abstract int find_custom (XNode target, CompareFunc<XNode> cmp);
+		/**
+		 * Obtain index for node target in the list.
+		 */
 		public abstract int position (XNode target);
+		/**
+		 * Obtain index for node target in the list.
+		 */
 		public abstract int index (XNode target);
+		// TODO: wow, lots of those GList compatibility methods are the same in a case like this.
 
-		internal abstract XNode? insert_before (XNode new_child, XNode ref_child) /*throws DomError*/;
-		internal abstract XNode? replace_child (XNode new_child, XNode old_child) /*throws DomError*/;
+		/* These exist to support management of a node's children */
+		internal abstract XNode? insert_before (XNode new_child, XNode? ref_child) throws DomError;
+		internal abstract XNode? replace_child (XNode new_child, XNode old_child) throws DomError;
 		internal abstract XNode? remove_child (XNode old_child) /*throws DomError*/;
 		internal abstract XNode? append_child (XNode new_child) /*throws DomError*/;
 
+		/**
+		 * Provide a string representation of the list.
+		 */
+		// TODO: convert it to valid XML
 		public abstract string to_string ();
 	}
 
@@ -83,7 +134,7 @@ namespace GXml.Dom {
 				/* This is disgusting, but we do this for the case where
 				   xmlAttr*'s immediate children list the xmlAttr as their
 				   parent, but claim that xmlAttr is an xmlNode* (since
-				   the parent field is of type xmlNode*).  We need to get
+				   the parent field is of type xmlNode*). We need to get
 				   an Xml.Node*ish parent for when we append new children
 				   here, whether we're the list of children of an Attr
 				   or not. */
@@ -113,7 +164,7 @@ namespace GXml.Dom {
 				/* This is disgusting, but we do this for the case where
 				   xmlAttr*'s immediate children list the xmlAttr as their
 				   parent, but claim that xmlAttr is an xmlNode* (since
-				   the parent field is of type xmlNode*).  We need to get
+				   the parent field is of type xmlNode*). We need to get
 				   an Xml.Node*ish parent for when we append new children
 				   here, whether we're the list of children of an Attr
 				   or not. */
@@ -282,20 +333,30 @@ namespace GXml.Dom {
 
 
 		/** Node's child methods, implemented here **/
-		internal new XNode? insert_before (XNode new_child, XNode ref_child) /* throws DomError */ {
+		internal new XNode? insert_before (XNode new_child, XNode? ref_child) throws DomError {
 			Xml.Node *child = head;
 
+			if (ref_child == null) {
+				this.append_child (ref_child);
+			}
+
+
 			while (child != ((BackedNode)ref_child).node && child != null) {
 				child = child->next;
 			}
 			if (child == null) {
-				// TODO: couldn't insert before ref, since ref not found
+				throw new DomError.NOT_FOUND ("ref_child not found.");
+				// TODO: provide a more useful description of ref_child, but there are so many different types
 			} else {
 				child->add_prev_sibling (((BackedNode)new_child).node);
 			}
 			return new_child;
 		}
-		internal new XNode? replace_child (XNode new_child, XNode old_child) /* throws DomError */ {
+		internal new XNode? replace_child (XNode new_child, XNode old_child) throws DomError {
+			// TODO: verify that libxml2 already removes
+			// new_child first if it is found elsewhere in
+			// the tree.
+
 			// TODO: nuts, if Node as an iface can't have properties,
 			//       then I have to cast these to XNodes, ugh.
 			// TODO: need to handle errors?
@@ -310,15 +371,24 @@ namespace GXml.Dom {
 			if (child != null) {
 				// it is a valid child
 				child->replace (((BackedNode)new_child).node);
+			} else {
+				throw new DomError.NOT_FOUND ("old_child not found");
+				// TODO: provide more useful descr. of old_child
 			}
 			return old_child;
 		}
 		internal new XNode? remove_child (XNode old_child) /* throws DomError */ {
+			// TODO: verify that old_child is a valid child here and then unlink
+
 			((BackedNode)old_child).node->unlink (); // TODO: do we need to free libxml2 stuff manually?
 			return old_child;
 		}
 
 		internal virtual XNode? append_child (XNode new_child) /* throws DomError */ {
+			// TODO: verify that libxml2 will first remove
+			// new_child if it already exists elsewhere in
+			// the tree.
+
 			parent_as_xmlnode->add_child (((BackedNode)new_child).node);
 
 			return new_child;