From df00e4522d7337513f254459f06bf37355830336 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Thu, 23 May 2024 15:44:30 +0200 Subject: [PATCH] Make remove_metadata_images() returns a boolean indicating changes like similar methods --- picard/item.py | 6 +++++- picard/metadata.py | 15 ++++++++++----- test/test_imagelist.py | 18 +++++++++--------- 3 files changed, 24 insertions(+), 15 deletions(-) diff --git a/picard/item.py b/picard/item.py index 360b767d8..c54971d69 100644 --- a/picard/item.py +++ b/picard/item.py @@ -206,11 +206,15 @@ class MetadataItem(Item): Args: removed_sources: List of child objects (`Track` or `File`) which's metadata images should be removed from """ + changed = False + for metadata_attr in self.update_children_metadata_attrs: removed_images = self.get_sources_metadata_images(getattr(s, metadata_attr) for s in removed_sources) sources_metadata = list(self.iter_children_items_metadata(metadata_attr)) metadata = getattr(self, metadata_attr) - metadata.remove_images(sources_metadata, removed_images) + changed |= metadata.remove_images(sources_metadata, removed_images) + + return changed def add_metadata_images(self, added_sources): """Add the images in the metadata of `added_sources` to the metadata. diff --git a/picard/metadata.py b/picard/metadata.py index 2c6e4566a..3a2bcd4c8 100644 --- a/picard/metadata.py +++ b/picard/metadata.py @@ -604,19 +604,22 @@ class Metadata(MutableMapping): Args: sources: List of source `Metadata` objects removed_images: Set of `CoverArt` to removed + + Returns: + True if self.images was modified, False else """ if not self.images or not removed_images: - return + return False if not sources: self.images = ImageList() self.has_common_images = True - return + return True current_images = set(self.images) if self.has_common_images and current_images == removed_images: - return + return False common_images = True # True, if all children share the same images previous_images = None @@ -631,10 +634,12 @@ class Metadata(MutableMapping): previous_images = set(source_metadata.images) # Remember for next iteration removed_images = removed_images.difference(source_images) if not removed_images and not common_images: - return # No images left to remove, abort immediately + return False # No images left to remove, abort immediately - self.images = ImageList(current_images.difference(removed_images)) + new_images = current_images.difference(removed_images) + self.images = ImageList(new_images) self.has_common_images = common_images + return True class MultiMetadataProxy: diff --git a/test/test_imagelist.py b/test/test_imagelist.py index 874540e93..952df36ee 100644 --- a/test/test_imagelist.py +++ b/test/test_imagelist.py @@ -143,7 +143,7 @@ class RemoveMetadataImagesTest(PicardTestCase): cluster.files = list(self.test_files) cluster.update_metadata_images_from_children() cluster.files.remove(self.test_files[0]) - cluster.remove_metadata_images([self.test_files[0]]) + self.assertTrue(cluster.remove_metadata_images([self.test_files[0]])) self.assertEqual(set(self.test_images[1:]), set(cluster.metadata.images)) self.assertTrue(cluster.metadata.has_common_images) @@ -152,7 +152,7 @@ class RemoveMetadataImagesTest(PicardTestCase): cluster.files = list(self.test_files[1:]) cluster.update_metadata_images_from_children() cluster.files.remove(self.test_files[1]) - cluster.remove_metadata_images([self.test_files[1]]) + self.assertFalse(cluster.remove_metadata_images([self.test_files[1]])) self.assertEqual(set(self.test_images[1:]), set(cluster.metadata.images)) self.assertTrue(cluster.metadata.has_common_images) @@ -160,7 +160,7 @@ class RemoveMetadataImagesTest(PicardTestCase): cluster = Cluster('Test') cluster.files.append(File('test1.flac')) cluster.update_metadata_images_from_children() - cluster.remove_metadata_images([cluster.files[0]]) + self.assertFalse(cluster.remove_metadata_images([cluster.files[0]])) self.assertEqual(set(), set(cluster.metadata.images)) self.assertTrue(cluster.metadata.has_common_images) @@ -169,7 +169,7 @@ class RemoveMetadataImagesTest(PicardTestCase): track.files = list(self.test_files) track.update_metadata_images_from_children() track.files.remove(self.test_files[0]) - track.remove_metadata_images([self.test_files[0]]) + self.assertTrue(track.remove_metadata_images([self.test_files[0]])) self.assertEqual(set(self.test_images[1:]), set(track.orig_metadata.images)) self.assertTrue(track.orig_metadata.has_common_images) @@ -178,7 +178,7 @@ class RemoveMetadataImagesTest(PicardTestCase): track.files = list(self.test_files[1:]) track.update_metadata_images_from_children() track.files.remove(self.test_files[1]) - track.remove_metadata_images([self.test_files[1]]) + self.assertFalse(track.remove_metadata_images([self.test_files[1]])) self.assertEqual(set(self.test_images[1:]), set(track.orig_metadata.images)) self.assertTrue(track.orig_metadata.has_common_images) @@ -186,7 +186,7 @@ class RemoveMetadataImagesTest(PicardTestCase): track = Track('00000000-0000-0000-0000-000000000000') track.files.append(File('test1.flac')) track.update_metadata_images_from_children() - track.remove_metadata_images([track.files[0]]) + self.assertFalse(track.remove_metadata_images([track.files[0]])) self.assertEqual(set(), set(track.orig_metadata.images)) self.assertTrue(track.orig_metadata.has_common_images) @@ -195,7 +195,7 @@ class RemoveMetadataImagesTest(PicardTestCase): album.unmatched_files.files = list(self.test_files) album.update_metadata_images_from_children() album.unmatched_files.files.remove(self.test_files[0]) - album.remove_metadata_images([self.test_files[0]]) + self.assertTrue(album.remove_metadata_images([self.test_files[0]])) self.assertEqual(set(self.test_images[1:]), set(album.metadata.images)) self.assertEqual(set(self.test_images[1:]), set(album.orig_metadata.images)) self.assertTrue(album.metadata.has_common_images) @@ -206,7 +206,7 @@ class RemoveMetadataImagesTest(PicardTestCase): album.unmatched_files.files = list(self.test_files[1:]) album.update_metadata_images_from_children() album.unmatched_files.files.remove(self.test_files[1]) - album.remove_metadata_images([self.test_files[1]]) + self.assertFalse(album.remove_metadata_images([self.test_files[1]])) self.assertEqual(set(self.test_images[1:]), set(album.metadata.images)) self.assertEqual(set(self.test_images[1:]), set(album.orig_metadata.images)) self.assertTrue(album.metadata.has_common_images) @@ -216,7 +216,7 @@ class RemoveMetadataImagesTest(PicardTestCase): album = Album('00000000-0000-0000-0000-000000000000') album.unmatched_files.files.append(File('test1.flac')) album.update_metadata_images_from_children() - album.remove_metadata_images([album.unmatched_files.files[0]]) + self.assertFalse(album.remove_metadata_images([album.unmatched_files.files[0]])) self.assertEqual(set(), set(album.metadata.images)) self.assertEqual(set(), set(album.orig_metadata.images)) self.assertTrue(album.metadata.has_common_images)