Merge remote-tracking branch 'upstream/master' into build_ui_improvements

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)
@@ -34,6 +35,8 @@
  * 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)
+ * Fixed a bug where Picard crashed if a MP3 file had malformed TRCK or TPOS tags (PICARD-112)
 
 
 Version 1.2 - 2013-03-30

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"),

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

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 PRESERVED_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) + PRESERVED_TAGS:
             values = self.orig_metadata.getall(tag)
             if values:
                 saved_metadata[tag] = values

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,10 @@ class ID3File(File):
 
     __other_supported_tags = ("discnumber", "tracknumber",
                               "totaldiscs", "totaltracks")
+    __tag_re_parse = {
+        'TRCK': re.compile(r'^(?P<tracknumber>\d+)(?:/(?P<totaltracks>\d+))?$'),
+        'TPOS': re.compile(r'^(?P<discnumber>\d+)(?:/(?P<totaldiscs>\d+))?$')
+    }
 
     def __init__(self, filename):
         super(ID3File, self).__init__(filename)
@@ -239,18 +244,14 @@ 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:
-                    metadata['tracknumber'], metadata['totaltracks'] = value[:2]
+            elif frameid in self.__tag_re_parse.keys():
+                m = self.__tag_re_parse[frameid].search(frame.text[0])
+                if m:
+                    for name, value in m.groupdict().iteritems():
+                        if value is not None:
+                            metadata[name] = value
                 else:
-                    metadata['tracknumber'] = value[0]
-            elif frameid == 'TPOS':
-                value = frame.text[0].split('/')
-                if len(value) > 1:
-                    metadata['discnumber'], metadata['totaldiscs'] = value[:2]
-                else:
-                    metadata['discnumber'] = value[0]
+                    log.error("Invalid %s value '%s' dropped in %r", frameid, frame.text[0], filename)
             elif frameid == 'APIC':
                 extras = {
                     'desc': frame.desc,
@@ -436,7 +437,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):

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)

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):
@@ -226,7 +231,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':

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)

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)
 

picard/ui/options/scripting.py

@@ -28,7 +28,7 @@ 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)
@@ -41,6 +41,12 @@ class TaggerScriptSyntaxHighlighter(QtGui.QSyntaxHighlighter):
         self.special_re = QtCore.QRegExp(r"[^\\][(),]")
         self.special_fmt = QtGui.QTextCharFormat()
         self.special_fmt.setForeground(QtCore.Qt.blue)
+        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)
+        self.noop_fmt.setFontItalic(True)
+        self.noop_fmt.setForeground(QtCore.Qt.darkGray)
         self.rules = [
             (self.func_re, self.func_fmt, 0, -1),
             (self.var_re, self.var_fmt, 0, 0),
@@ -49,6 +55,8 @@ class TaggerScriptSyntaxHighlighter(QtGui.QSyntaxHighlighter):
         ]
 
     def highlightBlock(self, text):
+        self.setCurrentBlockState(0)
+
         for expr, fmt, a, b in self.rules:
             index = expr.indexIn(text)
             while index >= 0:
@@ -56,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_re.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):
 

picard/util/tags.py

@@ -91,6 +91,11 @@ TAG_NAMES = {
     'work': N_('Work'),
 }
 
+PRESERVED_TAGS = [
+    "~bitrate", "~bits_per_sample", "~format", "~channels", "~sample_rate",
+    "~dirname", "~filename", "~extension",
+]
+
 
 def display_tag_name(name):
     if ':' in name:

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)