From a52cf0392fe7ace9376215d38e4d4a33a74845af Mon Sep 17 00:00:00 2001 From: m42i Date: Wed, 19 Mar 2014 12:43:42 +0100 Subject: [PATCH 01/24] Change color of $noop to be dark gray to better distinguish comments --- picard/ui/options/scripting.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/picard/ui/options/scripting.py b/picard/ui/options/scripting.py index 9df9c6e66..370b79689 100644 --- a/picard/ui/options/scripting.py +++ b/picard/ui/options/scripting.py @@ -28,10 +28,15 @@ class TaggerScriptSyntaxHighlighter(QtGui.QSyntaxHighlighter): def __init__(self, document): QtGui.QSyntaxHighlighter.__init__(self, document) - self.func_re = QtCore.QRegExp(r"\$[a-zA-Z][_a-zA-Z0-9]*\(") + self.func_re = QtCore.QRegExp(r"\$(?!noop)[a-zA-Z][_a-zA-Z0-9]*\(") self.func_fmt = QtGui.QTextCharFormat() self.func_fmt.setFontWeight(QtGui.QFont.Bold) self.func_fmt.setForeground(QtCore.Qt.blue) + self.noop_re = QtCore.QRegExp(r"\$noop\([^\)]*\)?") + self.noop_fmt = QtGui.QTextCharFormat() + self.noop_fmt.setFontWeight(QtGui.QFont.Bold) + self.noop_fmt.setFontItalic(True) + self.noop_fmt.setForeground(QtCore.Qt.darkGray) self.var_re = QtCore.QRegExp(r"%[_a-zA-Z0-9:]*%") self.var_fmt = QtGui.QTextCharFormat() self.var_fmt.setForeground(QtCore.Qt.darkCyan) @@ -43,6 +48,7 @@ class TaggerScriptSyntaxHighlighter(QtGui.QSyntaxHighlighter): self.special_fmt.setForeground(QtCore.Qt.blue) self.rules = [ (self.func_re, self.func_fmt, 0, -1), + (self.noop_re, self.noop_fmt, 0, 0), (self.var_re, self.var_fmt, 0, 0), (self.escape_re, self.escape_fmt, 0, 0), (self.special_re, self.special_fmt, 1, -1), From 1dffd71bb40d26a62710dce5e919cf9ce68c0ef4 Mon Sep 17 00:00:00 2001 From: m42i Date: Wed, 19 Mar 2014 12:43:42 +0100 Subject: [PATCH 02/24] Follow noop function highlighting over multiple lines --- picard/ui/options/scripting.py | 41 +++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/picard/ui/options/scripting.py b/picard/ui/options/scripting.py index 370b79689..e716404b8 100644 --- a/picard/ui/options/scripting.py +++ b/picard/ui/options/scripting.py @@ -32,11 +32,6 @@ class TaggerScriptSyntaxHighlighter(QtGui.QSyntaxHighlighter): self.func_fmt = QtGui.QTextCharFormat() self.func_fmt.setFontWeight(QtGui.QFont.Bold) self.func_fmt.setForeground(QtCore.Qt.blue) - self.noop_re = QtCore.QRegExp(r"\$noop\([^\)]*\)?") - self.noop_fmt = QtGui.QTextCharFormat() - self.noop_fmt.setFontWeight(QtGui.QFont.Bold) - self.noop_fmt.setFontItalic(True) - self.noop_fmt.setForeground(QtCore.Qt.darkGray) self.var_re = QtCore.QRegExp(r"%[_a-zA-Z0-9:]*%") self.var_fmt = QtGui.QTextCharFormat() self.var_fmt.setForeground(QtCore.Qt.darkCyan) @@ -46,15 +41,22 @@ class TaggerScriptSyntaxHighlighter(QtGui.QSyntaxHighlighter): self.special_re = QtCore.QRegExp(r"[^\\][(),]") self.special_fmt = QtGui.QTextCharFormat() self.special_fmt.setForeground(QtCore.Qt.blue) + self.bracket = QtCore.QRegExp(r"[()]") + self.noop_re = QtCore.QRegExp(r"\$noop\(") + self.noop_fmt = QtGui.QTextCharFormat() + self.noop_fmt.setFontWeight(QtGui.QFont.Bold) + self.noop_fmt.setFontItalic(True) + self.noop_fmt.setForeground(QtCore.Qt.darkGray) self.rules = [ (self.func_re, self.func_fmt, 0, -1), - (self.noop_re, self.noop_fmt, 0, 0), (self.var_re, self.var_fmt, 0, 0), (self.escape_re, self.escape_fmt, 0, 0), (self.special_re, self.special_fmt, 1, -1), ] def highlightBlock(self, text): + self.setCurrentBlockState(0) + for expr, fmt, a, b in self.rules: index = expr.indexIn(text) while index >= 0: @@ -62,6 +64,33 @@ class TaggerScriptSyntaxHighlighter(QtGui.QSyntaxHighlighter): self.setFormat(index + a, length + b, fmt) index = expr.indexIn(text, index + length + b) + # Ignore everything if we're already in a noop function + index = self.noop_re.indexIn(text) if self.previousBlockState() <= 0 else 0 + open_brackets = self.previousBlockState() if self.previousBlockState() > 0 else 0 + while index >= 0: + next_index = self.bracket.indexIn(text, index) + + # Skip escaped brackets + if (next_index > 0) and text[next_index - 1] == '\\': + next_index += 1 + + if (next_index > -1) and text[next_index] == '(': + open_brackets += 1 + elif (next_index > -1) and text[next_index] == ')': + open_brackets -= 1 + + if (next_index > -1): + self.setFormat(index, next_index - index + 1, self.noop_fmt) + elif (next_index == -1) and (open_brackets > 0): + self.setFormat(index, len(text) - index, self.noop_fmt) + + # Check for next noop operation, necessary for multiple noops in one line + if open_brackets == 0: + next_index = self.noop_re.indexIn(text, next_index) + + index = next_index + 1 if (next_index > -1) and (next_index < len(text)) else -1 + + self.setCurrentBlockState(open_brackets) class ScriptingOptionsPage(OptionsPage): From 2e7e24c4f7962a4833d64df59dd82766fff8e288 Mon Sep 17 00:00:00 2001 From: m42i Date: Thu, 20 Mar 2014 12:33:13 +0100 Subject: [PATCH 03/24] Rename regex variable to bracket_re --- picard/ui/options/scripting.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/picard/ui/options/scripting.py b/picard/ui/options/scripting.py index e716404b8..4611b3a29 100644 --- a/picard/ui/options/scripting.py +++ b/picard/ui/options/scripting.py @@ -41,7 +41,7 @@ class TaggerScriptSyntaxHighlighter(QtGui.QSyntaxHighlighter): self.special_re = QtCore.QRegExp(r"[^\\][(),]") self.special_fmt = QtGui.QTextCharFormat() self.special_fmt.setForeground(QtCore.Qt.blue) - self.bracket = QtCore.QRegExp(r"[()]") + self.bracket_re = QtCore.QRegExp(r"[()]") self.noop_re = QtCore.QRegExp(r"\$noop\(") self.noop_fmt = QtGui.QTextCharFormat() self.noop_fmt.setFontWeight(QtGui.QFont.Bold) @@ -68,7 +68,7 @@ class TaggerScriptSyntaxHighlighter(QtGui.QSyntaxHighlighter): index = self.noop_re.indexIn(text) if self.previousBlockState() <= 0 else 0 open_brackets = self.previousBlockState() if self.previousBlockState() > 0 else 0 while index >= 0: - next_index = self.bracket.indexIn(text, index) + next_index = self.bracket_re.indexIn(text, index) # Skip escaped brackets if (next_index > 0) and text[next_index - 1] == '\\': From 2fbde0ee13f6862ac9fe4ab00ca0a6a83ca8b13a Mon Sep 17 00:00:00 2001 From: Sophist Date: Wed, 26 Mar 2014 11:43:24 +0000 Subject: [PATCH 04/24] Move and rename default preserved tags so that they can be used by other modules. --- picard/file.py | 8 ++------ picard/util/tags.py | 5 +++++ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/picard/file.py b/picard/file.py index fb225b612..5b5b60501 100644 --- a/picard/file.py +++ b/picard/file.py @@ -46,6 +46,7 @@ from picard.util import ( unaccent, ) from picard.util.filenaming import make_short_filename +from picard.util.tags import MEDIA_TAGS class File(QtCore.QObject, Item): @@ -119,17 +120,12 @@ class File(QtCore.QObject, Item): self.orig_metadata = metadata self.metadata.copy(metadata) - _default_preserved_tags = [ - "~bitrate", "~bits_per_sample", "~format", "~channels", "~filename", - "~dirname", "~extension" - ] - def copy_metadata(self, metadata): acoustid = self.metadata["acoustid_id"] preserve = config.setting["preserved_tags"].strip() saved_metadata = {} - for tag in re.split(r"\s*,\s*", preserve) + File._default_preserved_tags: + for tag in re.split(r"\s*,\s*", preserve) + MEDIA_TAGS: values = self.orig_metadata.getall(tag) if values: saved_metadata[tag] = values diff --git a/picard/util/tags.py b/picard/util/tags.py index a391cdb27..2c1deebdb 100644 --- a/picard/util/tags.py +++ b/picard/util/tags.py @@ -91,6 +91,11 @@ TAG_NAMES = { 'work': N_('Work'), } +MEDIA_TAGS = [ + "~bitrate", "~bits_per_sample", "~format", "~channels", "~sample_rate", + "~dirname", "~filename", "~extension", +] + def display_tag_name(name): if ':' in name: From 35ba51351bfb1da755e24e59b752c646afcb6cbd Mon Sep 17 00:00:00 2001 From: Sophist Date: Fri, 28 Mar 2014 14:46:22 +0000 Subject: [PATCH 05/24] Fix and extend artists tag ... 1. artists tag is incorrectly added to a release - now added only to a track 2. add _albumartists and _albumartists_sort for use in scripts and plugins. --- picard/mbxml.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/picard/mbxml.py b/picard/mbxml.py index 3157913af..14f76855d 100644 --- a/picard/mbxml.py +++ b/picard/mbxml.py @@ -141,6 +141,7 @@ def artist_credit_from_node(node): artist = "" artistsort = "" artists = [] + artistssort = [] standardize_artists = config.setting["standardize_artists"] for credit in node.name_credit: a = credit.artist[0] @@ -154,24 +155,28 @@ def artist_credit_from_node(node): artist += name artistsort += translated_sort artists.append(name) + artistssort.append(translated_sort) if 'joinphrase' in credit.attribs: artist += credit.joinphrase artistsort += credit.joinphrase - return (artist, artistsort, artists) + return (artist, artistsort, artists, artistssort) def artist_credit_to_metadata(node, m, release=False): ids = [n.artist[0].id for n in node.name_credit] - artist, artistsort, artists = artist_credit_from_node(node) - m["artists"] = artists + artist, artistsort, artists, artistssort = artist_credit_from_node(node) if release: m["musicbrainz_albumartistid"] = ids m["albumartist"] = artist m["albumartistsort"] = artistsort + m["~albumartists"] = artists + m["~albumartists_sort"] = artistssort else: m["musicbrainz_artistid"] = ids m["artist"] = artist m["artistsort"] = artistsort + m["artists"] = artists + m["~artists_sort"] = artistsort def label_info_from_node(node): From af693808d7227f368bd07f286dfc5382cfa5a443 Mon Sep 17 00:00:00 2001 From: DanMan Date: Fri, 28 Mar 2014 21:21:06 +0100 Subject: [PATCH 06/24] Store ReplayGain info for MP3s in ID3v2 instead of APE tags (supported for years now) --- contrib/plugins/replaygain/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/plugins/replaygain/__init__.py b/contrib/plugins/replaygain/__init__.py index 54fb951ae..1bc4a36a7 100644 --- a/contrib/plugins/replaygain/__init__.py +++ b/contrib/plugins/replaygain/__init__.py @@ -130,7 +130,7 @@ class ReplayGainOptionsPage(OptionsPage): TextOption("setting", "replaygain_vorbisgain_command", "vorbisgain"), TextOption("setting", "replaygain_vorbisgain_options", "-asf"), TextOption("setting", "replaygain_mp3gain_command", "mp3gain"), - TextOption("setting", "replaygain_mp3gain_options", "-a"), + TextOption("setting", "replaygain_mp3gain_options", "-a -s i"), TextOption("setting", "replaygain_metaflac_command", "metaflac"), TextOption("setting", "replaygain_metaflac_options", "--add-replay-gain"), TextOption("setting", "replaygain_wvgain_command", "wvgain"), From 070f8939fd341cf6612b865d72718ee185226730 Mon Sep 17 00:00:00 2001 From: Sophist Date: Sun, 30 Mar 2014 11:27:26 +0100 Subject: [PATCH 07/24] Make code clearer (as suggested by jesus2099) --- picard/album.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/picard/album.py b/picard/album.py index 8cc654b86..f4638b8a2 100644 --- a/picard/album.py +++ b/picard/album.py @@ -195,7 +195,7 @@ class Album(DataObject, Item): if not self._tracks_loaded: totalalbumtracks = 0 - albumtracknumber = 0 + absolutetracknumber = 0 va = self._new_metadata['musicbrainz_albumartistid'] == VARIOUS_ARTISTS_ID djmix_ars = {} @@ -219,8 +219,8 @@ class Album(DataObject, Item): tm = track.metadata tm.copy(mm) track_to_metadata(track_node, track) - albumtracknumber += 1 - tm["~absolutetracknumber"] = albumtracknumber + absolutetracknumber += 1 + tm["~absolutetracknumber"] = absolutetracknumber track._customize_metadata() self._new_metadata.length += tm.length From 207e7de8b174be8598295254d73dfd392e84fe95 Mon Sep 17 00:00:00 2001 From: Sophist Date: Mon, 31 Mar 2014 07:24:50 +0100 Subject: [PATCH 08/24] Switch back to PRESERVED_TAGS --- picard/file.py | 4 ++-- picard/util/tags.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/picard/file.py b/picard/file.py index 5b5b60501..6ae3f2f58 100644 --- a/picard/file.py +++ b/picard/file.py @@ -46,7 +46,7 @@ from picard.util import ( unaccent, ) from picard.util.filenaming import make_short_filename -from picard.util.tags import MEDIA_TAGS +from picard.util.tags import PRESERVED_TAGS class File(QtCore.QObject, Item): @@ -125,7 +125,7 @@ class File(QtCore.QObject, Item): preserve = config.setting["preserved_tags"].strip() saved_metadata = {} - for tag in re.split(r"\s*,\s*", preserve) + MEDIA_TAGS: + for tag in re.split(r"\s*,\s*", preserve) + PRESERVED_TAGS: values = self.orig_metadata.getall(tag) if values: saved_metadata[tag] = values diff --git a/picard/util/tags.py b/picard/util/tags.py index 2c1deebdb..2b50e25c8 100644 --- a/picard/util/tags.py +++ b/picard/util/tags.py @@ -91,7 +91,7 @@ TAG_NAMES = { 'work': N_('Work'), } -MEDIA_TAGS = [ +PRESERVED_TAGS = [ "~bitrate", "~bits_per_sample", "~format", "~channels", "~sample_rate", "~dirname", "~filename", "~extension", ] From 56ac2849b9faff5d1d9817334037f25983ce50fc Mon Sep 17 00:00:00 2001 From: Sophist Date: Mon, 31 Mar 2014 17:47:44 +0100 Subject: [PATCH 09/24] Fix sub-menu capability for plugins... 1. xrange(x, y) goes from x to y-1. If len(MENU) is 1 this is never called. So add 1 to get correct range. 2. Exception handling is typically slower and code is less clear than if we just test for the key existing. --- picard/ui/itemviews.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/picard/ui/itemviews.py b/picard/ui/itemviews.py index 67c6a4d7e..aaa280850 100644 --- a/picard/ui/itemviews.py +++ b/picard/ui/itemviews.py @@ -370,11 +370,11 @@ class BaseTreeView(QtGui.QTreeWidget): plugin_menus = {} for action in plugin_actions: action_menu = plugin_menu - for index in xrange(1, len(action.MENU)): + for index in xrange(1, len(action.MENU)+1): key = tuple(action.MENU[:index]) - try: + if key in plugin_menus: action_menu = plugin_menus[key] - except KeyError: + else: action_menu = plugin_menus[key] = action_menu.addMenu(key[-1]) action_menu.addAction(action) From 704c68f76f83548891af0055476352f4901a1547 Mon Sep 17 00:00:00 2001 From: Sophist Date: Mon, 31 Mar 2014 22:25:11 +0100 Subject: [PATCH 10/24] Minor style improvement --- picard/ui/itemviews.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/picard/ui/itemviews.py b/picard/ui/itemviews.py index aaa280850..d22ac3db0 100644 --- a/picard/ui/itemviews.py +++ b/picard/ui/itemviews.py @@ -370,7 +370,7 @@ class BaseTreeView(QtGui.QTreeWidget): plugin_menus = {} for action in plugin_actions: action_menu = plugin_menu - for index in xrange(1, len(action.MENU)+1): + for index in xrange(1, len(action.MENU) + 1): key = tuple(action.MENU[:index]) if key in plugin_menus: action_menu = plugin_menus[key] From 8ebb2e1b0de993647a9e62e7e3474a75e527eeb0 Mon Sep 17 00:00:00 2001 From: Sophist Date: Tue, 1 Apr 2014 08:01:47 +0100 Subject: [PATCH 11/24] Add NEWS.txt --- NEWS.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/NEWS.txt b/NEWS.txt index f55907486..091d34dc1 100644 --- a/NEWS.txt +++ b/NEWS.txt @@ -19,8 +19,9 @@ * autodetect the CD drive on Mac OS X (PICARD-123) * Ignore directories and files while indexing when show_hidden_files option is set to False (PICARD-528) * Add ignore_regex option which allows one to ignore matching paths, can be set in Options > Advanced (PICARD-528) - * Added an "artists" tag to track metadata, based on the one in Jaikoz, which - contains the individual artist names from the artist credit. + * Added an "artists" multi-value tag to track metadata, based on the one in Jaikoz, which contains the individual + artist names from the artist credit. Also useful in scripts (joining phrases like 'feat:' are omitted) and plugins. + * Added "_artists_sort", "_albumartists", "_albumartists_sort" variables for scripts and plugins. * Made Picard use the country names also used on the MusicBrainz website (PICARD-205) * New setup.py command `get_po_files` (Retrieve po files from transifex) * New setup.py command `update_countries` (Regenerate countries.py) From 0f274c8ab0e03367ddc89fa5699e9afce50875b8 Mon Sep 17 00:00:00 2001 From: Sophist Date: Tue, 1 Apr 2014 20:17:05 +0100 Subject: [PATCH 12/24] Show id3 version in info - resolves PICARD-218 --- picard/formats/id3.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/picard/formats/id3.py b/picard/formats/id3.py index 1d402373f..5b480e927 100644 --- a/picard/formats/id3.py +++ b/picard/formats/id3.py @@ -436,7 +436,10 @@ class MP3File(ID3File): def _info(self, metadata, file): super(MP3File, self)._info(metadata, file) - metadata['~format'] = 'MPEG-1 Layer %d' % file.info.layer + id3version = '' + if file.info.layer == 3: + id3version = ' - ID3v%d.%d' % (file.tags.version[0], file.tags.version[1]) + metadata['~format'] = 'MPEG-1 Layer %d%s' % (file.info.layer, id3version) class TrueAudioFile(ID3File): From c3ee57ee645143b43be2c5aacc83f37af0cf8e5a Mon Sep 17 00:00:00 2001 From: Sophist Date: Thu, 3 Apr 2014 10:29:20 +0100 Subject: [PATCH 13/24] Add NEWS --- NEWS.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.txt b/NEWS.txt index 6880a76bf..2b3c8b61c 100644 --- a/NEWS.txt +++ b/NEWS.txt @@ -34,7 +34,7 @@ * Support dropping image directly from Google image results to cover art box * Add %_musicbrainz_tracknumber% to hold track # as shown on MusicBrainz release web-page e.g. vinyl/cassette style A1, A2, B1, B2 - + * Show the ID3 version of the file in the Info... dialog (Ctrl-I) (PICARD-218) Version 1.2 - 2013-03-30 * Picard now requires at least Python 2.6 From 8dec7fb20f6a60e9721890b0b8cdc7ffebb89e53 Mon Sep 17 00:00:00 2001 From: Sophist Date: Thu, 3 Apr 2014 10:57:09 +0100 Subject: [PATCH 14/24] Tweak NEWS --- NEWS.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS.txt b/NEWS.txt index 2b3c8b61c..37ffab757 100644 --- a/NEWS.txt +++ b/NEWS.txt @@ -36,6 +36,7 @@ e.g. vinyl/cassette style A1, A2, B1, B2 * Show the ID3 version of the file in the Info... dialog (Ctrl-I) (PICARD-218) + Version 1.2 - 2013-03-30 * Picard now requires at least Python 2.6 * Removed support for AmpliFIND/PUIDs From 14c8a68ad82b41fb0537ffada1011a54b5b37360 Mon Sep 17 00:00:00 2001 From: Sophist Date: Thu, 3 Apr 2014 10:13:44 +0100 Subject: [PATCH 15/24] Fix typo in commit d65ecf4bdd2269632a53b1fed5606ef22860e974 --- picard/mbxml.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/picard/mbxml.py b/picard/mbxml.py index 4f9bd3894..c90d7ad74 100644 --- a/picard/mbxml.py +++ b/picard/mbxml.py @@ -226,7 +226,7 @@ def track_to_metadata(node, track): elif name == 'position': m['tracknumber'] = nodes[0].text elif name == 'number': - m['~~musicbrainz_tracknumber'] = nodes[0].text + m['~musicbrainz_tracknumber'] = nodes[0].text elif name == 'length' and nodes[0].text: m.length = int(nodes[0].text) elif name == 'artist_credit': From 0f2e62796c0d5589f85be66f50fccf7905477d56 Mon Sep 17 00:00:00 2001 From: Sophist Date: Thu, 3 Apr 2014 13:54:46 +0100 Subject: [PATCH 16/24] Fix error on malformed TRCK tag. --- NEWS.txt | 2 +- picard/formats/id3.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/NEWS.txt b/NEWS.txt index 6880a76bf..e60c9408d 100644 --- a/NEWS.txt +++ b/NEWS.txt @@ -34,7 +34,7 @@ * Support dropping image directly from Google image results to cover art box * Add %_musicbrainz_tracknumber% to hold track # as shown on MusicBrainz release web-page e.g. vinyl/cassette style A1, A2, B1, B2 - + * Fixed a bug where Picard crashed if a MP3 file had a malformed TRCK tag (PICARD-112) Version 1.2 - 2013-03-30 * Picard now requires at least Python 2.6 diff --git a/picard/formats/id3.py b/picard/formats/id3.py index 1d402373f..25de35686 100644 --- a/picard/formats/id3.py +++ b/picard/formats/id3.py @@ -241,10 +241,10 @@ class ID3File(File): metadata['musicbrainz_recordingid'] = frame.data.decode('ascii', 'ignore') elif frameid == 'TRCK': value = frame.text[0].split('/') - if len(value) > 1: - metadata['tracknumber'], metadata['totaltracks'] = value[:2] - else: + if len(value) == 1 and value[0].isdigit(): metadata['tracknumber'] = value[0] + elif len(value) == 2 and value[0].isdigit() and value[1].isdigit(): + metadata['tracknumber'], metadata['totaltracks'] = value elif frameid == 'TPOS': value = frame.text[0].split('/') if len(value) > 1: From 388a48219ebe8fd097cdc9da5fd8f76c3d6b5e6f Mon Sep 17 00:00:00 2001 From: Sophist Date: Thu, 3 Apr 2014 13:57:54 +0100 Subject: [PATCH 17/24] Provide error log on invalid format. --- picard/formats/id3.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/picard/formats/id3.py b/picard/formats/id3.py index 25de35686..3522a5592 100644 --- a/picard/formats/id3.py +++ b/picard/formats/id3.py @@ -245,6 +245,8 @@ class ID3File(File): metadata['tracknumber'] = value[0] elif len(value) == 2 and value[0].isdigit() and value[1].isdigit(): metadata['tracknumber'], metadata['totaltracks'] = value + else: + log.error("Invalid TRCK value '%s' dropped in %r", frame.text[0], filename) elif frameid == 'TPOS': value = frame.text[0].split('/') if len(value) > 1: From b3d92437be299f95ecc9ca7e6ac7c7fedb41648d Mon Sep 17 00:00:00 2001 From: Sophist Date: Thu, 3 Apr 2014 14:21:39 +0100 Subject: [PATCH 18/24] Fix broken test_mbxml broken by commit 69228485452dfb7f8e665ea8996e75b9db66234a --- test/test_mbxml.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/test_mbxml.py b/test/test_mbxml.py index c60d2e363..18a6ee9c5 100644 --- a/test/test_mbxml.py +++ b/test/test_mbxml.py @@ -160,7 +160,8 @@ class ArtistTest(unittest.TestCase): })] })] }) - artist, artist_sort, artists = artist_credit_from_node(node) - self.assertEqual(['Foo Bar', 'Baz'], artists) + artist, artist_sort, artists, artists_sort = artist_credit_from_node(node) self.assertEqual('Foo Bar & Baz', artist) + self.assertEqual(['Foo Bar', 'Baz'], artists) self.assertEqual('Bar, Foo & Baz', artist_sort) + self.assertEqual(['Bar, Foo', 'Baz'], artists_sort) From 02168dc50d5df68fd0ea1e7a825a038b580598cb Mon Sep 17 00:00:00 2001 From: Sophist Date: Thu, 3 Apr 2014 18:03:26 +0100 Subject: [PATCH 19/24] Provide same checks for TPOS --- picard/formats/id3.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/picard/formats/id3.py b/picard/formats/id3.py index 3522a5592..1d8bf4706 100644 --- a/picard/formats/id3.py +++ b/picard/formats/id3.py @@ -249,10 +249,12 @@ class ID3File(File): log.error("Invalid TRCK value '%s' dropped in %r", frame.text[0], filename) elif frameid == 'TPOS': value = frame.text[0].split('/') - if len(value) > 1: - metadata['discnumber'], metadata['totaldiscs'] = value[:2] - else: + if len(value) == 1 and value[0].isdigit(): metadata['discnumber'] = value[0] + elif len(value) == 2 and value[0].isdigit() and value[1].isdigit(): + metadata['discnumber'], metadata['totaldiscs'] = value + else: + log.error("Invalid TPOS value '%s' dropped in %r", frame.text[0], filename) elif frameid == 'APIC': extras = { 'desc': frame.desc, From 09f726fcdca38ce6771437f5165962281908b9a5 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 4 Apr 2014 12:07:50 +0200 Subject: [PATCH 20/24] Match TRCK and TPOS using regex and reduce code redundancy. --- picard/formats/id3.py | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/picard/formats/id3.py b/picard/formats/id3.py index 1d8bf4706..6f67fe82c 100644 --- a/picard/formats/id3.py +++ b/picard/formats/id3.py @@ -20,6 +20,7 @@ import mutagen.apev2 import mutagen.mp3 import mutagen.trueaudio +import re from collections import defaultdict from mutagen import id3 from picard import config, log @@ -175,6 +176,11 @@ class ID3File(File): __other_supported_tags = ("discnumber", "tracknumber", "totaldiscs", "totaltracks") + __tag_subvalues = { + 'TRCK': ('tracknumber', 'totaltracks'), + 'TPOS': ('discnumber', 'totaldiscs') + } + __tag_subvalues_re = re.compile(r'^(\d+)(?:/(\d+))?$') def __init__(self, filename): super(ID3File, self).__init__(filename) @@ -239,22 +245,17 @@ class ID3File(File): metadata.add(name, unicode(frame.text)) elif frameid == 'UFID' and frame.owner == 'http://musicbrainz.org': metadata['musicbrainz_recordingid'] = frame.data.decode('ascii', 'ignore') - elif frameid == 'TRCK': - value = frame.text[0].split('/') - if len(value) == 1 and value[0].isdigit(): - metadata['tracknumber'] = value[0] - elif len(value) == 2 and value[0].isdigit() and value[1].isdigit(): - metadata['tracknumber'], metadata['totaltracks'] = value + elif frameid in ('TRCK', 'TPOS'): + # frame is a numeric string, eventually extended with a "/" character + # and a numeric string containing the total number. + # E.g: "1" or "1/2" + m = self.__tag_subvalues_re.search(frame.text[0]) + if m: + metadata[self.__tag_subvalues[frameid][0]] = m.group(1) + if m.group(2) is not None: + metadata[self.__tag_subvalues[frameid][1]] = m.group(2) else: - log.error("Invalid TRCK value '%s' dropped in %r", frame.text[0], filename) - elif frameid == 'TPOS': - value = frame.text[0].split('/') - if len(value) == 1 and value[0].isdigit(): - metadata['discnumber'] = value[0] - elif len(value) == 2 and value[0].isdigit() and value[1].isdigit(): - metadata['discnumber'], metadata['totaldiscs'] = value - else: - log.error("Invalid TPOS value '%s' dropped in %r", frame.text[0], filename) + log.error("Invalid %s value '%s' dropped in %r", frameid, frame.text[0], filename) elif frameid == 'APIC': extras = { 'desc': frame.desc, From 8f91a9f7eb20dec07351c28a51583f2354574adc Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 4 Apr 2014 15:06:41 +0200 Subject: [PATCH 21/24] Use re named groups to match values and metadata keys. --- picard/formats/id3.py | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/picard/formats/id3.py b/picard/formats/id3.py index 6f67fe82c..bd40679c4 100644 --- a/picard/formats/id3.py +++ b/picard/formats/id3.py @@ -176,11 +176,10 @@ class ID3File(File): __other_supported_tags = ("discnumber", "tracknumber", "totaldiscs", "totaltracks") - __tag_subvalues = { - 'TRCK': ('tracknumber', 'totaltracks'), - 'TPOS': ('discnumber', 'totaldiscs') + __tag_re_parse = { + 'TRCK': re.compile(r'^(?P\d+)(?:/(?P\d+))?$'), + 'TPOS': re.compile(r'^(?P\d+)(?:/(?P\d+))?$') } - __tag_subvalues_re = re.compile(r'^(\d+)(?:/(\d+))?$') def __init__(self, filename): super(ID3File, self).__init__(filename) @@ -245,15 +244,12 @@ class ID3File(File): metadata.add(name, unicode(frame.text)) elif frameid == 'UFID' and frame.owner == 'http://musicbrainz.org': metadata['musicbrainz_recordingid'] = frame.data.decode('ascii', 'ignore') - elif frameid in ('TRCK', 'TPOS'): - # frame is a numeric string, eventually extended with a "/" character - # and a numeric string containing the total number. - # E.g: "1" or "1/2" - m = self.__tag_subvalues_re.search(frame.text[0]) + elif frameid in self.__tag_re_parse.keys(): + m = self.__tag_re_parse[frameid].search(frame.text[0]) if m: - metadata[self.__tag_subvalues[frameid][0]] = m.group(1) - if m.group(2) is not None: - metadata[self.__tag_subvalues[frameid][1]] = m.group(2) + for name, value in m.groupdict().iteritems(): + if value is not None: + metadata[name] = value else: log.error("Invalid %s value '%s' dropped in %r", frameid, frame.text[0], filename) elif frameid == 'APIC': From acbebc3eb6042a5b7d9de329f4bb09edfe331069 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Fri, 4 Apr 2014 21:21:30 +0200 Subject: [PATCH 22/24] Get rid of 'if logdebug: logdebug(...)' pattern using a noop function Suggested by @Mineo in https://github.com/musicbrainz/picard/pull/254/files#r11290500 --- picard/i18n.py | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/picard/i18n.py b/picard/i18n.py index 87b322046..85161d0e5 100644 --- a/picard/i18n.py +++ b/picard/i18n.py @@ -26,8 +26,10 @@ import __builtin__ __builtin__.__dict__['N_'] = lambda a: a -def setup_gettext(localedir, ui_language=None, logdebug=None): +def setup_gettext(localedir, ui_language=None, logger=None): """Setup locales, load translations, install gettext functions.""" + if not logger: + logger = lambda *a, **b: None # noop current_locale = '' if ui_language: os.environ['LANGUAGE'] = '' @@ -61,21 +63,17 @@ def setup_gettext(localedir, ui_language=None, logdebug=None): current_locale = locale.setlocale(locale.LC_ALL, "") except: pass - if logdebug: - logdebug("Using locale %r", current_locale) + logger("Using locale %r", current_locale) try: - if logdebug: - logdebug("Loading gettext translation, localedir=%r", localedir) + logger("Loading gettext translation, localedir=%r", localedir) trans = gettext.translation("picard", localedir) trans.install(True) _ungettext = trans.ungettext - if logdebug: - logdebug("Loading gettext translation (picard-countries), localedir=%r", localedir) + logger("Loading gettext translation (picard-countries), localedir=%r", localedir) trans_countries = gettext.translation("picard-countries", localedir) _ugettext_countries = trans_countries.ugettext except IOError as e: - if logdebug: - logdebug(e) + logger(e) __builtin__.__dict__['_'] = lambda a: a def _ungettext(a, b, c): @@ -89,8 +87,8 @@ def setup_gettext(localedir, ui_language=None, logdebug=None): __builtin__.__dict__['ungettext'] = _ungettext __builtin__.__dict__['ugettext_countries'] = _ugettext_countries - if logdebug: - logdebug("_ = %r", _) - logdebug("N_ = %r", N_) - logdebug("ungettext = %r", ungettext) - logdebug("ugettext_countries = %r", ugettext_countries) + + logger("_ = %r", _) + logger("N_ = %r", N_) + logger("ungettext = %r", ungettext) + logger("ugettext_countries = %r", ugettext_countries) From 0a87b62ee7045faff71a2eb1096b833ca7a7a211 Mon Sep 17 00:00:00 2001 From: Sophist Date: Fri, 4 Apr 2014 20:25:02 +0100 Subject: [PATCH 23/24] Update NEWS --- NEWS.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.txt b/NEWS.txt index e60c9408d..39d6d44e3 100644 --- a/NEWS.txt +++ b/NEWS.txt @@ -34,7 +34,7 @@ * Support dropping image directly from Google image results to cover art box * Add %_musicbrainz_tracknumber% to hold track # as shown on MusicBrainz release web-page e.g. vinyl/cassette style A1, A2, B1, B2 - * Fixed a bug where Picard crashed if a MP3 file had a malformed TRCK tag (PICARD-112) + * Fixed a bug where Picard crashed if a MP3 file had malformed TRCK or TPOS tags (PICARD-112) Version 1.2 - 2013-03-30 * Picard now requires at least Python 2.6 From 871e2467e89f793f8b55165fbbacedb2a39be647 Mon Sep 17 00:00:00 2001 From: Michael Wiencek Date: Sun, 6 Apr 2014 13:40:20 -0500 Subject: [PATCH 24/24] Fix instance where QStringList is now a list (post-fa66e9f) --- picard/ui/edittagdialog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/picard/ui/edittagdialog.py b/picard/ui/edittagdialog.py index d2783b9c1..e7fcea140 100644 --- a/picard/ui/edittagdialog.py +++ b/picard/ui/edittagdialog.py @@ -147,7 +147,7 @@ class EditTagDialog(PicardDialog): self._modified_tag()[row] = value # add tags to the completer model once they get values cm = self.completer.model() - if not cm.stringList().contains(self.tag): + if self.tag not in cm.stringList(): cm.insertRows(0, 1) cm.setData(cm.index(0, 0), self.tag) cm.sort(0)