Improve ListProxy detaching behavior.

CHANGELOG

@@ -18,10 +18,13 @@ v0.4 (unreleased):
   template now raises ValueError instead of doing nothing.
 - Parameters with non-integer keys can no longer be created with
   'showkey=False', nor have the value of this attribute be set to False later.
+- _ListProxy.destroy() has been changed to _ListProxy.detach(), and now works
+  in a more useful way.
 - If something goes wrong while parsing, ParserError will now be raised.
   Previously, the parser would produce an unclear BadRoute exception or allow
   an incorrect node tree to be build.
-- Fixed a parser bug involving nested tags.
+- Fixed a parser bug involving nested tags, and another involving comments in
+  template names.
 - Test coverage has been improved, and some minor related bugs have been fixed.
 - Updated and fixed some documentation.
 

docs/changelog.rst

@@ -29,10 +29,13 @@ Unreleased
 - :py:class:`.Parameter`\ s with non-integer keys can no longer be created with
   *showkey=False*, nor have the value of this attribute be set to *False*
   later.
+- :py:meth:`._ListProxy.destroy` has been changed to
+  :py:meth:`._ListProxy.detach`, and now works in a more useful way.
 - If something goes wrong while parsing, :py:exc:`.ParserError` will now be
   raised. Previously, the parser would produce an unclear :py:exc:`.BadRoute`
   exception or allow an incorrect node tree to be build.
-- Fixed a parser bug involving nested tags.
+- Fixed a parser bug involving nested tags, and another involving comments in
+  template names.
 - Test coverage has been improved, and some minor related bugs have been fixed.
 - Updated and fixed some documentation.
 

mwparserfromhell/smart_list.py

@@ -41,6 +41,7 @@ def inheritdoc(method):
     method.__doc__ = getattr(list, method.__name__).__doc__
     return method
 
+
 class _SliceNormalizerMixIn(object):
     """MixIn that provides a private method to normalize slices."""
 
@@ -83,7 +84,9 @@ class SmartList(_SliceNormalizerMixIn, list):
     The parent needs to keep a list of its children in order to update them,
     which prevents them from being garbage-collected. If you are keeping the
     parent around for a while but creating many children, it is advisable to
-    call :py:meth:`~._ListProxy.destroy` when you're finished with them.
+    call :py:meth:`~._ListProxy.detach` when you're finished with them. Certain
+    parent methods, like :py:meth:`reverse` and :py:meth:`sort`, will do this
+    automatically.
     """
 
     def __init__(self, iterable=None):
@@ -151,10 +154,10 @@ class SmartList(_SliceNormalizerMixIn, list):
         self.extend(other)
         return self
 
-    def _release_children(self):
-        copy = list(self)
-        for child in self._children:
-            child._parent = copy
+    def _detach_children(self):
+        children = [val[0] for val in self._children.values()]
+        for child in children:
+            child.detach()
 
     @inheritdoc
     def append(self, item):
@@ -184,13 +187,13 @@ class SmartList(_SliceNormalizerMixIn, list):
 
     @inheritdoc
     def reverse(self):
-        self._release_children()
+        self._detach_children()
         super(SmartList, self).reverse()
 
     if py3k:
         @inheritdoc
         def sort(self, key=None, reverse=None):
-            self._release_children()
+            self._detach_children()
             kwargs = {}
             if key is not None:
                 kwargs["key"] = key
@@ -200,7 +203,7 @@ class SmartList(_SliceNormalizerMixIn, list):
     else:
         @inheritdoc
         def sort(self, cmp=None, key=None, reverse=None):
-            self._release_children()
+            self._detach_children()
             kwargs = {}
             if cmp is not None:
                 kwargs["cmp"] = cmp
@@ -223,6 +226,7 @@ class _ListProxy(_SliceNormalizerMixIn, list):
         super(_ListProxy, self).__init__()
         self._parent = parent
         self._sliceinfo = sliceinfo
+        self._detached = False
 
     def __repr__(self):
         return repr(self._render())
@@ -452,9 +456,17 @@ class _ListProxy(_SliceNormalizerMixIn, list):
             item.sort(**kwargs)
             self._parent[self._start:self._stop:self._step] = item
 
-    def destroy(self):
-        """Make the parent forget this child. The child will no longer work."""
-        self._parent._children.pop(id(self))
+    def detach(self):
+        """Detach the child so it operates like a normal list.
+
+        This allows children to be properly garbage-collected if their parent
+        is being kept around for a long time. This method has no effect if the
+        child is already detached.
+        """
+        if not self._detached:
+            self._parent._children.pop(id(self))
+            self._parent = list(self._parent)
+            self._detached = True
 
 
 del inheritdoc

tests/test_smart_list.py

@@ -88,6 +88,10 @@ class TestSmartList(unittest.TestCase):
         self.assertEqual([0, 1, 2, 3, 4, 5, 6], list2)
         self.assertRaises(ValueError, assign, list2, 0, 5, 2,
                           [100, 102, 104, 106])
+        with self.assertRaises(IndexError):
+            list2[7] = "foo"
+        with self.assertRaises(IndexError):
+            list2[-8] = "foo"
 
         del list2[2]
         self.assertEqual([0, 1, 3, 4, 5, 6], list2)
@@ -271,6 +275,13 @@ class TestSmartList(unittest.TestCase):
         list3.sort(key=lambda i: i[1], reverse=True)
         self.assertEqual([("b", 8), ("a", 5), ("c", 3), ("d", 2)], list3)
 
+    def _dispatch_test_for_children(self, meth):
+        """Run a test method on various different types of children."""
+        meth(lambda L: SmartList(list(L))[:])
+        meth(lambda L: SmartList([999] + list(L))[1:])
+        meth(lambda L: SmartList(list(L) + [999])[:-1])
+        meth(lambda L: SmartList([101, 102] + list(L) + [201, 202])[2:-2])
+
     def test_docs(self):
         """make sure the methods of SmartList/_ListProxy have docstrings"""
         methods = ["append", "count", "extend", "index", "insert", "pop",
@@ -300,8 +311,8 @@ class TestSmartList(unittest.TestCase):
         """make sure SmartList's add/radd/iadd work"""
         self._test_add_radd_iadd(SmartList)
 
