fixed reparent()

git-svn-id: http://svn.devjavu.com/gaphor/gaphas/trunk@2682 a8418922-720d-0410-834f-a69b97ada669

2 files changed, 224 insertions, 196 deletions

diff --git a/gaphas/tests/test_tree.py b/gaphas/tests/test_tree.py
index 6c3fedc..718006c 100644
--- a/gaphas/tests/test_tree.py
+++ b/gaphas/tests/test_tree.py
@@ -1,157 +1,163 @@
-

-import unittest

-from gaphas.tree import Tree

-

-

-class TreeTestCase(unittest.TestCase):

-

-    def test_add(self):

-        """

-        Test creating node trees.

-        """

-        tree = Tree()

-        n1 = 'n1'

-        n2 = 'n2'

-        n3 = 'n3'

-

-        tree.add(n1)

-        assert len(tree._nodes) == 1

-        assert len(tree._children) == 2

-        assert len(tree._children[None]) == 1

-        assert len(tree._children[n1]) == 0

-

-        tree.add(n2)

-        tree.add(n3, parent=n1)

-        assert len(tree._nodes) == 3

-        assert len(tree._children) == 4

-        assert len(tree._children[None]) == 2

-        assert len(tree._children[n1]) == 1

-        assert len(tree._children[n2]) == 0

-        assert len(tree._children[n2]) == 0

-        assert tree._nodes == [n1, n3, n2]

-

-        n4 = 'n4'

-        tree.add(n4, parent=n3)

-        assert tree._nodes == [n1, n3, n4, n2]

-

-        n5 = 'n5'

-        tree.add(n5, parent=n3)

-        assert tree._nodes == [n1, n3, n4, n5, n2]

-

-        n6 = 'n6'

-        tree.add(n6, parent=n2)

-        assert tree._nodes == [n1, n3, n4, n5, n2, n6]

-

-        n7 = 'n7'

-        tree.add(n7, parent=n1)

-        assert len(tree._children) == 8

-        assert tree._nodes == [n1, n3, n4, n5, n7, n2, n6]

-        assert tree.get_parent(n7) is n1

-        assert tree.get_parent(n6) is n2

-        assert tree.get_parent(n5) is n3

-        assert tree.get_parent(n4) is n3

-        assert tree.get_parent(n3) is n1

-        assert tree.get_parent(n2) is None

-        assert tree.get_parent(n1) is None

-

-    def test_add_on_index(self):

-        tree = Tree()

-        n1 = 'n1'

-        n2 = 'n2'

-        n3 = 'n3'

-        n4 = 'n4'

-        n5 = 'n5'

-

-        tree.add(n1)

-        tree.add(n2)

-        tree.add(n3, index=1)

-        assert tree.get_children(None) == [n1, n3, n2], tree.get_children(None)

-        assert tree.nodes == [n1, n3, n2], tree.nodes

-        

-        tree.add(n4, parent=n3)

-        tree.add(n5, parent=n3, index=0)

-        assert tree.get_children(None) == [n1, n3, n2], tree.get_children(None)

-        assert tree.nodes == [n1, n3, n5, n4, n2], tree.nodes

-        assert tree.get_children(n3) == [n5, n4], tree.get_children(n3)

-

-

-    def test_remove(self):

-        """

-        Test removal of nodes.

-        """

-        tree = Tree()

-        n1 = 'n1'

-        n2 = 'n2'

-        n3 = 'n3'

-        n4 = 'n4'

-        n5 = 'n5'

-

-        tree.add(n1)

-        tree.add(n2)

-        tree.add(n3, parent=n1)

-        tree.add(n4, parent=n3)

-        tree.add(n5, parent=n4)

-

-        assert tree._nodes == [n1, n3, n4, n5, n2]

-

-        all_ch = list(tree.get_all_children(n1))

-        assert all_ch == [ n3, n4, n5 ], all_ch

-

-        tree.remove(n4)

-        assert tree._nodes == [n1, n3, n2]

-

-        tree.remove(n1)

-        assert len(tree._children) == 2

-        assert tree._children[None] == [n2]

-        assert tree._children[n2] == []

-        assert tree._nodes == [n2]

-

-    def test_siblings(self):

-        tree = Tree()

-        n1 = 'n1'

-        n2 = 'n2'

-        n3 = 'n3'

-        

-        tree.add(n1)

-        tree.add(n2)

-        tree.add(n3)

-        

-        assert tree.get_next_sibling(n1) is n2

-        assert tree.get_next_sibling(n2) is n3

-        try:

-            tree.get_next_sibling(n3)

-        except IndexError:

-            pass # okay

-        else:

-            raise AssertionError, 'Index should be out of range, not %s' % tree.get_next_sibling(n3)

-

-        assert tree.get_previous_sibling(n3) is n2

-        assert tree.get_previous_sibling(n2) is n1

-        try:

-            tree.get_previous_sibling(n1)

-        except IndexError:

-            pass # okay

-        else:

-            raise AssertionError, 'Index should be out of range, not %s' % tree.get_previous_sibling(n1)

-

-    def test_reparent(self):

-        tree = Tree()

-        n1 = 'n1'

