From 786d6192746284ef19c166c4d9eb95050c661b1c Mon Sep 17 00:00:00 2001 From: Ben Kurtovic Date: Wed, 24 Apr 2013 10:28:17 -0400 Subject: [PATCH] Drop force_no_field in template.remove(); implement test_remove(). - Also add tests for spacing in param names. --- mwparserfromhell/nodes/template.py | 27 ++++++++++-------- tests/test_template.py | 56 ++++++++++++++++++++++++++++++++++---- 2 files changed, 66 insertions(+), 17 deletions(-) diff --git a/mwparserfromhell/nodes/template.py b/mwparserfromhell/nodes/template.py index eb7f3a8..751c2b1 100644 --- a/mwparserfromhell/nodes/template.py +++ b/mwparserfromhell/nodes/template.py @@ -142,9 +142,9 @@ class Template(Node): return False return True - def _remove_without_field(self, param, i, force_no_field): + def _remove_without_field(self, param, i): """Return False if a parameter name should be kept, otherwise True.""" - if not param.showkey and not force_no_field: + if not param.showkey: dependents = [not after.showkey for after in self.params[i+1:]] if any(dependents): return False @@ -266,22 +266,23 @@ class Template(Node): self.params.append(param) return param - def remove(self, name, keep_field=False, force_no_field=False): + def remove(self, name, keep_field=False): """Remove a parameter from the template whose name is *name*. If *keep_field* is ``True``, we will keep the parameter's name, but blank its value. Otherwise, we will remove the parameter completely *unless* other parameters are dependent on it (e.g. removing ``bar`` from ``{{foo|bar|baz}}`` is unsafe because ``{{foo|baz}}`` is not what - we expected, so ``{{foo||baz}}`` will be produced instead), unless - *force_no_field* is also ``True``. If the parameter shows up multiple - times in the template, we will remove all instances of it (and keep - one if *keep_field* is ``True`` - that being the first instance if - none of the instances have dependents, otherwise that instance will be - kept). + we expected, so ``{{foo||baz}}`` will be produced instead). + + If the parameter shows up multiple times in the template, we will + remove all instances of it (and keep one if *keep_field* is ``True`` - + the first instance if none have dependents, otherwise the one with + dependents will be kept). """ name = name.strip() if isinstance(name, basestring) else str(name) removed = False + to_remove =[] for i, param in enumerate(self.params): if param.name.strip() == name: if keep_field: @@ -289,13 +290,15 @@ class Template(Node): self._blank_param_value(param.value) keep_field = False else: - self.params.remove(param) + to_remove.append(param) else: - if self._remove_without_field(param, i, force_no_field): - self.params.remove(param) + if self._remove_without_field(param, i): + to_remove.append(param) else: self._blank_param_value(param.value) if not removed: removed = True if not removed: raise ValueError(name) + for param in to_remove: + self.params.remove(param) diff --git a/tests/test_template.py b/tests/test_template.py index fde7522..ecac917 100644 --- a/tests/test_template.py +++ b/tests/test_template.py @@ -98,7 +98,7 @@ class TestTemplate(TreeEqualityTestCase): """test Template.has_param()""" node1 = Template(wrap([Text("foobar")])) node2 = Template(wrap([Text("foo")]), - [pgenh("1", "bar"), pgens("abc", "def")]) + [pgenh("1", "bar"), pgens("\nabc ", "def")]) node3 = Template(wrap([Text("foo")]), [pgenh("1", "a"), pgens("b", "c"), pgens("1", "d")]) node4 = Template(wrap([Text("foo")]), @@ -108,7 +108,7 @@ class TestTemplate(TreeEqualityTestCase): self.assertTrue(node2.has_param("abc")) self.assertFalse(node2.has_param("def")) self.assertTrue(node3.has_param("1")) - self.assertTrue(node3.has_param("b")) + self.assertTrue(node3.has_param(" b ")) self.assertFalse(node4.has_param("b")) self.assertTrue(node3.has_param("b", False)) self.assertTrue(node4.has_param("b", False)) @@ -123,7 +123,7 @@ class TestTemplate(TreeEqualityTestCase): node3p2 = pgens("1", "d") node3 = Template(wrap([Text("foo")]), [pgenh("1", "a"), node3p1, node3p2]) - node4p1 = pgens("b", " ") + node4p1 = pgens(" b", " ") node4 = Template(wrap([Text("foo")]), [pgenh("1", "a"), node4p1]) self.assertRaises(ValueError, node1.get, "foobar") self.assertIs(node2p1, node2.get(1)) @@ -131,10 +131,56 @@ class TestTemplate(TreeEqualityTestCase): self.assertRaises(ValueError, node2.get, "def") self.assertIs(node3p1, node3.get("b")) self.assertIs(node3p2, node3.get("1")) - self.assertIs(node4p1, node4.get("b")) + self.assertIs(node4p1, node4.get("b ")) # add - # remove + + def test_remove(self): + """test Template.remove()""" + node1 = Template(wrap([Text("foobar")])) + node2 = Template(wrap([Text("foo")]), [pgenh("1", "bar"), + pgens("abc", "def")]) + node3 = Template(wrap([Text("foo")]), [pgenh("1", "bar"), + pgens("abc", "def")]) + node4 = Template(wrap([Text("foo")]), [pgenh("1", "bar"), + pgenh("2", "baz")]) + node5 = Template(wrap([Text("foo")]), [ + pgens(" a", "b"), pgens("b", "c"), pgens("a ", "d")]) + node6 = Template(wrap([Text("foo")]), [ + pgens(" a", "b"), pgens("b", "c"), pgens("a ", "d")]) + node7 = Template(wrap([Text("foo")]), [ + pgens("1 ", "a"), pgens(" 1", "b"), pgens("2", "c")]) + node8 = Template(wrap([Text("foo")]), [ + pgens("1 ", "a"), pgens(" 1", "b"), pgens("2", "c")]) + node9 = Template(wrap([Text("foo")]), [ + pgens("1 ", "a"), pgenh("1", "b"), pgenh("2", "c")]) + node10 = Template(wrap([Text("foo")]), [ + pgens("1 ", "a"), pgenh("1", "b"), pgenh("2", "c")]) + + node2.remove("1") + node2.remove("abc") + node3.remove(1, keep_field=True) + node3.remove("abc", keep_field=True) + node4.remove("1", keep_field=False) + node5.remove("a", keep_field=False) + node6.remove("a", keep_field=True) + node7.remove(1, keep_field=True) + node8.remove(1, keep_field=False) + node9.remove(1, keep_field=True) + node10.remove(1, keep_field=False) + + self.assertRaises(ValueError, node1.remove, 1) + self.assertRaises(ValueError, node1.remove, "a") + self.assertRaises(ValueError, node2.remove, "1") + self.assertEquals("{{foo}}", node2) + self.assertEquals("{{foo||abc=}}", node3) + self.assertEquals("{{foo||baz}}", node4) + self.assertEquals("{{foo|b=c}}", node5) + self.assertEquals("{{foo| a=|b=c}}", node6) + self.assertEquals("{{foo|1 =|2=c}}", node7) + self.assertEquals("{{foo|2=c}}", node8) + self.assertEquals("{{foo||c}}", node9) + self.assertEquals("{{foo||c}}", node10) if __name__ == "__main__": unittest.main(verbosity=2)