-    def test_parent_unaffected_magics(self):
-        """sanity checks against SmartList features that were not modified"""
+    def test_parent_other_magics(self):
+        """make sure SmartList's other magically implemented features work"""
         self._test_other_magic_methods(SmartList)
 
     def test_parent_methods(self):
@@ -310,41 +321,29 @@ class TestSmartList(unittest.TestCase):
 
     def test_child_get_set_del(self):
         """make sure _ListProxy's getitem/setitem/delitem work"""
-        self._test_get_set_del_item(lambda L: SmartList(list(L))[:])
-        self._test_get_set_del_item(lambda L: SmartList([999] + list(L))[1:])
-        self._test_get_set_del_item(lambda L: SmartList(list(L) + [999])[:-1])
-        builder = lambda L: SmartList([101, 102] + list(L) + [201, 202])[2:-2]
-        self._test_get_set_del_item(builder)
+        self._dispatch_test_for_children(self._test_get_set_del_item)
 
     def test_child_add(self):
         """make sure _ListProxy's add/radd/iadd work"""
-        self._test_add_radd_iadd(lambda L: SmartList(list(L))[:])
-        self._test_add_radd_iadd(lambda L: SmartList([999] + list(L))[1:])
-        self._test_add_radd_iadd(lambda L: SmartList(list(L) + [999])[:-1])
-        builder = lambda L: SmartList([101, 102] + list(L) + [201, 202])[2:-2]
-        self._test_add_radd_iadd(builder)
+        self._dispatch_test_for_children(self._test_add_radd_iadd)
 
     def test_child_other_magics(self):
         """make sure _ListProxy's other magically implemented features work"""
-        self._test_other_magic_methods(lambda L: SmartList(list(L))[:])
-        self._test_other_magic_methods(lambda L: SmartList([999] + list(L))[1:])
-        self._test_other_magic_methods(lambda L: SmartList(list(L) + [999])[:-1])
-        builder = lambda L: SmartList([101, 102] + list(L) + [201, 202])[2:-2]
-        self._test_other_magic_methods(builder)
+        self._dispatch_test_for_children(self._test_other_magic_methods)
 
     def test_child_methods(self):
         """make sure _ListProxy's non-magic methods work, like append()"""
-        self._test_list_methods(lambda L: SmartList(list(L))[:])
-        self._test_list_methods(lambda L: SmartList([999] + list(L))[1:])
-        self._test_list_methods(lambda L: SmartList(list(L) + [999])[:-1])
-        builder = lambda L: SmartList([101, 102] + list(L) + [201, 202])[2:-2]
-        self._test_list_methods(builder)
+        self._dispatch_test_for_children(self._test_list_methods)
 
     def test_influence(self):
         """make sure changes are propagated from parents to children"""
         parent = SmartList([0, 1, 2, 3, 4, 5])
         child1 = parent[2:]
         child2 = parent[2:5]
+        self.assertEqual([0, 1, 2, 3, 4, 5], parent)
+        self.assertEqual([2, 3, 4, 5], child1)
+        self.assertEqual([2, 3, 4], child2)
+        self.assertEqual(2, len(parent._children))
 
         parent.append(6)
         child1.append(7)
@@ -390,5 +389,28 @@ class TestSmartList(unittest.TestCase):
         self.assertEqual([4, 3, 2, 1.9, 1.8, 5, 6, 7, 8, 8.1, 8.2], child1)
         self.assertEqual([4, 3, 2, 1.9, 1.8], child2)
 
+        child1.detach()
+        self.assertEqual([1, 4, 3, 2, 1.9, 1.8, 5, 6, 7, 8, 8.1, 8.2], parent)
+        self.assertEqual([4, 3, 2, 1.9, 1.8, 5, 6, 7, 8, 8.1, 8.2], child1)
+        self.assertEqual([4, 3, 2, 1.9, 1.8], child2)
+        self.assertEqual(1, len(parent._children))
+
+        parent.remove(1.9)
+        parent.remove(1.8)
+        self.assertEqual([1, 4, 3, 2, 5, 6, 7, 8, 8.1, 8.2], parent)
+        self.assertEqual([4, 3, 2, 1.9, 1.8, 5, 6, 7, 8, 8.1, 8.2], child1)
+        self.assertEqual([4, 3, 2], child2)
+
+        parent.reverse()
+        self.assertEqual([8.2, 8.1, 8, 7, 6, 5, 2, 3, 4, 1], parent)
+        self.assertEqual([4, 3, 2, 1.9, 1.8, 5, 6, 7, 8, 8.1, 8.2], child1)
+        self.assertEqual([4, 3, 2], child2)
+        self.assertEqual(0, len(parent._children))
+
+        child2.detach()
+        self.assertEqual([8.2, 8.1, 8, 7, 6, 5, 2, 3, 4, 1], parent)
+        self.assertEqual([4, 3, 2, 1.9, 1.8, 5, 6, 7, 8, 8.1, 8.2], child1)
+        self.assertEqual([4, 3, 2], child2)
+
 if __name__ == "__main__":
     unittest.main(verbosity=2)