-        n2 = 'n2'

-        n3 = 'n3'

-        n4 = 'n4'

-        n5 = 'n5'

-

-        tree.add(n1)

-        tree.add(n2)

-        tree.add(n3)

-        tree.add(n4, parent=n2)

-        tree.add(n5, parent=n4)

-

-        assert tree.nodes == [n1, n2, n4, n5, n3], tree.nodes

-

-        tree.reparent(n4, parent=None, index=0)

-        assert tree.nodes == [n4, n5, n1, n2, n3], tree.nodes

-

-

-# vi:sw=4:et:ai

+
+import unittest
+from gaphas.tree import Tree
+
+
+class TreeTestCase(unittest.TestCase):
+
+    def test_add(self):
+        """
+        Test creating node trees.
+        """
+        tree = Tree()
+        n1 = 'n1'
+        n2 = 'n2'
+        n3 = 'n3'
+
+        tree.add(n1)
+        assert len(tree._nodes) == 1
+        assert len(tree._children) == 2
+        assert len(tree._children[None]) == 1
+        assert len(tree._children[n1]) == 0
+
+        tree.add(n2)
+        tree.add(n3, parent=n1)
+        assert len(tree._nodes) == 3
+        assert len(tree._children) == 4
+        assert len(tree._children[None]) == 2
+        assert len(tree._children[n1]) == 1
+        assert len(tree._children[n2]) == 0
+        assert len(tree._children[n2]) == 0
+        assert tree._nodes == [n1, n3, n2]
+
+        n4 = 'n4'
+        tree.add(n4, parent=n3)
+        assert tree._nodes == [n1, n3, n4, n2]
+
+        n5 = 'n5'
+        tree.add(n5, parent=n3)
+        assert tree._nodes == [n1, n3, n4, n5, n2]
+
+        n6 = 'n6'
+        tree.add(n6, parent=n2)
+        assert tree._nodes == [n1, n3, n4, n5, n2, n6]
+
+        n7 = 'n7'
+        tree.add(n7, parent=n1)
+        assert len(tree._children) == 8
+        assert tree._nodes == [n1, n3, n4, n5, n7, n2, n6]
+        assert tree.get_parent(n7) is n1
+        assert tree.get_parent(n6) is n2
+        assert tree.get_parent(n5) is n3
+        assert tree.get_parent(n4) is n3
+        assert tree.get_parent(n3) is n1
+        assert tree.get_parent(n2) is None
+        assert tree.get_parent(n1) is None
+
+    def test_add_on_index(self):
+        tree = Tree()
+        n1 = 'n1'
+        n2 = 'n2'
+        n3 = 'n3'
+        n4 = 'n4'
+        n5 = 'n5'
+
+        tree.add(n1)
+        tree.add(n2)
+        tree.add(n3, index=1)
+        assert tree.get_children(None) == [n1, n3, n2], tree.get_children(None)
+        assert tree.nodes == [n1, n3, n2], tree.nodes
+        
+        tree.add(n4, parent=n3)
+        tree.add(n5, parent=n3, index=0)
+        assert tree.get_children(None) == [n1, n3, n2], tree.get_children(None)
+        assert tree.nodes == [n1, n3, n5, n4, n2], tree.nodes
+        assert tree.get_children(n3) == [n5, n4], tree.get_children(n3)
+
+
+    def test_remove(self):
+        """
+        Test removal of nodes.
+        """
+        tree = Tree()
+        n1 = 'n1'
+        n2 = 'n2'
+        n3 = 'n3'
+        n4 = 'n4'
+        n5 = 'n5'
+
+        tree.add(n1)
+        tree.add(n2)
+        tree.add(n3, parent=n1)
+        tree.add(n4, parent=n3)
+        tree.add(n5, parent=n4)
+
+        assert tree._nodes == [n1, n3, n4, n5, n2]
+
+        all_ch = list(tree.get_all_children(n1))
+        assert all_ch == [ n3, n4, n5 ], all_ch
+
+        tree.remove(n4)
+        assert tree._nodes == [n1, n3, n2]
+
+        tree.remove(n1)
+        assert len(tree._children) == 2
+        assert tree._children[None] == [n2]
+        assert tree._children[n2] == []
+        assert tree._nodes == [n2]
+
+    def test_siblings(self):
+        tree = Tree()
+        n1 = 'n1'
+        n2 = 'n2'
+        n3 = 'n3'
+        
+        tree.add(n1)
+        tree.add(n2)
+        tree.add(n3)
+        
+        assert tree.get_next_sibling(n1) is n2
+        assert tree.get_next_sibling(n2) is n3
+        try:
+            tree.get_next_sibling(n3)
+        except IndexError:
+            pass # okay
+        else:
+            raise AssertionError, 'Index should be out of range, not %s' % tree.get_next_sibling(n3)
+
+        assert tree.get_previous_sibling(n3) is n2
+        assert tree.get_previous_sibling(n2) is n1
+        try:
+            tree.get_previous_sibling(n1)
+        except IndexError:
+            pass # okay
+        else:
+            raise AssertionError, 'Index should be out of range, not %s' % tree.get_previous_sibling(n1)
+
+    def test_reparent(self):
+        tree = Tree()
+        n1 = 'n1'
+        n2 = 'n2'
+        n3 = 'n3'
+        n4 = 'n4'
+        n5 = 'n5'
+
+        tree.add(n1)
+        tree.add(n2)
+        tree.add(n3)
+        tree.add(n4, parent=n2)
+        tree.add(n5, parent=n4)
+
+        assert tree.nodes == [n1, n2, n4, n5, n3], tree.nodes
+
+        tree.reparent(n4, parent=n1, index=0)
+        assert tree.nodes == [n1, n4, n5, n2, n3], tree.nodes
+        assert tree.get_children(n2) == [], tree.get_children(n2)
+        assert tree.get_children(n1) == [n4], tree.get_children(n1)
+        assert tree.get_children(n4) == [n5], tree.get_children(n4)
+
+        tree.reparent(n4, parent=None, index=0)
+        assert tree.nodes == [n4, n5, n1, n2, n3], tree.nodes
+
+
+# vi:sw=4:et:ai
diff --git a/gaphas/tree.py b/gaphas/tree.py
index 63565b7..469afce 100644
--- a/gaphas/tree.py
+++ b/gaphas/tree.py
@@ -209,51 +209,67 @@ class Tree(object):
         else:
             raise NotImplemented('index_key should be provided.')
 
