From e294ee6298f50783dc0671341f9c9d0ec26ef9ea Mon Sep 17 00:00:00 2001 From: Ben Kurtovic Date: Thu, 10 Jul 2014 22:34:51 -0400 Subject: [PATCH] Improve ListProxy detaching behavior. --- CHANGELOG | 5 +++- docs/changelog.rst | 5 +++- mwparserfromhell/smart_list.py | 34 +++++++++++++++------- tests/test_smart_list.py | 66 ++++++++++++++++++++++++++++-------------- 4 files changed, 75 insertions(+), 35 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index f7dcb8a..b4b01d6 100644 --- a/CHANGELOG +++ b/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. diff --git a/docs/changelog.rst b/docs/changelog.rst index 3bc4ce7..d793db1 100644 --- a/docs/changelog.rst +++ b/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. diff --git a/mwparserfromhell/smart_list.py b/mwparserfromhell/smart_list.py index 5fa9055..cedfb5c 100644 --- a/mwparserfromhell/smart_list.py +++ b/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 diff --git a/tests/test_smart_list.py b/tests/test_smart_list.py index b739d62..13d96d2 100644 --- a/tests/test_smart_list.py +++ b/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)