From 3d2a5cf2506d92e6291da8d2c6bcabfff7b6056f Mon Sep 17 00:00:00 2001
From: Philipp Wolfer <ph.wolfer@gmail.com>
Date: Fri, 31 Jan 2020 17:32:25 +0100
Subject: [PATCH] PICARD-1719: Fix $unset must not mark tags for deletion.

This regressed in Picard 2.2.0 with commit 503b520 and $unset behaves the same as $delete.
---
 picard/metadata.py    | 12 ++++++++++++
 picard/script.py      |  4 ++--
 test/test_metadata.py | 11 +++++++++++
 test/test_script.py   | 11 ++++++++++-
 4 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/picard/metadata.py b/picard/metadata.py
index e0ce5175b..cea33f598 100644
--- a/picard/metadata.py
+++ b/picard/metadata.py
@@ -381,6 +381,18 @@ class Metadata(MutableMapping):
         """Deprecated: use del directly"""
         del self[self.normalize_tag(name)]
 
+    def unset(self, name):
+        """Removes a tag from the metadata, but does not mark it for deletion.
+
+        Args:
+            name: name of the tag to unset
+
+        Raises:
+            KeyError: name not set
+        """
+        name = self.normalize_tag(name)
+        del self._store[name]
+
     def __iter__(self):
         return iter(self._store)
 
diff --git a/picard/script.py b/picard/script.py
index b9df0fb4c..8686be035 100644
--- a/picard/script.py
+++ b/picard/script.py
@@ -489,10 +489,10 @@ def func_unset(parser, name):
         name = name[:-1]
         for key in list(parser.context.keys()):
             if key.startswith(name):
-                del parser.context[key]
+                parser.context.unset(key)
         return ""
     try:
-        del parser.context[name]
+        parser.context.unset(name)
     except KeyError:
         pass
     return ""
diff --git a/test/test_metadata.py b/test/test_metadata.py
index 12c09b28b..1d4d45014 100644
--- a/test/test_metadata.py
+++ b/test/test_metadata.py
@@ -108,7 +108,18 @@ class MetadataTest(PicardTestCase):
         metadata_items = [(x, z) for (x, y) in self.metadata.rawitems() for z in y]
         self.assertEqual(metadata_items, list(self.metadata.items()))
 
+    def test_metadata_unset(self):
+        self.metadata.unset("single1")
+        self.assertNotIn("single1", self.metadata)
+        self.assertNotIn("single1", self.metadata.deleted_tags)
+        self.assertRaises(KeyError, self.metadata.unset, 'unknown_tag')
+
     def test_metadata_delete(self):
+        del self.metadata["single1"]
+        self.assertNotIn("single1", self.metadata)
+        self.assertIn("single1", self.metadata.deleted_tags)
+
+    def test_metadata_legacy_delete(self):
         self.metadata.delete("single1")
         self.assertNotIn("single1", self.metadata)
         self.assertIn("single1", self.metadata.deleted_tags)
diff --git a/test/test_script.py b/test/test_script.py
index c360c5706..242b43357 100644
--- a/test/test_script.py
+++ b/test/test_script.py
@@ -69,7 +69,9 @@ class ScriptParserTest(PicardTestCase):
         self.assertScriptResultEquals("$right(abcd,x)", "")
 
     def test_cmd_set(self):
-        self.assertScriptResultEquals("$set(test,aaa)%test%", "aaa")
+        context = Metadata()
+        self.assertScriptResultEquals("$set(test,aaa)%test%", "aaa", context)
+        self.assertEqual(context['test'], 'aaa')
 
     def test_cmd_set_empty(self):
         self.assertScriptResultEquals("$set(test,)%test%", "")
@@ -464,6 +466,13 @@ class ScriptParserTest(PicardTestCase):
         self.assertNotIn('performer:bar', context)
         self.assertNotIn('performer:foo', context)
 
+    def test_cmd_unset(self):
+        context = Metadata()
+        context['title'] = 'Foo'
+        self.parser.eval("$unset(title)", context)
+        self.assertNotIn('title', context)
+        self.assertNotIn('title', context.deleted_tags)
+
     def test_cmd_delete(self):
         context = Metadata()
         context['title'] = 'Foo'