diff --git a/picard/metadata.py b/picard/metadata.py index dca8ffb8a..180da615a 100644 --- a/picard/metadata.py +++ b/picard/metadata.py @@ -433,8 +433,9 @@ class Metadata(MutableMapping): name = self.normalize_tag(name) if isinstance(values, str) or not isinstance(values, Iterable): values = [values] - values = [str(value) for value in values if value or value == 0] - if values: + values = [str(value) for value in values if value or value == 0 or value == ''] + # Remove if there is only a single empty or blank element. + if values and (len(values) > 1 or values[0]): self._store[name] = values self.deleted_tags.discard(name) elif name in self._store: diff --git a/picard/script/functions.py b/picard/script/functions.py index 091a1a023..93c3a4d10 100644 --- a/picard/script/functions.py +++ b/picard/script/functions.py @@ -1494,3 +1494,24 @@ _Since Picard 2.7_""" def func_is_multi(parser, multi): multi_value = MultiValue(parser, multi, MULTI_VALUED_JOINER) return '' if len(multi_value) < 2 else '1' + + +@script_function(eval_args=True, documentation=N_( + """`$clean_multi(name)` + +Removes all empty string elements from the multi-value variable. + +Example: + + $$setmulti(test,one; ; two; three) + $clean_multi(%test%) + +Result: Sets the value of 'test' to ["one", "two", "three"]. + +_Since Picard 2.8_""" +)) +def func_clean_multi(parser, multi): + name = normalize_tagname(multi) + values = [str(value) for value in parser.context.getall(name) if value or value == 0] + parser.context[multi] = values + return "" diff --git a/test/test_script.py b/test/test_script.py index 49d18bb7e..f72714360 100644 --- a/test/test_script.py +++ b/test/test_script.py @@ -427,16 +427,16 @@ class ScriptParserTest(PicardTestCase): self.assertEqual("", self.parser.eval("$setmulti(test,%source%)", context)) # no return value self.assertEqual(context.getall("source"), context.getall("test")) - def test_cmd_setmulti_will_remove_empty_items(self): + def test_cmd_setmulti_will_keep_empty_items(self): context = Metadata() context["source"] = ["", "multi", ""] self.assertEqual("", self.parser.eval("$setmulti(test,%source%)", context)) # no return value - self.assertEqual(["multi"], context.getall("test")) + self.assertEqual(["", "multi", ""], context.getall("test")) def test_cmd_setmulti_custom_splitter_string(self): context = Metadata() self.assertEqual("", self.parser.eval("$setmulti(test,multi##valued##test##,##)", context)) # no return value - self.assertEqual(["multi", "valued", "test"], context.getall("test")) + self.assertEqual(["multi", "valued", "test", ""], context.getall("test")) def test_cmd_setmulti_empty_splitter_does_nothing(self): context = Metadata() @@ -1785,3 +1785,56 @@ class ScriptParserTest(PicardTestCase): self.parser.eval("$is_multi()") with self.assertRaisesRegex(ScriptError, areg): self.parser.eval("$is_multi(a,)") + + def test_cmd_clean_multi(self): + context = Metadata() + context["foo"] = ["", "one", "two"] + context["bar"] = ["one", "", "two"] + context["baz"] = ["one", "two", ""] + + # Confirm initial values + self.assertScriptResultEquals("%foo%", "; one; two", context) + self.assertScriptResultEquals("%bar%", "one; ; two", context) + self.assertScriptResultEquals("%baz%", "one; two; ", context) + # Test cleaned values + self.assertScriptResultEquals("$clean_multi(foo)%foo%", "one; two", context) + self.assertScriptResultEquals("$clean_multi(bar)%bar%", "one; two", context) + self.assertScriptResultEquals("$clean_multi(baz)%baz%", "one; two", context) + + # Text clean with only empty string elements + context["foo"] = ["", "", ""] + + # Confirm initial values + self.assertScriptResultEquals("%foo%", "; ; ", context) + # Test cleaned values + self.assertScriptResultEquals("$clean_multi(foo)%foo%", "", context) + + # Test clean with indirect argument + context["foo"] = ["", "one", "two"] + context["bar"] = "foo" + + # Confirm initial values + self.assertScriptResultEquals("%foo%", "; one; two", context) + # Test cleaned values + self.assertScriptResultEquals("$clean_multi(%bar%)%foo%", "one; two", context) + + # Test clean with non-multi argument + context["foo"] = "one" + context["bar"] = "one; ; two" + context["baz"] = "" + + # Confirm initial values + self.assertScriptResultEquals("%foo%", "one", context) + self.assertScriptResultEquals("%bar%", "one; ; two", context) + self.assertScriptResultEquals("%baz%", "", context) + # Test cleaned values + self.assertScriptResultEquals("$clean_multi(foo)%foo%", "one", context) + self.assertScriptResultEquals("$clean_multi(bar)%bar%", "one; ; two", context) + self.assertScriptResultEquals("$clean_multi(baz)%baz%", "", context) + + # Tests with invalid number of arguments + areg = r"^\d+:\d+:\$clean_multi: Wrong number of arguments for \$clean_multi: Expected exactly 1, " + with self.assertRaisesRegex(ScriptError, areg): + self.parser.eval("$clean_multi()") + with self.assertRaisesRegex(ScriptError, areg): + self.parser.eval("$clean_multi(foo,)")