From 6c837327a44678d00c3b3555affe9620a5e16eb5 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 14:30:14 +0200 Subject: [PATCH 01/29] MetadataBox: use self as parent of QtGui.QAction actions --- picard/ui/metadatabox.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index a8525a33cb..47e396ea72 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -235,9 +235,9 @@ def __init__(self, parent): self.selection_dirty = False self.editing = None # the QTableWidgetItem being edited self.clipboard = [""] - self.add_tag_action = QtGui.QAction(_("Add New Tag…"), parent) + self.add_tag_action = QtGui.QAction(_("Add New Tag…"), self) self.add_tag_action.triggered.connect(partial(self.edit_tag, "")) - self.changes_first_action = QtGui.QAction(_("Show Changes First"), parent) + self.changes_first_action = QtGui.QAction(_("Show Changes First"), self) self.changes_first_action.setCheckable(True) self.changes_first_action.setChecked(config.persist['show_changes_first']) self.changes_first_action.toggled.connect(self.toggle_changes_first) @@ -367,18 +367,18 @@ def contextMenuEvent(self, event): if single_tag: selected_tag = tags[0] editable = self.tag_is_editable(selected_tag) - edit_tag_action = QtGui.QAction(_("Edit…"), self.parent) + edit_tag_action = QtGui.QAction(_("Edit…"), self) edit_tag_action.triggered.connect(partial(self.edit_tag, selected_tag)) edit_tag_action.setShortcut(self.edit_tag_shortcut.key()) edit_tag_action.setEnabled(editable) menu.addAction(edit_tag_action) if selected_tag not in self.preserved_tags: - add_to_preserved_tags_action = QtGui.QAction(_("Add to 'Preserve Tags' List"), self.parent) + add_to_preserved_tags_action = QtGui.QAction(_("Add to 'Preserve Tags' List"), self) add_to_preserved_tags_action.triggered.connect(partial(self.preserved_tags.add, selected_tag)) add_to_preserved_tags_action.setEnabled(editable) menu.addAction(add_to_preserved_tags_action) else: - remove_from_preserved_tags_action = QtGui.QAction(_("Remove from 'Preserve Tags' List"), self.parent) + remove_from_preserved_tags_action = QtGui.QAction(_("Remove from 'Preserve Tags' List"), self) remove_from_preserved_tags_action.triggered.connect(partial(self.preserved_tags.discard, selected_tag)) remove_from_preserved_tags_action.setEnabled(editable) menu.addAction(remove_from_preserved_tags_action) @@ -394,7 +394,7 @@ def contextMenuEvent(self, event): values = self.tag_diff.orig[tag] else: values = self.tag_diff.new[tag] - lookup_action = QtGui.QAction(_("Lookup in &Browser"), self.parent) + lookup_action = QtGui.QAction(_("Lookup in &Browser"), self) lookup_action.triggered.connect(partial(self.open_link, values, tag)) menu.addAction(lookup_action) if self.tag_is_removable(tag): @@ -420,14 +420,14 @@ def contextMenuEvent(self, event): objects = [album] orig_values = list(album.orig_metadata.getall(tag)) or [""] useorigs.append(partial(self.set_tag_values, tag, orig_values, objects)) - remove_tag_action = QtGui.QAction(_("Remove"), self.parent) + remove_tag_action = QtGui.QAction(_("Remove"), self) remove_tag_action.triggered.connect(partial(self._apply_update_funcs, removals)) remove_tag_action.setShortcut(self.remove_tag_shortcut.key()) remove_tag_action.setEnabled(bool(removals)) menu.addAction(remove_tag_action) if useorigs: name = ngettext("Use Original Value", "Use Original Values", len(useorigs)) - use_orig_value_action = QtGui.QAction(name, self.parent) + use_orig_value_action = QtGui.QAction(name, self) use_orig_value_action.triggered.connect(partial(self._apply_update_funcs, useorigs)) menu.addAction(use_orig_value_action) menu.addSeparator() From e9f90e61aa616a5e90fa46f3a232b225eb163f9d Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 15:46:32 +0200 Subject: [PATCH 02/29] lookup_tags() -> _lookup_tags() --- picard/ui/metadatabox.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index 47e396ea72..ca31488763 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -261,7 +261,7 @@ def get_file_lookup(self): config.setting['server_port'], self.tagger.browser_integration.port) - def lookup_tags(self): + def _lookup_tags(self): lookup = self.get_file_lookup() LOOKUP_TAGS = { 'musicbrainz_recordingid': lookup.recording_lookup, @@ -277,7 +277,7 @@ def lookup_tags(self): return LOOKUP_TAGS def open_link(self, values, tag): - lookup = self.lookup_tags() + lookup = self._lookup_tags() lookup_func = lookup[tag] for v in values: lookup_func(v) @@ -388,7 +388,7 @@ def contextMenuEvent(self, event): if item: column = item.column() for tag in tags: - if tag in self.lookup_tags().keys(): + if tag in self._lookup_tags().keys(): if (column == self.COLUMN_ORIG or column == self.COLUMN_NEW) and single_tag and item.text(): if column == self.COLUMN_ORIG: values = self.tag_diff.orig[tag] From bb8091c5cd795a483fc1f35e0f0ee8e72d883f1e Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 15:48:25 +0200 Subject: [PATCH 03/29] get_file_lookup() -> _get_file_lookup() --- picard/ui/metadatabox.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index ca31488763..a609b8c364 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -254,7 +254,7 @@ def __init__(self, parent): self.ignore_updates = IgnoreUpdatesContext(onexit=self.update) self.tagger.clipboard().dataChanged.connect(self.update_clipboard) - def get_file_lookup(self): + def _get_file_lookup(self): """Return a FileLookup object.""" config = get_config() return FileLookup(self, config.setting['server_host'], @@ -262,7 +262,7 @@ def get_file_lookup(self): self.tagger.browser_integration.port) def _lookup_tags(self): - lookup = self.get_file_lookup() + lookup = self._get_file_lookup() LOOKUP_TAGS = { 'musicbrainz_recordingid': lookup.recording_lookup, 'musicbrainz_trackid': lookup.track_lookup, From 74eb11052431b3c335417938ed5afa94b7469647 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 15:49:12 +0200 Subject: [PATCH 04/29] open_link() -> _open_link() --- picard/ui/metadatabox.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index a609b8c364..92c59fbec9 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -276,7 +276,7 @@ def _lookup_tags(self): } return LOOKUP_TAGS - def open_link(self, values, tag): + def _open_link(self, values, tag): lookup = self._lookup_tags() lookup_func = lookup[tag] for v in values: @@ -395,7 +395,7 @@ def contextMenuEvent(self, event): else: values = self.tag_diff.new[tag] lookup_action = QtGui.QAction(_("Lookup in &Browser"), self) - lookup_action.triggered.connect(partial(self.open_link, values, tag)) + lookup_action.triggered.connect(partial(self._open_link, values, tag)) menu.addAction(lookup_action) if self.tag_is_removable(tag): removals.append(partial(self.remove_tag, tag)) From c94599108769bc103d372e2d9f201b4aacfab893 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 15:52:17 +0200 Subject: [PATCH 05/29] copy/paste_value() -> _copy/_paste_value() --- picard/ui/metadatabox.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index 92c59fbec9..ed5efca2f8 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -303,13 +303,13 @@ def edit(self, index, trigger, event): def keyPressEvent(self, event): if event.matches(QtGui.QKeySequence.StandardKey.Copy): - self.copy_value() + self._copy_value() elif event.matches(QtGui.QKeySequence.StandardKey.Paste): - self.paste_value() + self._paste_value() else: super().keyPressEvent(event) - def copy_value(self): + def _copy_value(self): item = self.currentItem() if item: column = item.column() @@ -325,7 +325,7 @@ def copy_value(self): self.tagger.clipboard().setText(MULTI_VALUED_JOINER.join(value)) self.clipboard = value - def paste_value(self): + def _paste_value(self): item = self.currentItem() if item: column = item.column() @@ -434,11 +434,11 @@ def contextMenuEvent(self, event): if single_tag: menu.addSeparator() copy_action = QtGui.QAction(icontheme.lookup('edit-copy', icontheme.ICON_SIZE_MENU), _("&Copy"), self) - copy_action.triggered.connect(self.copy_value) + copy_action.triggered.connect(self._copy_value) copy_action.setShortcut(QtGui.QKeySequence.StandardKey.Copy) menu.addAction(copy_action) paste_action = QtGui.QAction(icontheme.lookup('edit-paste', icontheme.ICON_SIZE_MENU), _("&Paste"), self) - paste_action.triggered.connect(self.paste_value) + paste_action.triggered.connect(self._paste_value) paste_action.setShortcut(QtGui.QKeySequence.StandardKey.Paste) paste_action.setEnabled(editable) menu.addAction(paste_action) From 385bd3c2c427b843a1f32b7b33ebf5267a5e797f Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 15:54:30 +0200 Subject: [PATCH 06/29] update_clipboard() -> _update_clipboard() --- picard/ui/metadatabox.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index ed5efca2f8..77fadec672 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -252,7 +252,7 @@ def __init__(self, parent): self._single_file_album = False self._single_track_album = False self.ignore_updates = IgnoreUpdatesContext(onexit=self.update) - self.tagger.clipboard().dataChanged.connect(self.update_clipboard) + self.tagger.clipboard().dataChanged.connect(self._update_clipboard) def _get_file_lookup(self): """Return a FileLookup object.""" @@ -334,7 +334,7 @@ def _paste_value(self): self.set_tag_values(tag, self.clipboard) self.update() - def update_clipboard(self): + def _update_clipboard(self): clipboard = self.tagger.clipboard().text().split(MULTI_VALUED_JOINER) if clipboard: self.clipboard = clipboard From 903d3dc07abb5f50cefdbfe3a8d9f84767ad433a Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 16:20:43 +0200 Subject: [PATCH 07/29] edit_tag() -> _edit_tag() # Conflicts: # picard/ui/metadatabox.py --- picard/ui/metadatabox.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index 77fadec672..1b9cf65dc4 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -236,13 +236,13 @@ def __init__(self, parent): self.editing = None # the QTableWidgetItem being edited self.clipboard = [""] self.add_tag_action = QtGui.QAction(_("Add New Tag…"), self) - self.add_tag_action.triggered.connect(partial(self.edit_tag, "")) + self.add_tag_action.triggered.connect(partial(self._edit_tag, "")) self.changes_first_action = QtGui.QAction(_("Show Changes First"), self) self.changes_first_action.setCheckable(True) self.changes_first_action.setChecked(config.persist['show_changes_first']) self.changes_first_action.toggled.connect(self.toggle_changes_first) # TR: Keyboard shortcut for "Add New Tag…" - self.add_tag_shortcut = QtGui.QShortcut(QtGui.QKeySequence(_("Alt+Shift+A")), self, partial(self.edit_tag, "")) + self.add_tag_shortcut = QtGui.QShortcut(QtGui.QKeySequence(_("Alt+Shift+A")), self, partial(self._edit_tag, "")) self.add_tag_action.setShortcut(self.add_tag_shortcut.key()) # TR: Keyboard shortcut for "Edit…" (tag) self.edit_tag_shortcut = QtGui.QShortcut(QtGui.QKeySequence(_("Alt+Shift+E")), self, partial(self.edit_selected_tag)) @@ -293,7 +293,7 @@ def edit(self, index, trigger, event): tag = self.tag_diff.tag_names[item.row()] values = self.tag_diff.new[tag] if len(values) > 1: - self.edit_tag(tag) + self._edit_tag(tag) return False else: self.editing = item @@ -368,7 +368,7 @@ def contextMenuEvent(self, event): selected_tag = tags[0] editable = self.tag_is_editable(selected_tag) edit_tag_action = QtGui.QAction(_("Edit…"), self) - edit_tag_action.triggered.connect(partial(self.edit_tag, selected_tag)) + edit_tag_action.triggered.connect(partial(self._edit_tag, selected_tag)) edit_tag_action.setShortcut(self.edit_tag_shortcut.key()) edit_tag_action.setEnabled(editable) menu.addAction(edit_tag_action) @@ -456,14 +456,14 @@ def _apply_update_funcs(self, funcs): f() self.parent.update_selection(new_selection=False, drop_album_caches=True) - def edit_tag(self, tag): + def _edit_tag(self, tag): if self.tag_diff is not None: EditTagDialog(self.parent, tag).exec() def edit_selected_tag(self): tags = self.selected_tags(filter_func=self.tag_is_editable) if len(tags) == 1: - self.edit_tag(tags[0]) + self._edit_tag(tags[0]) def toggle_changes_first(self, checked): config = get_config() From a106bc97ee03f7b054e2180b424d9220fccbdb83 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 15:57:23 +0200 Subject: [PATCH 08/29] edit_selected_tag() -> _edit_selected_tag() --- picard/ui/metadatabox.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index 1b9cf65dc4..d1103cb749 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -245,7 +245,7 @@ def __init__(self, parent): self.add_tag_shortcut = QtGui.QShortcut(QtGui.QKeySequence(_("Alt+Shift+A")), self, partial(self._edit_tag, "")) self.add_tag_action.setShortcut(self.add_tag_shortcut.key()) # TR: Keyboard shortcut for "Edit…" (tag) - self.edit_tag_shortcut = QtGui.QShortcut(QtGui.QKeySequence(_("Alt+Shift+E")), self, partial(self.edit_selected_tag)) + self.edit_tag_shortcut = QtGui.QShortcut(QtGui.QKeySequence(_("Alt+Shift+E")), self, partial(self._edit_selected_tag)) # TR: Keyboard shortcut for "Remove" (tag) self.remove_tag_shortcut = QtGui.QShortcut(QtGui.QKeySequence(_("Alt+Shift+R")), self, self.remove_selected_tags) self.preserved_tags = PreservedTags() @@ -460,7 +460,7 @@ def _edit_tag(self, tag): if self.tag_diff is not None: EditTagDialog(self.parent, tag).exec() - def edit_selected_tag(self): + def _edit_selected_tag(self): tags = self.selected_tags(filter_func=self.tag_is_editable) if len(tags) == 1: self._edit_tag(tags[0]) From 02d191d2d75e7bbc18fbf3827b896ec037219ea1 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 16:00:16 +0200 Subject: [PATCH 09/29] toggle_changes_first() -> _toggle_changes_first() --- picard/ui/metadatabox.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index d1103cb749..c9e388670c 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -240,7 +240,7 @@ def __init__(self, parent): self.changes_first_action = QtGui.QAction(_("Show Changes First"), self) self.changes_first_action.setCheckable(True) self.changes_first_action.setChecked(config.persist['show_changes_first']) - self.changes_first_action.toggled.connect(self.toggle_changes_first) + self.changes_first_action.toggled.connect(self._toggle_changes_first) # TR: Keyboard shortcut for "Add New Tag…" self.add_tag_shortcut = QtGui.QShortcut(QtGui.QKeySequence(_("Alt+Shift+A")), self, partial(self._edit_tag, "")) self.add_tag_action.setShortcut(self.add_tag_shortcut.key()) @@ -465,7 +465,7 @@ def _edit_selected_tag(self): if len(tags) == 1: self._edit_tag(tags[0]) - def toggle_changes_first(self, checked): + def _toggle_changes_first(self, checked): config = get_config() config.persist['show_changes_first'] = checked self.update() From d1a9f0d519502695524f9ab514addb723f8676d7 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 16:00:43 +0200 Subject: [PATCH 10/29] set_tag_values() -> _set_tag_values() --- picard/ui/metadatabox.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index c9e388670c..c99d295a26 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -331,7 +331,7 @@ def _paste_value(self): column = item.column() tag = self.tag_diff.tag_names[item.row()] if column == self.COLUMN_NEW and self.tag_is_editable(tag): - self.set_tag_values(tag, self.clipboard) + self._set_tag_values(tag, self.clipboard) self.update() def _update_clipboard(self): @@ -347,7 +347,7 @@ def closeEditor(self, editor, hint): if old == new: self.editing.setText(old[0]) else: - self.set_tag_values(tag, new) + self._set_tag_values(tag, new) self.editing = None self.update(drop_album_caches=tag == 'album') @@ -410,16 +410,16 @@ def contextMenuEvent(self, event): file_tracks.append(file.parent) track_albums.add(file.parent.album) orig_values = list(file.orig_metadata.getall(tag)) or [""] - useorigs.append(partial(self.set_tag_values, tag, orig_values, objects)) + useorigs.append(partial(self._set_tag_values, tag, orig_values, objects)) for track in set(self.tracks)-set(file_tracks): objects = [track] orig_values = list(track.orig_metadata.getall(tag)) or [""] - useorigs.append(partial(self.set_tag_values, tag, orig_values, objects)) + useorigs.append(partial(self._set_tag_values, tag, orig_values, objects)) track_albums.add(track.album) for album in track_albums: objects = [album] orig_values = list(album.orig_metadata.getall(tag)) or [""] - useorigs.append(partial(self.set_tag_values, tag, orig_values, objects)) + useorigs.append(partial(self._set_tag_values, tag, orig_values, objects)) remove_tag_action = QtGui.QAction(_("Remove"), self) remove_tag_action.triggered.connect(partial(self._apply_update_funcs, removals)) remove_tag_action.setShortcut(self.remove_tag_shortcut.key()) @@ -470,7 +470,7 @@ def _toggle_changes_first(self, checked): config.persist['show_changes_first'] = checked self.update() - def set_tag_values(self, tag, values, objects=None): + def _set_tag_values(self, tag, values, objects=None): if objects is None: objects = self.objects with self.parent.ignore_selection_changes: @@ -486,7 +486,7 @@ def set_tag_values(self, tag, values, objects=None): obj.update() def remove_tag(self, tag): - self.set_tag_values(tag, []) + self._set_tag_values(tag, []) def remove_selected_tags(self): for tag in self.selected_tags(filter_func=self.tag_is_removable): From 901acc91a5c1d9d486963015f5a4ad6050f7c54c Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 16:02:33 +0200 Subject: [PATCH 11/29] remove_tag() -> _remove_tag() --- picard/ui/metadatabox.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index c99d295a26..c5489d2e09 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -398,7 +398,7 @@ def contextMenuEvent(self, event): lookup_action.triggered.connect(partial(self._open_link, values, tag)) menu.addAction(lookup_action) if self.tag_is_removable(tag): - removals.append(partial(self.remove_tag, tag)) + removals.append(partial(self._remove_tag, tag)) status = self.tag_diff.status[tag] & TagStatus.CHANGED if status == TagStatus.CHANGED or status == TagStatus.REMOVED: file_tracks = [] @@ -485,13 +485,13 @@ def _set_tag_values(self, tag, values, objects=None): obj.metadata[tag] = values obj.update() - def remove_tag(self, tag): + def _remove_tag(self, tag): self._set_tag_values(tag, []) def remove_selected_tags(self): for tag in self.selected_tags(filter_func=self.tag_is_removable): if self.tag_is_removable(tag): - self.remove_tag(tag) + self._remove_tag(tag) self.parent.update_selection(new_selection=False, drop_album_caches=True) def tag_is_removable(self, tag): From 8e53ebbd0594cd010f6bff4c6c61fafa7756053c Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 16:06:34 +0200 Subject: [PATCH 12/29] tag_is_(removable|editable)() -> _tag_is_(removable|editable)() --- picard/ui/metadatabox.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index c5489d2e09..15776b5a49 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -330,7 +330,7 @@ def _paste_value(self): if item: column = item.column() tag = self.tag_diff.tag_names[item.row()] - if column == self.COLUMN_NEW and self.tag_is_editable(tag): + if column == self.COLUMN_NEW and self._tag_is_editable(tag): self._set_tag_values(tag, self.clipboard) self.update() @@ -366,7 +366,7 @@ def contextMenuEvent(self, event): single_tag = len(tags) == 1 if single_tag: selected_tag = tags[0] - editable = self.tag_is_editable(selected_tag) + editable = self._tag_is_editable(selected_tag) edit_tag_action = QtGui.QAction(_("Edit…"), self) edit_tag_action.triggered.connect(partial(self._edit_tag, selected_tag)) edit_tag_action.setShortcut(self.edit_tag_shortcut.key()) @@ -397,7 +397,7 @@ def contextMenuEvent(self, event): lookup_action = QtGui.QAction(_("Lookup in &Browser"), self) lookup_action.triggered.connect(partial(self._open_link, values, tag)) menu.addAction(lookup_action) - if self.tag_is_removable(tag): + if self._tag_is_removable(tag): removals.append(partial(self._remove_tag, tag)) status = self.tag_diff.status[tag] & TagStatus.CHANGED if status == TagStatus.CHANGED or status == TagStatus.REMOVED: @@ -461,7 +461,7 @@ def _edit_tag(self, tag): EditTagDialog(self.parent, tag).exec() def _edit_selected_tag(self): - tags = self.selected_tags(filter_func=self.tag_is_editable) + tags = self.selected_tags(filter_func=self._tag_is_editable) if len(tags) == 1: self._edit_tag(tags[0]) @@ -476,7 +476,7 @@ def _set_tag_values(self, tag, values, objects=None): with self.parent.ignore_selection_changes: if values == [""]: values = [] - if not values and self.tag_is_removable(tag): + if not values and self._tag_is_removable(tag): for obj in objects: del obj.metadata[tag] obj.update() @@ -489,15 +489,15 @@ def _remove_tag(self, tag): self._set_tag_values(tag, []) def remove_selected_tags(self): - for tag in self.selected_tags(filter_func=self.tag_is_removable): - if self.tag_is_removable(tag): + for tag in self.selected_tags(filter_func=self._tag_is_removable): + if self._tag_is_removable(tag): self._remove_tag(tag) self.parent.update_selection(new_selection=False, drop_album_caches=True) - def tag_is_removable(self, tag): + def _tag_is_removable(self, tag): return self.tag_diff.status[tag] & TagStatus.NOTREMOVABLE == 0 - def tag_is_editable(self, tag): + def _tag_is_editable(self, tag): return self.tag_diff.status[tag] & TagStatus.READONLY == 0 def selected_tags(self, filter_func=None): From 03968083c4816d64ab48b783587ce9d8ad0c1547 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 16:08:31 +0200 Subject: [PATCH 13/29] remove_selected_tags(): no need to check if tag is removable, as the list is filtered already --- picard/ui/metadatabox.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index 15776b5a49..13367c1f13 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -490,8 +490,7 @@ def _remove_tag(self, tag): def remove_selected_tags(self): for tag in self.selected_tags(filter_func=self._tag_is_removable): - if self._tag_is_removable(tag): - self._remove_tag(tag) + self._remove_tag(tag) self.parent.update_selection(new_selection=False, drop_album_caches=True) def _tag_is_removable(self, tag): From 888df2349e33eb049cb8f1ca1c86d78889a765b3 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 16:10:23 +0200 Subject: [PATCH 14/29] selected_tags() -> _selected_tags() --- picard/ui/metadatabox.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index 13367c1f13..f539dae6bf 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -362,7 +362,7 @@ def _get_editor_value(editor): def contextMenuEvent(self, event): menu = QtWidgets.QMenu(self) if self.objects: - tags = self.selected_tags() + tags = self._selected_tags() single_tag = len(tags) == 1 if single_tag: selected_tag = tags[0] @@ -461,7 +461,7 @@ def _edit_tag(self, tag): EditTagDialog(self.parent, tag).exec() def _edit_selected_tag(self): - tags = self.selected_tags(filter_func=self._tag_is_editable) + tags = self._selected_tags(filter_func=self._tag_is_editable) if len(tags) == 1: self._edit_tag(tags[0]) @@ -489,7 +489,7 @@ def _remove_tag(self, tag): self._set_tag_values(tag, []) def remove_selected_tags(self): - for tag in self.selected_tags(filter_func=self._tag_is_removable): + for tag in self._selected_tags(filter_func=self._tag_is_removable): self._remove_tag(tag) self.parent.update_selection(new_selection=False, drop_album_caches=True) @@ -499,7 +499,7 @@ def _tag_is_removable(self, tag): def _tag_is_editable(self, tag): return self.tag_diff.status[tag] & TagStatus.READONLY == 0 - def selected_tags(self, filter_func=None): + def _selected_tags(self, filter_func=None): tags = set(self.tag_diff.tag_names[item.row()] for item in self.selectedItems()) if filter_func: From 7a620f0fbd02cb2c00d955e64a1f8ba26ed7fcf3 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 16:19:20 +0200 Subject: [PATCH 15/29] _selected_tags(): make it a generator --- picard/ui/metadatabox.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index f539dae6bf..a08f38abdd 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -362,7 +362,7 @@ def _get_editor_value(editor): def contextMenuEvent(self, event): menu = QtWidgets.QMenu(self) if self.objects: - tags = self._selected_tags() + tags = list(self._selected_tags()) single_tag = len(tags) == 1 if single_tag: selected_tag = tags[0] @@ -461,7 +461,7 @@ def _edit_tag(self, tag): EditTagDialog(self.parent, tag).exec() def _edit_selected_tag(self): - tags = self._selected_tags(filter_func=self._tag_is_editable) + tags = list(self._selected_tags(filter_func=self._tag_is_editable)) if len(tags) == 1: self._edit_tag(tags[0]) @@ -500,11 +500,10 @@ def _tag_is_editable(self, tag): return self.tag_diff.status[tag] & TagStatus.READONLY == 0 def _selected_tags(self, filter_func=None): - tags = set(self.tag_diff.tag_names[item.row()] - for item in self.selectedItems()) - if filter_func: - tags = filter(filter_func, tags) - return list(tags) + for tag in set(self.tag_diff.tag_names[item.row()] + for item in self.selectedItems()): + if filter_func is None or filter_func(tag): + yield tag def _update_selection(self): files = set() From 953bdbcbbc7f66a4087a73a0d656f9cb627b20d7 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 16:22:47 +0200 Subject: [PATCH 16/29] set_item_value() -> _set_item_value() --- picard/ui/metadatabox.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index a08f38abdd..57459dfed8 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -680,12 +680,12 @@ def _update_items(self, result=None, error=None): new_item = QtWidgets.QTableWidgetItem() self.setItem(i, 2, new_item) tag_item.setText(display_tag_name(name)) - self.set_item_value(orig_item, self.tag_diff.orig, name) + self._set_item_value(orig_item, self.tag_diff.orig, name) if name == '~length': new_item.setFlags(orig_flags) else: new_item.setFlags(new_flags) - self.set_item_value(new_item, self.tag_diff.new, name) + self._set_item_value(new_item, self.tag_diff.new, name) font = new_item.font() if result.tag_status(name) == TagStatus.REMOVED: @@ -708,7 +708,7 @@ def _update_items(self, result=None, error=None): # Adjust row height to content size self.setRowHeight(i, self.sizeHintForRow(i)) - def set_item_value(self, item, tags, name): + def _set_item_value(self, item, tags, name): text, italic = tags.display_value(name) item.setData(QtCore.Qt.ItemDataRole.UserRole, name) item.setText(text) From 94ff51ec37a1fc1e32a068f0a26d29c0e464daf7 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 16:27:16 +0200 Subject: [PATCH 17/29] Drop _lookup_tags() in favor of new _lookup_tag(tag) - define methods to use as strings and use getattr() --- picard/ui/metadatabox.py | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index 57459dfed8..ef744d8521 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -206,6 +206,21 @@ class MetadataBox(QtWidgets.QTableWidget): COLUMN_ORIG = 1 COLUMN_NEW = 2 + # keys are tags + # values are FileLookup methods (as string) + # to use to look up for the matching tag + LOOKUP_TAGS = { + 'acoustid_id': 'acoust_lookup', + 'musicbrainz_albumartistid': 'artist_lookup', + 'musicbrainz_albumid': 'album_lookup', + 'musicbrainz_artistid': 'artist_lookup', + 'musicbrainz_discid': 'discid_lookup', + 'musicbrainz_recordingid': 'recording_lookup', + 'musicbrainz_releasegroupid': 'release_group_lookup', + 'musicbrainz_trackid': 'track_lookup', + 'musicbrainz_workid': 'work_lookup', + } + def __init__(self, parent): super().__init__(parent) self.tagger = QtCore.QCoreApplication.instance() @@ -261,24 +276,12 @@ def _get_file_lookup(self): config.setting['server_port'], self.tagger.browser_integration.port) - def _lookup_tags(self): + def _lookup_tag(self, tag): lookup = self._get_file_lookup() - LOOKUP_TAGS = { - 'musicbrainz_recordingid': lookup.recording_lookup, - 'musicbrainz_trackid': lookup.track_lookup, - 'musicbrainz_albumid': lookup.album_lookup, - 'musicbrainz_workid': lookup.work_lookup, - 'musicbrainz_artistid': lookup.artist_lookup, - 'musicbrainz_albumartistid': lookup.artist_lookup, - 'musicbrainz_releasegroupid': lookup.release_group_lookup, - 'musicbrainz_discid': lookup.discid_lookup, - 'acoustid_id': lookup.acoust_lookup - } - return LOOKUP_TAGS + return getattr(lookup, self.LOOKUP_TAGS[tag]) def _open_link(self, values, tag): - lookup = self._lookup_tags() - lookup_func = lookup[tag] + lookup_func = self._lookup_tag(tag) for v in values: lookup_func(v) @@ -388,7 +391,7 @@ def contextMenuEvent(self, event): if item: column = item.column() for tag in tags: - if tag in self._lookup_tags().keys(): + if tag in self.LOOKUP_TAGS: if (column == self.COLUMN_ORIG or column == self.COLUMN_NEW) and single_tag and item.text(): if column == self.COLUMN_ORIG: values = self.tag_diff.orig[tag] From 6a1d568152afd89b8d7fd3f56a4c61304cc53942 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 17:11:00 +0200 Subject: [PATCH 18/29] Test if methods listed in MetadataBox.LOOKUP_TAGS are valid FileLookup methods --- test/test_metadatabox.py | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 test/test_metadatabox.py diff --git a/test/test_metadatabox.py b/test/test_metadatabox.py new file mode 100644 index 0000000000..69e60dbe08 --- /dev/null +++ b/test/test_metadatabox.py @@ -0,0 +1,35 @@ +# -*- coding: utf-8 -*- +# +# Picard, the next-generation MusicBrainz tagger +# +# Copyright (C) 2024 Laurent Monin +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + + +from test.picardtestcase import PicardTestCase + +from picard.browser.filelookup import FileLookup + +from picard.ui.metadatabox import MetadataBox + + +class MetadataBoxFileLookupTest(PicardTestCase): + def test_filelook_methods(self): + """Test if methods listed in MetadataBox.LOOKUP_TAGS are valid FileLookup methods""" + for method_as_string in MetadataBox.LOOKUP_TAGS.values(): + method = getattr(FileLookup, method_as_string, None) + self.assertIsNotNone(method, f"No such FileLookup.{method_as_string}") + self.assertTrue(callable(method), f"FileLookup.{method_as_string} is not callable") From 1d06036d1b944f53a855fd444656b56144bd5f99 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 17:30:16 +0200 Subject: [PATCH 19/29] Use named columns instead of numerical values --- picard/ui/metadatabox.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index ef744d8521..622418e340 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -203,6 +203,7 @@ def get_tag_name(self, index): class MetadataBox(QtWidgets.QTableWidget): + COLUMN_TAG = 0 COLUMN_ORIG = 1 COLUMN_NEW = 2 @@ -665,23 +666,23 @@ def _update_items(self, result=None, error=None): new_flags = orig_flags | QtCore.Qt.ItemFlag.ItemIsEditable for i, name in enumerate(result.tag_names): - tag_item = self.item(i, 0) - orig_item = self.item(i, 1) - new_item = self.item(i, 2) + tag_item = self.item(i, self.COLUMN_TAG) + orig_item = self.item(i, self.COLUMN_ORIG) + new_item = self.item(i, self.COLUMN_NEW) if not tag_item: tag_item = QtWidgets.QTableWidgetItem() tag_item.setFlags(orig_flags) font = tag_item.font() font.setBold(True) tag_item.setFont(font) - self.setItem(i, 0, tag_item) + self.setItem(i, self.COLUMN_TAG, tag_item) if not orig_item: orig_item = QtWidgets.QTableWidgetItem() orig_item.setFlags(orig_flags) - self.setItem(i, 1, orig_item) + self.setItem(i, self.COLUMN_ORIG, orig_item) if not new_item: new_item = QtWidgets.QTableWidgetItem() - self.setItem(i, 2, new_item) + self.setItem(i, self.COLUMN_NEW, new_item) tag_item.setText(display_tag_name(name)) self._set_item_value(orig_item, self.tag_diff.orig, name) if name == '~length': From 6157063671653acd874fce126838773fdeafee8f Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 17:36:58 +0200 Subject: [PATCH 20/29] name -> tag (as we iterate among tags) --- picard/ui/metadatabox.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index 622418e340..34bfe759b8 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -598,28 +598,28 @@ def _update_tags(self, new_selection=True, drop_album_caches=False): orig_metadata = file.orig_metadata tags = set(new_metadata) | set(orig_metadata) - for name in filter(lambda x: not x.startswith("~") and file.supports_tag(x), tags): - new_values = file.format_specific_metadata(new_metadata, name, settings) - orig_values = file.format_specific_metadata(orig_metadata, name, settings) + for tag in filter(lambda x: not x.startswith("~") and file.supports_tag(x), tags): + new_values = file.format_specific_metadata(new_metadata, tag, settings) + orig_values = file.format_specific_metadata(orig_metadata, tag, settings) if not clear_existing_tags and not new_values: new_values = list(orig_values or [""]) - removed = name in new_metadata.deleted_tags - tag_diff.add(name, orig_values, new_values, True, removed, top_tags=top_tags_set) + removed = tag in new_metadata.deleted_tags + tag_diff.add(tag, orig_values, new_values, True, removed, top_tags=top_tags_set) tag_diff.add('~length', str(orig_metadata.length), str(new_metadata.length), removable=False, readonly=True) for track in tracks: if track.num_linked_files == 0: - for name, new_values in track.metadata.rawitems(): - if not name.startswith("~"): - if name in track.orig_metadata: - orig_values = track.orig_metadata.getall(name) + for tag, new_values in track.metadata.rawitems(): + if not tag.startswith("~"): + if tag in track.orig_metadata: + orig_values = track.orig_metadata.getall(tag) else: orig_values = new_values - tag_diff.add(name, orig_values, new_values, True) + tag_diff.add(tag, orig_values, new_values, True) length = str(track.metadata.length) tag_diff.add('~length', length, length, removable=False, readonly=True) From 6ed7802777c5173d7fbed05f5b7e5b3c0b640d41 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 17:40:53 +0200 Subject: [PATCH 21/29] Use if/continue instead of filter(lambda...), more readable --- picard/ui/metadatabox.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index 34bfe759b8..2add970be7 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -598,7 +598,11 @@ def _update_tags(self, new_selection=True, drop_album_caches=False): orig_metadata = file.orig_metadata tags = set(new_metadata) | set(orig_metadata) - for tag in filter(lambda x: not x.startswith("~") and file.supports_tag(x), tags): + for tag in tags: + if tag.startswith("~"): + continue + if not file.supports_tag(tag): + continue new_values = file.format_specific_metadata(new_metadata, tag, settings) orig_values = file.format_specific_metadata(orig_metadata, tag, settings) From f82c5ac1dd8ca12ee203961756c6747503b9813d Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 17:46:03 +0200 Subject: [PATCH 22/29] Use self.tag_diff after it was set to result It will make it easier to move code --- picard/ui/metadatabox.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index 2add970be7..a0c91ec918 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -660,16 +660,16 @@ def _update_items(self, result=None, error=None): self.tag_diff = result - if result is None: + if self.tag_diff is None: self.setRowCount(0) return - self.setRowCount(len(result.tag_names)) + self.setRowCount(len(self.tag_diff.tag_names)) orig_flags = QtCore.Qt.ItemFlag.ItemIsSelectable | QtCore.Qt.ItemFlag.ItemIsEnabled new_flags = orig_flags | QtCore.Qt.ItemFlag.ItemIsEditable - for i, name in enumerate(result.tag_names): + for i, name in enumerate(self.tag_diff.tag_names): tag_item = self.item(i, self.COLUMN_TAG) orig_item = self.item(i, self.COLUMN_ORIG) new_item = self.item(i, self.COLUMN_NEW) @@ -696,14 +696,14 @@ def _update_items(self, result=None, error=None): self._set_item_value(new_item, self.tag_diff.new, name) font = new_item.font() - if result.tag_status(name) == TagStatus.REMOVED: + if self.tag_diff.tag_status(name) == TagStatus.REMOVED: font.setStrikeOut(True) else: font.setStrikeOut(False) new_item.setFont(font) - color = self.colors.get(result.tag_status(name), + color = self.colors.get(self.tag_diff.tag_status(name), self.colors[TagStatus.NOCHANGE]) orig_item.setForeground(color) new_item.setForeground(color) From 43a11032df90dd6589409bb3f4331fea9898da47 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 17:48:48 +0200 Subject: [PATCH 23/29] Minor code redundancy reduction --- picard/ui/metadatabox.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index a0c91ec918..e10a11f9fc 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -696,11 +696,8 @@ def _update_items(self, result=None, error=None): self._set_item_value(new_item, self.tag_diff.new, name) font = new_item.font() - if self.tag_diff.tag_status(name) == TagStatus.REMOVED: - font.setStrikeOut(True) - else: - font.setStrikeOut(False) - + strikeout = self.tag_diff.tag_status(name) == TagStatus.REMOVED + font.setStrikeOut(strikeout) new_item.setFont(font) color = self.colors.get(self.tag_diff.tag_status(name), From 4e1ff5527400b58d85d53bcd88c6804c575f625c Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 17:52:13 +0200 Subject: [PATCH 24/29] Consistency: we use tag instead of name everywhere else in the file --- picard/ui/metadatabox.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index e10a11f9fc..5b4e65ef60 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -669,7 +669,7 @@ def _update_items(self, result=None, error=None): orig_flags = QtCore.Qt.ItemFlag.ItemIsSelectable | QtCore.Qt.ItemFlag.ItemIsEnabled new_flags = orig_flags | QtCore.Qt.ItemFlag.ItemIsEditable - for i, name in enumerate(self.tag_diff.tag_names): + for i, tag in enumerate(self.tag_diff.tag_names): tag_item = self.item(i, self.COLUMN_TAG) orig_item = self.item(i, self.COLUMN_ORIG) new_item = self.item(i, self.COLUMN_NEW) @@ -687,20 +687,20 @@ def _update_items(self, result=None, error=None): if not new_item: new_item = QtWidgets.QTableWidgetItem() self.setItem(i, self.COLUMN_NEW, new_item) - tag_item.setText(display_tag_name(name)) - self._set_item_value(orig_item, self.tag_diff.orig, name) - if name == '~length': + tag_item.setText(display_tag_name(tag)) + self._set_item_value(orig_item, self.tag_diff.orig, tag) + if tag == '~length': new_item.setFlags(orig_flags) else: new_item.setFlags(new_flags) - self._set_item_value(new_item, self.tag_diff.new, name) + self._set_item_value(new_item, self.tag_diff.new, tag) font = new_item.font() - strikeout = self.tag_diff.tag_status(name) == TagStatus.REMOVED + strikeout = self.tag_diff.tag_status(tag) == TagStatus.REMOVED font.setStrikeOut(strikeout) new_item.setFont(font) - color = self.colors.get(self.tag_diff.tag_status(name), + color = self.colors.get(self.tag_diff.tag_status(tag), self.colors[TagStatus.NOCHANGE]) orig_item.setForeground(color) new_item.setForeground(color) @@ -713,9 +713,9 @@ def _update_items(self, result=None, error=None): # Adjust row height to content size self.setRowHeight(i, self.sizeHintForRow(i)) - def _set_item_value(self, item, tags, name): - text, italic = tags.display_value(name) - item.setData(QtCore.Qt.ItemDataRole.UserRole, name) + def _set_item_value(self, item, tags, tag): + text, italic = tags.display_value(tag) + item.setData(QtCore.Qt.ItemDataRole.UserRole, tag) item.setText(text) font = item.font() font.setItalic(italic) From be261bb9d3bb90d004f4ea53a210c6d6df9a17a2 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 18:18:37 +0200 Subject: [PATCH 25/29] _update_items(): regroup code per column - alignment never changes, move it outside the loop - set new_item flags once at object creation - it makes the code a bit easier to read imho --- picard/ui/metadatabox.py | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index 5b4e65ef60..17a3ef136a 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -668,48 +668,48 @@ def _update_items(self, result=None, error=None): orig_flags = QtCore.Qt.ItemFlag.ItemIsSelectable | QtCore.Qt.ItemFlag.ItemIsEnabled new_flags = orig_flags | QtCore.Qt.ItemFlag.ItemIsEditable + alignment = QtCore.Qt.AlignmentFlag.AlignLeft | QtCore.Qt.AlignmentFlag.AlignTop for i, tag in enumerate(self.tag_diff.tag_names): + color = self.colors.get(self.tag_diff.tag_status(tag), + self.colors[TagStatus.NOCHANGE]) + tag_item = self.item(i, self.COLUMN_TAG) - orig_item = self.item(i, self.COLUMN_ORIG) - new_item = self.item(i, self.COLUMN_NEW) if not tag_item: tag_item = QtWidgets.QTableWidgetItem() tag_item.setFlags(orig_flags) font = tag_item.font() font.setBold(True) tag_item.setFont(font) + tag_item.setTextAlignment(alignment) self.setItem(i, self.COLUMN_TAG, tag_item) + tag_item.setText(display_tag_name(tag)) + + orig_item = self.item(i, self.COLUMN_ORIG) if not orig_item: orig_item = QtWidgets.QTableWidgetItem() orig_item.setFlags(orig_flags) + orig_item.setTextAlignment(alignment) self.setItem(i, self.COLUMN_ORIG, orig_item) + self._set_item_value(orig_item, self.tag_diff.orig, tag) + orig_item.setForeground(color) + + new_item = self.item(i, self.COLUMN_NEW) if not new_item: new_item = QtWidgets.QTableWidgetItem() + new_item.setTextAlignment(alignment) + if tag == '~length': + new_item.setFlags(orig_flags) + else: + new_item.setFlags(new_flags) self.setItem(i, self.COLUMN_NEW, new_item) - tag_item.setText(display_tag_name(tag)) - self._set_item_value(orig_item, self.tag_diff.orig, tag) - if tag == '~length': - new_item.setFlags(orig_flags) - else: - new_item.setFlags(new_flags) self._set_item_value(new_item, self.tag_diff.new, tag) - font = new_item.font() strikeout = self.tag_diff.tag_status(tag) == TagStatus.REMOVED font.setStrikeOut(strikeout) new_item.setFont(font) - - color = self.colors.get(self.tag_diff.tag_status(tag), - self.colors[TagStatus.NOCHANGE]) - orig_item.setForeground(color) new_item.setForeground(color) - alignment = QtCore.Qt.AlignmentFlag.AlignLeft | QtCore.Qt.AlignmentFlag.AlignTop - tag_item.setTextAlignment(alignment) - orig_item.setTextAlignment(alignment) - new_item.setTextAlignment(alignment) - # Adjust row height to content size self.setRowHeight(i, self.sizeHintForRow(i)) From 138996d21938f219f52ca7719ee41eff990a06df Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 19:41:00 +0200 Subject: [PATCH 26/29] Use a namedtuple for display values, with fields text and is_grouped - it makes things a bit clearer - it will be easier to add new fields --- picard/ui/edittagdialog.py | 14 +++++++++++--- picard/ui/metadatabox.py | 21 +++++++++++++++------ 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/picard/ui/edittagdialog.py b/picard/ui/edittagdialog.py index 9d59862ff4..47f903768c 100644 --- a/picard/ui/edittagdialog.py +++ b/picard/ui/edittagdialog.py @@ -240,9 +240,17 @@ def tag_changed(self, tag): values = self.modified_tags.get(self.tag, None) if values is None: new_tags = self.metadata_box.tag_diff.new - display_value, self.different = new_tags.display_value(self.tag) - values = [display_value] if self.different else new_tags[self.tag] - self.ui.add_value.setEnabled(not self.different) + display_value = new_tags.display_value(self.tag) + if display_value.is_grouped: + # grouped values have a special text, which isn't a valid tag value + self.different = True + values = [display_value.text] + self.ui.add_value.setEnabled(False) + else: + # normal tag values + self.different = False + values = new_tags[self.tag] + self.ui.add_value.setEnabled(True) self.value_list.model().rowsInserted.disconnect(self.on_rows_inserted) self._add_value_items(values) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index 17a3ef136a..a537dea05f 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -34,6 +34,7 @@ from collections import ( Counter, defaultdict, + namedtuple, ) from functools import partial @@ -83,6 +84,9 @@ class TagStatus: READONLY = 32 +TagCounterDisplayValue = namedtuple('TagCounterDisplayValue', ('text', 'is_grouped')) + + class TagCounter(dict): __slots__ = ('parent', 'counts', 'different') @@ -109,7 +113,8 @@ def display_value(self, tag): missing = self.parent.objects - count if tag in self.different: - return (ngettext("(different across %d item)", "(different across %d items)", count) % count, True) + text = ngettext("(different across %d item)", "(different across %d items)", count) % count + is_grouped = True else: if tag == '~length': msg = format_time(self.get(tag, 0)) @@ -117,9 +122,13 @@ def display_value(self, tag): msg = MULTI_VALUED_JOINER.join(self[tag]) if count > 0 and missing > 0: - return (msg + " " + (ngettext("(missing from %d item)", "(missing from %d items)", missing) % missing), True) + text = msg + " " + (ngettext("(missing from %d item)", "(missing from %d items)", missing) % missing) + is_grouped = True else: - return (msg, False) + text = msg + is_grouped = False + + return TagCounterDisplayValue(text, is_grouped) class TagDiff(object): @@ -714,11 +723,11 @@ def _update_items(self, result=None, error=None): self.setRowHeight(i, self.sizeHintForRow(i)) def _set_item_value(self, item, tags, tag): - text, italic = tags.display_value(tag) + display_value = tags.display_value(tag) item.setData(QtCore.Qt.ItemDataRole.UserRole, tag) - item.setText(text) + item.setText(display_value.text) font = item.font() - font.setItalic(italic) + font.setItalic(display_value.is_grouped) item.setFont(font) @restore_method From a095e31f42c4aaec6a47dbcdfcccc4378fc887ac Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 19:49:41 +0200 Subject: [PATCH 27/29] different -> is_grouped --- picard/ui/edittagdialog.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/picard/ui/edittagdialog.py b/picard/ui/edittagdialog.py index 47f903768c..2fbbf36563 100644 --- a/picard/ui/edittagdialog.py +++ b/picard/ui/edittagdialog.py @@ -104,7 +104,7 @@ def __init__(self, window, tag): self.metadata_box = window.metadata_box self.tag = tag self.modified_tags = {} - self.different = False + self.is_grouped = False self.default_tags = sorted( set(list(TAG_NAMES.keys()) + self.metadata_box.tag_diff.tag_names)) if len(self.metadata_box.files) == 1: @@ -165,8 +165,8 @@ def add_or_edit_value(self): def remove_value(self): value_list = self.value_list row = value_list.currentRow() - if row == 0 and self.different: - self.different = False + if row == 0 and self.is_grouped: + self.is_grouped = False self.ui.add_value.setEnabled(True) value_list.takeItem(row) @@ -241,14 +241,13 @@ def tag_changed(self, tag): if values is None: new_tags = self.metadata_box.tag_diff.new display_value = new_tags.display_value(self.tag) - if display_value.is_grouped: + self.is_grouped = display_value.is_grouped + if self.is_grouped: # grouped values have a special text, which isn't a valid tag value - self.different = True values = [display_value.text] self.ui.add_value.setEnabled(False) else: # normal tag values - self.different = False values = new_tags[self.tag] self.ui.add_value.setEnabled(True) @@ -264,16 +263,16 @@ def _add_value_items(self, values): item = QtWidgets.QListWidgetItem(value) item.setFlags(QtCore.Qt.ItemFlag.ItemIsSelectable | QtCore.Qt.ItemFlag.ItemIsEnabled | QtCore.Qt.ItemFlag.ItemIsEditable | QtCore.Qt.ItemFlag.ItemIsDragEnabled) font = item.font() - font.setItalic(self.different) + font.setItalic(self.is_grouped) item.setFont(font) self.value_list.addItem(item) def value_edited(self, item): row = self.value_list.row(item) value = item.text() - if row == 0 and self.different: + if row == 0 and self.is_grouped: self.modified_tags[self.tag] = [value] - self.different = False + self.is_grouped = False font = item.font() font.setItalic(False) item.setFont(font) From 14db96d5d1485232f3c85b216a66d76d989d83ee Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 20:04:10 +0200 Subject: [PATCH 28/29] Add two helper functions to toggle between grouped and normal values --- picard/ui/edittagdialog.py | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/picard/ui/edittagdialog.py b/picard/ui/edittagdialog.py index 2fbbf36563..43010fc97c 100644 --- a/picard/ui/edittagdialog.py +++ b/picard/ui/edittagdialog.py @@ -162,12 +162,15 @@ def add_or_edit_value(self): else: self.add_value() + def _group(self, is_grouped): + self.is_grouped = is_grouped + self.ui.add_value.setEnabled(not is_grouped) + def remove_value(self): value_list = self.value_list row = value_list.currentRow() if row == 0 and self.is_grouped: - self.is_grouped = False - self.ui.add_value.setEnabled(True) + self._group(False) value_list.takeItem(row) def on_rows_inserted(self, parent, first, last): @@ -241,15 +244,14 @@ def tag_changed(self, tag): if values is None: new_tags = self.metadata_box.tag_diff.new display_value = new_tags.display_value(self.tag) - self.is_grouped = display_value.is_grouped - if self.is_grouped: + if display_value.is_grouped: # grouped values have a special text, which isn't a valid tag value values = [display_value.text] - self.ui.add_value.setEnabled(False) + self._group(True) else: # normal tag values values = new_tags[self.tag] - self.ui.add_value.setEnabled(True) + self._group(False) self.value_list.model().rowsInserted.disconnect(self.on_rows_inserted) self._add_value_items(values) @@ -257,14 +259,17 @@ def tag_changed(self, tag): self.value_list.setCurrentItem(self.value_list.item(0), QtCore.QItemSelectionModel.SelectionFlag.SelectCurrent) tag_names.editTextChanged.connect(self.tag_changed) + def _set_item_style(self, item): + font = item.font() + font.setItalic(self.is_grouped) + item.setFont(font) + def _add_value_items(self, values): values = [v for v in values if v] or [""] for value in values: item = QtWidgets.QListWidgetItem(value) item.setFlags(QtCore.Qt.ItemFlag.ItemIsSelectable | QtCore.Qt.ItemFlag.ItemIsEnabled | QtCore.Qt.ItemFlag.ItemIsEditable | QtCore.Qt.ItemFlag.ItemIsDragEnabled) - font = item.font() - font.setItalic(self.is_grouped) - item.setFont(font) + self._set_item_style(item) self.value_list.addItem(item) def value_edited(self, item): @@ -272,11 +277,8 @@ def value_edited(self, item): value = item.text() if row == 0 and self.is_grouped: self.modified_tags[self.tag] = [value] - self.is_grouped = False - font = item.font() - font.setItalic(False) - item.setFont(font) - self.ui.add_value.setEnabled(True) + self._group(False) + self._set_item_style(item) else: self._modified_tag()[row] = value # add tags to the completer model once they get values From 72c7c60fd509814b1c10cb9e5fb335643409d12c Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 17 May 2024 22:33:19 +0200 Subject: [PATCH 29/29] Variables orig_tags and new_tags aren't needed, drop keys() as it's the default for dicts --- picard/ui/metadatabox.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/picard/ui/metadatabox.py b/picard/ui/metadatabox.py index a537dea05f..1efadc6e2a 100644 --- a/picard/ui/metadatabox.py +++ b/picard/ui/metadatabox.py @@ -592,8 +592,6 @@ def _update_tags(self, new_selection=True, drop_album_caches=False): config = get_config() tag_diff = TagDiff(max_length_diff=config.setting['ignore_track_duration_difference_under']) - orig_tags = tag_diff.orig - new_tags = tag_diff.new tag_diff.objects = len(files) clear_existing_tags = config.setting['clear_existing_tags'] @@ -639,7 +637,7 @@ def _update_tags(self, new_selection=True, drop_album_caches=False): tag_diff.objects += 1 - all_tags = set(list(orig_tags.keys()) + list(new_tags.keys())) + all_tags = set(list(tag_diff.orig) + list(tag_diff.new)) common_tags = [tag for tag in top_tags if tag in all_tags] tag_names = common_tags + sorted(all_tags.difference(common_tags), key=lambda x: display_tag_name(x).lower())