From 85282935f60f30b37295a607caa410430a7277bb Mon Sep 17 00:00:00 2001 From: Antonio Larrosa Date: Sun, 12 Feb 2017 20:47:15 +0100 Subject: [PATCH 1/4] Add test for the inmulti function Add a test to check that the inmulti function works as expected. --- test/test_script.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/test_script.py b/test/test_script.py index 35e45cffd..e9846256f 100644 --- a/test/test_script.py +++ b/test/test_script.py @@ -370,3 +370,22 @@ class ScriptParserTest(unittest.TestCase): self.parser.eval("$unset(performer:*)", context) self.assertNotIn('performer:bar', context) self.assertNotIn('performer:foo', context) + + def test_cmd_inmulti(self): + context = Metadata() + self.parser.eval("$set(foo,First; Second; Third)", context) + self.assertEqual(self.parser.eval("$in(%foo%,Second)", context), "1") + self.assertEqual(self.parser.eval("$in(%foo%,irst; Second; Thi)", context), "1") + self.assertEqual(self.parser.eval("$in(%foo%,First; Second; Third)", context), "1") + self.assertEqual(self.parser.eval("$inmulti(%foo%,Second)", context), "") + self.assertEqual(self.parser.eval("$inmulti(%foo%,irst; Second; Thi)", context), "") + self.assertEqual(self.parser.eval("$inmulti(%foo%,First; Second; Third)", context), "1") + + self.parser.eval("$setmulti(foo,First; Second; Third)", context) + self.assertEqual(self.parser.eval("$in(%foo%,Second)", context), "1") + self.assertEqual(self.parser.eval("$in(%foo%,irst; Second; Thi)", context), "1") + self.assertEqual(self.parser.eval("$in(%foo%,First; Second; Third)", context), "1") + self.assertEqual(self.parser.eval("$inmulti(%foo%,Second)", context), "1") + self.assertEqual(self.parser.eval("$inmulti(%foo%,irst; Second; Thi)", context), "") + self.assertEqual(self.parser.eval("$inmulti(%foo%,First; Second; Third)", context), "") + From 9155c0bd8f5e25797e9a8f7b71b6615c5b5369e2 Mon Sep 17 00:00:00 2001 From: Antonio Larrosa Date: Sun, 12 Feb 2017 20:50:55 +0100 Subject: [PATCH 2/4] Fix inmulti behaviour with multi-values inmulti arguments where evaluated before inmulti was executed, which resulted in strings being passed as arguments, which breaks cases in which the strings in multivalue variables contain the separator used to separate multiple values (see PICARD-922). This commit changes the behaviour of inmulti so arguments are not evaluated before they're passed, so it can extract the argument value as a list and work with that. This should fix PICARD-922 --- picard/script.py | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/picard/script.py b/picard/script.py index 1fca623c0..54ad48131 100644 --- a/picard/script.py +++ b/picard/script.py @@ -387,9 +387,30 @@ def func_in(parser, text, needle): return "" -def func_inmulti(parser, text, value, separator=MULTI_VALUED_JOINER): - """Splits ``text`` by ``separator``, and returns true if the resulting list contains ``value``.""" - return func_in(parser, text.split(separator) if separator else [text], value) +def func_inmulti(parser, haystack, needle, separator=MULTI_VALUED_JOINER): + """Searches for ``needle`` in ``haystack``, supporting a list variable for ``haystack``. + If a string is used instead, then a ``separator`` can be used to split it. + In both cases, it returns true if the resulting list contains ``needle``.""" + + needle = needle.eval(parser) + if type(text)==ScriptExpression and len(text)==1 and type(text[0])==ScriptVariable: + text=text[0] + if type(text)==ScriptVariable: + if text.name.startswith(u"_"): + name = u"~" + text.name[1:] + else: + name = text.name + values=parser.context.getall(name) + + if needle in values: + return "1" + else: + return "" + + # I'm not sure if it is actually possible to continue in this code path, + # but just in case, it's better to have a fallback to correct behaviour + text=text.eval(parser) + return func_in(parser, text.split(separator) if separator else [text], needle) def func_rreplace(parser, text, old, new): @@ -819,7 +840,7 @@ register_script_function(func_lte, "lte") register_script_function(func_gt, "gt") register_script_function(func_gte, "gte") register_script_function(func_in, "in") -register_script_function(func_inmulti, "inmulti") +register_script_function(func_inmulti, "inmulti", eval_args=False) register_script_function(func_copy, "copy") register_script_function(func_copymerge, "copymerge") register_script_function(func_len, "len") From b7c324b4de2dab0b8facd402ab8aa12f44cbb3cd Mon Sep 17 00:00:00 2001 From: Antonio Larrosa Date: Mon, 13 Feb 2017 15:41:20 +0100 Subject: [PATCH 3/4] Fix PEP8 issues (mostly lines too large) Add a few empty lines and wrap some lines so they're not so large --- test/test_script.py | 41 ++++++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/test/test_script.py b/test/test_script.py index e9846256f..c7102c581 100644 --- a/test/test_script.py +++ b/test/test_script.py @@ -6,15 +6,19 @@ from picard.script import ScriptParser, ScriptError, register_script_function from picard.metadata import Metadata from picard.ui.options.renaming import _DEFAULT_FILE_NAMING_FORMAT + class ScriptParserTest(unittest.TestCase): def setUp(self): config.setting = { 'enabled_plugins': '', } + self.parser = ScriptParser() + def func_noargstest(parser): return "" + register_script_function(func_noargstest, "noargstest") def test_cmd_noop(self): @@ -374,18 +378,29 @@ class ScriptParserTest(unittest.TestCase): def test_cmd_inmulti(self): context = Metadata() self.parser.eval("$set(foo,First; Second; Third)", context) - self.assertEqual(self.parser.eval("$in(%foo%,Second)", context), "1") - self.assertEqual(self.parser.eval("$in(%foo%,irst; Second; Thi)", context), "1") - self.assertEqual(self.parser.eval("$in(%foo%,First; Second; Third)", context), "1") - self.assertEqual(self.parser.eval("$inmulti(%foo%,Second)", context), "") - self.assertEqual(self.parser.eval("$inmulti(%foo%,irst; Second; Thi)", context), "") - self.assertEqual(self.parser.eval("$inmulti(%foo%,First; Second; Third)", context), "1") + self.assertEqual( + self.parser.eval("$in(%foo%,Second)", context), "1") + self.assertEqual( + self.parser.eval("$in(%foo%,irst; Second; Thi)", context), "1") + self.assertEqual( + self.parser.eval("$in(%foo%,First; Second; Third)", context), "1") + self.assertEqual( + self.parser.eval("$inmulti(%foo%,Second)", context), "") + self.assertEqual( + self.parser.eval("$inmulti(%foo%,irst; Second; Thi)", context), "") + self.assertEqual( + self.parser.eval("$inmulti(%foo%,First; Second; Third)", context), "1") self.parser.eval("$setmulti(foo,First; Second; Third)", context) - self.assertEqual(self.parser.eval("$in(%foo%,Second)", context), "1") - self.assertEqual(self.parser.eval("$in(%foo%,irst; Second; Thi)", context), "1") - self.assertEqual(self.parser.eval("$in(%foo%,First; Second; Third)", context), "1") - self.assertEqual(self.parser.eval("$inmulti(%foo%,Second)", context), "1") - self.assertEqual(self.parser.eval("$inmulti(%foo%,irst; Second; Thi)", context), "") - self.assertEqual(self.parser.eval("$inmulti(%foo%,First; Second; Third)", context), "") - + self.assertEqual( + self.parser.eval("$in(%foo%,Second)", context), "1") + self.assertEqual( + self.parser.eval("$in(%foo%,irst; Second; Thi)", context), "1") + self.assertEqual( + self.parser.eval("$in(%foo%,First; Second; Third)", context), "1") + self.assertEqual( + self.parser.eval("$inmulti(%foo%,Second)", context), "1") + self.assertEqual( + self.parser.eval("$inmulti(%foo%,irst; Second; Thi)", context), "") + self.assertEqual( + self.parser.eval("$inmulti(%foo%,First; Second; Third)", context), "") From 5daf2fecda9d06c96824ed322487fcf40f105768 Mon Sep 17 00:00:00 2001 From: Antonio Larrosa Date: Mon, 13 Feb 2017 16:24:52 +0100 Subject: [PATCH 4/4] Fix PEP8 issues and rename text to haystack everywhere The text variable was renamed to haystack except in some lines. This commit fixes this and also fixes some coding style issues. --- picard/script.py | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/picard/script.py b/picard/script.py index 54ad48131..487d99b52 100644 --- a/picard/script.py +++ b/picard/script.py @@ -388,29 +388,32 @@ def func_in(parser, text, needle): def func_inmulti(parser, haystack, needle, separator=MULTI_VALUED_JOINER): - """Searches for ``needle`` in ``haystack``, supporting a list variable for ``haystack``. - If a string is used instead, then a ``separator`` can be used to split it. - In both cases, it returns true if the resulting list contains ``needle``.""" + """Searches for ``needle`` in ``haystack``, supporting a list variable for + ``haystack``. If a string is used instead, then a ``separator`` can be + used to split it. In both cases, it returns true if the resulting list + contains ``needle``.""" needle = needle.eval(parser) - if type(text)==ScriptExpression and len(text)==1 and type(text[0])==ScriptVariable: - text=text[0] - if type(text)==ScriptVariable: - if text.name.startswith(u"_"): - name = u"~" + text.name[1:] - else: - name = text.name - values=parser.context.getall(name) + if (isinstance(haystack, ScriptExpression) and + len(haystack) == 1 and + isinstance(haystack[0], ScriptVariable)): + haystack = haystack[0] - if needle in values: - return "1" + if isinstance(haystack, ScriptVariable): + if haystack.name.startswith(u"_"): + name = u"~" + haystack.name[1:] else: - return "" + name = haystack.name + values = parser.context.getall(name) + + return func_in(parser, values, needle) # I'm not sure if it is actually possible to continue in this code path, # but just in case, it's better to have a fallback to correct behaviour - text=text.eval(parser) - return func_in(parser, text.split(separator) if separator else [text], needle) + haystack = haystack.eval(parser) + return func_in(parser, + haystack.split(separator) if separator else [haystack], + needle) def func_rreplace(parser, text, old, new):