-    def _add_to_nodes(self, node, parent):
+    def _add_to_nodes(self, node, parent, index=None):
         """
-        Called only from add()
+        Helper method to place nodes on the right location in the nodes list
+        Called only from add() and reparent()
         """
         nodes = self._nodes
-        if parent:
-            try:
-                next_uncle = self.get_next_sibling(parent)
-            except IndexError:
-                # parent has no younger brothers..
-                # place it before the next uncle of grant_parent:
-                self._add_to_nodes(node, self.get_parent(parent))
+        siblings = self._children[parent]
+        try:
+            atnode = siblings[index]
+        except (TypeError, IndexError):
+            index = len(siblings)
+            #self._add_to_nodes(node, parent)
+            if parent:
+                try:
+                    next_uncle = self.get_next_sibling(parent)
+                except IndexError:
+                    # parent has no younger brothers..
+                    # place it before the next uncle of grant_parent:
+                    return self._add_to_nodes(node, self.get_parent(parent))
+                else:
+                    nodes.insert(nodes.index(next_uncle), node)
             else:
-                nodes.insert(nodes.index(next_uncle), node)
+                # append to root node:
+                nodes.append(node)
         else:
-            # append to root node:
-            nodes.append(node)
+            print nodes, siblings, atnode, node
+            nodes.insert(nodes.index(atnode), node)
 
-    def add(self, node, parent=None, index=None):
-        """
-        Add node to the tree. parent is the parent node, which may
-        be None if the item should be added to the root item.
 
-        For usage, see the unit tests.
+    def _add(self, node, parent=None, index=None):
+        """
+        Helper method for both add() and reparent().
         """
-
         assert node not in self._nodes
 
         siblings = self._children[parent]
-        try:
-            atnode = siblings[index]
-        except (TypeError, IndexError):
-            index = len(siblings)
-            self._add_to_nodes(node, parent)
-        else:
-            self._nodes.insert(self._nodes.index(atnode), node)
+
+        self._add_to_nodes(node, parent, index)
         
         # Fix parent-child and child-parent relationship
-        siblings.insert(index, node)
+        try:
+            siblings.insert(index, node)
+        except TypeError:
+            siblings.append(node)
 
         # Create new entry for it's own children:
-        self._children[node] = []
         if parent:
             self._parents[node] = parent
 
+
+    def add(self, node, parent=None, index=None):
+        """
+        Add node to the tree. parent is the parent node, which may
+        be None if the item should be added to the root item.
+
+        For usage, see the unit tests.
+        """
+        self._add(node, parent, index)
+        self._children[node] = []
+
+
     def _remove(self, node):
         # Remove from parent item
         self.get_siblings(node).remove(node)
@@ -320,23 +336,29 @@ class Tree(object):
         >>> tree.nodes
         ['n4', 'n1', 'n3', 'n2']
         """
+        if parent is self.get_parent(node):
+            return
+
+	siblings = self._children[parent]
+
         # Add to new parent's children:
         self.get_siblings(node).remove(node)
+        self._nodes.remove(node)
 
-        self._parents[node] = parent
-
-        # Change this to get index working:
-	siblings = self._children[parent]
         try:
-            atnode = siblings[index]
-        except (TypeError, IndexError):
-            siblings.append(node)
-            # reorganize nodes
-            self._reparent_nodes(node, parent)
-        else:
-            self._nodes.insert(self._nodes.index(atnode), node)
+            del self._parents[node]
+        except KeyError:
+            pass
 
-        
+        self._add(node, parent, index)
+
+        assert self._parents.get(node) is parent
+        assert node in self._children[parent]
+        assert node in self.get_siblings(node)
+
+        # reorganize children in nodes list
+        for c in self._children[node]:
+            self._reparent_nodes(c, node)
 
 
 # vi: sw=4:et:ai