From 1303a915c17030ccacd8ccb5695070a4ded8442b Mon Sep 17 00:00:00 2001
From: Pierre Rust <pierre.rust@gmail.com>
Date: Wed, 11 Jun 2014 16:21:43 +0200
Subject: [PATCH 01/63] Sort implementation

 * sort can be sepcified using the 'field_name'(+|-) syntax
 * supports fixed fields and flexible attributes
 * includes plugins fix for API changes (might have missed some)
---
 beets/dbcore/__init__.py   |   1 +
 beets/dbcore/db.py         |  25 +++---
 beets/dbcore/query.py      | 160 +++++++++++++++++++++++++++++++++++++
 beets/dbcore/queryparse.py |  28 +++++++
 beets/library.py           |  50 +++++++-----
 beetsplug/ihate.py         |   4 +-
 beetsplug/smartplaylist.py |   2 +-
 test/test_dbcore.py        |  31 +++++++
 8 files changed, 263 insertions(+), 38 deletions(-)

diff --git a/beets/dbcore/__init__.py b/beets/dbcore/__init__.py
index fed65f482..fdf6b4695 100644
--- a/beets/dbcore/__init__.py
+++ b/beets/dbcore/__init__.py
@@ -19,5 +19,6 @@ from .db import Model, Database
 from .query import Query, FieldQuery, MatchQuery, AndQuery, OrQuery
 from .types import Type
 from .queryparse import query_from_strings
+from .queryparse import sort_from_strings
 
 # flake8: noqa
diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py
index 68f144b7a..e12178784 100644
--- a/beets/dbcore/db.py
+++ b/beets/dbcore/db.py
@@ -24,7 +24,7 @@ import collections
 
 import beets
 from beets.util.functemplate import Template
-from .query import MatchQuery
+from .query import MatchQuery, build_sql
 from .types import BASE_TYPE
 
 
@@ -509,7 +509,10 @@ class Results(object):
                     ),
                     (row['id'],)
                 )
-            values = dict(row)
+
+            cols = dict(row)
+            values = dict((k, v) for (k, v) in cols.items()
+                          if not k[:4] == 'flex')
             flex_values = dict((row['key'], row['value']) for row in flex_rows)
 
             # Construct the Python object and yield it if it passes the
@@ -739,24 +742,20 @@ class Database(object):
 
     # Querying.
 
-    def _fetch(self, model_cls, query, order_by=None):
+    def _fetch(self, model_cls, query, sort_order=None):
         """Fetch the objects of type `model_cls` matching the given
         query. The query may be given as a string, string sequence, a
         Query object, or None (to fetch everything). If provided,
-        `order_by` is a SQLite ORDER BY clause for sorting.
-        """
-        where, subvals = query.clause()
+       `sort_order` is either a SQLite ORDER BY clause for sorting or a
+        Sort object.
+         """
+
+        sql, subvals, is_slow = build_sql(model_cls, query, sort_order)
 
-        sql = "SELECT * FROM {0} WHERE {1}".format(
-            model_cls._table,
-            where or '1',
-        )
-        if order_by:
-            sql += " ORDER BY {0}".format(order_by)
         with self.transaction() as tx:
             rows = tx.query(sql, subvals)
 
-        return Results(model_cls, rows, self, None if where else query)
+        return Results(model_cls, rows, self, None if not is_slow else query)
 
     def _get(self, model_cls, id):
         """Get a Model object by its id or None if the id does not
diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py
index 805f2cac9..54f6df920 100644
--- a/beets/dbcore/query.py
+++ b/beets/dbcore/query.py
@@ -497,3 +497,163 @@ class DateQuery(FieldQuery):
             # Match any date.
             clause = '1'
         return clause, subvals
+
+
+class Sort(object):
+    """An abstract class representing a sort opertation for a query into the
+    item database.
+    """
+    def select_clause(self):
+        """ Generates a select sql fragment.
+        """
+        return None
+
+    def union_clause(self):
+        """ Generates a union sql fragment or None if the Sort is a slow sort.
+        """
+        return None
+
+    def order_clause(self):
+        """Generates a sql fragment to be use in a ORDER BY clause
+        or None if it's a slow query
+        """
+        return None
+
+    def sort(self, items):
+        """Sort the given items list. Meant to be used with slow queries.
+        """
+        return items
+
+
+class MultipleSort(Sort):
+    """ Sort class that combines several sort criteria.
+    """
+
+    def __init__(self):
+        self.sorts = []
+
+    def add_criteria(self, sort):
+        self.sorts.append(sort)
+
+    def select_clause(self):
+        """ Generate a select sql fragment.
+        """
+        select_strings = []
+        index = 0
+        for sort in self.sorts:
+            select = sort.select_clause()
+            if select is None:
+                # FIXME : sort for slow sort
+                break
+            else:
+                select_strings.append(select)
+                index = index + 1
+
+        select_string = ",".join(select_strings)
+        return "" if not select_string else ", " + select_string
+
+    def union_clause(self):
+        """ Returns a union sql fragment.
+        """
+        union_strings = []
+        for sort in self.sorts:
+            union = sort.union_clause()
+            if union is None:
+                pass
+            else:
+                union_strings.append(union)
+
+        return "".join(union_strings)
+
+    def order_clause(self):
+        """Returns a sql fragment to be use in a ORDER BY clause
+        or None if it's a slow query
+        """
+        order_strings = []
+        index = 0
+        for sort in self.sorts:
+            order = sort.order_clause()
+            if order is None:
+                break
+            else:
+                order_strings.append(order)
+                index = index + 1
+
+        return ",".join(order_strings)
+
+    def sort(self, items):
+        # FIXME : sort according to criteria following the first slow sort
+        return items
+
+
+class FlexFieldSort(Sort):
+
+    def __init__(self, model_cls, field, is_ascending):
+        self.model_cls = model_cls
+        self.field = field
+        self.is_ascending = is_ascending
+
+    def select_clause(self):
+        """ Return a select sql fragment.
+        """
+        return "sort_flexattr{0!s}.value as flex_{0!s} ".format(self.field)
+
+    def union_clause(self):
+        """ Returns an union sql fragment.
+        """
+        return "LEFT JOIN {flextable} as sort_flexattr{index!s} \
+               ON {table}.id = sort_flexattr{index!s}.entity_id \
+               AND sort_flexattr{index!s}.key='{flexattr}' ".format(
+            flextable=self.model_cls._flex_table,
+            table=self.model_cls._table,
+            index=self.field, flexattr=self.field)
+
+    def order_clause(self):
+        """ Returns an order sql fragment.
+        """
+        order = "ASC" if self.is_ascending else "DESC"
+        return "flex_{0} {1} ".format(self.field, order)
+
+
+class FixedFieldSort(Sort):
+
+    def __init__(self, field, is_ascending=True):
+        self.field = field
+        self.is_ascending = is_ascending
+
+    """ Sort on a fixed field
+    """
+    def order_clause(self):
+        order = "ASC" if self.is_ascending else "DESC"
+        return "{0} {1}".format(self.field, order)
+
+
+def build_sql(model_cls, query, sort_order):
+    """ Generate a sql statement (and the values that must be injected into it)
+    from a query, sort and a model class.
+    """
+    where, subvals = query.clause()
+
+    if not sort_order:
+        sort_select = ""
+        sort_union = ""
+        sort_order = ""
+    elif isinstance(sort_order, basestring):
+        sort_select = ""
+        sort_union = ""
+        sort_order = " ORDER BY {0}".format(sort_order)
+    elif isinstance(sort_order, Sort):
+        sort_select = sort_order.select_clause()
+        sort_union = sort_order.union_clause()
+        sort_order = " ORDER BY {0}".format(sort_order.order_clause())
+
+    sql = "SELECT {table}.* {sort_select} FROM {table} {sort_union} WHERE \
+            {query_clause} {sort_order}".format(
+        sort_select=sort_select,
+        sort_union=sort_union,
+        table=model_cls._table,
+        query_clause=where or '1',
+        sort_order=sort_order
+    )
+
+    return (sql, subvals, where is None) 
\ No newline at end of file
diff --git a/beets/dbcore/queryparse.py b/beets/dbcore/queryparse.py
index a767b56d1..05ee28add 100644
--- a/beets/dbcore/queryparse.py
+++ b/beets/dbcore/queryparse.py
@@ -121,3 +121,31 @@ def query_from_strings(query_cls, model_cls, prefixes, query_parts):
     if not subqueries:  # No terms in query.
         subqueries = [query.TrueQuery()]
     return query_cls(subqueries)
+
+
+def construct_sort_part(model_cls, part):
+    """ Creates a Sort object from a single criteria. Returns a `Sort` instance.
+    """
+    sort = None
+    field = part[:-1]
+    is_ascending = (part[-1] == '+')
+    if field in model_cls._fields:
+        sort = query.FixedFieldSort(field, is_ascending)
+    elif field in model_cls._getters():
+        # Computed field, all following fields must use the slow path.
+        pass
+    else:
+        # Neither fixed nor computed : must be a flex attr.
+        sort = query.FlexFieldSort(model_cls, field, is_ascending)
+    return sort
+
+
+def sort_from_strings(model_cls, sort_parts):
+    """Creates a Sort object from a list of sort criteria strings.
+    """
+    if not sort_parts:
+        return None
+    sort = query.MultipleSort()
+    for part in sort_parts:
+        sort.add_criteria(construct_sort_part(model_cls, part))
+    return sort
\ No newline at end of file
diff --git a/beets/library.py b/beets/library.py
index 8e3d5f1e9..61ddfab96 100644
--- a/beets/library.py
+++ b/beets/library.py
@@ -546,7 +546,7 @@ class Item(LibModel):
         for query, path_format in path_formats:
             if query == PF_KEY_DEFAULT:
                 continue
-            query = get_query(query, type(self))
+            (query, _) = get_query(query, type(self))
             if query.match(self):
                 # The query matches the item! Use the corresponding path
                 # format.
@@ -889,7 +889,8 @@ class Album(LibModel):
 
 def get_query(val, model_cls):
     """Take a value which may be None, a query string, a query string
-    list, or a Query object, and return a suitable Query object.
+    list, or a Query object, and return a suitable Query object and Sort
+    object.
 
     `model_cls` is the subclass of Model indicating which entity this
     is a query for (i.e., Album or Item) and is used to determine which
@@ -910,7 +911,7 @@ def get_query(val, model_cls):
         val = [s.decode('utf8') for s in shlex.split(val)]
 
     if val is None:
-        return dbcore.query.TrueQuery()
+        return (dbcore.query.TrueQuery(), None)
 
     elif isinstance(val, list) or isinstance(val, tuple):
         # Special-case path-like queries, which are non-field queries
@@ -928,18 +929,23 @@ def get_query(val, model_cls):
             path_parts = ()
             non_path_parts = val
 
+        # separate query token and sort token
+        query_val = [s for s in non_path_parts if not s.endswith(('+', '-'))]
+        sort_val = [s for s in non_path_parts if s.endswith(('+', '-'))]
+
         # Parse remaining parts and construct an AndQuery.
         query = dbcore.query_from_strings(
-            dbcore.AndQuery, model_cls, prefixes, non_path_parts
+            dbcore.AndQuery, model_cls, prefixes, query_val
         )
+        sort = dbcore.sort_from_strings(model_cls, sort_val)
 
         # Add path queries to aggregate query.
         if path_parts:
             query.subqueries += [PathQuery('path', s) for s in path_parts]
-        return query
+        return (query, sort)
 
     elif isinstance(val, dbcore.Query):
-        return val
+        return (val, None)
 
     else:
         raise ValueError('query must be None or have type Query or str')
@@ -1006,30 +1012,30 @@ class Library(dbcore.Database):
 
     # Querying.
 
-    def _fetch(self, model_cls, query, order_by=None):
-        """Parse a query and fetch.
-        """
+    def _fetch(self, model_cls, query, sort_order=None):
+        """Parse a query and fetch. If a sort_order is explicitly given,
+        any sort order specification present in the query string is ignored.
+          """
+        (query, sort) = get_query(query, model_cls)
+        sort = sort if sort_order is None else sort_order
+
         return super(Library, self)._fetch(
-            model_cls, get_query(query, model_cls), order_by
+            model_cls, query, sort
         )
 
-    def albums(self, query=None):
+    def albums(self, query=None, sort_order=None):
         """Get a sorted list of :class:`Album` objects matching the
-        given query.
+        given sort order. If a sort_order is explicitly given,
+        any sort order specification present in the query string is ignored.
         """
-        order = '{0}, album'.format(
-            _orelse("albumartist_sort", "albumartist")
-        )
-        return self._fetch(Album, query, order)
+        return self._fetch(Album, query, sort_order)
 
-    def items(self, query=None):
+    def items(self, query=None, sort_order=None):
         """Get a sorted list of :class:`Item` objects matching the given
-        query.
+        given sort order. If a sort_order is explicitly given,
+        any sort order specification present in the query string is ignored.
         """
-        order = '{0}, album, disc, track'.format(
-            _orelse("artist_sort", "artist")
-        )
-        return self._fetch(Item, query, order)
+        return self._fetch(Item, query, sort_order)
 
     # Convenience accessors.
 
diff --git a/beetsplug/ihate.py b/beetsplug/ihate.py
index 3cd6ace41..85e3382d4 100644
--- a/beetsplug/ihate.py
+++ b/beetsplug/ihate.py
@@ -57,9 +57,9 @@ class IHatePlugin(BeetsPlugin):
             for query_string in action_patterns:
                 query = None
                 if task.is_album:
-                    query = get_query(query_string, Album)
+                    (query, _) = get_query(query_string, Album)
                 else:
-                    query = get_query(query_string, Item)
+                    (query, _) = get_query(query_string, Item)
                 if any(query.match(item) for item in task.imported_items()):
                     return True
         return False
diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py
index 118dc361b..12672ce42 100644
--- a/beetsplug/smartplaylist.py
+++ b/beetsplug/smartplaylist.py
@@ -42,7 +42,7 @@ def _items_for_query(lib, playlist, album=False):
         query_strings = [query_strings]
     model = library.Album if album else library.Item
     query = dbcore.OrQuery(
-        [library.get_query(q, model) for q in query_strings]
+        [library.get_query(q, model)[0] for q in query_strings]
     )
 
     # Execute query, depending on type.
diff --git a/test/test_dbcore.py b/test/test_dbcore.py
index a4be181e4..e55bd84db 100644
--- a/test/test_dbcore.py
+++ b/test/test_dbcore.py
@@ -412,6 +412,37 @@ class QueryFromStringsTest(_common.TestCase):
         self.assertIsInstance(q.subqueries[0], dbcore.query.NumericQuery)
 
 
+class SortFromStringsTest(_common.TestCase):
+    def sfs(self, strings):
+        return dbcore.queryparse.sort_from_strings(
+            TestModel1,
+            strings,
+        )
+
+    def test_zero_parts(self):
+        s = self.sfs([])
+        self.assertIsNone(s)
+
+    def test_one_parts(self):
+        s = self.sfs(['field+'])
+        self.assertIsInstance(s, dbcore.query.Sort)
+
+    def test_two_parts(self):
+        s = self.sfs(['field+', 'another_field-'])
+        self.assertIsInstance(s, dbcore.query.MultipleSort)
+        self.assertEqual(len(s.sorts), 2)
+
+    def test_fixed_field_sort(self):
+        s = self.sfs(['field_one+'])
+        self.assertIsInstance(s, dbcore.query.MultipleSort)
+        self.assertIsInstance(s.sorts[0], dbcore.query.FixedFieldSort)
+
+    def test_flex_field_sort(self):
+        s = self.sfs(['flex_field+'])
+        self.assertIsInstance(s, dbcore.query.MultipleSort)
+        self.assertIsInstance(s.sorts[0], dbcore.query.FlexFieldSort)
+
+
 def suite():
     return unittest.TestLoader().loadTestsFromName(__name__)
 

From 3d7814c32eadf1966f21a199ccc82011011eb807 Mon Sep 17 00:00:00 2001
From: Pierre Rust <pierre.rust@gmail.com>
Date: Wed, 11 Jun 2014 22:26:22 +0200
Subject: [PATCH 02/63] Use implicit continuation for sql generation.

Avoid newlines in the query string.
---
 beets/dbcore/query.py | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py
index 54f6df920..21ba3b572 100644
--- a/beets/dbcore/query.py
+++ b/beets/dbcore/query.py
@@ -601,12 +601,13 @@ class FlexFieldSort(Sort):
     def union_clause(self):
         """ Returns an union sql fragment.
         """
-        return "LEFT JOIN {flextable} as sort_flexattr{index!s} \
-               ON {table}.id = sort_flexattr{index!s}.entity_id \
-               AND sort_flexattr{index!s}.key='{flexattr}' ".format(
+        union = ("LEFT JOIN {flextable} as sort_flexattr{index!s} "
+                 "ON {table}.id = sort_flexattr{index!s}.entity_id "
+                 "AND sort_flexattr{index!s}.key='{flexattr}' ").format(
             flextable=self.model_cls._flex_table,
             table=self.model_cls._table,
             index=self.field, flexattr=self.field)
+        return union
 
     def order_clause(self):
         """ Returns an order sql fragment.
@@ -647,8 +648,8 @@ def build_sql(model_cls, query, sort_order):
         sort_union = sort_order.union_clause()
         sort_order = " ORDER BY {0}".format(sort_order.order_clause())
 
-    sql = "SELECT {table}.* {sort_select} FROM {table} {sort_union} WHERE \
-            {query_clause} {sort_order}".format(
+    sql = ("SELECT {table}.* {sort_select} FROM {table} {sort_union} WHERE "
+           "{query_clause} {sort_order}").format(
         sort_select=sort_select,
         sort_union=sort_union,
         table=model_cls._table,

From 4958ce83c3b81e2c5fc8b104c0cec437415ea4e5 Mon Sep 17 00:00:00 2001
From: Pierre Rust <pierre.rust@gmail.com>
Date: Wed, 11 Jun 2014 22:54:39 +0200
Subject: [PATCH 03/63] Add defaut sort on artist_sort field.

For items and albums, defaulting on artist field if artist field
is empty.
---
 beets/dbcore/query.py | 30 ++++++++++++++++++++++++++++++
 beets/library.py      | 19 ++++++++++++-------
 2 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py
index 21ba3b572..97b8833bb 100644
--- a/beets/dbcore/query.py
+++ b/beets/dbcore/query.py
@@ -628,6 +628,36 @@ class FixedFieldSort(Sort):
         order = "ASC" if self.is_ascending else "DESC"
         return "{0} {1}".format(self.field, order)
 
+class SmartArtistSort(Sort):
+    """ Sort Album or Item on artist sort fields, defaulting back on
+        artist field if the sort specific field is empty.
+    """
+    def __init__(self, model_cls, is_ascending=True):
+        self.model_cls = model_cls
+        self.is_ascending = is_ascending
+
+    def select_clause(self):
+        return ""
+
+    def union_clause(self):
+        return ""
+
+    def order_clause(self):
+        order = "ASC" if self.is_ascending else "DESC"
+        if 'albumartist_sort' in self.model_cls._fields:
+            exp1 = 'albumartist_sort'
+            exp2 = 'albumartist'
+        elif 'artist_sort' in self.model_cls_fields:
+            exp1 = 'artist_sort'
+            exp2 = 'artist'
+        else:
+            return ""
+
+        order_str = ('(CASE {0} WHEN NULL THEN {1} '
+                     'WHEN "" THEN {1} '
+                     'ELSE {0} END) {2} ').format(exp1, exp2, order)
+        return order_str
+
 
 def build_sql(model_cls, query, sort_order):
     """ Generate a sql statement (and the values that must be injected into it)
diff --git a/beets/library.py b/beets/library.py
index 61ddfab96..3965b6fb9 100644
--- a/beets/library.py
+++ b/beets/library.py
@@ -29,6 +29,7 @@ from beets.util import bytestring_path, syspath, normpath, samefile
 from beets.util.functemplate import Template
 from beets import dbcore
 from beets.dbcore import types
+from beets.dbcore.query import SmartArtistSort
 import beets
 
 
@@ -1013,11 +1014,11 @@ class Library(dbcore.Database):
     # Querying.
 
     def _fetch(self, model_cls, query, sort_order=None):
-        """Parse a query and fetch. If a sort_order is explicitly given,
-        any sort order specification present in the query string is ignored.
+        """Parse a query and fetch. If a order specification is present in the
+        query string the sort_order argument is ignored.
           """
         (query, sort) = get_query(query, model_cls)
-        sort = sort if sort_order is None else sort_order
+        sort = sort_order if sort is None else sort
 
         return super(Library, self)._fetch(
             model_cls, query, sort
@@ -1025,16 +1026,20 @@ class Library(dbcore.Database):
 
     def albums(self, query=None, sort_order=None):
         """Get a sorted list of :class:`Album` objects matching the
-        given sort order. If a sort_order is explicitly given,
-        any sort order specification present in the query string is ignored.
+        given sort order. If a order specification is present in the query
+        string the sort_order argument is ignored. 
         """
+        if sort_order is None:
+            sort_order = SmartArtistSort(Album)
         return self._fetch(Album, query, sort_order)
 
     def items(self, query=None, sort_order=None):
         """Get a sorted list of :class:`Item` objects matching the given
-        given sort order. If a sort_order is explicitly given,
-        any sort order specification present in the query string is ignored.
+        given sort order. If a order specification is present in the query
+        string the sort_order argument is ignored.
         """
+        if sort_order is None:
+            sort_order = SmartArtistSort(Item)
         return self._fetch(Item, query, sort_order)
 
     # Convenience accessors.

From 342636377513e0f1fb4010b366fee44fcf0300df Mon Sep 17 00:00:00 2001
From: Pierre Rust <pierre.rust@gmail.com>
Date: Thu, 12 Jun 2014 09:14:05 +0200
Subject: [PATCH 04/63] Add documentation about sort specification.

---
 docs/reference/cli.rst   |  5 +++--
 docs/reference/query.rst | 22 ++++++++++++++++++++++
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/docs/reference/cli.rst b/docs/reference/cli.rst
index 4ff0346a6..317dbd00d 100644
--- a/docs/reference/cli.rst
+++ b/docs/reference/cli.rst
@@ -169,8 +169,9 @@ list
 
 Want to search for "Gronlandic Edit" by of Montreal? Try ``beet list
 gronlandic``.  Maybe you want to see everything released in 2009 with
-"vegetables" in the title? Try ``beet list year:2009 title:vegetables``. (Read
-more in :doc:`query`.)
+"vegetables" in the title? Try ``beet list year:2009 title:vegetables``. You 
+can also specify the order used when outputting the results (Read more in 
+:doc:`query`.)
 
 You can use the ``-a`` switch to search for albums instead of individual items.
 In this case, the queries you use are restricted to album-level fields: for
diff --git a/docs/reference/query.rst b/docs/reference/query.rst
index cf981bed2..a224c8dc9 100644
--- a/docs/reference/query.rst
+++ b/docs/reference/query.rst
@@ -183,3 +183,25 @@ equivalent::
 Note that this only matches items that are *already in your library*, so a path
 query won't necessarily find *all* the audio files in a directory---just the
 ones you've already added to your beets library.
+
+
+Sort Order
+----------
+
+You can also specify the order used when outputting the results. Of course, this
+is only useful when displaying the result, for example with the ``list``
+command, and is useless when the query is used as a filter for an command. Use
+the name of the `field` you want to sort on, followed by a ``+`` or ``-`` sign
+if you want ascending or descending sort. For example this command::
+
+    $ beet list -a year+
+
+will list all albums in chronological order.
+
+You can also specify several sort orders, which will be used in the same order at
+which they appear in your query::
+
+    $ beet list -a genre+ year+
+ 
+This command will sort all albums by genre and, in each genre, in chronological
+order.

From 86c34f8740224cf053e536aeb38d5387066d86dc Mon Sep 17 00:00:00 2001
From: Pierre Rust <pierre.rust@gmail.com>
Date: Thu, 12 Jun 2014 10:07:56 +0200
Subject: [PATCH 05/63] Moves default sort specification from code to conf

Default sort order can be set in configuration file with the
`sort_album` and `sort_item` keys.
---
 beets/config_default.yaml  |  3 +++
 beets/dbcore/query.py      |  4 ++++
 beets/dbcore/queryparse.py |  2 ++
 beets/library.py           |  6 +-----
 beets/ui/commands.py       | 10 ++++++++--
 5 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/beets/config_default.yaml b/beets/config_default.yaml
index d35368ea5..e43345933 100644
--- a/beets/config_default.yaml
+++ b/beets/config_default.yaml
@@ -54,6 +54,9 @@ list_format_item: $artist - $album - $title
 list_format_album: $albumartist - $album
 time_format: '%Y-%m-%d %H:%M:%S'
 
+sort_album: smartartist+
+sort_item: smartartist+
+
 paths:
     default: $albumartist/$album%aunique{}/$track $title
     singleton: Non-Album/$artist/$title
diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py
index 97b8833bb..9952fcf4b 100644
--- a/beets/dbcore/query.py
+++ b/beets/dbcore/query.py
@@ -628,6 +628,7 @@ class FixedFieldSort(Sort):
         order = "ASC" if self.is_ascending else "DESC"
         return "{0} {1}".format(self.field, order)
 
+
 class SmartArtistSort(Sort):
     """ Sort Album or Item on artist sort fields, defaulting back on
         artist field if the sort specific field is empty.
@@ -659,6 +660,9 @@ class SmartArtistSort(Sort):
         return order_str
 
 
+special_sorts = {'smartartist': SmartArtistSort}
+
+
 def build_sql(model_cls, query, sort_order):
     """ Generate a sql statement (and the values that must be injected into it)
     from a query, sort and a model class.
diff --git a/beets/dbcore/queryparse.py b/beets/dbcore/queryparse.py
index 05ee28add..1a94c1831 100644
--- a/beets/dbcore/queryparse.py
+++ b/beets/dbcore/queryparse.py
@@ -134,6 +134,8 @@ def construct_sort_part(model_cls, part):
     elif field in model_cls._getters():
         # Computed field, all following fields must use the slow path.
         pass
+    elif field in query.special_sorts:
+        sort = query.special_sorts[field](model_cls, is_ascending)
     else:
         # Neither fixed nor computed : must be a flex attr.
         sort = query.FlexFieldSort(model_cls, field, is_ascending)
diff --git a/beets/library.py b/beets/library.py
index 3965b6fb9..16aeed772 100644
--- a/beets/library.py
+++ b/beets/library.py
@@ -1027,10 +1027,8 @@ class Library(dbcore.Database):
     def albums(self, query=None, sort_order=None):
         """Get a sorted list of :class:`Album` objects matching the
         given sort order. If a order specification is present in the query
-        string the sort_order argument is ignored. 
+        string the sort_order argument is ignored.
         """
-        if sort_order is None:
-            sort_order = SmartArtistSort(Album)
         return self._fetch(Album, query, sort_order)
 
     def items(self, query=None, sort_order=None):
@@ -1038,8 +1036,6 @@ class Library(dbcore.Database):
         given sort order. If a order specification is present in the query
         string the sort_order argument is ignored.
         """
-        if sort_order is None:
-            sort_order = SmartArtistSort(Item)
         return self._fetch(Item, query, sort_order)
 
     # Convenience accessors.
diff --git a/beets/ui/commands.py b/beets/ui/commands.py
index c32c1f313..dbd6806f4 100644
--- a/beets/ui/commands.py
+++ b/beets/ui/commands.py
@@ -38,6 +38,7 @@ from beets.util.functemplate import Template
 from beets import library
 from beets import config
 from beets.util.confit import _package_path
+from beets.dbcore import sort_from_strings
 
 VARIOUS_ARTISTS = u'Various Artists'
 
@@ -898,11 +899,16 @@ def list_items(lib, query, album, fmt):
     albums instead of single items.
     """
     tmpl = Template(ui._pick_format(album, fmt))
+
     if album:
-        for album in lib.albums(query):
+        sort_order = sort_from_strings(library.Album,
+                                       [str(config['sort_album'])])
+        for album in lib.albums(query, sort_order):
             ui.print_obj(album, lib, tmpl)
     else:
-        for item in lib.items(query):
+        sort_order = sort_from_strings(library.Item,
+                                       [str(config['sort_item'])])
+        for item in lib.items(query, sort_order):
             ui.print_obj(item, lib, tmpl)
 
 

From 486289b11ae577defb9a70482ef269b99afa3064 Mon Sep 17 00:00:00 2001
From: Pierre Rust <pierre.rust@gmail.com>
Date: Thu, 12 Jun 2014 10:23:23 +0200
Subject: [PATCH 06/63] Add documentation about default sort configuration.

---
 docs/reference/config.rst | 18 ++++++++++++++++++
 docs/reference/query.rst  |  5 +++++
 2 files changed, 23 insertions(+)

diff --git a/docs/reference/config.rst b/docs/reference/config.rst
index edf0f5027..cf10ef48d 100644
--- a/docs/reference/config.rst
+++ b/docs/reference/config.rst
@@ -163,6 +163,24 @@ Format to use when listing *albums* with :ref:`list-cmd` and other
 commands. Defaults to ``$albumartist - $album``. The ``-f`` command-line
 option overrides this setting.
 
+.. _sort_item:
+
+sort_item
+~~~~~~~~~
+
+Sort order to use when listing *individual items* with the :ref:`list-cmd`
+command and other commands that need to print out items. Defaults to
+``smartartist+``. Any command-line sort order overrides this setting.
+
+.. _sort_album:
+
+sort_album
+~~~~~~~~~~
+
+Sort order to use when listing *albums* with the :ref:`list-cmd`
+command. Defaults to ``smartartist+``. Any command-line sort order overrides
+this setting.
+
 .. _original_date:
 
 original_date
diff --git a/docs/reference/query.rst b/docs/reference/query.rst
index a224c8dc9..1702aa215 100644
--- a/docs/reference/query.rst
+++ b/docs/reference/query.rst
@@ -198,6 +198,11 @@ if you want ascending or descending sort. For example this command::
 
 will list all albums in chronological order.
 
+There is a special ``smartartist`` sort that uses sort-specific field (
+``artist_sort`` for items and ``albumartist_sort`` for albums) but falls back to
+standard artist fields if these are empty. When no sort order is specified,
+``smartartist+`` is used (but this is configurable).
+
 You can also specify several sort orders, which will be used in the same order at
 which they appear in your query::
 

From 1d61088cfc9e2e5233cc537ebd6c0aefda43e172 Mon Sep 17 00:00:00 2001
From: Pierre Rust <pierre.rust@gmail.com>
Date: Fri, 13 Jun 2014 13:52:14 +0200
Subject: [PATCH 07/63] Sort on computed field implementation.

Sort is implemented in python.
When combining sorts on fixed, flex and computed field, make as
much as possible in sql and finishes the job in python.
---
 beets/dbcore/db.py         |  72 +++++++++++------
 beets/dbcore/query.py      | 155 +++++++++++++++++++++++++------------
 beets/dbcore/queryparse.py |   2 +-
 3 files changed, 154 insertions(+), 75 deletions(-)

diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py
index e12178784..c50d2d572 100644
--- a/beets/dbcore/db.py
+++ b/beets/dbcore/db.py
@@ -483,43 +483,64 @@ class Results(object):
     """An item query result set. Iterating over the collection lazily
     constructs LibModel objects that reflect database rows.
     """
-    def __init__(self, model_class, rows, db, query=None):
+    def __init__(self, model_class, rows, db, query=None, sort=None):
         """Create a result set that will construct objects of type
         `model_class`, which should be a subclass of `LibModel`, out of
         the query result mapping in `rows`. The new objects are
-        associated with the database `db`. If `query` is provided, it is
-        used as a predicate to filter the results for a "slow query" that
-        cannot be evaluated by the database directly.
+        associated with the database `db`.
+        If `query` is provided, it is used as a predicate to filter the results
+        for a "slow query" that cannot be evaluated by the database directly.
+        If `sort` is provided, it is used to sort the full list of results
+        before returning. This means it is a "slow sort" and all objects must
+        be built before returning the first one.
         """
         self.model_class = model_class
         self.rows = rows
         self.db = db
         self.query = query
+        self.sort = sort
 
     def __iter__(self):
         """Construct Python objects for all rows that pass the query
         predicate.
         """
-        for row in self.rows:
-            # Get the flexible attributes for the object.
-            with self.db.transaction() as tx:
-                flex_rows = tx.query(
-                    'SELECT * FROM {0} WHERE entity_id=?'.format(
-                        self.model_class._flex_table
-                    ),
-                    (row['id'],)
-                )
+        if self.sort:
+            # slow sort, must build the full list first
+            objects = []
+            for row in self.rows:
+                obj = self._generate_results(row)
+                # check the predicate if any
+                if not self.query or self.query.match(obj):
+                    objects.append(obj)
+            # Now that we have the full list, we can sort it
+            objects = self.sort.sort(objects)
+            for o in objects:
+                yield o
+        else:
+            for row in self.rows:
+                obj = self._generate_results(row)
+                # check the predicate if any
+                if not self.query or self.query.match(obj):
+                    yield obj
 
-            cols = dict(row)
-            values = dict((k, v) for (k, v) in cols.items()
-                          if not k[:4] == 'flex')
-            flex_values = dict((row['key'], row['value']) for row in flex_rows)
+    def _generate_results(self, row):
+        # Get the flexible attributes for the object.
+        with self.db.transaction() as tx:
+            flex_rows = tx.query(
+                'SELECT * FROM {0} WHERE entity_id=?'.format(
+                    self.model_class._flex_table
+                ),
+                (row['id'],)
+            )
 
-            # Construct the Python object and yield it if it passes the
-            # predicate.
-            obj = self.model_class._awaken(self.db, values, flex_values)
-            if not self.query or self.query.match(obj):
-                yield obj
+        cols = dict(row)
+        values = dict((k, v) for (k, v) in cols.items()
+                      if not k[:4] == 'flex')
+        flex_values = dict((row['key'], row['value']) for row in flex_rows)
+
+        # Construct the Python object
+        obj = self.model_class._awaken(self.db, values, flex_values)
+        return obj
 
     def __len__(self):
         """Get the number of matching objects.
@@ -750,12 +771,15 @@ class Database(object):
         Sort object.
          """
 
-        sql, subvals, is_slow = build_sql(model_cls, query, sort_order)
+        sql, subvals, slow_query, slow_sort = build_sql(model_cls, query,
+                                                        sort_order)
 
         with self.transaction() as tx:
             rows = tx.query(sql, subvals)
 
-        return Results(model_cls, rows, self, None if not is_slow else query)
+        return Results(model_cls, rows, self,
+                       None if not slow_query else query,
+                       None if not slow_sort else sort_order)
 
     def _get(self, model_cls, id):
         """Get a Model object by its id or None if the id does not
diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py
index 9952fcf4b..14cb2af82 100644
--- a/beets/dbcore/query.py
+++ b/beets/dbcore/query.py
@@ -15,6 +15,7 @@
 """The Query type hierarchy for DBCore.
 """
 import re
+from operator import attrgetter
 from beets import util
 from datetime import datetime, timedelta
 
@@ -500,33 +501,43 @@ class DateQuery(FieldQuery):
 
 
 class Sort(object):
-    """An abstract class representing a sort opertation for a query into the
+    """An abstract class representing a sort operation for a query into the
     item database.
     """
     def select_clause(self):
-        """ Generates a select sql fragment.
+        """ Generates a select sql fragment if the sort operation requires one,
+        an empty string otherwise.
         """
-        return None
+        return ""
 
     def union_clause(self):
-        """ Generates a union sql fragment or None if the Sort is a slow sort.
+        """ Generates a union sql fragment if the sort operation requires one,
+        an empty string otherwise.
         """
-        return None
+        return ""
 
     def order_clause(self):
-        """Generates a sql fragment to be use in a ORDER BY clause
-        or None if it's a slow query
+        """Generates a sql fragment to be use in a ORDER BY clause or None if
+        it's a slow query.
         """
         return None
 
     def sort(self, items):
-        """Sort the given items list. Meant to be used with slow queries.
+        """Return a key function that can be used with the list.sort() method.
+        Meant to be used with slow sort, it must be implemented even for sort
+        that can be done with sql, as they might be used in conjunction with
+        slow sort.
         """
-        return items
+        return sorted(items, key=lambda x: x)
+
+    def is_slow(self):
+        return False
 
 
 class MultipleSort(Sort):
     """ Sort class that combines several sort criteria.
+    This implementation tries to implement as many sort operation in sql,
+    falling back to python sort only when necessary.
     """
 
     def __init__(self):
@@ -535,59 +546,75 @@ class MultipleSort(Sort):
     def add_criteria(self, sort):
         self.sorts.append(sort)
 
-    def select_clause(self):
-        """ Generate a select sql fragment.
+    def _sql_sorts(self):
+        """ Returns the list of sort for which sql can be used
         """
-        select_strings = []
-        index = 0
-        for sort in self.sorts:
-            select = sort.select_clause()
-            if select is None:
-                # FIXME : sort for slow sort
-                break
+        # with several Sort, we can use SQL sorting only if there is only
+        # SQL-capable Sort or if the list ends with SQl-capable Sort.
+        sql_sorts = []
+        for sort in reversed(self.sorts):
+            if not sort.order_clause() is None:
+                sql_sorts.append(sort)
             else:
+                break
+        sql_sorts.reverse()
+        return sql_sorts
+
+    def select_clause(self):
+        sql_sorts = self._sql_sorts()
+        select_strings = []
+        for sort in sql_sorts:
+            select = sort.select_clause()
+            if select:
                 select_strings.append(select)
-                index = index + 1
 
         select_string = ",".join(select_strings)
         return "" if not select_string else ", " + select_string
 
     def union_clause(self):
-        """ Returns a union sql fragment.
-        """
+        sql_sorts = self._sql_sorts()
         union_strings = []
-        for sort in self.sorts:
+        for sort in sql_sorts:
             union = sort.union_clause()
-            if union is None:
-                pass
-            else:
-                union_strings.append(union)
+            union_strings.append(union)
 
         return "".join(union_strings)
 
     def order_clause(self):
-        """Returns a sql fragment to be use in a ORDER BY clause
-        or None if it's a slow query
-        """
+        sql_sorts = self._sql_sorts()
         order_strings = []
-        index = 0
-        for sort in self.sorts:
+        for sort in sql_sorts:
             order = sort.order_clause()
-            if order is None:
-                break
-            else:
-                order_strings.append(order)
-                index = index + 1
+            order_strings.append(order)
 
         return ",".join(order_strings)
 
+    def is_slow(self):
+        for sort in self.sorts:
+            if sort.is_slow():
+                return True
+        return False
+
     def sort(self, items):
-        # FIXME : sort according to criteria following the first slow sort
+        slow_sorts = []
+        switch_slow = False
+        for sort in reversed(self.sorts):
+            if switch_slow:
+                slow_sorts.append(sort)
+            elif sort.order_clause() is None:
+                switch_slow = True
+                slow_sorts.append(sort)
+            else:
+                pass
+
+        for sort in slow_sorts:
+            items = sort.sort(items)
         return items
 
 
 class FlexFieldSort(Sort):
-
+    """Sort object to sort on a flexible attribute field
+    """
     def __init__(self, model_cls, field, is_ascending):
         self.model_cls = model_cls
         self.field = field
@@ -615,19 +642,26 @@ class FlexFieldSort(Sort):
         order = "ASC" if self.is_ascending else "DESC"
         return "flex_{0} {1} ".format(self.field, order)
 
+    def sort(self, items):
+        return sorted(items, key=attrgetter(self.field),
+                      reverse=(not self.is_ascending))
+
 
 class FixedFieldSort(Sort):
-
+    """Sort object to sort on a fixed field
+    """
     def __init__(self, field, is_ascending=True):
         self.field = field
         self.is_ascending = is_ascending
 
-    """ Sort on a fixed field
-    """
     def order_clause(self):
         order = "ASC" if self.is_ascending else "DESC"
         return "{0} {1}".format(self.field, order)
 
+    def sort(self, items):
+        return sorted(items, key=attrgetter(self.field),
+                      reverse=(not self.is_ascending))
+
 
 class SmartArtistSort(Sort):
     """ Sort Album or Item on artist sort fields, defaulting back on
@@ -660,27 +694,48 @@ class SmartArtistSort(Sort):
         return order_str
 
 
+class ComputedFieldSort(Sort):
+
+    def __init__(self, model_cls, field, is_ascending=True):
+        self.is_ascending = is_ascending
+        self.field = field
+        self._getters = model_cls._getters()
+
+    def is_slow(self):
+        return True
+
+    def sort(self, items):
+        return sorted(items, key=lambda x: self._getters[self.field](x),
+                      reverse=(not self.is_ascending))
+
 special_sorts = {'smartartist': SmartArtistSort}
 
 
-def build_sql(model_cls, query, sort_order):
+def build_sql(model_cls, query, sort):
     """ Generate a sql statement (and the values that must be injected into it)
     from a query, sort and a model class.
     """
     where, subvals = query.clause()
+    slow_query = where is None
 
-    if not sort_order:
+    if not sort:
         sort_select = ""
         sort_union = ""
         sort_order = ""
-    elif isinstance(sort_order, basestring):
+        slow_sort = False
+    elif isinstance(sort, basestring):
         sort_select = ""
         sort_union = ""
-        sort_order = " ORDER BY {0}".format(sort_order)
-    elif isinstance(sort_order, Sort):
-        sort_select = sort_order.select_clause()
-        sort_union = sort_order.union_clause()
-        sort_order = " ORDER BY {0}".format(sort_order.order_clause())
+        sort_order = " ORDER BY {0}".format(sort) \
+            if sort else ""
+        slow_sort = False
+    elif isinstance(sort, Sort):
+        sort_select = sort.select_clause()
+        sort_union = sort.union_clause()
+        slow_sort = sort.is_slow()
+        order_clause = sort.order_clause()
+        sort_order = " ORDER BY {0}".format(order_clause) \
+            if order_clause else ""
 
     sql = ("SELECT {table}.* {sort_select} FROM {table} {sort_union} WHERE "
            "{query_clause} {sort_order}").format(
@@ -691,4 +746,4 @@ def build_sql(model_cls, query, sort_order):
         sort_order=sort_order
     )
 
-    return (sql, subvals, where is None) 
\ No newline at end of file
+    return (sql, subvals, slow_query, slow_sort) 
\ No newline at end of file
diff --git a/beets/dbcore/queryparse.py b/beets/dbcore/queryparse.py
index 1a94c1831..03536f8ca 100644
--- a/beets/dbcore/queryparse.py
+++ b/beets/dbcore/queryparse.py
@@ -133,7 +133,7 @@ def construct_sort_part(model_cls, part):
         sort = query.FixedFieldSort(field, is_ascending)
     elif field in model_cls._getters():
         # Computed field, all following fields must use the slow path.
-        pass
+        sort = query.ComputedFieldSort(model_cls, field, is_ascending)
     elif field in query.special_sorts:
         sort = query.special_sorts[field](model_cls, is_ascending)
     else:

From ebd2da14dc85ad71dbd48ce4685b9121bac5479e Mon Sep 17 00:00:00 2001
From: Pierre Rust <pierre.rust@gmail.com>
Date: Fri, 13 Jun 2014 13:55:51 +0200
Subject: [PATCH 08/63] Fix error on default sort configuration

Fix crash when using several criteria for default sort in the
configuration file.
---
 beets/ui/commands.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/beets/ui/commands.py b/beets/ui/commands.py
index dbd6806f4..29d80a74b 100644
--- a/beets/ui/commands.py
+++ b/beets/ui/commands.py
@@ -901,13 +901,15 @@ def list_items(lib, query, album, fmt):
     tmpl = Template(ui._pick_format(album, fmt))
 
     if album:
+        sort_parts = str(config['sort_album']).split()
         sort_order = sort_from_strings(library.Album,
-                                       [str(config['sort_album'])])
+                                       sort_parts)
         for album in lib.albums(query, sort_order):
             ui.print_obj(album, lib, tmpl)
     else:
+        sort_parts = str(config['sort_item']).split()
         sort_order = sort_from_strings(library.Item,
-                                       [str(config['sort_item'])])
+                                       sort_parts)
         for item in lib.items(query, sort_order):
             ui.print_obj(item, lib, tmpl)
 

From a90991b29667fe93eb28036e4ccb08816bf1024a Mon Sep 17 00:00:00 2001
From: Pierre Rust <pierre.rust@gmail.com>
Date: Fri, 13 Jun 2014 13:57:16 +0200
Subject: [PATCH 09/63] Removes unused import (flake8 warning)

---
 beets/library.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/beets/library.py b/beets/library.py
index 16aeed772..9e566626c 100644
--- a/beets/library.py
+++ b/beets/library.py
@@ -29,7 +29,6 @@ from beets.util import bytestring_path, syspath, normpath, samefile
 from beets.util.functemplate import Template
 from beets import dbcore
 from beets.dbcore import types
-from beets.dbcore.query import SmartArtistSort
 import beets
 
 

From 1a995ed8d45d872efbfa324f0bd68eb0d64ee6b4 Mon Sep 17 00:00:00 2001
From: Pierre Rust <pierre.rust@gmail.com>
Date: Sat, 14 Jun 2014 17:11:57 +0200
Subject: [PATCH 10/63] Fix sorting without using the MultipleSort object.

SQL syntax error when using a simple FlexFieldSort.
---
 beets/dbcore/query.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py
index 14cb2af82..e240d3252 100644
--- a/beets/dbcore/query.py
+++ b/beets/dbcore/query.py
@@ -569,7 +569,7 @@ class MultipleSort(Sort):
                 select_strings.append(select)
 
         select_string = ",".join(select_strings)
-        return "" if not select_string else ", " + select_string
+        return select_string
 
     def union_clause(self):
         sql_sorts = self._sql_sorts()
@@ -730,7 +730,9 @@ def build_sql(model_cls, query, sort):
             if sort else ""
         slow_sort = False
     elif isinstance(sort, Sort):
-        sort_select = sort.select_clause()
+        select_clause = sort.select_clause() 
+        sort_select = " ,{0} ".format(select_clause) \
+            if select_clause else ""
         sort_union = sort.union_clause()
         slow_sort = sort.is_slow()
         order_clause = sort.order_clause()

From ad1c0b38319209f9a965c8c668c2734c75915b2c Mon Sep 17 00:00:00 2001
From: Pierre Rust <pierre.rust@gmail.com>
Date: Sat, 14 Jun 2014 21:24:54 +0200
Subject: [PATCH 11/63] Add unit tests for fixef and flex field sort

---
 test/test_sort.py | 155 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 155 insertions(+)
 create mode 100644 test/test_sort.py

diff --git a/test/test_sort.py b/test/test_sort.py
new file mode 100644
index 000000000..43f43e221
--- /dev/null
+++ b/test/test_sort.py
@@ -0,0 +1,155 @@
+# This file is part of beets.
+# Copyright 2013, Adrian Sampson.
+#
+# Permission is hereby granted, free of charge, to any person obtaining
+# a copy of this software and associated documentation files (the
+# "Software"), to deal in the Software without restriction, including
+# without limitation the rights to use, copy, modify, merge, publish,
+# distribute, sublicense, and/or sell copies of the Software, and to
+# permit persons to whom the Software is furnished to do so, subject to
+# the following conditions:
+#
+# The above copyright notice and this permission notice shall be
+# included in all copies or substantial portions of the Software.
+
+"""Various tests for querying the library database.
+"""
+import _common
+from _common import unittest
+import beets.library
+from beets import dbcore
+
+
+
+# A test case class providing a library with some dummy data and some
+# assertions involving that data.
+class DummyDataTestCase(_common.TestCase):
+    def setUp(self):
+        super(DummyDataTestCase, self).setUp()
+        self.lib = beets.library.Library(':memory:')
+        items = [_common.item() for _ in range(3)]
+        items[0].title = 'foo bar'
+        items[0].artist = 'one'
+        items[0].album = 'baz'
+        items[0].year = 2001
+        items[0].comp = True
+        items[0].flex1 = "flex1-0"
+        items[0].flex2 = "flex2-A"
+        items[1].title = 'baz qux'
+        items[1].artist = 'two'
+        items[1].album = 'baz'
+        items[1].year = 2002
+        items[1].comp = True
+        items[1].flex1 = "flex1-1"
+        items[1].flex2 = "flex2-A"
+        items[2].title = 'beets 4 eva'
+        items[2].artist = 'three'
+        items[2].album = 'foo'
+        items[2].year = 2003
+        items[2].comp = False
+        items[2].flex1 = "flex1-2"
+        items[2].flex2 = "flex1-B"
+        for item in items:
+            self.lib.add(item)
+        self.lib.add_album(items[:2])
+
+
+class SortFixedFieldTest(DummyDataTestCase):
+    def test_sort_asc(self):
+        q = ''
+        sort = dbcore.query.FixedFieldSort("year", True)
+        results = self.lib.items(q, sort)
+        self.assertTrue(results[0]['year']<results[1]['year'])
+        self.assertEqual(results[0]['year'],2001)
+        # same thing with query string
+        q = 'year+'
+        results2 = self.lib.items(q)
+        for r1, r2 in zip(results, results2) :
+            self.assertEqual(r1.id, r2.id)
+
+    def test_sort_desc(self):
+        q = ''
+        sort = dbcore.query.FixedFieldSort("year", False)
+        results = self.lib.items(q, sort)
+        self.assertTrue(results[0]['year']>results[1]['year'])
+        self.assertEqual(results[0]['year'],2003)
+        # same thing with query string
+        q = 'year-'
+        results2 = self.lib.items(q)
+        for r1, r2 in zip(results, results2) :
+            self.assertEqual(r1.id, r2.id)
+
+    def test_sort_two_field_asc(self):
+        q = ''
+        s1 = dbcore.query.FixedFieldSort("album", True)
+        s2 = dbcore.query.FixedFieldSort("year", True)
+        sort = dbcore.query.MultipleSort()
+        sort.add_criteria(s1)
+        sort.add_criteria(s2)
+        results = self.lib.items(q, sort)
+        self.assertTrue(results[0]['album']<=results[1]['album'])
+        self.assertTrue(results[1]['album']<=results[2]['album'])
+        self.assertEqual(results[0]['album'],'baz')
+        self.assertEqual(results[1]['album'],'baz')
+        self.assertTrue(results[0]['year']<=results[1]['year'])
+        #same thing with query string
+        q = 'album+ year+'
+        results2 = self.lib.items(q)
+        for r1, r2 in zip(results, results2) :
+            self.assertEqual(r1.id, r2.id)
+
+
+class SortFlexFieldTest(DummyDataTestCase):
+    def test_sort_asc(self):
+        q = ''
+        sort = dbcore.query.FlexFieldSort(beets.library.Item, "flex1", True)
+        results = self.lib.items(q, sort)
+        self.assertTrue(results[0]['flex1']<results[1]['flex1'])
+        self.assertEqual(results[0]['flex1'],'flex1-0')
+        # same thing with query string
+        q = 'flex1+'
+        results2 = self.lib.items(q)
+        for r1, r2 in zip(results, results2) :
+            self.assertEqual(r1.id, r2.id)
+
+    def test_sort_desc(self):
+        q = ''
+        sort = dbcore.query.FlexFieldSort(beets.library.Item, "flex1", False)
+        results = self.lib.items(q, sort)
+        self.assertTrue(results[0]['flex1']>results[1]['flex1'])
+        self.assertEqual(results[0]['flex1'],'flex1-2')
+        # same thing with query string
+        q = 'flex1-'
+        results2 = self.lib.items(q)
+        for r1, r2 in zip(results, results2) :
+            self.assertEqual(r1.id, r2.id)
+
+    def test_sort_two_field(self):
+        q = ''
+        s1 = dbcore.query.FlexFieldSort(beets.library.Item, "flex2", False)
+        s2 = dbcore.query.FlexFieldSort(beets.library.Item, "flex1", True)
+        sort = dbcore.query.MultipleSort()
+        sort.add_criteria(s1)
+        sort.add_criteria(s2)
+        results = self.lib.items(q, sort)
+        self.assertTrue(results[0]['flex2']>=results[1]['flex2'])
+        self.assertTrue(results[1]['flex2']>=results[2]['flex2'])
+        self.assertEqual(results[0]['flex2'],'flex2-A')
+        self.assertEqual(results[1]['flex2'],'flex2-A')
+        self.assertTrue(results[0]['flex1']<=results[1]['flex1'])
+        #same thing with query string
+        q = 'flex2- flex1+'
+        results2 = self.lib.items(q)
+        for r1, r2 in zip(results, results2) :
+            self.assertEqual(r1.id, r2.id)
+
+
+
+
+
+def suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)
+
+
+if __name__ == '__main__':
+    unittest.main(defaultTest='suite')

From 728797718ecdd5001048da85b9a618aba94bcf5c Mon Sep 17 00:00:00 2001
From: Pierre Rust <pierre.rust@gmail.com>
Date: Sun, 15 Jun 2014 09:08:28 +0200
Subject: [PATCH 12/63] Add tests cases for sort on albums.

On fixed and flex attributes.
---
 test/_common.py   |  26 +++++++-
 test/test_sort.py | 149 ++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 164 insertions(+), 11 deletions(-)

diff --git a/test/_common.py b/test/_common.py
index d209bf4ef..0187cc25b 100644
--- a/test/_common.py
+++ b/test/_common.py
@@ -82,9 +82,33 @@ def item(lib=None):
         album_id =         None,
     )
     if lib:
-        lib.add(i)
+        lib.add_album(i)
     return i
 
+_album_ident = 0
+def album(lib=None):
+    global _item_ident
+    _item_ident += 1
+    i = beets.library.Album(
+        artpath= None,
+        albumartist = 'some album artist',
+        albumartist_sort = 'some sort album artist',
+        albumartist_credit = 'some album artist credit',
+        album = 'the album',
+        genre = 'the genre',
+        year = 2014,
+        month = 2,
+        day = 5,
+        tracktotal = 0,
+        disctotal = 1,
+        comp = False,
+        mb_albumid = 'someID-1',
+        mb_albumartistid = 'someID-1'
+    )
+    if lib:
+        lib.add(i)
+    return i    
+    
 # Dummy import session.
 def import_session(lib=None, logfile=None, paths=[], query=[], cli=False):
     cls = commands.TerminalImportSession if cli else importer.ImportSession
diff --git a/test/test_sort.py b/test/test_sort.py
index 43f43e221..bb6047ddc 100644
--- a/test/test_sort.py
+++ b/test/test_sort.py
@@ -27,6 +27,26 @@ class DummyDataTestCase(_common.TestCase):
     def setUp(self):
         super(DummyDataTestCase, self).setUp()
         self.lib = beets.library.Library(':memory:')
+        
+        albums = [_common.album() for _ in range(3)]
+        albums[0].album = "album A"
+        albums[0].genre = "Rock"
+        albums[0].year = "2001"
+        albums[0].flex1 = "flex1-1"
+        albums[0].flex2 = "flex2-A"
+        albums[1].album = "album B"
+        albums[1].genre = "Rock"
+        albums[1].year = "2001"
+        albums[1].flex1 = "flex1-2"
+        albums[1].flex2 = "flex2-A"
+        albums[2].album = "album C"
+        albums[2].genre = "Jazz"
+        albums[2].year = "2005"
+        albums[2].flex1 = "flex1-1"
+        albums[2].flex2 = "flex2-B"
+        for album in albums:
+            self.lib.add(album)
+
         items = [_common.item() for _ in range(3)]
         items[0].title = 'foo bar'
         items[0].artist = 'one'
@@ -35,6 +55,7 @@ class DummyDataTestCase(_common.TestCase):
         items[0].comp = True
         items[0].flex1 = "flex1-0"
         items[0].flex2 = "flex2-A"
+        items[0].album_id = albums[0].id
         items[1].title = 'baz qux'
         items[1].artist = 'two'
         items[1].album = 'baz'
@@ -42,6 +63,7 @@ class DummyDataTestCase(_common.TestCase):
         items[1].comp = True
         items[1].flex1 = "flex1-1"
         items[1].flex2 = "flex2-A"
+        items[1].album_id = albums[0].id
         items[2].title = 'beets 4 eva'
         items[2].artist = 'three'
         items[2].album = 'foo'
@@ -49,9 +71,29 @@ class DummyDataTestCase(_common.TestCase):
         items[2].comp = False
         items[2].flex1 = "flex1-2"
         items[2].flex2 = "flex1-B"
+        items[2].album_id = albums[1].id
         for item in items:
             self.lib.add(item)
-        self.lib.add_album(items[:2])
+
+        albums = [_common.album() for _ in range(3)]
+        albums[0].album = "album A"
+        albums[0].genre = "Jazz"
+        albums[0].year = "2001"
+        albums[0].flex1 = "flex1-1"
+        albums[0].flex2 = "flex2-A"
+        albums[1].album = "album B"
+        albums[1].genre = "Jazz"
+        albums[1].year = "2001"
+        albums[1].flex1 = "flex1-2"
+        albums[1].flex2 = "flex2-A"
+        albums[2].album = "album C"
+        albums[2].genre = "Rock"
+        albums[2].year = "2005"
+        albums[2].flex1 = "flex1-1"
+        albums[2].flex2 = "flex2-B"
+        
+        for album in albums:
+            self.lib.add(album)
 
 
 class SortFixedFieldTest(DummyDataTestCase):
@@ -59,7 +101,7 @@ class SortFixedFieldTest(DummyDataTestCase):
         q = ''
         sort = dbcore.query.FixedFieldSort("year", True)
         results = self.lib.items(q, sort)
-        self.assertTrue(results[0]['year']<results[1]['year'])
+        self.assertLessEqual(results[0]['year'], results[1]['year'])
         self.assertEqual(results[0]['year'],2001)
         # same thing with query string
         q = 'year+'
@@ -71,7 +113,7 @@ class SortFixedFieldTest(DummyDataTestCase):
         q = ''
         sort = dbcore.query.FixedFieldSort("year", False)
         results = self.lib.items(q, sort)
-        self.assertTrue(results[0]['year']>results[1]['year'])
+        self.assertGreaterEqual(results[0]['year'], results[1]['year'])
         self.assertEqual(results[0]['year'],2003)
         # same thing with query string
         q = 'year-'
@@ -87,11 +129,11 @@ class SortFixedFieldTest(DummyDataTestCase):
         sort.add_criteria(s1)
         sort.add_criteria(s2)
         results = self.lib.items(q, sort)
-        self.assertTrue(results[0]['album']<=results[1]['album'])
-        self.assertTrue(results[1]['album']<=results[2]['album'])
+        self.assertLessEqual(results[0]['album'], results[1]['album'])
+        self.assertLessEqual(results[1]['album'], results[2]['album'])
         self.assertEqual(results[0]['album'],'baz')
         self.assertEqual(results[1]['album'],'baz')
-        self.assertTrue(results[0]['year']<=results[1]['year'])
+        self.assertLessEqual(results[0]['year'], results[1]['year'])
         #same thing with query string
         q = 'album+ year+'
         results2 = self.lib.items(q)
@@ -104,7 +146,7 @@ class SortFlexFieldTest(DummyDataTestCase):
         q = ''
         sort = dbcore.query.FlexFieldSort(beets.library.Item, "flex1", True)
         results = self.lib.items(q, sort)
-        self.assertTrue(results[0]['flex1']<results[1]['flex1'])
+        self.assertLessEqual(results[0]['flex1'], results[1]['flex1'])
         self.assertEqual(results[0]['flex1'],'flex1-0')
         # same thing with query string
         q = 'flex1+'
@@ -132,11 +174,11 @@ class SortFlexFieldTest(DummyDataTestCase):
         sort.add_criteria(s1)
         sort.add_criteria(s2)
         results = self.lib.items(q, sort)
-        self.assertTrue(results[0]['flex2']>=results[1]['flex2'])
-        self.assertTrue(results[1]['flex2']>=results[2]['flex2'])
+        self.assertGreaterEqual(results[0]['flex2'], results[1]['flex2'])
+        self.assertGreaterEqual(results[1]['flex2'], results[2]['flex2'])
         self.assertEqual(results[0]['flex2'],'flex2-A')
         self.assertEqual(results[1]['flex2'],'flex2-A')
-        self.assertTrue(results[0]['flex1']<=results[1]['flex1'])
+        self.assertLessEqual(results[0]['flex1'], results[1]['flex1'])
         #same thing with query string
         q = 'flex2- flex1+'
         results2 = self.lib.items(q)
@@ -144,7 +186,94 @@ class SortFlexFieldTest(DummyDataTestCase):
             self.assertEqual(r1.id, r2.id)
 
 
+class SortAlbumFixedFieldTest(DummyDataTestCase):
+    def test_sort_asc(self):
+        q = ''
+        sort = dbcore.query.FixedFieldSort("year", True)
+        results = self.lib.albums(q, sort)
+        self.assertLessEqual(results[0]['year'], results[1]['year'])
+        self.assertEqual(results[0]['year'],2001)
+        # same thing with query string
+        q = 'year+'
+        results2 = self.lib.albums(q)
+        for r1, r2 in zip(results, results2) :
+            self.assertEqual(r1.id, r2.id)
 
+    def test_sort_desc(self):
+        q = ''
+        sort = dbcore.query.FixedFieldSort("year", False)
+        results = self.lib.albums(q, sort)
+        self.assertGreaterEqual(results[0]['year'], results[1]['year'])
+        self.assertEqual(results[0]['year'],2005)
+        # same thing with query string
+        q = 'year-'
+        results2 = self.lib.albums(q)
+        for r1, r2 in zip(results, results2) :
+            self.assertEqual(r1.id, r2.id)
+
+    def test_sort_two_field_asc(self):
+        q = ''
+        s1 = dbcore.query.FixedFieldSort("genre", True)
+        s2 = dbcore.query.FixedFieldSort("album", True)
+        sort = dbcore.query.MultipleSort()
+        sort.add_criteria(s1)
+        sort.add_criteria(s2)
+        results = self.lib.albums(q, sort)
+        self.assertLessEqual(results[0]['genre'], results[1]['genre'])
+        self.assertLessEqual(results[1]['genre'], results[2]['genre'])
+        self.assertEqual(results[0]['genre'],'Jazz')
+        self.assertEqual(results[1]['genre'],'Jazz')
+        self.assertLessEqual(results[0]['album'], results[1]['album'])
+        #same thing with query string
+        q = 'genre+ album+'
+        results2 = self.lib.albums(q)
+        for r1, r2 in zip(results, results2) :
+            self.assertEqual(r1.id, r2.id)
+
+
+class SortAlbumFlexdFieldTest(DummyDataTestCase):
+    def test_sort_asc(self):
+        q = ''
+        sort = dbcore.query.FlexFieldSort(beets.library.Album, "flex1", True)
+        results = self.lib.albums(q, sort)
+        self.assertLessEqual(results[0]['flex1'], results[1]['flex1'])
+        self.assertLessEqual(results[1]['flex1'], results[2]['flex1'])
+        # same thing with query string
+        q = 'flex1+'
+        results2 = self.lib.albums(q)
+        for r1, r2 in zip(results, results2) :
+            self.assertEqual(r1.id, r2.id)
+
+    def test_sort_desc(self):
+        q = ''
+        sort = dbcore.query.FlexFieldSort(beets.library.Album, "flex1", False)
+        results = self.lib.albums(q, sort)
+        self.assertGreaterEqual(results[0]['flex1'], results[1]['flex1'])
+        self.assertGreaterEqual(results[1]['flex1'], results[2]['flex1'])
+        # same thing with query string
+        q = 'flex1-'
+        results2 = self.lib.albums(q)
+        for r1, r2 in zip(results, results2) :
+            self.assertEqual(r1.id, r2.id)
+
+    def test_sort_two_field_asc(self):
+        q = ''
+        s1 = dbcore.query.FlexFieldSort(beets.library.Album, "flex2", True)
+        s2 = dbcore.query.FlexFieldSort(beets.library.Album, "flex1", True)
+        sort = dbcore.query.MultipleSort()
+        sort.add_criteria(s1)
+        sort.add_criteria(s2)
+        results = self.lib.albums(q, sort)
+        self.assertLessEqual(results[0]['flex2'], results[1]['flex2'])
+        self.assertLessEqual(results[1]['flex2'], results[2]['flex2'])
+        self.assertEqual(results[0]['flex2'],'flex2-A')
+        self.assertEqual(results[1]['flex2'],'flex2-A')
+        self.assertLessEqual(results[0]['flex1'], results[1]['flex1'])
+        #same thing with query string
+        q = 'flex2+ flex1+'
+        results2 = self.lib.albums(q)
+        for r1, r2 in zip(results, results2) :
+            self.assertEqual(r1.id, r2.id)
 
 
 def suite():

From 936acf02362c0c82fe59a068b2685603ef37bc45 Mon Sep 17 00:00:00 2001
From: Pierre Rust <pierre.rust@gmail.com>
Date: Sun, 15 Jun 2014 09:16:37 +0200
Subject: [PATCH 13/63] Fix typo in common utility function for tests

---
 test/_common.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/_common.py b/test/_common.py
index 0187cc25b..7891f6e80 100644
--- a/test/_common.py
+++ b/test/_common.py
@@ -82,7 +82,7 @@ def item(lib=None):
         album_id =         None,
     )
     if lib:
-        lib.add_album(i)
+        lib.add(i)
     return i
 
 _album_ident = 0
@@ -106,7 +106,7 @@ def album(lib=None):
         mb_albumartistid = 'someID-1'
     )
     if lib:
-        lib.add(i)
+        lib.add_album(i)
     return i    
     
 # Dummy import session.

From 69213ad574147113648ea4520fc214229726e339 Mon Sep 17 00:00:00 2001
From: Pierre Rust <pierre.rust@gmail.com>
Date: Sun, 15 Jun 2014 22:06:05 +0200
Subject: [PATCH 14/63] Add tests for computed attributes.

---
 test/_common.py   |  2 +-
 test/test_sort.py | 93 ++++++++++++++++++++++++++++++++++-------------
 2 files changed, 68 insertions(+), 27 deletions(-)

diff --git a/test/_common.py b/test/_common.py
index 7891f6e80..94b6255b8 100644
--- a/test/_common.py
+++ b/test/_common.py
@@ -106,7 +106,7 @@ def album(lib=None):
         mb_albumartistid = 'someID-1'
     )
     if lib:
-        lib.add_album(i)
+        lib.add(i)
     return i    
     
 # Dummy import session.
diff --git a/test/test_sort.py b/test/test_sort.py
index bb6047ddc..472136ce0 100644
--- a/test/test_sort.py
+++ b/test/test_sort.py
@@ -47,7 +47,7 @@ class DummyDataTestCase(_common.TestCase):
         for album in albums:
             self.lib.add(album)
 
-        items = [_common.item() for _ in range(3)]
+        items = [_common.item() for _ in range(4)]
         items[0].title = 'foo bar'
         items[0].artist = 'one'
         items[0].album = 'baz'
@@ -72,29 +72,17 @@ class DummyDataTestCase(_common.TestCase):
         items[2].flex1 = "flex1-2"
         items[2].flex2 = "flex1-B"
         items[2].album_id = albums[1].id
+        items[3].title = 'beets 4 eva'
+        items[3].artist = 'three'
+        items[3].album = 'foo2'
+        items[3].year = 2004
+        items[3].comp = False
+        items[3].flex1 = "flex1-2"
+        items[3].flex2 = "flex1-C"
+        items[3].album_id = albums[2].id
         for item in items:
             self.lib.add(item)
 
-        albums = [_common.album() for _ in range(3)]
-        albums[0].album = "album A"
-        albums[0].genre = "Jazz"
-        albums[0].year = "2001"
-        albums[0].flex1 = "flex1-1"
-        albums[0].flex2 = "flex2-A"
-        albums[1].album = "album B"
-        albums[1].genre = "Jazz"
-        albums[1].year = "2001"
-        albums[1].flex1 = "flex1-2"
-        albums[1].flex2 = "flex2-A"
-        albums[2].album = "album C"
-        albums[2].genre = "Rock"
-        albums[2].year = "2005"
-        albums[2].flex1 = "flex1-1"
-        albums[2].flex2 = "flex2-B"
-        
-        for album in albums:
-            self.lib.add(album)
-
 
 class SortFixedFieldTest(DummyDataTestCase):
     def test_sort_asc(self):
@@ -114,7 +102,7 @@ class SortFixedFieldTest(DummyDataTestCase):
         sort = dbcore.query.FixedFieldSort("year", False)
         results = self.lib.items(q, sort)
         self.assertGreaterEqual(results[0]['year'], results[1]['year'])
-        self.assertEqual(results[0]['year'],2003)
+        self.assertEqual(results[0]['year'],2004)
         # same thing with query string
         q = 'year-'
         results2 = self.lib.items(q)
@@ -158,7 +146,9 @@ class SortFlexFieldTest(DummyDataTestCase):
         q = ''
         sort = dbcore.query.FlexFieldSort(beets.library.Item, "flex1", False)
         results = self.lib.items(q, sort)
-        self.assertTrue(results[0]['flex1']>results[1]['flex1'])
+        self.assertGreaterEqual(results[0]['flex1'], results[1]['flex1'])
+        self.assertGreaterEqual(results[1]['flex1'], results[2]['flex1'])
+        self.assertGreaterEqual(results[2]['flex1'], results[3]['flex1'])
         self.assertEqual(results[0]['flex1'],'flex1-2')
         # same thing with query string
         q = 'flex1-'
@@ -221,9 +211,9 @@ class SortAlbumFixedFieldTest(DummyDataTestCase):
         results = self.lib.albums(q, sort)
         self.assertLessEqual(results[0]['genre'], results[1]['genre'])
         self.assertLessEqual(results[1]['genre'], results[2]['genre'])
-        self.assertEqual(results[0]['genre'],'Jazz')
-        self.assertEqual(results[1]['genre'],'Jazz')
-        self.assertLessEqual(results[0]['album'], results[1]['album'])
+        self.assertEqual(results[1]['genre'],'Rock')
+        self.assertEqual(results[2]['genre'],'Rock')
+        self.assertLessEqual(results[1]['album'], results[2]['album'])
         #same thing with query string
         q = 'genre+ album+'
         results2 = self.lib.albums(q)
@@ -276,6 +266,57 @@ class SortAlbumFlexdFieldTest(DummyDataTestCase):
             self.assertEqual(r1.id, r2.id)
 
 
+class SortAlbumComputedFieldTest(DummyDataTestCase):
+    def test_sort_asc(self):
+        q = ''
+        sort = dbcore.query.ComputedFieldSort(beets.library.Album, "path", True)
+        results = self.lib.albums(q, sort)
+        self.assertLessEqual(results[0]['path'], results[1]['path'])
+        self.assertLessEqual(results[1]['path'], results[2]['path'])
+        # same thing with query string
+        q = 'path+'
+        results2 = self.lib.albums(q)
+        for r1, r2 in zip(results, results2) :
+            self.assertEqual(r1.id, r2.id)
+
+    def test_sort_asc(self):
+        q = ''
+        sort = dbcore.query.ComputedFieldSort(beets.library.Album, "path", False)
+        results = self.lib.albums(q, sort)
+        self.assertGreaterEqual(results[0]['path'], results[1]['path'])
+        self.assertGreaterEqual(results[1]['path'], results[2]['path'])
+        # same thing with query string
+        q = 'path-'
+        results2 = self.lib.albums(q)
+        for r1, r2 in zip(results, results2) :
+            self.assertEqual(r1.id, r2.id)
+
+
+class SortCombinedFieldTest(DummyDataTestCase):
+    def test_computed_first(self):
+        q=''
+        s1 = dbcore.query.ComputedFieldSort(beets.library.Album, "path", True)
+        s2 = dbcore.query.FixedFieldSort("year", True)
+        sort = dbcore.query.MultipleSort()
+        sort.add_criteria(s1)
+        sort.add_criteria(s2)
+        results = self.lib.albums(q, sort)
+        self.assertLessEqual(results[0]['path'], results[1]['path'])
+        self.assertLessEqual(results[1]['path'], results[2]['path'])
+
+    def test_computed_second(self):
+        q=''
+        s1 = dbcore.query.FixedFieldSort("year", True)
+        s2 = dbcore.query.ComputedFieldSort(beets.library.Album, "path", True)
+        sort = dbcore.query.MultipleSort()
+        sort.add_criteria(s1)
+        sort.add_criteria(s2)
+        results = self.lib.albums(q, sort)
+        self.assertLessEqual(results[0]['year'], results[1]['year'])
+        self.assertLessEqual(results[1]['year'], results[2]['year'])
+        self.assertLessEqual(results[0]['path'], results[1]['path'])
+
+
 def suite():
     return unittest.TestLoader().loadTestsFromName(__name__)
 

From 18539b0bd190f6cbb2197f9a257928d18048c24d Mon Sep 17 00:00:00 2001
From: Pierre Rust <pierre.rust@gmail.com>
Date: Sun, 15 Jun 2014 22:28:26 +0200
Subject: [PATCH 15/63] Fix flake8 warnings.

---
 beets/dbcore/query.py |  2 +-
 test/test_sort.py     | 87 ++++++++++++++++++++++++-------------------
 2 files changed, 49 insertions(+), 40 deletions(-)

diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py
index e240d3252..458bb7f91 100644
--- a/beets/dbcore/query.py
+++ b/beets/dbcore/query.py
@@ -730,7 +730,7 @@ def build_sql(model_cls, query, sort):
             if sort else ""
         slow_sort = False
     elif isinstance(sort, Sort):
-        select_clause = sort.select_clause() 
+        select_clause = sort.select_clause()
         sort_select = " ,{0} ".format(select_clause) \
             if select_clause else ""
         sort_union = sort.union_clause()
diff --git a/test/test_sort.py b/test/test_sort.py
index 472136ce0..76e5a35cb 100644
--- a/test/test_sort.py
+++ b/test/test_sort.py
@@ -20,14 +20,13 @@ import beets.library
 from beets import dbcore
 
 
-
 # A test case class providing a library with some dummy data and some
 # assertions involving that data.
 class DummyDataTestCase(_common.TestCase):
     def setUp(self):
         super(DummyDataTestCase, self).setUp()
         self.lib = beets.library.Library(':memory:')
-        
+
         albums = [_common.album() for _ in range(3)]
         albums[0].album = "album A"
         albums[0].genre = "Rock"
@@ -90,11 +89,11 @@ class SortFixedFieldTest(DummyDataTestCase):
         sort = dbcore.query.FixedFieldSort("year", True)
         results = self.lib.items(q, sort)
         self.assertLessEqual(results[0]['year'], results[1]['year'])
-        self.assertEqual(results[0]['year'],2001)
+        self.assertEqual(results[0]['year'], 2001)
         # same thing with query string
         q = 'year+'
         results2 = self.lib.items(q)
-        for r1, r2 in zip(results, results2) :
+        for r1, r2 in zip(results, results2):
             self.assertEqual(r1.id, r2.id)
 
     def test_sort_desc(self):
@@ -102,11 +101,11 @@ class SortFixedFieldTest(DummyDataTestCase):
         sort = dbcore.query.FixedFieldSort("year", False)
         results = self.lib.items(q, sort)
         self.assertGreaterEqual(results[0]['year'], results[1]['year'])
-        self.assertEqual(results[0]['year'],2004)
+        self.assertEqual(results[0]['year'], 2004)
         # same thing with query string
         q = 'year-'
         results2 = self.lib.items(q)
-        for r1, r2 in zip(results, results2) :
+        for r1, r2 in zip(results, results2):
             self.assertEqual(r1.id, r2.id)
 
     def test_sort_two_field_asc(self):
@@ -119,13 +118,13 @@ class SortFixedFieldTest(DummyDataTestCase):
         results = self.lib.items(q, sort)
         self.assertLessEqual(results[0]['album'], results[1]['album'])
         self.assertLessEqual(results[1]['album'], results[2]['album'])
-        self.assertEqual(results[0]['album'],'baz')
-        self.assertEqual(results[1]['album'],'baz')
+        self.assertEqual(results[0]['album'], 'baz')
+        self.assertEqual(results[1]['album'], 'baz')
         self.assertLessEqual(results[0]['year'], results[1]['year'])
-        #same thing with query string
+        # same thing with query string
         q = 'album+ year+'
         results2 = self.lib.items(q)
-        for r1, r2 in zip(results, results2) :
+        for r1, r2 in zip(results, results2):
             self.assertEqual(r1.id, r2.id)
 
 
@@ -135,11 +134,11 @@ class SortFlexFieldTest(DummyDataTestCase):
         sort = dbcore.query.FlexFieldSort(beets.library.Item, "flex1", True)
         results = self.lib.items(q, sort)
         self.assertLessEqual(results[0]['flex1'], results[1]['flex1'])
-        self.assertEqual(results[0]['flex1'],'flex1-0')
+        self.assertEqual(results[0]['flex1'], 'flex1-0')
         # same thing with query string
         q = 'flex1+'
         results2 = self.lib.items(q)
-        for r1, r2 in zip(results, results2) :
+        for r1, r2 in zip(results, results2):
             self.assertEqual(r1.id, r2.id)
 
     def test_sort_desc(self):
@@ -149,11 +148,11 @@ class SortFlexFieldTest(DummyDataTestCase):
         self.assertGreaterEqual(results[0]['flex1'], results[1]['flex1'])
         self.assertGreaterEqual(results[1]['flex1'], results[2]['flex1'])
         self.assertGreaterEqual(results[2]['flex1'], results[3]['flex1'])
-        self.assertEqual(results[0]['flex1'],'flex1-2')
+        self.assertEqual(results[0]['flex1'], 'flex1-2')
         # same thing with query string
         q = 'flex1-'
         results2 = self.lib.items(q)
-        for r1, r2 in zip(results, results2) :
+        for r1, r2 in zip(results, results2):
             self.assertEqual(r1.id, r2.id)
 
     def test_sort_two_field(self):
@@ -166,13 +165,13 @@ class SortFlexFieldTest(DummyDataTestCase):
         results = self.lib.items(q, sort)
         self.assertGreaterEqual(results[0]['flex2'], results[1]['flex2'])
         self.assertGreaterEqual(results[1]['flex2'], results[2]['flex2'])
-        self.assertEqual(results[0]['flex2'],'flex2-A')
-        self.assertEqual(results[1]['flex2'],'flex2-A')
+        self.assertEqual(results[0]['flex2'], 'flex2-A')
+        self.assertEqual(results[1]['flex2'], 'flex2-A')
         self.assertLessEqual(results[0]['flex1'], results[1]['flex1'])
-        #same thing with query string
+        # same thing with query string
         q = 'flex2- flex1+'
         results2 = self.lib.items(q)
-        for r1, r2 in zip(results, results2) :
+        for r1, r2 in zip(results, results2):
             self.assertEqual(r1.id, r2.id)
 
 
@@ -182,11 +181,11 @@ class SortAlbumFixedFieldTest(DummyDataTestCase):
         sort = dbcore.query.FixedFieldSort("year", True)
         results = self.lib.albums(q, sort)
         self.assertLessEqual(results[0]['year'], results[1]['year'])
-        self.assertEqual(results[0]['year'],2001)
+        self.assertEqual(results[0]['year'], 2001)
         # same thing with query string
         q = 'year+'
         results2 = self.lib.albums(q)
-        for r1, r2 in zip(results, results2) :
+        for r1, r2 in zip(results, results2):
             self.assertEqual(r1.id, r2.id)
 
     def test_sort_desc(self):
@@ -194,11 +193,11 @@ class SortAlbumFixedFieldTest(DummyDataTestCase):
         sort = dbcore.query.FixedFieldSort("year", False)
         results = self.lib.albums(q, sort)
         self.assertGreaterEqual(results[0]['year'], results[1]['year'])
-        self.assertEqual(results[0]['year'],2005)
+        self.assertEqual(results[0]['year'], 2005)
         # same thing with query string
         q = 'year-'
         results2 = self.lib.albums(q)
-        for r1, r2 in zip(results, results2) :
+        for r1, r2 in zip(results, results2):
             self.assertEqual(r1.id, r2.id)
 
     def test_sort_two_field_asc(self):
@@ -211,13 +210,13 @@ class SortAlbumFixedFieldTest(DummyDataTestCase):
         results = self.lib.albums(q, sort)
         self.assertLessEqual(results[0]['genre'], results[1]['genre'])
         self.assertLessEqual(results[1]['genre'], results[2]['genre'])
-        self.assertEqual(results[1]['genre'],'Rock')
-        self.assertEqual(results[2]['genre'],'Rock')
+        self.assertEqual(results[1]['genre'], 'Rock')
+        self.assertEqual(results[2]['genre'], 'Rock')
         self.assertLessEqual(results[1]['album'], results[2]['album'])
-        #same thing with query string
+        # same thing with query string
         q = 'genre+ album+'
         results2 = self.lib.albums(q)
-        for r1, r2 in zip(results, results2) :
+        for r1, r2 in zip(results, results2):
             self.assertEqual(r1.id, r2.id)
 
 
@@ -231,7 +230,7 @@ class SortAlbumFlexdFieldTest(DummyDataTestCase):
         # same thing with query string
         q = 'flex1+'
         results2 = self.lib.albums(q)
-        for r1, r2 in zip(results, results2) :
+        for r1, r2 in zip(results, results2):
             self.assertEqual(r1.id, r2.id)
 
     def test_sort_desc(self):
@@ -243,7 +242,7 @@ class SortAlbumFlexdFieldTest(DummyDataTestCase):
         # same thing with query string
         q = 'flex1-'
         results2 = self.lib.albums(q)
-        for r1, r2 in zip(results, results2) :
+        for r1, r2 in zip(results, results2):
             self.assertEqual(r1.id, r2.id)
 
     def test_sort_two_field_asc(self):
@@ -256,45 +255,47 @@ class SortAlbumFlexdFieldTest(DummyDataTestCase):
         results = self.lib.albums(q, sort)
         self.assertLessEqual(results[0]['flex2'], results[1]['flex2'])
         self.assertLessEqual(results[1]['flex2'], results[2]['flex2'])
-        self.assertEqual(results[0]['flex2'],'flex2-A')
-        self.assertEqual(results[1]['flex2'],'flex2-A')
+        self.assertEqual(results[0]['flex2'], 'flex2-A')
+        self.assertEqual(results[1]['flex2'], 'flex2-A')
         self.assertLessEqual(results[0]['flex1'], results[1]['flex1'])
-        #same thing with query string
+        # same thing with query string
         q = 'flex2+ flex1+'
         results2 = self.lib.albums(q)
-        for r1, r2 in zip(results, results2) :
+        for r1, r2 in zip(results, results2):
             self.assertEqual(r1.id, r2.id)
 
 
 class SortAlbumComputedFieldTest(DummyDataTestCase):
     def test_sort_asc(self):
         q = ''
-        sort = dbcore.query.ComputedFieldSort(beets.library.Album, "path", True)
+        sort = dbcore.query.ComputedFieldSort(beets.library.Album, "path",
+                                              True)
         results = self.lib.albums(q, sort)
         self.assertLessEqual(results[0]['path'], results[1]['path'])
         self.assertLessEqual(results[1]['path'], results[2]['path'])
         # same thing with query string
         q = 'path+'
         results2 = self.lib.albums(q)
-        for r1, r2 in zip(results, results2) :
+        for r1, r2 in zip(results, results2):
             self.assertEqual(r1.id, r2.id)
 
-    def test_sort_asc(self):
+    def test_sort_desc(self):
         q = ''
-        sort = dbcore.query.ComputedFieldSort(beets.library.Album, "path", False)
+        sort = dbcore.query.ComputedFieldSort(beets.library.Album, "path",
+                                              False)
         results = self.lib.albums(q, sort)
         self.assertGreaterEqual(results[0]['path'], results[1]['path'])
         self.assertGreaterEqual(results[1]['path'], results[2]['path'])
         # same thing with query string
         q = 'path-'
         results2 = self.lib.albums(q)
-        for r1, r2 in zip(results, results2) :
+        for r1, r2 in zip(results, results2):
             self.assertEqual(r1.id, r2.id)
 
 
 class SortCombinedFieldTest(DummyDataTestCase):
     def test_computed_first(self):
-        q=''
+        q = ''
         s1 = dbcore.query.ComputedFieldSort(beets.library.Album, "path", True)
         s2 = dbcore.query.FixedFieldSort("year", True)
         sort = dbcore.query.MultipleSort()
@@ -303,9 +304,13 @@ class SortCombinedFieldTest(DummyDataTestCase):
         results = self.lib.albums(q, sort)
         self.assertLessEqual(results[0]['path'], results[1]['path'])
         self.assertLessEqual(results[1]['path'], results[2]['path'])
+        q = 'path+ year+'
+        results2 = self.lib.albums(q)
+        for r1, r2 in zip(results, results2):
+            self.assertEqual(r1.id, r2.id)
 
     def test_computed_second(self):
-        q=''
+        q = ''
         s1 = dbcore.query.FixedFieldSort("year", True)
         s2 = dbcore.query.ComputedFieldSort(beets.library.Album, "path", True)
         sort = dbcore.query.MultipleSort()
@@ -315,6 +320,10 @@ class SortCombinedFieldTest(DummyDataTestCase):
         self.assertLessEqual(results[0]['year'], results[1]['year'])
         self.assertLessEqual(results[1]['year'], results[2]['year'])
         self.assertLessEqual(results[0]['path'], results[1]['path'])
+        q = 'year+ path+'
+        results2 = self.lib.albums(q)
+        for r1, r2 in zip(results, results2):
+            self.assertEqual(r1.id, r2.id)
 
 
 def suite():

From cd1dcf69690a95cc4eecb2e4a7aba609691291ff Mon Sep 17 00:00:00 2001
From: Pierre Rust <pierre.rust@gmail.com>
Date: Wed, 2 Jul 2014 13:05:33 +0200
Subject: [PATCH 16/63] Fix minor formatting and comments issues.

---
 beets/dbcore/db.py         | 2 +-
 beets/dbcore/query.py      | 2 +-
 beets/dbcore/queryparse.py | 2 +-
 beets/library.py           | 4 ++--
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py
index c50d2d572..5dff1c0f1 100644
--- a/beets/dbcore/db.py
+++ b/beets/dbcore/db.py
@@ -505,7 +505,7 @@ class Results(object):
         predicate.
         """
         if self.sort:
-            # slow sort, must build the full list first
+            # Slow sort. Must build the full list first.
             objects = []
             for row in self.rows:
                 obj = self._generate_results(row)
diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py
index 458bb7f91..b66496e2d 100644
--- a/beets/dbcore/query.py
+++ b/beets/dbcore/query.py
@@ -535,7 +535,7 @@ class Sort(object):
 
 
 class MultipleSort(Sort):
-    """ Sort class that combines several sort criteria.
+    """Sort class that combines several sort criteria.
     This implementation tries to implement as many sort operation in sql,
     falling back to python sort only when necessary.
     """
diff --git a/beets/dbcore/queryparse.py b/beets/dbcore/queryparse.py
index 03536f8ca..b51194b33 100644
--- a/beets/dbcore/queryparse.py
+++ b/beets/dbcore/queryparse.py
@@ -150,4 +150,4 @@ def sort_from_strings(model_cls, sort_parts):
     sort = query.MultipleSort()
     for part in sort_parts:
         sort.add_criteria(construct_sort_part(model_cls, part))
-    return sort
\ No newline at end of file
+    return sort
diff --git a/beets/library.py b/beets/library.py
index 9e566626c..7270dc8c0 100644
--- a/beets/library.py
+++ b/beets/library.py
@@ -942,10 +942,10 @@ def get_query(val, model_cls):
         # Add path queries to aggregate query.
         if path_parts:
             query.subqueries += [PathQuery('path', s) for s in path_parts]
-        return (query, sort)
+        return query, sort
 
     elif isinstance(val, dbcore.Query):
-        return (val, None)
+        return val, None
 
     else:
         raise ValueError('query must be None or have type Query or str')

From 451f17c99ada663b42b0982a05eee32b3f0db4dc Mon Sep 17 00:00:00 2001
From: Pierre Rust <pierre.rust@gmail.com>
Date: Wed, 2 Jul 2014 19:06:27 +0200
Subject: [PATCH 17/63] Fix formatting issue

---
 beets/dbcore/query.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py
index b66496e2d..14ce0a2d4 100644
--- a/beets/dbcore/query.py
+++ b/beets/dbcore/query.py
@@ -748,4 +748,4 @@ def build_sql(model_cls, query, sort):
         sort_order=sort_order
     )
 
-    return (sql, subvals, slow_query, slow_sort) 
\ No newline at end of file
+    return sql, subvals, slow_query, slow_sort
\ No newline at end of file

From d168c6222b246a30393be9cfd35fe8b79684739d Mon Sep 17 00:00:00 2001
From: Pierre Rust <pierre.rust@gmail.com>
Date: Wed, 2 Jul 2014 19:26:27 +0200
Subject: [PATCH 18/63] Fix formatting (again ...)

---
 beets/dbcore/query.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py
index 14ce0a2d4..6b1b2541e 100644
--- a/beets/dbcore/query.py
+++ b/beets/dbcore/query.py
@@ -748,4 +748,4 @@ def build_sql(model_cls, query, sort):
         sort_order=sort_order
     )
 
-    return sql, subvals, slow_query, slow_sort
\ No newline at end of file
+    return sql, subvals, slow_query, slow_sort

From 3130a6add9a607f39511371e0a06944070af6ff1 Mon Sep 17 00:00:00 2001
From: PierreRust <pierre.rust@gmail.com>
Date: Thu, 24 Jul 2014 12:51:21 +0200
Subject: [PATCH 19/63] Minor changes from review (mostly style)

---
 beets/dbcore/db.py         | 13 +++++--------
 beets/dbcore/query.py      | 15 +++++++++------
 beets/library.py           |  8 ++++----
 beetsplug/ihate.py         |  6 +++---
 beetsplug/smartplaylist.py |  2 +-
 5 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py
index 5dff1c0f1..5172ad523 100644
--- a/beets/dbcore/db.py
+++ b/beets/dbcore/db.py
@@ -508,7 +508,7 @@ class Results(object):
             # Slow sort. Must build the full list first.
             objects = []
             for row in self.rows:
-                obj = self._generate_results(row)
+                obj = self._make_model(row)
                 # check the predicate if any
                 if not self.query or self.query.match(obj):
                     objects.append(obj)
@@ -518,12 +518,12 @@ class Results(object):
                 yield o
         else:
             for row in self.rows:
-                obj = self._generate_results(row)
+                obj = self._make_model(row)
                 # check the predicate if any
                 if not self.query or self.query.match(obj):
                     yield obj
 
-    def _generate_results(self, row):
+    def _make_model(self, row):
         # Get the flexible attributes for the object.
         with self.db.transaction() as tx:
             flex_rows = tx.query(
@@ -771,15 +771,12 @@ class Database(object):
         Sort object.
          """
 
-        sql, subvals, slow_query, slow_sort = build_sql(model_cls, query,
-                                                        sort_order)
+        sql, subvals, query, sort = build_sql(model_cls, query, sort_order)
 
         with self.transaction() as tx:
             rows = tx.query(sql, subvals)
 
-        return Results(model_cls, rows, self,
-                       None if not slow_query else query,
-                       None if not slow_sort else sort_order)
+        return Results(model_cls, rows, self, query, sort)
 
     def _get(self, model_cls, id):
         """Get a Model object by its id or None if the id does not
diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py
index 6b1b2541e..6a10f2533 100644
--- a/beets/dbcore/query.py
+++ b/beets/dbcore/query.py
@@ -713,31 +713,34 @@ special_sorts = {'smartartist': SmartArtistSort}
 
 def build_sql(model_cls, query, sort):
     """ Generate a sql statement (and the values that must be injected into it)
-    from a query, sort and a model class.
+    from a query, sort and a model class. Query and sort objects are returned
+    only for slow query and slow sort operation.
     """
     where, subvals = query.clause()
-    slow_query = where is None
+    if where is not None:
+        query = None
 
     if not sort:
         sort_select = ""
         sort_union = ""
         sort_order = ""
-        slow_sort = False
+        sort = None
     elif isinstance(sort, basestring):
         sort_select = ""
         sort_union = ""
         sort_order = " ORDER BY {0}".format(sort) \
             if sort else ""
-        slow_sort = False
+        sort = None
     elif isinstance(sort, Sort):
         select_clause = sort.select_clause()
         sort_select = " ,{0} ".format(select_clause) \
             if select_clause else ""
         sort_union = sort.union_clause()
-        slow_sort = sort.is_slow()
         order_clause = sort.order_clause()
         sort_order = " ORDER BY {0}".format(order_clause) \
             if order_clause else ""
+        if sort.is_slow():
+            sort = None
 
     sql = ("SELECT {table}.* {sort_select} FROM {table} {sort_union} WHERE "
            "{query_clause} {sort_order}").format(
@@ -748,4 +751,4 @@ def build_sql(model_cls, query, sort):
         sort_order=sort_order
     )
 
-    return sql, subvals, slow_query, slow_sort
+    return sql, subvals, query, sort
diff --git a/beets/library.py b/beets/library.py
index 7270dc8c0..c0c9ebe1a 100644
--- a/beets/library.py
+++ b/beets/library.py
@@ -546,7 +546,7 @@ class Item(LibModel):
         for query, path_format in path_formats:
             if query == PF_KEY_DEFAULT:
                 continue
-            (query, _) = get_query(query, type(self))
+            (query, _) = get_query_sort(query, type(self))
             if query.match(self):
                 # The query matches the item! Use the corresponding path
                 # format.
@@ -887,7 +887,7 @@ class Album(LibModel):
 
 # Query construction helper.
 
-def get_query(val, model_cls):
+def get_query_sort(val, model_cls):
     """Take a value which may be None, a query string, a query string
     list, or a Query object, and return a suitable Query object and Sort
     object.
@@ -1016,8 +1016,8 @@ class Library(dbcore.Database):
         """Parse a query and fetch. If a order specification is present in the
         query string the sort_order argument is ignored.
           """
-        (query, sort) = get_query(query, model_cls)
-        sort = sort_order if sort is None else sort
+        query, sort = get_query_sort(query, model_cls)
+        sort = sort or sort_order
 
         return super(Library, self)._fetch(
             model_cls, query, sort
diff --git a/beetsplug/ihate.py b/beetsplug/ihate.py
index 85e3382d4..3e86759c6 100644
--- a/beetsplug/ihate.py
+++ b/beetsplug/ihate.py
@@ -17,7 +17,7 @@
 import logging
 from beets.plugins import BeetsPlugin
 from beets.importer import action
-from beets.library import get_query
+from beets.library import get_query_sort
 from beets.library import Item
 from beets.library import Album
 
@@ -57,9 +57,9 @@ class IHatePlugin(BeetsPlugin):
             for query_string in action_patterns:
                 query = None
                 if task.is_album:
-                    (query, _) = get_query(query_string, Album)
+                    (query, _) = get_query_sort(query_string, Album)
                 else:
-                    (query, _) = get_query(query_string, Item)
+                    (query, _) = get_query_sort(query_string, Item)
                 if any(query.match(item) for item in task.imported_items()):
                     return True
         return False
diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py
index 12672ce42..6beb0ad59 100644
--- a/beetsplug/smartplaylist.py
+++ b/beetsplug/smartplaylist.py
@@ -42,7 +42,7 @@ def _items_for_query(lib, playlist, album=False):
         query_strings = [query_strings]
     model = library.Album if album else library.Item
     query = dbcore.OrQuery(
-        [library.get_query(q, model)[0] for q in query_strings]
+        [library.get_query_sort(q, model)[0] for q in query_strings]
     )
 
     # Execute query, depending on type.

From 7c196799e5034ade47c829fb22a4bcc786b76241 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rui=20Gon=C3=A7alves?= <ruippeixotog@gmail.com>
Date: Thu, 21 Aug 2014 23:35:27 +0100
Subject: [PATCH 20/63] Add support for releases with multiple versions of the
 same recording

---
 beetsplug/mbsync.py | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/beetsplug/mbsync.py b/beetsplug/mbsync.py
index 76226f08c..1e5857bba 100644
--- a/beetsplug/mbsync.py
+++ b/beetsplug/mbsync.py
@@ -64,13 +64,29 @@ def mbsync_albums(lib, query, move, pretend, write):
             log.info(u'Release ID not found: {0}'.format(a.mb_albumid))
             continue
 
+        # Construct an hash mapping recording MBIDs to their information. A
+        # release can have recording MBIDs that appear multiple times in the
+        # same release.
+        track_index = {}
+        for track_info in album_info.tracks:
+            if track_info.track_id in track_index:
+                track_index[track_info.track_id].append(track_info)
+            else:
+                track_index[track_info.track_id] = [track_info]
+
         # Construct a track mapping according to MBIDs. This should work
-        # for albums that have missing or extra tracks.
+        # for albums that have missing or extra tracks. If a mapping is
+        # ambiguous, the items' disc and track number need to match in order
+        # for an item to be mapped.
         mapping = {}
         for item in items:
-            for track_info in album_info.tracks:
-                if item.mb_trackid == track_info.track_id:
-                    mapping[item] = track_info
+            candidates = track_index.get(item.mb_trackid, [])
+            if len(candidates) == 1:
+                mapping[item] = candidates[0]
+                continue
+            for c in candidates:
+                if c.medium_index == item.track and c.medium == item.disc:
+                    mapping[item] = c
                     break
 
         # Apply.

From e52ca41456bfedea6d1c09810e32b457f9c0e094 Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Thu, 21 Aug 2014 23:11:23 -0700
Subject: [PATCH 21/63] Changelog and style for #908

Use a defaultdict for more idiomatic collection.
---
 beetsplug/mbsync.py | 32 +++++++++++++++-----------------
 docs/changelog.rst  |  2 ++
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/beetsplug/mbsync.py b/beetsplug/mbsync.py
index 1e5857bba..989caeb99 100644
--- a/beetsplug/mbsync.py
+++ b/beetsplug/mbsync.py
@@ -20,6 +20,7 @@ from beets.plugins import BeetsPlugin
 from beets import autotag, library, ui, util
 from beets.autotag import hooks
 from beets import config
+from collections import defaultdict
 
 log = logging.getLogger('beets')
 
@@ -64,30 +65,27 @@ def mbsync_albums(lib, query, move, pretend, write):
             log.info(u'Release ID not found: {0}'.format(a.mb_albumid))
             continue
 
-        # Construct an hash mapping recording MBIDs to their information. A
-        # release can have recording MBIDs that appear multiple times in the
-        # same release.
-        track_index = {}
+        # Map recording MBIDs to their information. Recordings can appear
+        # multiple times on a release, so each MBID maps to a list of TrackInfo
+        # objects.
+        track_index = defaultdict(list)
         for track_info in album_info.tracks:
-            if track_info.track_id in track_index:
-                track_index[track_info.track_id].append(track_info)
-            else:
-                track_index[track_info.track_id] = [track_info]
+            track_index[track_info.track_id].append(track_info)
 
         # Construct a track mapping according to MBIDs. This should work
-        # for albums that have missing or extra tracks. If a mapping is
-        # ambiguous, the items' disc and track number need to match in order
-        # for an item to be mapped.
+        # for albums that have missing or extra tracks. If there are multiple
+        # copies of a recording, they are disambiguated using their disc and
+        # track number.
         mapping = {}
         for item in items:
-            candidates = track_index.get(item.mb_trackid, [])
+            candidates = track_index[item.mb_trackid]
             if len(candidates) == 1:
                 mapping[item] = candidates[0]
-                continue
-            for c in candidates:
-                if c.medium_index == item.track and c.medium == item.disc:
-                    mapping[item] = c
-                    break
+            else:
+                for c in candidates:
+                    if c.medium_index == item.track and c.medium == item.disc:
+                        mapping[item] = c
+                        break
 
         # Apply.
         with lib.transaction():
diff --git a/docs/changelog.rst b/docs/changelog.rst
index dbddadd6c..114fbba67 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -90,6 +90,8 @@ Little improvements and fixes:
   work in ``auto`` mode. Thanks to Harry Khanna.
 * The :ref:`write-cmd` command now has a ``--force`` flag. Thanks again to
   Harry Khanna.
+* :doc:`/plugins/mbsync`: Track alignment now works with albums that have
+  multiple copies of the same recording. Thanks to Rui Gonçalves.
 
 
 1.3.6 (May 10, 2014)

From 91295d8d7bd4eb2d14cf8dfe657e653992c60506 Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Fri, 22 Aug 2014 12:47:52 -0700
Subject: [PATCH 22/63] Refine changelog for 1.3.7 release

---
 docs/changelog.rst | 47 +++++++++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 19 deletions(-)

diff --git a/docs/changelog.rst b/docs/changelog.rst
index 114fbba67..cc8d279d7 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -1,35 +1,46 @@
 Changelog
 =========
 
-1.3.7 (in development)
-----------------------
+1.3.7 (August 22, 2014)
+-----------------------
 
-New stuff
+This release of beets fixes all the bugs, and you can be confident that you
+will never again find any bugs in beets, ever.
+It also adds support for plain old AIFF files and adds three more plugins,
+including a nifty one that lets you measure a song's tempo by tapping out the
+beat on your keyboard.
+The importer deals more elegantly with duplicates and you can broaden your
+cover art search to the entire web with Google Image Search.
 
+The big new features are:
+
+* Support for AIFF files. Tags are stored as ID3 frames in one of the file's
+  IFF chunks. Thanks to Evan Purkhiser for contributing support to `Mutagen`_.
 * The new :doc:`/plugins/importadded` reads files' modification times to set
   their "added" date. Thanks to Stig Inge Lea Bjørnsen.
-* Support for AIFF files. Tags are stored as ID3 frames in one of the file's
-  IFF chunks.
-* A new :ref:`required` configuration option for the importer skips matches
-  that are missing certain data. Thanks to oprietop.
 * The new :doc:`/plugins/bpm` lets you manually measure the tempo of a playing
   song. Thanks to aroquen.
 * The new :doc:`/plugins/spotify` generates playlists for your `Spotify`_
   account. Thanks to Olin Gay.
+* A new :ref:`required` configuration option for the importer skips matches
+  that are missing certain data. Thanks to oprietop.
 * When the importer detects duplicates, it now shows you some details about
   the potentially-replaced music so you can make an informed decision. Thanks
   to Howard Jones.
+* :doc:`/plugins/fetchart`: You can now optionally search for cover art on
+  Google Image Search. Thanks to Lemutar.
+* A new :ref:`asciify-paths` configuration option replaces all non-ASCII
+  characters in paths.
 
+.. _Mutagen: https://bitbucket.org/lazka/mutagen
 .. _Spotify: https://www.spotify.com/
 
-Little improvements and fixes:
+And the multitude of little improvements and fixes:
 
+* Compatibility with the latest version of `Mutagen`_, 1.23.
 * :doc:`/plugins/web`: Lyrics now display readably with correct line breaks.
   Also, the detail view scrolls to reveal all of the lyrics. Thanks to Meet
   Udeshi.
-* Compatibility with the latest version of Mutagen, 1.23.
-* :doc:`/plugins/fetchart`: You can now optionally search for cover art on
-  Google Image Search. Thanks to Lemutar.
 * :doc:`/plugins/play`: The ``command`` config option can now contain
   arguments (rather than just an executable). Thanks to Alessandro Ghedini.
 * Fix an error when using the :ref:`modify-cmd` command to remove a flexible
@@ -51,11 +62,11 @@ Little improvements and fixes:
 * Don't display changes for fields that are not in the restricted field set.
   This fixes :ref:`write-cmd` showing changes for fields that are not written
   to the file.
-* :ref:`write-cmd` command: Don't display the item name if there are no
-  changes for it.
-* When using both :doc:`/plugins/convert` and :doc:`/plugins/scrub`, avoid
-  scrubbing the source file of conversions. (Fix a regression introduced in
-  the previous release.)
+* The :ref:`write-cmd` command avoids displaying the item name if there are
+  no changes for it.
+* When using both the :doc:`/plugins/convert` and the :doc:`/plugins/scrub`,
+  avoid scrubbing the source file of conversions. (Fix a regression introduced
+  in the previous release.)
 * :doc:`/plugins/replaygain`: Logging is now quieter during import. Thanks to
   Yevgeny Bezman.
 * :doc:`/plugins/fetchart`: When loading art from the filesystem, we now
@@ -71,7 +82,7 @@ Little improvements and fixes:
 * :doc:`/plugins/bucket`: You can now customize the definition of alphanumeric
   "ranges" using regular expressions. And the heuristic for detecting years
   has been improved. Thanks to sotho.
-* Already imported singleton tracks are skipped when resuming an
+* Already-imported singleton tracks are skipped when resuming an
   import.
 * :doc:`/plugins/chroma`: A new ``auto`` configuration option disables
   fingerprinting on import. Thanks to ddettrittus.
@@ -79,8 +90,6 @@ Little improvements and fixes:
   transcoding preset from the command-line.
 * :doc:`/plugins/convert`: Transcoding presets can now omit their filename
   extensions (extensions default to the name of the preset).
-* A new :ref:`asciify-paths` configuration option replaces all non-ASCII
-  characters in paths.
 * :doc:`/plugins/convert`: A new ``--pretend`` option lets you preview the
   commands the plugin will execute without actually taking any action. Thanks
   to Dietrich Daroch.

From 787f0e25c529043201d8397250c542a5c0d6f479 Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Fri, 22 Aug 2014 12:52:03 -0700
Subject: [PATCH 23/63] Version bump: 1.3.8

---
 beets/__init__.py  | 2 +-
 docs/changelog.rst | 6 ++++++
 docs/conf.py       | 2 +-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/beets/__init__.py b/beets/__init__.py
index d4f4c5db8..a2a0bfbd5 100644
--- a/beets/__init__.py
+++ b/beets/__init__.py
@@ -12,7 +12,7 @@
 # The above copyright notice and this permission notice shall be
 # included in all copies or substantial portions of the Software.
 
-__version__ = '1.3.7'
+__version__ = '1.3.8'
 __author__ = 'Adrian Sampson <adrian@radbox.org>'
 
 import beets.library
diff --git a/docs/changelog.rst b/docs/changelog.rst
index cc8d279d7..072dc57a6 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -1,6 +1,12 @@
 Changelog
 =========
 
+1.3.8 (in development)
+----------------------
+
+Changelog goes here!
+
+
 1.3.7 (August 22, 2014)
 -----------------------
 
diff --git a/docs/conf.py b/docs/conf.py
index 0040583c1..291685116 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -12,7 +12,7 @@ project = u'beets'
 copyright = u'2012, Adrian Sampson'
 
 version = '1.3'
-release = '1.3.7'
+release = '1.3.8'
 
 pygments_style = 'sphinx'
 

From 132fad847b0dabcbd1de911b8d2212e404392aef Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Fri, 22 Aug 2014 15:02:50 -0700
Subject: [PATCH 24/63] Changelog for sorting (#823)

---
 docs/changelog.rst       | 2 +-
 docs/reference/query.rst | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/docs/changelog.rst b/docs/changelog.rst
index 072dc57a6..53ed48e0d 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -4,7 +4,7 @@ Changelog
 1.3.8 (in development)
 ----------------------
 
-Changelog goes here!
+This release adds **sorting** to beets queries. See :ref:`query-sort`.
 
 
 1.3.7 (August 22, 2014)
diff --git a/docs/reference/query.rst b/docs/reference/query.rst
index 7fdf14640..b85a03962 100644
--- a/docs/reference/query.rst
+++ b/docs/reference/query.rst
@@ -185,6 +185,8 @@ query won't necessarily find *all* the audio files in a directory---just the
 ones you've already added to your beets library.
 
 
+.. _query-sort:
+
 Sort Order
 ----------
 
@@ -207,6 +209,6 @@ You can also specify several sort orders, which will be used in the same order a
 which they appear in your query::
 
     $ beet list -a genre+ year+
- 
+
 This command will sort all albums by genre and, in each genre, in chronological
 order.

From a663e52bad76066ec6ee44ffd698361899b62ad0 Mon Sep 17 00:00:00 2001
From: Andrii Kohut <kogut.andriy@gmail.com>
Date: Sat, 23 Aug 2014 13:00:25 +0300
Subject: [PATCH 25/63] Fix problems with new discogs_client version

---
 beetsplug/discogs.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py
index d33991144..8d95cb47c 100644
--- a/beetsplug/discogs.py
+++ b/beetsplug/discogs.py
@@ -17,7 +17,8 @@ discogs-client library.
 """
 from beets.autotag.hooks import AlbumInfo, TrackInfo, Distance
 from beets.plugins import BeetsPlugin
-from discogs_client import DiscogsAPIError, Release, Search
+from discogs_client import Release, Client
+from discogs_client.exceptions import DiscogsAPIError
 import beets
 import discogs_client
 import logging
@@ -42,6 +43,8 @@ class DiscogsPlugin(BeetsPlugin):
         self.config.add({
             'source_weight': 0.5,
         })
+        self.discogs_client = Client('beets/%s +http://beets.radbox.org/' %
+                                     beets.__version__)
 
     def album_distance(self, items, album_info, mapping):
         """Returns the album distance.
@@ -101,7 +104,7 @@ class DiscogsPlugin(BeetsPlugin):
         # can also negate an otherwise positive result.
         query = re.sub(r'(?i)\b(CD|disc)\s*\d+', '', query)
         albums = []
-        for result in Search(query).results():
+        for result in self.discogs_client.search(query):
             if isinstance(result, Release):
                 albums.append(self.get_album_info(result))
             if len(albums) >= 5:
@@ -113,7 +116,7 @@ class DiscogsPlugin(BeetsPlugin):
         """
         album = re.sub(r' +', ' ', result.title)
         album_id = result.data['id']
-        artist, artist_id = self.get_artist(result.data['artists'])
+        artist, artist_id = self.get_artist([a.data for a in result.artists])
         # Use `.data` to access the tracklist directly instead of the
         # convenient `.tracklist` property, which will strip out useful artist
         # information and leave us with skeleton `Artist` objects that will

From d0115f1110e276479deb83cc30a48ecaf5df0d1d Mon Sep 17 00:00:00 2001
From: Andrii Kohut <kogut.andriy@gmail.com>
Date: Sat, 23 Aug 2014 13:12:28 +0300
Subject: [PATCH 26/63] set user agent as Client parameter, remove imports

---
 beetsplug/discogs.py | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py
index 8d95cb47c..a898d927d 100644
--- a/beetsplug/discogs.py
+++ b/beetsplug/discogs.py
@@ -20,7 +20,6 @@ from beets.plugins import BeetsPlugin
 from discogs_client import Release, Client
 from discogs_client.exceptions import DiscogsAPIError
 import beets
-import discogs_client
 import logging
 import re
 import time
@@ -31,10 +30,6 @@ log = logging.getLogger('beets')
 urllib3_logger = logging.getLogger('requests.packages.urllib3')
 urllib3_logger.setLevel(logging.CRITICAL)
 
-# Set user-agent for discogs client.
-discogs_client.user_agent = 'beets/%s +http://beets.radbox.org/' % \
-    beets.__version__
-
 
 class DiscogsPlugin(BeetsPlugin):
 

From dd58f1b4528438f7d7dd5e976359f92cfcc151e3 Mon Sep 17 00:00:00 2001
From: Jocelyn De La Rosa <dreewoo@gmail.com>
Date: Sun, 24 Aug 2014 00:15:13 +0200
Subject: [PATCH 27/63] Catch UnreadableFileError when reading files in a
 (try_)write context

---
 beets/library.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/beets/library.py b/beets/library.py
index 35bd4b3fc..456f51f7b 100644
--- a/beets/library.py
+++ b/beets/library.py
@@ -398,7 +398,7 @@ class Item(LibModel):
         try:
             mediafile = MediaFile(syspath(path),
                                   id3v23=beets.config['id3v23'].get(bool))
-        except (OSError, IOError) as exc:
+        except (OSError, IOError, UnreadableFileError) as exc:
             raise ReadError(self.path, exc)
 
         mediafile.update(self)

From 225ce62a3370cd8b2f6d00f5e1f6f18a3c343d74 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Sun, 24 Aug 2014 14:37:36 +0200
Subject: [PATCH 28/63] Catch all errors when loading state file.

A crash during the multi-threaded import process may leave the pickled
state invalid (see #913). We recover from all these errors.
---
 beets/importer.py     | 2 +-
 docs/changelog.rst    | 4 ++++
 test/test_importer.py | 7 +++++++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/beets/importer.py b/beets/importer.py
index 76c90dbea..ba534b0ea 100644
--- a/beets/importer.py
+++ b/beets/importer.py
@@ -65,7 +65,7 @@ def _open_state():
     try:
         with open(config['statefile'].as_filename()) as f:
             return pickle.load(f)
-    except (IOError, EOFError):
+    except:
         return {}
 
 
diff --git a/docs/changelog.rst b/docs/changelog.rst
index 53ed48e0d..0a5ba81ac 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -6,6 +6,10 @@ Changelog
 
 This release adds **sorting** to beets queries. See :ref:`query-sort`.
 
+Fixes:
+
+* Invalid state files don't crash the importer.
+
 
 1.3.7 (August 22, 2014)
 -----------------------
diff --git a/test/test_importer.py b/test/test_importer.py
index 80a94833a..d3d1e686d 100644
--- a/test/test_importer.py
+++ b/test/test_importer.py
@@ -1197,6 +1197,13 @@ class IncrementalImportTest(unittest.TestCase, TestHelper):
         importer.run()
         self.assertEqual(len(self.lib.items()), 2)
 
+    def test_invalid_state_file(self):
+        importer = self.create_importer()
+        with open(self.config['statefile'].as_filename(), 'w') as f:
+            f.write('000')
+        importer.run()
+        self.assertEqual(len(self.lib.albums()), 1)
+
 
 def suite():
     return unittest.TestLoader().loadTestsFromName(__name__)

From 32626fedc44c430fd2bd2c7d676e420d9fcdff50 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Sun, 24 Aug 2014 14:59:58 +0200
Subject: [PATCH 29/63] mediafile: Catch IndexError when file has empty list
 tag

Fixes #913
---
 beets/mediafile.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/beets/mediafile.py b/beets/mediafile.py
index 4c8b59fe8..4e83d0543 100644
--- a/beets/mediafile.py
+++ b/beets/mediafile.py
@@ -411,7 +411,7 @@ class StorageStyle(object):
         """
         try:
             return mutagen_file[self.key][0]
-        except KeyError:
+        except (KeyError, IndexError):
             return None
 
     def deserialize(self, mutagen_value):
@@ -665,7 +665,7 @@ class MP3StorageStyle(StorageStyle):
     def fetch(self, mutagen_file):
         try:
             return mutagen_file[self.key].text[0]
-        except KeyError:
+        except (KeyError, IndexError):
             return None
 
     def store(self, mutagen_file, value):

From 9a801790594551d6b3783c47ad5b6729d4b493bd Mon Sep 17 00:00:00 2001
From: Andrii Kohut <kogut.andriy@gmail.com>
Date: Sun, 24 Aug 2014 16:37:24 +0300
Subject: [PATCH 30/63] this should fix UnicodeEncodeError with non-ascii
 queries

---
 beetsplug/discogs.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py
index a898d927d..868aa076f 100644
--- a/beetsplug/discogs.py
+++ b/beetsplug/discogs.py
@@ -94,7 +94,7 @@ class DiscogsPlugin(BeetsPlugin):
         # cause a query to return no results, even if they match the artist or
         # album title. Use `re.UNICODE` flag to avoid stripping non-english
         # word characters.
-        query = re.sub(r'(?u)\W+', ' ', query)
+        query = re.sub(r'(?u)\W+', ' ', query).encode('utf8')
         # Strip medium information from query, Things like "CD1" and "disk 1"
         # can also negate an otherwise positive result.
         query = re.sub(r'(?i)\b(CD|disc)\s*\d+', '', query)

From b512a0ce37660941c97f4f8d05a5bb1837005094 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Sun, 24 Aug 2014 16:15:14 +0200
Subject: [PATCH 31/63] lyrics: Use multiple lyrics search strings.

In particular we use the original artist and title before stripping
*and* and *featuring* suffixes.

Fixes #914.
---
 beetsplug/lyrics.py | 103 +++++++++++++++++++------------------
 docs/changelog.rst  |   3 ++
 test/test_lyrics.py | 120 +++++++++++++++++++++++++++-----------------
 3 files changed, 129 insertions(+), 97 deletions(-)

diff --git a/beetsplug/lyrics.py b/beetsplug/lyrics.py
index 5d44a35f3..01a188616 100644
--- a/beetsplug/lyrics.py
+++ b/beetsplug/lyrics.py
@@ -22,6 +22,7 @@ import urllib
 import json
 import unicodedata
 import difflib
+import itertools
 
 from beets.plugins import BeetsPlugin
 from beets import ui
@@ -130,33 +131,51 @@ def strip_cruft(lyrics, wscollapse=True):
     return lyrics
 
 
-def split_multi_titles(s):
-    """Check for a dual song (e.g. Pink Floyd - Speak to Me / Breathe)
-    and returns titles as a list or None if song is not dual."""
-    if '/' not in s:
-        return None
-    return [x.strip() for x in s.split('/')]
+def search_pairs(item):
+    """Yield a pairs of artists and titles to search for.
 
+    The first item in the pair is the name of the artist, the second
+    item is a list of song names.
 
-def remove_ft_artist_suffix(s):
-    """Remove any featuring artists from an artist string.
+    In addition to the artist and title obtained from the `item` the
+    method tries to strip extra information like paranthesized suffixes
+    and featured artists from the strings and add them as caniddates.
+    The method also tries to split multiple titles separated with `/`.
     """
-    pattern = r"(.*?) (&|\b(and|feat(uring)?\b))"
-    match = re.search(pattern, s, re.IGNORECASE)
+
+    title, artist = item.title, item.artist
+    titles = [title]
+    artists = [artist]
+
+    # Remove any featuring artists from the artists name
+    pattern = r"(.*?) (&|\b(and|ft|feat(uring)?\b))"
+    match = re.search(pattern, artist, re.IGNORECASE)
     if match:
-        s = match.group(1)
-    return s
+        artists.append(match.group(1))
 
-
-def remove_parenthesized_suffix(s):
-    """Remove a parenthesized suffix from a title string. Common
-    examples include (live), (remix), and (acoustic).
-    """
+    # Remove a parenthesized suffix from a title string. Common
+    # examples include (live), (remix), and (acoustic).
     pattern = r"(.+?)\s+[(].*[)]$"
-    match = re.search(pattern, s, re.IGNORECASE)
+    match = re.search(pattern, title, re.IGNORECASE)
     if match:
-        s = match.group(1)
-    return s
+        titles.append(match.group(1))
+
+    # Remove any featuring artists from the title
+    pattern = r"(.*?) \b(ft|feat(uring)?)\b"
+    for title in titles:
+        match = re.search(pattern, title, re.IGNORECASE)
+        if match:
+            titles.append(match.group(1))
+
+    # Check for a dual song (e.g. Pink Floyd - Speak to Me / Breathe)
+    # and each of them.
+    multi_titles = []
+    for title in titles:
+        multi_titles.append([title])
+        if '/' in title:
+            multi_titles.append([x.strip() for x in title.split('/')])
+
+    return itertools.product(artists, multi_titles)
 
 
 def _encode(s):
@@ -492,45 +511,31 @@ class LyricsPlugin(BeetsPlugin):
         parameter controls the visibility of the function's status log
         messages.
         """
-        fallback = self.config['fallback'].get()
-
         # Skip if the item already has lyrics.
         if not force and item.lyrics:
             log.log(loglevel, u'lyrics already present: %s - %s' %
                               (item.artist, item.title))
             return
 
-        artist = remove_ft_artist_suffix(item.artist)
-        title = remove_parenthesized_suffix(
-            remove_ft_artist_suffix(item.title)
-        )
+        lyrics = None
+        for artist, titles in search_pairs(item):
+            lyrics = [self.get_lyrics(artist, title) for title in titles]
+            if any(lyrics):
+                break
 
-        # Fetch lyrics.
-        lyrics = self.get_lyrics(artist, title)
+        lyrics = u"\n\n---\n\n".join([l for l in lyrics if l])
 
-        if not lyrics:
-            # Check for combined title.
-            # (e.g. Pink Floyd - Speak to Me / Breathe)
-            titles = split_multi_titles(title)
-            if titles:
-                for t in titles:
-                    lyrics_title = self.get_lyrics(artist, t)
-                    if lyrics_title:
-                        if lyrics:
-                            lyrics += u"\n\n---\n\n%s" % lyrics_title
-                        else:
-                            lyrics = lyrics_title
-
-        if not lyrics:
+        if lyrics:
+            log.log(loglevel, u'fetched lyrics: %s - %s' %
+                              (item.artist, item.title))
+        else:
             log.log(loglevel, u'lyrics not found: %s - %s' %
-                              (artist, title))
+                              (item.artist, item.title))
+            fallback = self.config['fallback'].get()
             if fallback:
                 lyrics = fallback
             else:
                 return
-        else:
-            log.log(loglevel, u'fetched lyrics : %s - %s' %
-                              (artist, title))
 
         item.lyrics = lyrics
 
@@ -542,12 +547,6 @@ class LyricsPlugin(BeetsPlugin):
         """Fetch lyrics, trying each source in turn. Return a string or
         None if no lyrics were found.
         """
-        # Remove featuring artists from search.
-        pattern = u"(.*) feat(uring|\.)?\s\S+"
-        match = re.search(pattern, artist, re.IGNORECASE)
-        if match:
-            artist = match.group(0)
-
         for backend in self.backends:
             lyrics = backend(artist, title)
             if lyrics:
diff --git a/docs/changelog.rst b/docs/changelog.rst
index 0a5ba81ac..49c7ef5a3 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -9,6 +9,9 @@ This release adds **sorting** to beets queries. See :ref:`query-sort`.
 Fixes:
 
 * Invalid state files don't crash the importer.
+* The :doc:`/plugins/lyrics` only strips featured artists and
+  parenthesized title suffixes if no lyrics for the original artist and
+  title were found.
 
 
 1.3.7 (August 22, 2014)
diff --git a/test/test_lyrics.py b/test/test_lyrics.py
index 49cd79e23..4750e825f 100644
--- a/test/test_lyrics.py
+++ b/test/test_lyrics.py
@@ -16,6 +16,7 @@
 
 from _common import unittest
 from beetsplug import lyrics
+from beets.library import Item
 
 
 class LyricsPluginTest(unittest.TestCase):
@@ -23,53 +24,82 @@ class LyricsPluginTest(unittest.TestCase):
         """Set up configuration"""
         lyrics.LyricsPlugin()
 
-    def test_split_multi_titles(self):
-        self.assertEqual(lyrics.split_multi_titles('song1 / song2 / song3'),
-                         ['song1', 'song2', 'song3'])
-        self.assertEqual(lyrics.split_multi_titles('song1/song2 song3'),
-                         ['song1', 'song2 song3'])
-        self.assertEqual(lyrics.split_multi_titles('song1 song2'),
-                         None)
+    def test_search_artist(self):
+        item = Item(artist='Alice ft. Bob', title='song')
+        self.assertIn(('Alice ft. Bob', ['song']),
+                      lyrics.search_pairs(item))
+        self.assertIn(('Alice', ['song']),
+                      lyrics.search_pairs(item))
 
-    def test_remove_ft_artist_suffix(self):
-        self.assertEqual(
-            lyrics.remove_ft_artist_suffix('Bob featuring Marcia'),
-            'Bob'
-        )
-        self.assertEqual(
-            lyrics.remove_ft_artist_suffix('Bob feat Marcia'),
-            'Bob'
-        )
-        self.assertEqual(
-            lyrics.remove_ft_artist_suffix('Bob and Marcia'),
-            'Bob'
-        )
-        self.assertEqual(
-            lyrics.remove_ft_artist_suffix('Bob feat. Marcia'),
-            'Bob'
-        )
-        self.assertEqual(
-            lyrics.remove_ft_artist_suffix('Bob & Marcia'),
-            'Bob'
-        )
-        self.assertEqual(
-            lyrics.remove_ft_artist_suffix('Bob feats Marcia'),
-            'Bob feats Marcia'
-        )
+        item = Item(artist='Alice feat Bob', title='song')
+        self.assertIn(('Alice feat Bob', ['song']),
+                      lyrics.search_pairs(item))
+        self.assertIn(('Alice', ['song']),
+                      lyrics.search_pairs(item))
 
-    def test_remove_parenthesized_suffix(self):
-        self.assertEqual(
-            lyrics.remove_parenthesized_suffix('Song (live)'),
-            'Song'
-        )
-        self.assertEqual(
-            lyrics.remove_parenthesized_suffix('Song (live) (new)'),
-            'Song'
-        )
-        self.assertEqual(
-            lyrics.remove_parenthesized_suffix('Song (live (new))'),
-            'Song'
-        )
+        item = Item(artist='Alice feat. Bob', title='song')
+        self.assertIn(('Alice feat. Bob', ['song']),
+                      lyrics.search_pairs(item))
+        self.assertIn(('Alice', ['song']),
+                      lyrics.search_pairs(item))
+
+        item = Item(artist='Alice feats Bob', title='song')
+        self.assertIn(('Alice feats Bob', ['song']),
+                      lyrics.search_pairs(item))
+        self.assertNotIn(('Alice', ['song']),
+                         lyrics.search_pairs(item))
+
+        item = Item(artist='Alice featuring Bob', title='song')
+        self.assertIn(('Alice featuring Bob', ['song']),
+                      lyrics.search_pairs(item))
+        self.assertIn(('Alice', ['song']),
+                      lyrics.search_pairs(item))
+
+        item = Item(artist='Alice & Bob', title='song')
+        self.assertIn(('Alice & Bob', ['song']),
+                      lyrics.search_pairs(item))
+        self.assertIn(('Alice', ['song']),
+                      lyrics.search_pairs(item))
+
+        item = Item(artist='Alice and Bob', title='song')
+        self.assertIn(('Alice and Bob', ['song']),
+                      lyrics.search_pairs(item))
+        self.assertIn(('Alice', ['song']),
+                      lyrics.search_pairs(item))
+
+    def test_search_pairs_multi_titles(self):
+        item = Item(title='1 / 2', artist='A')
+        self.assertIn(('A', ['1 / 2']), lyrics.search_pairs(item))
+        self.assertIn(('A', ['1', '2']), lyrics.search_pairs(item))
+
+        item = Item(title='1/2', artist='A')
+        self.assertIn(('A', ['1/2']), lyrics.search_pairs(item))
+        self.assertIn(('A', ['1', '2']), lyrics.search_pairs(item))
+
+    def test_search_pairs_titles(self):
+        item = Item(title='Song (live)', artist='A')
+        self.assertIn(('A', ['Song']), lyrics.search_pairs(item))
+        self.assertIn(('A', ['Song (live)']), lyrics.search_pairs(item))
+
+        item = Item(title='Song (live) (new)', artist='A')
+        self.assertIn(('A', ['Song']), lyrics.search_pairs(item))
+        self.assertIn(('A', ['Song (live) (new)']), lyrics.search_pairs(item))
+
+        item = Item(title='Song (live (new))', artist='A')
+        self.assertIn(('A', ['Song']), lyrics.search_pairs(item))
+        self.assertIn(('A', ['Song (live (new))']), lyrics.search_pairs(item))
+
+        item = Item(title='Song ft. B', artist='A')
+        self.assertIn(('A', ['Song']), lyrics.search_pairs(item))
+        self.assertIn(('A', ['Song ft. B']), lyrics.search_pairs(item))
+
+        item = Item(title='Song featuring B', artist='A')
+        self.assertIn(('A', ['Song']), lyrics.search_pairs(item))
+        self.assertIn(('A', ['Song featuring B']), lyrics.search_pairs(item))
+
+        item = Item(title='Song and B', artist='A')
+        self.assertNotIn(('A', ['Song']), lyrics.search_pairs(item))
+        self.assertIn(('A', ['Song and B']), lyrics.search_pairs(item))
 
     def test_remove_credits(self):
         self.assertEqual(

From f8af931caa48062299370b03df2059f81ebe110a Mon Sep 17 00:00:00 2001
From: Andrii Kohut <kogut.andriy@gmail.com>
Date: Sun, 24 Aug 2014 17:34:31 +0300
Subject: [PATCH 32/63] Seems like something broken in new version of
 discogs_client

---
 beetsplug/discogs.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py
index 868aa076f..0301cb67e 100644
--- a/beetsplug/discogs.py
+++ b/beetsplug/discogs.py
@@ -109,9 +109,9 @@ class DiscogsPlugin(BeetsPlugin):
     def get_album_info(self, result):
         """Returns an AlbumInfo object for a discogs Release object.
         """
+        artist, artist_id = self.get_artist([a.data for a in result.artists])
         album = re.sub(r' +', ' ', result.title)
         album_id = result.data['id']
-        artist, artist_id = self.get_artist([a.data for a in result.artists])
         # Use `.data` to access the tracklist directly instead of the
         # convenient `.tracklist` property, which will strip out useful artist
         # information and leave us with skeleton `Artist` objects that will

From 904baa6bc1b517033ebc3979aca0d1b3424c0f63 Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Sun, 24 Aug 2014 10:07:25 -0700
Subject: [PATCH 33/63] Debug log for unreadable state files (#913)

---
 beets/importer.py  | 7 ++++++-
 docs/changelog.rst | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/beets/importer.py b/beets/importer.py
index ba534b0ea..bf0cf7f4e 100644
--- a/beets/importer.py
+++ b/beets/importer.py
@@ -65,7 +65,12 @@ def _open_state():
     try:
         with open(config['statefile'].as_filename()) as f:
             return pickle.load(f)
-    except:
+    except Exception as exc:
+        # The `pickle` module can emit all sorts of exceptions during
+        # unpickling, including ImportError. We use a catch-all
+        # exception to avoid enumerating them all (the docs don't even have a
+        # full list!).
+        log.debug(u'state file could not be read: {0}'.format(exc))
         return {}
 
 
diff --git a/docs/changelog.rst b/docs/changelog.rst
index 49c7ef5a3..8368bb681 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -12,6 +12,7 @@ Fixes:
 * The :doc:`/plugins/lyrics` only strips featured artists and
   parenthesized title suffixes if no lyrics for the original artist and
   title were found.
+* Fix a crash when reading some files with missing tags.
 
 
 1.3.7 (August 22, 2014)

From 1263fbdf7ef50f4dbc2fdb232a3aa535c5bf9dcb Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Sun, 24 Aug 2014 10:22:23 -0700
Subject: [PATCH 34/63] Changelog for #910 (fix #915)

---
 docs/changelog.rst | 9 ++++++++-
 setup.py           | 2 +-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/docs/changelog.rst b/docs/changelog.rst
index 8368bb681..bc56970b5 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -9,10 +9,17 @@ This release adds **sorting** to beets queries. See :ref:`query-sort`.
 Fixes:
 
 * Invalid state files don't crash the importer.
-* The :doc:`/plugins/lyrics` only strips featured artists and
+* :doc:`/plugins/lyrics`: Only strip featured artists and
   parenthesized title suffixes if no lyrics for the original artist and
   title were found.
 * Fix a crash when reading some files with missing tags.
+* :doc:`/plugins/discogs`: Compatibility with the new 2.0 version of the
+  `discogs_client`_ Python library. If you were using the old version, you wil
+  need to upgrade to the latest version of the library to use the
+  correspondingly new version of the plugin (e.g., with
+  ``pip install -U discogs-client``). Thanks to Andriy Kohut.
+
+.. _discogs_client: https://github.com/discogs/discogs_client
 
 
 1.3.7 (August 22, 2014)
diff --git a/setup.py b/setup.py
index f2f9fde8e..ae62046fc 100755
--- a/setup.py
+++ b/setup.py
@@ -98,7 +98,7 @@ setup(
         'beatport': ['requests'],
         'fetchart': ['requests'],
         'chroma': ['pyacoustid'],
-        'discogs': ['discogs-client'],
+        'discogs': ['discogs-client>=2.0.0'],
         'echonest': ['pyechonest'],
         'echonest_tempo': ['pyechonest'],
         'lastgenre': ['pylast'],

From b5c9271baaf2d9a9358615e25847903dcb44b51d Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Sun, 24 Aug 2014 15:44:25 -0700
Subject: [PATCH 35/63] Changelog/thanks for #912

---
 docs/changelog.rst | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/docs/changelog.rst b/docs/changelog.rst
index bc56970b5..65ea010ff 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -18,6 +18,8 @@ Fixes:
   need to upgrade to the latest version of the library to use the
   correspondingly new version of the plugin (e.g., with
   ``pip install -U discogs-client``). Thanks to Andriy Kohut.
+* Fix a crash when writing files that can't be read. Thanks to Jocelyn De La
+  Rosa.
 
 .. _discogs_client: https://github.com/discogs/discogs_client
 

From 9e2b6d02531801d12431073fff9d6f2c167a2d57 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Mon, 25 Aug 2014 11:33:10 +0200
Subject: [PATCH 36/63] Show album artist stats

See #855
---
 beets/ui/commands.py | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/beets/ui/commands.py b/beets/ui/commands.py
index 0e8329d62..473b950e3 100644
--- a/beets/ui/commands.py
+++ b/beets/ui/commands.py
@@ -1186,6 +1186,7 @@ def show_stats(lib, query, exact):
     total_items = 0
     artists = set()
     albums = set()
+    album_artists = set()
 
     for item in items:
         if exact:
@@ -1195,6 +1196,7 @@ def show_stats(lib, query, exact):
         total_time += item.length
         total_items += 1
         artists.add(item.artist)
+        album_artists.add(item.albumartist)
         albums.add(item.album)
 
     size_str = '' + ui.human_bytes(total_size)
@@ -1205,8 +1207,10 @@ def show_stats(lib, query, exact):
 Total time: {1} ({2:.2f} seconds)
 Total size: {3}
 Artists: {4}
-Albums: {5}""".format(total_items, ui.human_seconds(total_time), total_time,
-                      size_str, len(artists), len(albums)))
+Albums: {5}
+Album artists: {6}""".format(total_items, ui.human_seconds(total_time),
+                             total_time, size_str, len(artists), len(albums),
+                             len(album_artists)))
 
 
 def stats_func(lib, opts, args):

From 2d2c84b505d120939ccc3923cf481da73a0960b7 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Mon, 25 Aug 2014 11:34:40 +0200
Subject: [PATCH 37/63] Stats only count albums in the database

Items might have the `album` field set without belonging to an album in
the beets database. We only count the albums that are represented in
the database.
---
 beets/ui/commands.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/beets/ui/commands.py b/beets/ui/commands.py
index 473b950e3..e3d0e4253 100644
--- a/beets/ui/commands.py
+++ b/beets/ui/commands.py
@@ -1197,7 +1197,8 @@ def show_stats(lib, query, exact):
         total_items += 1
         artists.add(item.artist)
         album_artists.add(item.albumartist)
-        albums.add(item.album)
+        if item.album_id:
+            albums.add(item.album_id)
 
     size_str = '' + ui.human_bytes(total_size)
     if exact:

From 0798af7774472a3e9bdbcdd1a000f7db6b7afcc1 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Mon, 25 Aug 2014 14:24:39 +0200
Subject: [PATCH 38/63] Refactor model formatting

Remove all formatting related code from models. It now lives in the
`FormattedMapping` class. Only API change is from `model.formatted` to
`model.formatted()`.
---
 beets/dbcore/db.py   | 122 ++++++++++++++++++-------------------------
 beets/library.py     | 108 ++++++++++++++++++--------------------
 beets/ui/__init__.py |   6 +--
 test/test_dbcore.py  |  22 ++++----
 test/test_library.py |  32 ++++++------
 5 files changed, 131 insertions(+), 159 deletions(-)

diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py
index 5172ad523..756448720 100644
--- a/beets/dbcore/db.py
+++ b/beets/dbcore/db.py
@@ -28,6 +28,50 @@ from .query import MatchQuery, build_sql
 from .types import BASE_TYPE
 
 
+class FormattedMapping(collections.Mapping):
+    """A `dict`-like formatted view of a model.
+
+    The accessor `mapping[key]` returns the formated version of
+    `model[key]` as a unicode string.
+
+    If `for_path` is true, all path separators in the formatted values
+    are replaced.
+    """
+
+    def __init__(self, model, for_path=False):
+        self.for_path = for_path
+        self.model = model
+        self.model_keys = model.keys(True)
+
+    def __getitem__(self, key):
+        if key in self.model_keys:
+            return self._get_formatted(self.model, key)
+        else:
+            raise KeyError(key)
+
+    def __iter__(self):
+        return iter(self.model_keys)
+
+    def __len__(self):
+        return len(self.model_keys)
+
+    def get(self, key, default=u''):
+        return super(FormattedMapping, self).get(key, default)
+
+    def _get_formatted(self, model, key):
+        value = model._type(key).format(model.get(key))
+        if isinstance(value, bytes):
+            value = value.decode('utf8', 'ignore')
+
+        if self.for_path:
+            sep_repl = beets.config['path_sep_replace'].get(unicode)
+            for sep in (os.path.sep, os.path.altsep):
+                if sep:
+                    value = value.replace(sep, sep_repl)
+
+        return value
+
+
 # Abstract base for model classes.
 
 class Model(object):
@@ -380,63 +424,24 @@ class Model(object):
 
     # Formatting and templating.
 
-    @classmethod
-    def _format(cls, key, value, for_path=False):
-        """Format a value as the given field for this model.
-        """
-        # Format the value as a string according to its type.
-        value = cls._type(key).format(value)
+    _formatter = FormattedMapping
 
-        # Formatting must result in a string. To deal with
-        # Python2isms, implicitly convert ASCII strings.
-        assert isinstance(value, basestring), \
-            u'field formatter must produce strings'
-        if isinstance(value, bytes):
-            value = value.decode('utf8', 'ignore')
-
-        if for_path:
-            sep_repl = beets.config['path_sep_replace'].get(unicode)
-            for sep in (os.path.sep, os.path.altsep):
-                if sep:
-                    value = value.replace(sep, sep_repl)
-
-        return value
-
-    def _get_formatted(self, key, for_path=False):
-        """Get a field value formatted as a string (`unicode` object)
-        for display to the user. If `for_path` is true, then the value
-        will be sanitized for inclusion in a pathname (i.e., path
-        separators will be removed from the value).
-        """
-        return self._format(key, self.get(key), for_path)
-
-    def _formatted_mapping(self, for_path=False):
+    def formatted(self, for_path=False):
         """Get a mapping containing all values on this object formatted
-        as human-readable strings.
+        as human-readable unicode strings.
         """
-        return FormattedMapping(self, for_path)
-
-    @property
-    def formatted(self):
-        """A `dict`-like view containing formatted values.
-        """
-        return self._formatted_mapping(False)
+        return self._formatter(self, for_path)
 
     def evaluate_template(self, template, for_path=False):
         """Evaluate a template (a string or a `Template` object) using
         the object's fields. If `for_path` is true, then no new path
         separators will be added to the template.
         """
-        # Build value mapping.
-        mapping = self._formatted_mapping(for_path)
-
-        # Get template functions.
-        funcs = self._template_funcs()
-
         # Perform substitution.
         if isinstance(template, basestring):
             template = Template(template)
-        return template.substitute(mapping, funcs)
+        return template.substitute(self.formatted(for_path),
+                                   self._template_funcs())
 
     # Parsing.
 
@@ -450,33 +455,6 @@ class Model(object):
         return cls._type(key).parse(string)
 
 
-class FormattedMapping(collections.Mapping):
-    """A `dict`-like formatted view of a model.
-
-    The accessor ``mapping[key]`` returns the formated version of
-    ``model[key]``. The formatting is handled by `model._format()`.
-    """
-    # TODO Move all formatting logic here
-    # TODO Add caching
-
-    def __init__(self, model, for_path=False):
-        self.for_path = for_path
-        self.model = model
-        self.model_keys = model.keys(True)
-
-    def __getitem__(self, key):
-        if key in self.model_keys:
-            return self.model._get_formatted(key, self.for_path)
-        else:
-            raise KeyError(key)
-
-    def __iter__(self):
-        return iter(self.model_keys)
-
-    def __len__(self):
-        return len(self.model_keys)
-
-
 # Database controller and supporting interfaces.
 
 class Results(object):
diff --git a/beets/library.py b/beets/library.py
index 456f51f7b..ee9184bb0 100644
--- a/beets/library.py
+++ b/beets/library.py
@@ -216,6 +216,54 @@ class LibModel(dbcore.Model):
         plugins.send('database_change', lib=self._db)
 
 
+class FormattedItemMapping(dbcore.db.FormattedMapping):
+    """Add lookup for album level fields.
+    """
+
+    def __init__(self, item, for_path=False):
+        super(FormattedItemMapping, self).__init__(item, for_path)
+        self.album = item.get_album()
+        self.album_keys = []
+        if self.album:
+            for key in self.album.keys(True):
+                if key in Album.item_keys or key not in item._fields.keys():
+                    self.album_keys.append(key)
+        self.all_keys = set(self.model_keys).union(self.album_keys)
+
+    def _get(self, key):
+        """Get the value for a key, either from the album or the item.
+        Raise a KeyError for invalid keys.
+        """
+        if key in self.album_keys:
+            return self._get_formatted(self.album, key)
+        elif key in self.model_keys:
+            return self._get_formatted(self.model, key)
+        else:
+            raise KeyError(key)
+
+    def __getitem__(self, key):
+        """Get the value for a key. Certain unset values are remapped.
+        """
+        value = self._get(key)
+
+        # `artist` and `albumartist` fields fall back to one another.
+        # This is helpful in path formats when the album artist is unset
+        # on as-is imports.
+        if key == 'artist' and not value:
+            return self._get('albumartist')
+        elif key == 'albumartist' and not value:
+            return self._get('artist')
+        else:
+            return value
+
+    def __iter__(self):
+        return iter(self.all_keys)
+
+    def __len__(self):
+        return len(self.all_keys)
+
+
+
 class Item(LibModel):
     _table = 'items'
     _flex_table = 'item_attributes'
@@ -296,6 +344,8 @@ class Item(LibModel):
     `write`.
     """
 
+    _formatter = FormattedItemMapping
+
     @classmethod
     def _getters(cls):
         getters = plugins.item_field_getters()
@@ -523,12 +573,6 @@ class Item(LibModel):
 
     # Templating.
 
-    def _formatted_mapping(self, for_path=False):
-        """Get a mapping containing string-formatted values from either
-        this item or the associated album, if any.
-        """
-        return FormattedItemMapping(self, for_path)
-
     def destination(self, fragment=False, basedir=None, platform=None,
                     path_formats=None):
         """Returns the path in the library directory designated for the
@@ -604,56 +648,6 @@ class Item(LibModel):
             return normpath(os.path.join(basedir, subpath))
 
 
-class FormattedItemMapping(dbcore.db.FormattedMapping):
-    """A `dict`-like formatted view of an item that inherits album fields.
-
-    The accessor ``mapping[key]`` returns the formated version of either
-    ``item[key]`` or ``album[key]``. Here `album` is the album
-    associated to `item` if it exists.
-    """
-    def __init__(self, item, for_path=False):
-        super(FormattedItemMapping, self).__init__(item, for_path)
-        self.album = item.get_album()
-        self.album_keys = []
-        if self.album:
-            for key in self.album.keys(True):
-                if key in Album.item_keys or key not in item._fields.keys():
-                    self.album_keys.append(key)
-        self.all_keys = set(self.model_keys).union(self.album_keys)
-
-    def _get(self, key):
-        """Get the value for a key, either from the album or the item.
-        Raise a KeyError for invalid keys.
-        """
-        if key in self.album_keys:
-            return self.album._get_formatted(key, self.for_path)
-        elif key in self.model_keys:
-            return self.model._get_formatted(key, self.for_path)
-        else:
-            raise KeyError(key)
-
-    def __getitem__(self, key):
-        """Get the value for a key. Certain unset values are remapped.
-        """
-        value = self._get(key)
-
-        # `artist` and `albumartist` fields fall back to one another.
-        # This is helpful in path formats when the album artist is unset
-        # on as-is imports.
-        if key == 'artist' and not value:
-            return self._get('albumartist')
-        elif key == 'albumartist' and not value:
-            return self._get('artist')
-        else:
-            return value
-
-    def __iter__(self):
-        return iter(self.all_keys)
-
-    def __len__(self):
-        return len(self.all_keys)
-
-
 class Album(LibModel):
     """Provides access to information about albums stored in a
     library. Reflects the library's "albums" table, including album
@@ -1219,7 +1213,7 @@ class DefaultTemplateFunctions(object):
             return res
 
         # Flatten disambiguation value into a string.
-        disam_value = album._get_formatted(disambiguator, True)
+        disam_value = album.formatted(True).get(disambiguator)
         res = u' [{0}]'.format(disam_value)
         self.lib._memotable[memokey] = res
         return res
diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py
index 1e64cd89c..829089200 100644
--- a/beets/ui/__init__.py
+++ b/beets/ui/__init__.py
@@ -576,8 +576,8 @@ def _field_diff(field, old, new):
         return None
 
     # Get formatted values for output.
-    oldstr = old.formatted.get(field, u'')
-    newstr = new.formatted.get(field, u'')
+    oldstr = old.formatted().get(field, u'')
+    newstr = new.formatted().get(field, u'')
 
     # For strings, highlight changes. For others, colorize the whole
     # thing.
@@ -620,7 +620,7 @@ def show_model_changes(new, old=None, fields=None, always=False):
 
         changes.append(u'  {0}: {1}'.format(
             field,
-            colorize('red', new.formatted[field])
+            colorize('red', new.formatted()[field])
         ))
 
     # Print changes.
diff --git a/test/test_dbcore.py b/test/test_dbcore.py
index e55bd84db..d6e92a1fb 100644
--- a/test/test_dbcore.py
+++ b/test/test_dbcore.py
@@ -255,54 +255,54 @@ class FormatTest(_common.TestCase):
     def test_format_fixed_field(self):
         model = TestModel1()
         model.field_one = u'caf\xe9'
-        value = model._get_formatted('field_one')
+        value = model.formatted().get('field_one')
         self.assertEqual(value, u'caf\xe9')
 
     def test_format_flex_field(self):
         model = TestModel1()
         model.other_field = u'caf\xe9'
-        value = model._get_formatted('other_field')
+        value = model.formatted().get('other_field')
         self.assertEqual(value, u'caf\xe9')
 
     def test_format_flex_field_bytes(self):
         model = TestModel1()
         model.other_field = u'caf\xe9'.encode('utf8')
-        value = model._get_formatted('other_field')
+        value = model.formatted().get('other_field')
         self.assertTrue(isinstance(value, unicode))
         self.assertEqual(value, u'caf\xe9')
 
     def test_format_unset_field(self):
         model = TestModel1()
-        value = model._get_formatted('other_field')
+        value = model.formatted().get('other_field')
         self.assertEqual(value, u'')
 
     def test_format_typed_flex_field(self):
         model = TestModel1()
         model.some_float_field = 3.14159265358979
-        value = model._get_formatted('some_float_field')
+        value = model.formatted().get('some_float_field')
         self.assertEqual(value, u'3.1')
 
 
 class FormattedMappingTest(_common.TestCase):
     def test_keys_equal_model_keys(self):
         model = TestModel1()
-        formatted = model._formatted_mapping()
+        formatted = model.formatted()
         self.assertEqual(set(model.keys(True)), set(formatted.keys()))
 
     def test_get_unset_field(self):
         model = TestModel1()
-        formatted = model._formatted_mapping()
+        formatted = model.formatted()
         with self.assertRaises(KeyError):
             formatted['other_field']
 
-    def test_get_method_with_none_default(self):
+    def test_get_method_with_default(self):
         model = TestModel1()
-        formatted = model._formatted_mapping()
-        self.assertIsNone(formatted.get('other_field'))
+        formatted = model.formatted()
+        self.assertEqual(formatted.get('other_field'), u'')
 
     def test_get_method_with_specified_default(self):
         model = TestModel1()
-        formatted = model._formatted_mapping()
+        formatted = model.formatted()
         self.assertEqual(formatted.get('other_field', 'default'), 'default')
 
 
diff --git a/test/test_library.py b/test/test_library.py
index aac58b9e9..7a29ea5db 100644
--- a/test/test_library.py
+++ b/test/test_library.py
@@ -340,37 +340,37 @@ class DestinationTest(_common.TestCase):
         with _common.platform_posix():
             name = os.path.join('a', 'b')
             self.i.title = name
-            newname = self.i._get_formatted('title')
+            newname = self.i.formatted().get('title')
         self.assertEqual(name, newname)
 
     def test_get_formatted_pads_with_zero(self):
         with _common.platform_posix():
             self.i.track = 1
-            name = self.i._get_formatted('track')
+            name = self.i.formatted().get('track')
         self.assertTrue(name.startswith('0'))
 
     def test_get_formatted_uses_kbps_bitrate(self):
         with _common.platform_posix():
             self.i.bitrate = 12345
-            val = self.i._get_formatted('bitrate')
+            val = self.i.formatted().get('bitrate')
         self.assertEqual(val, u'12kbps')
 
     def test_get_formatted_uses_khz_samplerate(self):
         with _common.platform_posix():
             self.i.samplerate = 12345
-            val = self.i._get_formatted('samplerate')
+            val = self.i.formatted().get('samplerate')
         self.assertEqual(val, u'12kHz')
 
     def test_get_formatted_datetime(self):
         with _common.platform_posix():
             self.i.added = 1368302461.210265
-            val = self.i._get_formatted('added')
+            val = self.i.formatted().get('added')
         self.assertTrue(val.startswith('2013'))
 
     def test_get_formatted_none(self):
         with _common.platform_posix():
             self.i.some_other_field = None
-            val = self.i._get_formatted('some_other_field')
+            val = self.i.formatted().get('some_other_field')
         self.assertEqual(val, u'')
 
     def test_artist_falls_back_to_albumartist(self):
@@ -462,20 +462,20 @@ class DestinationTest(_common.TestCase):
 
 class ItemFormattedMappingTest(_common.LibTestCase):
     def test_formatted_item_value(self):
-        formatted = self.i._formatted_mapping()
+        formatted = self.i.formatted()
         self.assertEqual(formatted['artist'], 'the artist')
 
     def test_get_unset_field(self):
-        formatted = self.i._formatted_mapping()
+        formatted = self.i.formatted()
         with self.assertRaises(KeyError):
             formatted['other_field']
 
-    def test_get_method_with_none_default(self):
-        formatted = self.i._formatted_mapping()
-        self.assertIsNone(formatted.get('other_field'))
+    def test_get_method_with_default(self):
+        formatted = self.i.formatted()
+        self.assertEqual(formatted.get('other_field'), u'')
 
     def test_get_method_with_specified_default(self):
-        formatted = self.i._formatted_mapping()
+        formatted = self.i.formatted()
         self.assertEqual(formatted.get('other_field', 'default'), 'default')
 
     def test_album_field_overrides_item_field(self):
@@ -487,23 +487,23 @@ class ItemFormattedMappingTest(_common.LibTestCase):
         self.i.store()
 
         # Ensure the album takes precedence.
-        formatted = self.i._formatted_mapping()
+        formatted = self.i.formatted()
         self.assertEqual(formatted['album'], 'foo')
 
     def test_artist_falls_back_to_albumartist(self):
         self.i.artist = ''
-        formatted = self.i._formatted_mapping()
+        formatted = self.i.formatted()
         self.assertEqual(formatted['artist'], 'the album artist')
 
     def test_albumartist_falls_back_to_artist(self):
         self.i.albumartist = ''
-        formatted = self.i._formatted_mapping()
+        formatted = self.i.formatted()
         self.assertEqual(formatted['albumartist'], 'the artist')
 
     def test_both_artist_and_albumartist_empty(self):
         self.i.artist = ''
         self.i.albumartist = ''
-        formatted = self.i._formatted_mapping()
+        formatted = self.i.formatted()
         self.assertEqual(formatted['albumartist'], '')
 
 

From 91c5d0ae1227bda07b6c3075d17830b10cc2d822 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Mon, 25 Aug 2014 14:37:10 +0200
Subject: [PATCH 39/63] Item fields take precedence over Album fields in
 formatting

For consistency, this is reversed when formatting paths. Fixes #858.
---
 beets/library.py     |  9 ++++++---
 test/test_library.py | 16 ++++++++++++++--
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/beets/library.py b/beets/library.py
index ee9184bb0..3119e1bfa 100644
--- a/beets/library.py
+++ b/beets/library.py
@@ -217,7 +217,9 @@ class LibModel(dbcore.Model):
 
 
 class FormattedItemMapping(dbcore.db.FormattedMapping):
-    """Add lookup for album level fields.
+    """Add lookup for album-level fields.
+
+    Album-level fields take precedence if `for_path` is true.
     """
 
     def __init__(self, item, for_path=False):
@@ -234,10 +236,12 @@ class FormattedItemMapping(dbcore.db.FormattedMapping):
         """Get the value for a key, either from the album or the item.
         Raise a KeyError for invalid keys.
         """
-        if key in self.album_keys:
+        if self.for_path and key in self.album_keys:
             return self._get_formatted(self.album, key)
         elif key in self.model_keys:
             return self._get_formatted(self.model, key)
+        elif key in self.album_keys:
+            return self._get_formatted(self.album, key)
         else:
             raise KeyError(key)
 
@@ -263,7 +267,6 @@ class FormattedItemMapping(dbcore.db.FormattedMapping):
         return len(self.all_keys)
 
 
-
 class Item(LibModel):
     _table = 'items'
     _flex_table = 'item_attributes'
diff --git a/test/test_library.py b/test/test_library.py
index 7a29ea5db..458863190 100644
--- a/test/test_library.py
+++ b/test/test_library.py
@@ -478,7 +478,19 @@ class ItemFormattedMappingTest(_common.LibTestCase):
         formatted = self.i.formatted()
         self.assertEqual(formatted.get('other_field', 'default'), 'default')
 
-    def test_album_field_overrides_item_field(self):
+    def test_item_precedence(self):
+        album = self.lib.add_album([self.i])
+        album['artist'] = 'foo'
+        album.store()
+        self.assertNotEqual('foo', self.i.formatted().get('artist'))
+
+    def test_album_flex_field(self):
+        album = self.lib.add_album([self.i])
+        album['flex'] = 'foo'
+        album.store()
+        self.assertEqual('foo', self.i.formatted().get('flex'))
+
+    def test_album_field_overrides_item_field_for_path(self):
         # Make the album inconsistent with the item.
         album = self.lib.add_album([self.i])
         album.album = 'foo'
@@ -487,7 +499,7 @@ class ItemFormattedMappingTest(_common.LibTestCase):
         self.i.store()
 
         # Ensure the album takes precedence.
-        formatted = self.i.formatted()
+        formatted = self.i.formatted(for_path=True)
         self.assertEqual(formatted['album'], 'foo')
 
     def test_artist_falls_back_to_albumartist(self):

From c0b248c4a2c120e986b97fcaf6c63136e098256e Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Mon, 25 Aug 2014 14:52:40 +0200
Subject: [PATCH 40/63] convert: Catch errors when writing tags

Fixes #878.
---
 beetsplug/convert.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/beetsplug/convert.py b/beetsplug/convert.py
index 2b1646b2c..44c0afa99 100644
--- a/beetsplug/convert.py
+++ b/beetsplug/convert.py
@@ -198,7 +198,7 @@ def convert_item(dest_dir, keep_new, path_formats, command, ext,
             continue
 
         # Write tags from the database to the converted file.
-        item.write(path=converted)
+        item.try_write(path=converted)
 
         if keep_new:
             # If we're keeping the transcoded file, read it again (after

From 9f4b3b811dc3b999ccb4e42c8f89b86bf6a267ed Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Mon, 25 Aug 2014 15:46:57 +0200
Subject: [PATCH 41/63] Refactor modfiy command and tests

Parsing of the modify command line is handled by `modfiy_parse_args()`.
Test use the command line.
---
 beets/ui/commands.py |  21 +++----
 test/test_ui.py      | 132 ++++++++++++++++++++++---------------------
 2 files changed, 79 insertions(+), 74 deletions(-)

diff --git a/beets/ui/commands.py b/beets/ui/commands.py
index 0e8329d62..e16c03ec8 100644
--- a/beets/ui/commands.py
+++ b/beets/ui/commands.py
@@ -1247,15 +1247,16 @@ default_commands.append(version_cmd)
 
 def modify_items(lib, mods, dels, query, write, move, album, confirm):
     """Modifies matching items according to user-specified assignments and
-    deletions. `mods` is a list of "field=value" strings indicating
+    deletions.
+
+    `mods` is a dictionary of field and value pairse indicating
     assignments. `dels` is a list of fields to be deleted.
     """
     # Parse key=value specifications into a dictionary.
     model_cls = library.Album if album else library.Item
-    fsets = {}
-    for mod in mods:
-        key, value = mod.split('=', 1)
-        fsets[key] = model_cls._parse(key, value)
+
+    for key, value in mods.items():
+        mods[key] = model_cls._parse(key, value)
 
     # Get the items to modify.
     items, albums = _do_query(lib, query, album, False)
@@ -1263,11 +1264,10 @@ def modify_items(lib, mods, dels, query, write, move, album, confirm):
 
     # Apply changes *temporarily*, preview them, and collect modified
     # objects.
-    print_('Modifying %i %ss.' % (len(objs), 'album' if album else 'item'))
+    print_('Modifying {0} {1}s.'.format(len(objs), 'album' if album else 'item'))
     changed = set()
     for obj in objs:
-        for field, value in fsets.iteritems():
-            obj[field] = value
+        obj.update(mods)
         for field in dels:
             del obj[field]
         if ui.show_model_changes(obj):
@@ -1318,14 +1318,15 @@ def modify_parse_args(args):
     assignments (field=value), and deletions (field!).  Returns the result as
     a three-tuple in that order.
     """
-    mods = []
+    mods = {}
     dels = []
     query = []
     for arg in args:
         if arg.endswith('!') and '=' not in arg and ':' not in arg:
             dels.append(arg[:-1])  # Strip trailing !.
         elif '=' in arg and ':' not in arg.split('=', 1)[0]:
-            mods.append(arg)
+            key, val = arg.split('=', 1)
+            mods[key] = val
         else:
             query.append(arg)
     return query, mods, dels
diff --git a/test/test_ui.py b/test/test_ui.py
index 45b7cfece..7e718550b 100644
--- a/test/test_ui.py
+++ b/test/test_ui.py
@@ -22,7 +22,7 @@ import platform
 
 import _common
 from _common import unittest
-from helper import capture_stdout, has_program, TestHelper
+from helper import capture_stdout, has_program, TestHelper, control_stdin
 
 from beets import library
 from beets import ui
@@ -141,100 +141,104 @@ class RemoveTest(_common.TestCase):
         self.assertFalse(os.path.exists(self.i.path))
 
 
-class ModifyTest(_common.TestCase):
+class ModifyTest(unittest.TestCase, TestHelper):
+
     def setUp(self):
-        super(ModifyTest, self).setUp()
+        self.setup_beets()
+        self.add_album_fixture()
 
-        self.io.install()
+    def tearDown(self):
+        self.teardown_beets()
 
-        self.libdir = os.path.join(self.temp_dir, 'testlibdir')
+    def modify(self, *args):
+        with control_stdin('y'):
+            ui._raw_main(['modify'] + list(args), self.lib)
 
-        # Copy a file into the library.
-        self.lib = library.Library(':memory:', self.libdir)
-        self.i = library.Item.from_path(os.path.join(_common.RSRC, 'full.mp3'))
-        self.lib.add(self.i)
-        self.i.move(copy=True)
-        self.album = self.lib.add_album([self.i])
+    # Item tests
 
-    def _modify(self, mods=(), dels=(), query=(), write=False, move=False,
-                album=False):
-        self.io.addinput('y')
-        commands.modify_items(self.lib, mods, dels, query,
-                              write, move, album, True)
-
-    def test_modify_item_dbdata(self):
-        self._modify(["title=newTitle"])
+    def test_modify_item(self):
+        self.modify("title=newTitle")
         item = self.lib.items().get()
         self.assertEqual(item.title, 'newTitle')
 
-    def test_modify_album_dbdata(self):
-        self._modify(["album=newAlbum"], album=True)
-        album = self.lib.albums()[0]
+    def test_modify_write_tags(self):
+        self.modify("title=newTitle")
+        item = self.lib.items().get()
+        item.read()
+        self.assertEqual(item.title, 'newTitle')
+
+    def test_modify_dont_write_tags(self):
+        self.modify("--nowrite", "title=newTitle")
+        item = self.lib.items().get()
+        item.read()
+        self.assertNotEqual(item.title, 'newTitle')
+
+    def test_move(self):
+        self.modify("title=newTitle")
+        item = self.lib.items().get()
+        self.assertIn('newTitle', item.path)
+
+    def test_not_move(self):
+        self.modify("--nomove", "title=newTitle")
+        item = self.lib.items().get()
+        self.assertNotIn('newTitle', item.path)
+
+    # Album Tests
+
+    def test_modify_album(self):
+        self.modify("--album", "album=newAlbum")
+        album = self.lib.albums().get()
         self.assertEqual(album.album, 'newAlbum')
 
-    def test_modify_item_tag_unmodified(self):
-        self._modify(["title=newTitle"], write=False)
-        item = self.lib.items().get()
-        item.read()
-        self.assertEqual(item.title, 'full')
-
-    def test_modify_album_tag_unmodified(self):
-        self._modify(["album=newAlbum"], write=False, album=True)
-        item = self.lib.items().get()
-        item.read()
-        self.assertEqual(item.album, 'the album')
-
-    def test_modify_item_tag(self):
-        self._modify(["title=newTitle"], write=True)
-        item = self.lib.items().get()
-        item.read()
-        self.assertEqual(item.title, 'newTitle')
-
-    def test_modify_album_tag(self):
-        self._modify(["album=newAlbum"], write=True, album=True)
+    def test_modify_album_write_tags(self):
+        self.modify("--album", "album=newAlbum")
         item = self.lib.items().get()
         item.read()
         self.assertEqual(item.album, 'newAlbum')
 
-    def test_item_move(self):
-        self._modify(["title=newTitle"], move=True)
+    def test_modify_album_dont_write_tags(self):
+        self.modify("--album", "--nowrite", "album=newAlbum")
         item = self.lib.items().get()
-        self.assertTrue('newTitle' in item.path)
+        item.read()
+        self.assertEqual(item.album, 'the album')
 
     def test_album_move(self):
-        self._modify(["album=newAlbum"], move=True, album=True)
+        self.modify("--album", "album=newAlbum")
         item = self.lib.items().get()
         item.read()
-        self.assertTrue('newAlbum' in item.path)
-
-    def test_item_not_move(self):
-        self._modify(["title=newTitle"], move=False)
-        item = self.lib.items().get()
-        self.assertFalse('newTitle' in item.path)
+        self.assertIn('newAlbum', item.path)
 
     def test_album_not_move(self):
-        self._modify(["album=newAlbum"], move=False, album=True)
+        self.modify("--nomove", "--album", "album=newAlbum")
         item = self.lib.items().get()
         item.read()
-        self.assertFalse('newAlbum' in item.path)
+        self.assertNotIn('newAlbum', item.path)
+
+    # Misc
 
     def test_write_initial_key_tag(self):
-        self._modify(["initial_key=C#m"], write=True)
+        self.modify("initial_key=C#m")
         item = self.lib.items().get()
         mediafile = MediaFile(item.path)
         self.assertEqual(mediafile.initial_key, 'C#m')
 
-    def test_remove_flexattr(self):
-        self._modify(["flexattr=testAttr"], write=True)
+    def test_set_flexattr(self):
+        self.modify("flexattr=testAttr")
         item = self.lib.items().get()
         self.assertEqual(item.flexattr, 'testAttr')
-        self._modify(dels=["flexattr"], write=True)
+
+    def test_remove_flexattr(self):
         item = self.lib.items().get()
-        self.assertTrue("flexattr" not in item)
+        item.flexattr = 'testAttr'
+        item.store()
+
+        self.modify("flexattr!")
+        item = self.lib.items().get()
+        self.assertNotIn("flexattr", item)
 
     @unittest.skip('not yet implemented')
     def test_delete_initial_key_tag(self):
-        item = self.i
+        item = self.lib.items().get()
         item.initial_key = 'C#m'
         item.write()
         item.store()
@@ -242,7 +246,7 @@ class ModifyTest(_common.TestCase):
         mediafile = MediaFile(item.path)
         self.assertEqual(mediafile.initial_key, 'C#m')
 
-        self._modify(dels=["initial_key!"], write=True)
+        self.modify("initial_key!")
         mediafile = MediaFile(item.path)
         self.assertIsNone(mediafile.initial_key)
 
@@ -250,7 +254,7 @@ class ModifyTest(_common.TestCase):
         (query, mods, dels) = commands.modify_parse_args(["title:oldTitle",
                                                           "title=newTitle"])
         self.assertEqual(query, ["title:oldTitle"])
-        self.assertEqual(mods, ["title=newTitle"])
+        self.assertEqual(mods, {"title": "newTitle"})
 
     def test_arg_parsing_delete(self):
         (query, mods, dels) = commands.modify_parse_args(["title:oldTitle",
@@ -262,13 +266,13 @@ class ModifyTest(_common.TestCase):
         (query, mods, dels) = commands.modify_parse_args(["title:oldTitle!",
                                                           "title=newTitle!"])
         self.assertEqual(query, ["title:oldTitle!"])
-        self.assertEqual(mods, ["title=newTitle!"])
+        self.assertEqual(mods, {"title": "newTitle!"})
 
     def test_arg_parsing_equals_in_value(self):
         (query, mods, dels) = commands.modify_parse_args(["title:foo=bar",
                                                           "title=newTitle"])
         self.assertEqual(query, ["title:foo=bar"])
-        self.assertEqual(mods, ["title=newTitle"])
+        self.assertEqual(mods, {"title": "newTitle"})
 
 
 class MoveTest(_common.TestCase):

From 6d67365df6110e23a84faf19765c760ab3be82e9 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Mon, 25 Aug 2014 17:17:02 +0200
Subject: [PATCH 42/63] PEP8 fix

---
 beets/ui/commands.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/beets/ui/commands.py b/beets/ui/commands.py
index e16c03ec8..24ca05ed3 100644
--- a/beets/ui/commands.py
+++ b/beets/ui/commands.py
@@ -1264,7 +1264,8 @@ def modify_items(lib, mods, dels, query, write, move, album, confirm):
 
     # Apply changes *temporarily*, preview them, and collect modified
     # objects.
-    print_('Modifying {0} {1}s.'.format(len(objs), 'album' if album else 'item'))
+    print_('Modifying {0} {1}s.'
+           .format(len(objs), 'album' if album else 'item'))
     changed = set()
     for obj in objs:
         obj.update(mods)

From 8880750b4f965e31bca3a7065da3d9b4a6ac63d6 Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Mon, 25 Aug 2014 08:49:21 -0700
Subject: [PATCH 43/63] Changelog for #917/#855

---
 docs/changelog.rst | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/docs/changelog.rst b/docs/changelog.rst
index 65ea010ff..025bcaa4d 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -20,6 +20,8 @@ Fixes:
   ``pip install -U discogs-client``). Thanks to Andriy Kohut.
 * Fix a crash when writing files that can't be read. Thanks to Jocelyn De La
   Rosa.
+* The :ref:`stats-cmd` command now counts album artists. The album count also
+  more accurately reflects the number of albums in the database.
 
 .. _discogs_client: https://github.com/discogs/discogs_client
 

From 22dc67f38269422c1bddf4279a7251823a5b579c Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Mon, 25 Aug 2014 08:51:18 -0700
Subject: [PATCH 44/63] Changelog for #878

---
 docs/changelog.rst | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/docs/changelog.rst b/docs/changelog.rst
index 025bcaa4d..5b11b85cc 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -22,6 +22,8 @@ Fixes:
   Rosa.
 * The :ref:`stats-cmd` command now counts album artists. The album count also
   more accurately reflects the number of albums in the database.
+* :doc:`/plugins/convert`: Avoid crashes when tags cannot be written to newly
+  converted files.
 
 .. _discogs_client: https://github.com/discogs/discogs_client
 

From 1a60e7d129350873d44711a654452d2a306989d1 Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Mon, 25 Aug 2014 09:03:37 -0700
Subject: [PATCH 45/63] Changelog for #858/#918

---
 docs/changelog.rst | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/docs/changelog.rst b/docs/changelog.rst
index 5b11b85cc..8fbb5e46d 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -24,6 +24,8 @@ Fixes:
   more accurately reflects the number of albums in the database.
 * :doc:`/plugins/convert`: Avoid crashes when tags cannot be written to newly
   converted files.
+* Formatting templates with item data no longer confusingly shows album-level
+  data when the two are inconsistent.
 
 .. _discogs_client: https://github.com/discogs/discogs_client
 

From 34584eadd1e962576aeff33b23b2ba43b528b7a1 Mon Sep 17 00:00:00 2001
From: Andrii Kohut <kogut.andriy@gmail.com>
Date: Mon, 25 Aug 2014 19:14:41 +0300
Subject: [PATCH 46/63] Works faster then checking each result item with
 isinstance

---
 beetsplug/discogs.py | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py
index 0301cb67e..75fe536c4 100644
--- a/beetsplug/discogs.py
+++ b/beetsplug/discogs.py
@@ -98,13 +98,8 @@ class DiscogsPlugin(BeetsPlugin):
         # Strip medium information from query, Things like "CD1" and "disk 1"
         # can also negate an otherwise positive result.
         query = re.sub(r'(?i)\b(CD|disc)\s*\d+', '', query)
-        albums = []
-        for result in self.discogs_client.search(query):
-            if isinstance(result, Release):
-                albums.append(self.get_album_info(result))
-            if len(albums) >= 5:
-                break
-        return albums
+        releases = self.discogs_client.search(query, type='release').page(1)
+        return [self.get_album_info(release) for release in releases[:5]]
 
     def get_album_info(self, result):
         """Returns an AlbumInfo object for a discogs Release object.

From b1f670ada375279ef77a9bf9946013798269845a Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Mon, 25 Aug 2014 18:00:05 +0200
Subject: [PATCH 47/63] Move albums_in_dir() from autotag to import module

Has nothing to do with the former
---
 beets/autotag/__init__.py | 123 +----------------------------------
 beets/importer.py         | 133 ++++++++++++++++++++++++++++++++++++--
 test/test_autotag.py      | 124 -----------------------------------
 test/test_importer.py     | 125 +++++++++++++++++++++++++++++++++++
 4 files changed, 254 insertions(+), 251 deletions(-)

diff --git a/beets/autotag/__init__.py b/beets/autotag/__init__.py
index 86c104b84..a32a8f6f9 100644
--- a/beets/autotag/__init__.py
+++ b/beets/autotag/__init__.py
@@ -19,7 +19,7 @@ import logging
 import re
 
 from beets import library, mediafile, config
-from beets.util import sorted_walk, ancestry, displayable_path
+from beets.util import ancestry, displayable_path
 
 # Parts of external interface.
 from .hooks import AlbumInfo, TrackInfo, AlbumMatch, TrackMatch  # noqa
@@ -29,130 +29,9 @@ from .match import Recommendation  # noqa
 # Global logger.
 log = logging.getLogger('beets')
 
-# Constants for directory walker.
-MULTIDISC_MARKERS = (r'dis[ck]', r'cd')
-MULTIDISC_PAT_FMT = r'^(.*%s[\W_]*)\d'
-
 
 # Additional utilities for the main interface.
 
-def albums_in_dir(path):
-    """Recursively searches the given directory and returns an iterable
-    of (paths, items) where paths is a list of directories and items is
-    a list of Items that is probably an album. Specifically, any folder
-    containing any media files is an album.
-    """
-    collapse_pat = collapse_paths = collapse_items = None
-
-    for root, dirs, files in sorted_walk(path,
-                                         ignore=config['ignore'].as_str_seq(),
-                                         logger=log):
-        # Get a list of items in the directory.
-        items = []
-        for filename in files:
-            try:
-                i = library.Item.from_path(os.path.join(root, filename))
-            except library.ReadError as exc:
-                if isinstance(exc.reason, mediafile.FileTypeError):
-                    # Silently ignore non-music files.
-                    pass
-                elif isinstance(exc.reason, mediafile.UnreadableFileError):
-                    log.warn(u'unreadable file: {0}'.format(
-                        displayable_path(filename))
-                    )
-                else:
-                    log.error(u'error reading {0}: {1}'.format(
-                        displayable_path(filename),
-                        exc,
-                    ))
-            else:
-                items.append(i)
-
-        # If we're currently collapsing the constituent directories in a
-        # multi-disc album, check whether we should continue collapsing
-        # and add the current directory. If so, just add the directory
-        # and move on to the next directory. If not, stop collapsing.
-        if collapse_paths:
-            if (not collapse_pat and collapse_paths[0] in ancestry(root)) or \
-                    (collapse_pat and
-                     collapse_pat.match(os.path.basename(root))):
-                # Still collapsing.
-                collapse_paths.append(root)
-                collapse_items += items
-                continue
-            else:
-                # Collapse finished. Yield the collapsed directory and
-                # proceed to process the current one.
-                if collapse_items:
-                    yield collapse_paths, collapse_items
-                collapse_pat = collapse_paths = collapse_items = None
-
-        # Check whether this directory looks like the *first* directory
-        # in a multi-disc sequence. There are two indicators: the file
-        # is named like part of a multi-disc sequence (e.g., "Title Disc
-        # 1") or it contains no items but only directories that are
-        # named in this way.
-        start_collapsing = False
-        for marker in MULTIDISC_MARKERS:
-            marker_pat = re.compile(MULTIDISC_PAT_FMT % marker, re.I)
-            match = marker_pat.match(os.path.basename(root))
-
-            # Is this directory the root of a nested multi-disc album?
-            if dirs and not items:
-                # Check whether all subdirectories have the same prefix.
-                start_collapsing = True
-                subdir_pat = None
-                for subdir in dirs:
-                    # The first directory dictates the pattern for
-                    # the remaining directories.
-                    if not subdir_pat:
-                        match = marker_pat.match(subdir)
-                        if match:
-                            subdir_pat = re.compile(
-                                r'^%s\d' % re.escape(match.group(1)), re.I
-                            )
-                        else:
-                            start_collapsing = False
-                            break
-
-                    # Subsequent directories must match the pattern.
-                    elif not subdir_pat.match(subdir):
-                        start_collapsing = False
-                        break
-
-                # If all subdirectories match, don't check other
-                # markers.
-                if start_collapsing:
-                    break
-
-            # Is this directory the first in a flattened multi-disc album?
-            elif match:
-                start_collapsing = True
-                # Set the current pattern to match directories with the same
-                # prefix as this one, followed by a digit.
-                collapse_pat = re.compile(
-                    r'^%s\d' % re.escape(match.group(1)), re.I
-                )
-                break
-
-        # If either of the above heuristics indicated that this is the
-        # beginning of a multi-disc album, initialize the collapsed
-        # directory and item lists and check the next directory.
-        if start_collapsing:
-            # Start collapsing; continue to the next iteration.
-            collapse_paths = [root]
-            collapse_items = items
-            continue
-
-        # If it's nonempty, yield it.
-        if items:
-            yield [root], items
-
-    # Clear out any unfinished collapse.
-    if collapse_paths and collapse_items:
-        yield collapse_paths, collapse_items
-
-
 def apply_item_metadata(item, track_info):
     """Set an item's metadata from its matched TrackInfo object.
     """
diff --git a/beets/importer.py b/beets/importer.py
index bf0cf7f4e..5ace5cfe0 100644
--- a/beets/importer.py
+++ b/beets/importer.py
@@ -18,6 +18,7 @@ autotagging music files.
 from __future__ import print_function
 
 import os
+import re
 import logging
 import pickle
 import itertools
@@ -33,7 +34,7 @@ from beets import dbcore
 from beets import plugins
 from beets import util
 from beets import config
-from beets.util import pipeline
+from beets.util import pipeline, sorted_walk, ancestry
 from beets.util import syspath, normpath, displayable_path
 from enum import Enum
 from beets import mediafile
@@ -108,8 +109,8 @@ def progress_add(toppath, *paths):
         for path in paths:
             # Normally `progress_add` will be called with the path
             # argument increasing. This is because of the ordering in
-            # `autotag.albums_in_dir`. We take advantage of that to make
-            # the code faster
+            # `albums_in_dir`. We take advantage of that to make the
+            # code faster
             if imported and imported[len(imported) - 1] <= path:
                 imported.append(path)
             else:
@@ -918,7 +919,7 @@ def read_tasks(session):
         # A flat album import merges all items into one album.
         if session.config['flat'] and not session.config['singletons']:
             all_items = []
-            for _, items in autotag.albums_in_dir(toppath):
+            for _, items in albums_in_dir(toppath):
                 all_items += items
             if all_items:
                 if session.already_imported(toppath, [toppath]):
@@ -931,7 +932,7 @@ def read_tasks(session):
             continue
 
         # Produce paths under this directory.
-        for paths, items in autotag.albums_in_dir(toppath):
+        for paths, items in albums_in_dir(toppath):
             if session.config['singletons']:
                 for item in items:
                     if session.already_imported(toppath, [item.path]):
@@ -1161,3 +1162,125 @@ def group_albums(session):
         tasks.append(SentinelImportTask(task.toppath, task.paths))
 
         task = pipeline.multiple(tasks)
+
+
+MULTIDISC_MARKERS = (r'dis[ck]', r'cd')
+MULTIDISC_PAT_FMT = r'^(.*%s[\W_]*)\d'
+
+
+def albums_in_dir(path):
+    """Recursively searches the given directory and returns an iterable
+    of (paths, items) where paths is a list of directories and items is
+    a list of Items that is probably an album. Specifically, any folder
+    containing any media files is an album.
+    """
+    collapse_pat = collapse_paths = collapse_items = None
+    ignore = config['ignore'].as_str_seq()
+
+    for root, dirs, files in sorted_walk(path, ignore=ignore, logger=log):
+        # Get a list of items in the directory.
+        items = []
+        for filename in files:
+            try:
+                i = library.Item.from_path(os.path.join(root, filename))
+            except library.ReadError as exc:
+                if isinstance(exc.reason, mediafile.FileTypeError):
+                    # Silently ignore non-music files.
+                    pass
+                elif isinstance(exc.reason, mediafile.UnreadableFileError):
+                    log.warn(u'unreadable file: {0}'.format(
+                        displayable_path(filename))
+                    )
+                else:
+                    log.error(u'error reading {0}: {1}'.format(
+                        displayable_path(filename),
+                        exc,
+                    ))
+            else:
+                items.append(i)
+
+        # If we're currently collapsing the constituent directories in a
+        # multi-disc album, check whether we should continue collapsing
+        # and add the current directory. If so, just add the directory
+        # and move on to the next directory. If not, stop collapsing.
+        if collapse_paths:
+            if (not collapse_pat and collapse_paths[0] in ancestry(root)) or \
+                    (collapse_pat and
+                     collapse_pat.match(os.path.basename(root))):
+                # Still collapsing.
+                collapse_paths.append(root)
+                collapse_items += items
+                continue
+            else:
+                # Collapse finished. Yield the collapsed directory and
+                # proceed to process the current one.
+                if collapse_items:
+                    yield collapse_paths, collapse_items
+                collapse_pat = collapse_paths = collapse_items = None
+
+        # Check whether this directory looks like the *first* directory
+        # in a multi-disc sequence. There are two indicators: the file
+        # is named like part of a multi-disc sequence (e.g., "Title Disc
+        # 1") or it contains no items but only directories that are
+        # named in this way.
+        start_collapsing = False
+        for marker in MULTIDISC_MARKERS:
+            marker_pat = re.compile(MULTIDISC_PAT_FMT % marker, re.I)
+            match = marker_pat.match(os.path.basename(root))
+
+            # Is this directory the root of a nested multi-disc album?
+            if dirs and not items:
+                # Check whether all subdirectories have the same prefix.
+                start_collapsing = True
+                subdir_pat = None
+                for subdir in dirs:
+                    # The first directory dictates the pattern for
+                    # the remaining directories.
+                    if not subdir_pat:
+                        match = marker_pat.match(subdir)
+                        if match:
+                            subdir_pat = re.compile(
+                                r'^%s\d' % re.escape(match.group(1)), re.I
+                            )
+                        else:
+                            start_collapsing = False
+                            break
+
+                    # Subsequent directories must match the pattern.
+                    elif not subdir_pat.match(subdir):
+                        start_collapsing = False
+                        break
+
+                # If all subdirectories match, don't check other
+                # markers.
+                if start_collapsing:
+                    break
+
+            # Is this directory the first in a flattened multi-disc album?
+            elif match:
+                start_collapsing = True
+                # Set the current pattern to match directories with the same
+                # prefix as this one, followed by a digit.
+                collapse_pat = re.compile(
+                    r'^%s\d' % re.escape(match.group(1)), re.I
+                )
+                break
+
+        # If either of the above heuristics indicated that this is the
+        # beginning of a multi-disc album, initialize the collapsed
+        # directory and item lists and check the next directory.
+        if start_collapsing:
+            # Start collapsing; continue to the next iteration.
+            collapse_paths = [root]
+            collapse_items = items
+            continue
+
+        # If it's nonempty, yield it.
+        if items:
+            yield [root], items
+
+    # Clear out any unfinished collapse.
+    if collapse_paths and collapse_items:
+        yield collapse_paths, collapse_items
+
+
diff --git a/test/test_autotag.py b/test/test_autotag.py
index 10a03bd2f..4599d6df2 100644
--- a/test/test_autotag.py
+++ b/test/test_autotag.py
@@ -14,8 +14,6 @@
 
 """Tests for autotagging functionality.
 """
-import os
-import shutil
 import re
 import copy
 
@@ -486,128 +484,6 @@ class AlbumDistanceTest(_common.TestCase):
         self.assertEqual(dist, 0)
 
 
-def _mkmp3(path):
-    shutil.copyfile(os.path.join(_common.RSRC, 'min.mp3'), path)
-
-
-class AlbumsInDirTest(_common.TestCase):
-    def setUp(self):
-        super(AlbumsInDirTest, self).setUp()
-
-        # create a directory structure for testing
-        self.base = os.path.abspath(os.path.join(self.temp_dir, 'tempdir'))
-        os.mkdir(self.base)
-
-        os.mkdir(os.path.join(self.base, 'album1'))
-        os.mkdir(os.path.join(self.base, 'album2'))
-        os.mkdir(os.path.join(self.base, 'more'))
-        os.mkdir(os.path.join(self.base, 'more', 'album3'))
-        os.mkdir(os.path.join(self.base, 'more', 'album4'))
-
-        _mkmp3(os.path.join(self.base, 'album1', 'album1song1.mp3'))
-        _mkmp3(os.path.join(self.base, 'album1', 'album1song2.mp3'))
-        _mkmp3(os.path.join(self.base, 'album2', 'album2song.mp3'))
-        _mkmp3(os.path.join(self.base, 'more', 'album3', 'album3song.mp3'))
-        _mkmp3(os.path.join(self.base, 'more', 'album4', 'album4song.mp3'))
-
-    def test_finds_all_albums(self):
-        albums = list(autotag.albums_in_dir(self.base))
-        self.assertEqual(len(albums), 4)
-
-    def test_separates_contents(self):
-        found = []
-        for _, album in autotag.albums_in_dir(self.base):
-            found.append(re.search(r'album(.)song', album[0].path).group(1))
-        self.assertTrue('1' in found)
-        self.assertTrue('2' in found)
-        self.assertTrue('3' in found)
-        self.assertTrue('4' in found)
-
-    def test_finds_multiple_songs(self):
-        for _, album in autotag.albums_in_dir(self.base):
-            n = re.search(r'album(.)song', album[0].path).group(1)
-            if n == '1':
-                self.assertEqual(len(album), 2)
-            else:
-                self.assertEqual(len(album), 1)
-
-
-class MultiDiscAlbumsInDirTest(_common.TestCase):
-    def setUp(self):
-        super(MultiDiscAlbumsInDirTest, self).setUp()
-
-        self.base = os.path.abspath(os.path.join(self.temp_dir, 'tempdir'))
-        os.mkdir(self.base)
-
-        self.dirs = [
-            # Nested album, multiple subdirs.
-            # Also, false positive marker in root dir, and subtitle for disc 3.
-            os.path.join(self.base, 'ABCD1234'),
-            os.path.join(self.base, 'ABCD1234', 'cd 1'),
-            os.path.join(self.base, 'ABCD1234', 'cd 3 - bonus'),
-
-            # Nested album, single subdir.
-            # Also, punctuation between marker and disc number.
-            os.path.join(self.base, 'album'),
-            os.path.join(self.base, 'album', 'cd _ 1'),
-
-            # Flattened album, case typo.
-            # Also, false positive marker in parent dir.
-            os.path.join(self.base, 'artist [CD5]'),
-            os.path.join(self.base, 'artist [CD5]', 'CAT disc 1'),
-            os.path.join(self.base, 'artist [CD5]', 'CAt disc 2'),
-
-            # Single disc album, sorted between CAT discs.
-            os.path.join(self.base, 'artist [CD5]', 'CATS'),
-        ]
-        self.files = [
-            os.path.join(self.base, 'ABCD1234', 'cd 1', 'song1.mp3'),
-            os.path.join(self.base, 'ABCD1234', 'cd 3 - bonus', 'song2.mp3'),
-            os.path.join(self.base, 'ABCD1234', 'cd 3 - bonus', 'song3.mp3'),
-            os.path.join(self.base, 'album', 'cd _ 1', 'song4.mp3'),
-            os.path.join(self.base, 'artist [CD5]', 'CAT disc 1', 'song5.mp3'),
-            os.path.join(self.base, 'artist [CD5]', 'CAt disc 2', 'song6.mp3'),
-            os.path.join(self.base, 'artist [CD5]', 'CATS', 'song7.mp3'),
-        ]
-
-        for path in self.dirs:
-            os.mkdir(path)
-        for path in self.files:
-            _mkmp3(path)
-
-    def test_coalesce_nested_album_multiple_subdirs(self):
-        albums = list(autotag.albums_in_dir(self.base))
-        self.assertEquals(len(albums), 4)
-        root, items = albums[0]
-        self.assertEquals(root, self.dirs[0:3])
-        self.assertEquals(len(items), 3)
-
-    def test_coalesce_nested_album_single_subdir(self):
-        albums = list(autotag.albums_in_dir(self.base))
-        root, items = albums[1]
-        self.assertEquals(root, self.dirs[3:5])
-        self.assertEquals(len(items), 1)
-
-    def test_coalesce_flattened_album_case_typo(self):
-        albums = list(autotag.albums_in_dir(self.base))
-        root, items = albums[2]
-        self.assertEquals(root, self.dirs[6:8])
-        self.assertEquals(len(items), 2)
-
-    def test_single_disc_album(self):
-        albums = list(autotag.albums_in_dir(self.base))
-        root, items = albums[3]
-        self.assertEquals(root, self.dirs[8:])
-        self.assertEquals(len(items), 1)
-
-    def test_do_not_yield_empty_album(self):
-        # Remove all the MP3s.
-        for path in self.files:
-            os.remove(path)
-        albums = list(autotag.albums_in_dir(self.base))
-        self.assertEquals(len(albums), 0)
-
-
 class AssignmentTest(unittest.TestCase):
     def item(self, title, track):
         return Item(
diff --git a/test/test_importer.py b/test/test_importer.py
index d3d1e686d..5920ae0a2 100644
--- a/test/test_importer.py
+++ b/test/test_importer.py
@@ -15,6 +15,7 @@
 """Tests for the general importer functionality.
 """
 import os
+import re
 import shutil
 import StringIO
 from tempfile import mkstemp
@@ -26,6 +27,7 @@ import _common
 from _common import unittest
 from helper import TestImportSession, TestHelper, has_program
 from beets import importer
+from beets.importer import albums_in_dir
 from beets.mediafile import MediaFile
 from beets import autotag
 from beets.autotag import AlbumInfo, TrackInfo, AlbumMatch
@@ -1205,6 +1207,129 @@ class IncrementalImportTest(unittest.TestCase, TestHelper):
         self.assertEqual(len(self.lib.albums()), 1)
 
 
+def _mkmp3(path):
+    shutil.copyfile(os.path.join(_common.RSRC, 'min.mp3'), path)
+
+
+class AlbumsInDirTest(_common.TestCase):
+    def setUp(self):
+        super(AlbumsInDirTest, self).setUp()
+
+        # create a directory structure for testing
+        self.base = os.path.abspath(os.path.join(self.temp_dir, 'tempdir'))
+        os.mkdir(self.base)
+
+        os.mkdir(os.path.join(self.base, 'album1'))
+        os.mkdir(os.path.join(self.base, 'album2'))
+        os.mkdir(os.path.join(self.base, 'more'))
+        os.mkdir(os.path.join(self.base, 'more', 'album3'))
+        os.mkdir(os.path.join(self.base, 'more', 'album4'))
+
+        _mkmp3(os.path.join(self.base, 'album1', 'album1song1.mp3'))
+        _mkmp3(os.path.join(self.base, 'album1', 'album1song2.mp3'))
+        _mkmp3(os.path.join(self.base, 'album2', 'album2song.mp3'))
+        _mkmp3(os.path.join(self.base, 'more', 'album3', 'album3song.mp3'))
+        _mkmp3(os.path.join(self.base, 'more', 'album4', 'album4song.mp3'))
+
+    def test_finds_all_albums(self):
+        albums = list(albums_in_dir(self.base))
+        self.assertEqual(len(albums), 4)
+
+    def test_separates_contents(self):
+        found = []
+        for _, album in albums_in_dir(self.base):
+            found.append(re.search(r'album(.)song', album[0].path).group(1))
+        self.assertTrue('1' in found)
+        self.assertTrue('2' in found)
+        self.assertTrue('3' in found)
+        self.assertTrue('4' in found)
+
+    def test_finds_multiple_songs(self):
+        for _, album in albums_in_dir(self.base):
+            n = re.search(r'album(.)song', album[0].path).group(1)
+            if n == '1':
+                self.assertEqual(len(album), 2)
+            else:
+                self.assertEqual(len(album), 1)
+
+
+class MultiDiscAlbumsInDirTest(_common.TestCase):
+    def setUp(self):
+        super(MultiDiscAlbumsInDirTest, self).setUp()
+
+        self.base = os.path.abspath(os.path.join(self.temp_dir, 'tempdir'))
+        os.mkdir(self.base)
+
+        self.dirs = [
+            # Nested album, multiple subdirs.
+            # Also, false positive marker in root dir, and subtitle for disc 3.
+            os.path.join(self.base, 'ABCD1234'),
+            os.path.join(self.base, 'ABCD1234', 'cd 1'),
+            os.path.join(self.base, 'ABCD1234', 'cd 3 - bonus'),
+
+            # Nested album, single subdir.
+            # Also, punctuation between marker and disc number.
+            os.path.join(self.base, 'album'),
+            os.path.join(self.base, 'album', 'cd _ 1'),
+
+            # Flattened album, case typo.
+            # Also, false positive marker in parent dir.
+            os.path.join(self.base, 'artist [CD5]'),
+            os.path.join(self.base, 'artist [CD5]', 'CAT disc 1'),
+            os.path.join(self.base, 'artist [CD5]', 'CAt disc 2'),
+
+            # Single disc album, sorted between CAT discs.
+            os.path.join(self.base, 'artist [CD5]', 'CATS'),
+        ]
+        self.files = [
+            os.path.join(self.base, 'ABCD1234', 'cd 1', 'song1.mp3'),
+            os.path.join(self.base, 'ABCD1234', 'cd 3 - bonus', 'song2.mp3'),
+            os.path.join(self.base, 'ABCD1234', 'cd 3 - bonus', 'song3.mp3'),
+            os.path.join(self.base, 'album', 'cd _ 1', 'song4.mp3'),
+            os.path.join(self.base, 'artist [CD5]', 'CAT disc 1', 'song5.mp3'),
+            os.path.join(self.base, 'artist [CD5]', 'CAt disc 2', 'song6.mp3'),
+            os.path.join(self.base, 'artist [CD5]', 'CATS', 'song7.mp3'),
+        ]
+
+        for path in self.dirs:
+            os.mkdir(path)
+        for path in self.files:
+            _mkmp3(path)
+
+    def test_coalesce_nested_album_multiple_subdirs(self):
+        albums = list(albums_in_dir(self.base))
+        self.assertEquals(len(albums), 4)
+        root, items = albums[0]
+        self.assertEquals(root, self.dirs[0:3])
+        self.assertEquals(len(items), 3)
+
+    def test_coalesce_nested_album_single_subdir(self):
+        albums = list(albums_in_dir(self.base))
+        root, items = albums[1]
+        self.assertEquals(root, self.dirs[3:5])
+        self.assertEquals(len(items), 1)
+
+    def test_coalesce_flattened_album_case_typo(self):
+        albums = list(albums_in_dir(self.base))
+        root, items = albums[2]
+        self.assertEquals(root, self.dirs[6:8])
+        self.assertEquals(len(items), 2)
+
+    def test_single_disc_album(self):
+        albums = list(albums_in_dir(self.base))
+        root, items = albums[3]
+        self.assertEquals(root, self.dirs[8:])
+        self.assertEquals(len(items), 1)
+
+    def test_do_not_yield_empty_album(self):
+        # Remove all the MP3s.
+        for path in self.files:
+            os.remove(path)
+        albums = list(albums_in_dir(self.base))
+        self.assertEquals(len(albums), 0)
+
+
+
 def suite():
     return unittest.TestLoader().loadTestsFromName(__name__)
 

From 4835475fb74ee9c5e3219fee57102ee5d3cf6906 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Mon, 25 Aug 2014 18:19:22 +0200
Subject: [PATCH 48/63] Tags are not read while walking the import tree.

This makes skipping directories on incremental imports much faster and
fixes #158.
---
 beets/importer.py     | 75 +++++++++++++++++++++++--------------------
 test/test_importer.py |  5 ++-
 2 files changed, 43 insertions(+), 37 deletions(-)

diff --git a/beets/importer.py b/beets/importer.py
index 5ace5cfe0..a050be27c 100644
--- a/beets/importer.py
+++ b/beets/importer.py
@@ -918,38 +918,41 @@ def read_tasks(session):
 
         # A flat album import merges all items into one album.
         if session.config['flat'] and not session.config['singletons']:
-            all_items = []
-            for _, items in albums_in_dir(toppath):
-                all_items += items
-            if all_items:
+            all_item_paths = []
+            for _, item_paths in albums_in_dir(toppath):
+                all_item_paths += item_paths
+            if all_item_paths:
                 if session.already_imported(toppath, [toppath]):
                     log.debug(u'Skipping previously-imported path: {0}'
                               .format(displayable_path(toppath)))
                     skipped += 1
                     continue
+                all_items = read_items(all_item_paths)
                 yield ImportTask(toppath, [toppath], all_items)
                 yield SentinelImportTask(toppath)
             continue
 
         # Produce paths under this directory.
-        for paths, items in albums_in_dir(toppath):
+        for dirs, paths in albums_in_dir(toppath):
             if session.config['singletons']:
-                for item in items:
-                    if session.already_imported(toppath, [item.path]):
+                for path in paths:
+                    if session.already_imported(toppath, [path]):
                         log.debug(u'Skipping previously-imported path: {0}'
-                                  .format(displayable_path(paths)))
+                                  .format(displayable_path(path)))
                         skipped += 1
                         continue
-                    yield SingletonImportTask(toppath, item)
-                yield SentinelImportTask(toppath, paths)
+                    yield SingletonImportTask(toppath, read_items([path])[0])
+                yield SentinelImportTask(toppath, dirs)
 
             else:
-                if session.already_imported(toppath, paths):
+                if session.already_imported(toppath, dirs):
                     log.debug(u'Skipping previously-imported path: {0}'
-                              .format(displayable_path(paths)))
+                              .format(displayable_path(dirs)))
                     skipped += 1
                     continue
-                yield ImportTask(toppath, paths, items)
+                print(paths)
+                print(read_items(paths))
+                yield ImportTask(toppath, dirs, read_items(paths))
 
         # Indicate the directory is finished.
         # FIXME hack to delete extracted archives
@@ -1178,27 +1181,7 @@ def albums_in_dir(path):
     ignore = config['ignore'].as_str_seq()
 
     for root, dirs, files in sorted_walk(path, ignore=ignore, logger=log):
-        # Get a list of items in the directory.
-        items = []
-        for filename in files:
-            try:
-                i = library.Item.from_path(os.path.join(root, filename))
-            except library.ReadError as exc:
-                if isinstance(exc.reason, mediafile.FileTypeError):
-                    # Silently ignore non-music files.
-                    pass
-                elif isinstance(exc.reason, mediafile.UnreadableFileError):
-                    log.warn(u'unreadable file: {0}'.format(
-                        displayable_path(filename))
-                    )
-                else:
-                    log.error(u'error reading {0}: {1}'.format(
-                        displayable_path(filename),
-                        exc,
-                    ))
-            else:
-                items.append(i)
-
+        items = [os.path.join(root, f) for f in files]
         # If we're currently collapsing the constituent directories in a
         # multi-disc album, check whether we should continue collapsing
         # and add the current directory. If so, just add the directory
@@ -1284,3 +1267,27 @@ def albums_in_dir(path):
         yield collapse_paths, collapse_items
 
 
+def read_items(paths):
+    """Return a list of items created from each path.
+
+    If an item could not be read it skips the item and logs an error.
+    """
+    # TODO remove this method. Should be handled in ImportTask creation.
+    items = []
+    for path in paths:
+        try:
+            items.append(library.Item.from_path(path))
+        except library.ReadError as exc:
+            if isinstance(exc.reason, mediafile.FileTypeError):
+                # Silently ignore non-music files.
+                pass
+            elif isinstance(exc.reason, mediafile.UnreadableFileError):
+                log.warn(u'unreadable file: {0}'.format(
+                    displayable_path(path))
+                )
+            else:
+                log.error(u'error reading {0}: {1}'.format(
+                    displayable_path(path),
+                    exc,
+                ))
+    return items
diff --git a/test/test_importer.py b/test/test_importer.py
index 5920ae0a2..66d3d5794 100644
--- a/test/test_importer.py
+++ b/test/test_importer.py
@@ -1238,7 +1238,7 @@ class AlbumsInDirTest(_common.TestCase):
     def test_separates_contents(self):
         found = []
         for _, album in albums_in_dir(self.base):
-            found.append(re.search(r'album(.)song', album[0].path).group(1))
+            found.append(re.search(r'album(.)song', album[0]).group(1))
         self.assertTrue('1' in found)
         self.assertTrue('2' in found)
         self.assertTrue('3' in found)
@@ -1246,7 +1246,7 @@ class AlbumsInDirTest(_common.TestCase):
 
     def test_finds_multiple_songs(self):
         for _, album in albums_in_dir(self.base):
-            n = re.search(r'album(.)song', album[0].path).group(1)
+            n = re.search(r'album(.)song', album[0]).group(1)
             if n == '1':
                 self.assertEqual(len(album), 2)
             else:
@@ -1329,7 +1329,6 @@ class MultiDiscAlbumsInDirTest(_common.TestCase):
         self.assertEquals(len(albums), 0)
 
 
-
 def suite():
     return unittest.TestLoader().loadTestsFromName(__name__)
 

From 659a0862f6dc2c8c46e503fdfa2574ed8becf006 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Mon, 25 Aug 2014 18:46:19 +0200
Subject: [PATCH 49/63] PEP8 fixes

---
 beets/autotag/__init__.py | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/beets/autotag/__init__.py b/beets/autotag/__init__.py
index a32a8f6f9..c9e6825e8 100644
--- a/beets/autotag/__init__.py
+++ b/beets/autotag/__init__.py
@@ -14,12 +14,9 @@
 
 """Facilities for automatically determining files' correct metadata.
 """
-import os
 import logging
-import re
 
-from beets import library, mediafile, config
-from beets.util import ancestry, displayable_path
+from beets import config
 
 # Parts of external interface.
 from .hooks import AlbumInfo, TrackInfo, AlbumMatch, TrackMatch  # noqa

From 1e207395b34f5833492765b642938438f8e9c3c2 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Mon, 25 Aug 2014 18:53:05 +0200
Subject: [PATCH 50/63] Formatted mapping uses field type to determine default

---
 beets/dbcore/db.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py
index 756448720..050ddbd81 100644
--- a/beets/dbcore/db.py
+++ b/beets/dbcore/db.py
@@ -55,7 +55,9 @@ class FormattedMapping(collections.Mapping):
     def __len__(self):
         return len(self.model_keys)
 
-    def get(self, key, default=u''):
+    def get(self, key, default=None):
+        if default is None:
+            default = self.model._type(key).format(None)
         return super(FormattedMapping, self).get(key, default)
 
     def _get_formatted(self, model, key):

From 71645ea7cda9911dbda1b87e8dc925cd0adc482c Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Mon, 25 Aug 2014 09:55:37 -0700
Subject: [PATCH 51/63] Changelog for #158

---
 docs/changelog.rst | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/docs/changelog.rst b/docs/changelog.rst
index 8fbb5e46d..9805db98c 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -26,6 +26,8 @@ Fixes:
   converted files.
 * Formatting templates with item data no longer confusingly shows album-level
   data when the two are inconsistent.
+* Resuming imports and beginning incremental imports should now be much faster
+  when there is a lot of previously-imported music to skip.
 
 .. _discogs_client: https://github.com/discogs/discogs_client
 

From 4279ac0b679e2a0b75294cd6733ed648047d6c8d Mon Sep 17 00:00:00 2001
From: Andrii Kohut <kogut.andriy@gmail.com>
Date: Mon, 25 Aug 2014 20:22:47 +0300
Subject: [PATCH 52/63] New api requires client and dict as parameters for
 Release initialization

---
 beetsplug/discogs.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py
index 75fe536c4..949888eff 100644
--- a/beetsplug/discogs.py
+++ b/beetsplug/discogs.py
@@ -76,7 +76,7 @@ class DiscogsPlugin(BeetsPlugin):
                           album_id)
         if not match:
             return None
-        result = Release(match.group(2))
+        result = Release(self.discogs_client, {'id': int(match.group(2))})
         # Try to obtain title to verify that we indeed have a valid Release
         try:
             getattr(result, 'title')

From a9f839bbf85437259f8643f147410ec05fb3d23f Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Mon, 25 Aug 2014 22:59:38 +0200
Subject: [PATCH 53/63] info: Specify files through library query

---
 beetsplug/info.py  | 92 +++++++++++++++++++++++++++++-----------------
 docs/changelog.rst |  3 ++
 test/helper.py     |  8 +++-
 3 files changed, 69 insertions(+), 34 deletions(-)

diff --git a/beetsplug/info.py b/beetsplug/info.py
index 87e21f916..3bd1a10fe 100644
--- a/beetsplug/info.py
+++ b/beetsplug/info.py
@@ -23,7 +23,51 @@ from beets import mediafile
 from beets import util
 
 
-def info(paths):
+def run(lib, opts, args):
+    """Print tag info for each file referenced by args.
+
+    Main entry point for the `beet info ARGS...` command.
+
+    If an argument is a path pointing to an existing file, then the tags
+    of that file are printed. All other arguments are considered
+    queries, and for each item matching all those queries the tags from
+    the file are printed.
+    """
+    paths, query = parse_args(args)
+
+    first = True
+    for path in paths:
+        if not first:
+            ui.print_()
+        print_tags(path)
+        first = False
+
+    if not query:
+        return
+
+    for item in lib.items(*query):
+        if not first:
+            ui.print_()
+        print_tags(item.path)
+        first = False
+
+
+def parse_args(args):
+    """Split `args` into a tuple of paths and querys.
+    """
+    if not args:
+        raise ui.UserError('no file specified')
+    paths = []
+    query = []
+    for arg in args:
+        if os.sep in arg and os.path.exists(arg):
+            paths.append(util.normpath(arg))
+        else:
+            query.append(arg)
+    return paths, query
+
+
+def print_tags(path):
     # Set up fields to output.
     fields = list(mediafile.MediaFile.readable_fields())
     fields.remove('art')
@@ -34,43 +78,25 @@ def info(paths):
     maxwidth = max(len(name) for name in fields + other_fields)
     lineformat = u'{{0:>{0}}}: {{1}}'.format(maxwidth)
 
-    first = True
-    for path in paths:
-        if not first:
-            ui.print_()
+    ui.print_(path)
+    try:
+        mf = mediafile.MediaFile(path)
+    except mediafile.UnreadableFileError:
+        ui.print_('cannot read file: {0}'.format(
+            util.displayable_path(path)
+        ))
+        return
 
-        path = util.normpath(path)
-        if not os.path.isfile(path):
-            ui.print_(u'not a file: {0}'.format(
-                util.displayable_path(path)
-            ))
-            continue
-        ui.print_(path)
-        try:
-            mf = mediafile.MediaFile(path)
-        except mediafile.UnreadableFileError:
-            ui.print_('cannot read file: {0}'.format(
-                util.displayable_path(path)
-            ))
-            continue
-
-        # Basic fields.
-        for name in fields:
-            ui.print_(lineformat.format(name, getattr(mf, name)))
-        # Extra stuff.
-        ui.print_(lineformat.format('album art', mf.art is not None))
-
-        first = False
+    # Basic fields.
+    for name in fields:
+        ui.print_(lineformat.format(name, getattr(mf, name)))
+    # Extra stuff.
+    ui.print_(lineformat.format('album art', mf.art is not None))
 
 
 class InfoPlugin(BeetsPlugin):
 
     def commands(self):
         cmd = ui.Subcommand('info', help='show file metadata')
-
-        def func(lib, opts, args):
-            if not args:
-                raise ui.UserError('no file specified')
-            info(args)
-        cmd.func = func
+        cmd.func = run
         return [cmd]
diff --git a/docs/changelog.rst b/docs/changelog.rst
index 9805db98c..5b7802cab 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -6,6 +6,9 @@ Changelog
 
 This release adds **sorting** to beets queries. See :ref:`query-sort`.
 
+Features:
+* :doc:`/plugins/info`: Files can be specified through library queries.
+
 Fixes:
 
 * Invalid state files don't crash the importer.
diff --git a/test/helper.py b/test/helper.py
index bfbd378d0..2e7732ede 100644
--- a/test/helper.py
+++ b/test/helper.py
@@ -80,12 +80,13 @@ def capture_stdout():
     'spam'
     """
     org = sys.stdout
-    sys.stdout = StringIO()
+    sys.stdout = capture = StringIO()
     sys.stdout.encoding = 'utf8'
     try:
         yield sys.stdout
     finally:
         sys.stdout = org
+        print(capture.getvalue())
 
 
 def has_program(cmd, args=['--version']):
@@ -289,6 +290,11 @@ class TestHelper(object):
             lib = Library(':memory:')
         beets.ui._raw_main(list(args), lib)
 
+    def run_with_output(self, *args):
+        with capture_stdout() as out:
+            self.run_command(*args)
+        return out.getvalue()
+
     def create_temp_dir(self):
         """Create a temporary directory and assign it into
         `self.temp_dir`. Call `remove_temp_dir` later to delete it.

From b278db42be42bb90b43f816dfc5c6390ed13a753 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Mon, 25 Aug 2014 23:47:16 +0200
Subject: [PATCH 54/63] info: print library fields and sort output

---
 beetsplug/info.py  | 105 ++++++++++++++++++++++++++-------------------
 docs/changelog.rst |   3 +-
 test/helper.py     |   4 +-
 3 files changed, 66 insertions(+), 46 deletions(-)

diff --git a/beetsplug/info.py b/beetsplug/info.py
index 3bd1a10fe..b6876a0a1 100644
--- a/beetsplug/info.py
+++ b/beetsplug/info.py
@@ -21,42 +21,30 @@ from beets.plugins import BeetsPlugin
 from beets import ui
 from beets import mediafile
 from beets import util
+from beets.util import displayable_path
 
 
 def run(lib, opts, args):
-    """Print tag info for each file referenced by args.
+    """Print tag info or library data for each file referenced by args.
 
     Main entry point for the `beet info ARGS...` command.
+    """
+    if opts.library:
+        print_library_info(lib, args)
+    else:
+        print_tag_info(lib, args)
+
+def print_tag_info(lib, args):
+    """Print tag info for each file referenced by args.
 
     If an argument is a path pointing to an existing file, then the tags
     of that file are printed. All other arguments are considered
     queries, and for each item matching all those queries the tags from
     the file are printed.
     """
-    paths, query = parse_args(args)
-
-    first = True
-    for path in paths:
-        if not first:
-            ui.print_()
-        print_tags(path)
-        first = False
-
-    if not query:
-        return
-
-    for item in lib.items(*query):
-        if not first:
-            ui.print_()
-        print_tags(item.path)
-        first = False
-
-
-def parse_args(args):
-    """Split `args` into a tuple of paths and querys.
-    """
     if not args:
         raise ui.UserError('no file specified')
+
     paths = []
     query = []
     for arg in args:
@@ -64,34 +52,63 @@ def parse_args(args):
             paths.append(util.normpath(arg))
         else:
             query.append(arg)
-    return paths, query
+
+    if query:
+        for item in lib.items(query):
+            paths.append(item.path)
+
+    first = True
+    for path in paths:
+        if not first:
+            ui.print_()
+        try:
+            data = tag_data(path)
+        except mediafile.UnreadableFileError:
+            ui.print_('cannot read file: {0}'.format(
+                util.displayable_path(path)
+            ))
+        else:
+            print_data(path, data)
+        first = False
 
 
-def print_tags(path):
-    # Set up fields to output.
+def print_library_info(lib, queries):
+    """Print library data for each item matching all queries
+    """
+    first = True
+    for item in lib.items(queries):
+        if not first:
+            ui.print_()
+        print_data(item.path, library_data(item))
+        first = False
+
+
+def tag_data(path):
     fields = list(mediafile.MediaFile.readable_fields())
-    fields.remove('art')
     fields.remove('images')
+    mf = mediafile.MediaFile(path)
+    tags = {}
+    for field in fields:
+        tags[field] = getattr(mf, field)
+    tags['art'] = mf.art is not None
+    return tags
 
-    # Line format.
-    other_fields = ['album art']
-    maxwidth = max(len(name) for name in fields + other_fields)
+
+def library_data(item):
+    return dict(item.formatted())
+
+
+def print_data(path, data):
+    maxwidth = max(len(key) for key in data)
     lineformat = u'{{0:>{0}}}: {{1}}'.format(maxwidth)
 
-    ui.print_(path)
-    try:
-        mf = mediafile.MediaFile(path)
-    except mediafile.UnreadableFileError:
-        ui.print_('cannot read file: {0}'.format(
-            util.displayable_path(path)
-        ))
-        return
+    ui.print_(displayable_path(path))
 
-    # Basic fields.
-    for name in fields:
-        ui.print_(lineformat.format(name, getattr(mf, name)))
-    # Extra stuff.
-    ui.print_(lineformat.format('album art', mf.art is not None))
+    for field in sorted(data):
+        value = data[field]
+        if isinstance(value, list):
+            value = u'; '.join(value)
+        ui.print_(lineformat.format(field, value))
 
 
 class InfoPlugin(BeetsPlugin):
@@ -99,4 +116,6 @@ class InfoPlugin(BeetsPlugin):
     def commands(self):
         cmd = ui.Subcommand('info', help='show file metadata')
         cmd.func = run
+        cmd.parser.add_option('-l', '--library', action='store_true',
+                              help='show library fields instead of tags')
         return [cmd]
diff --git a/docs/changelog.rst b/docs/changelog.rst
index 5b7802cab..dd0958476 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -7,7 +7,8 @@ Changelog
 This release adds **sorting** to beets queries. See :ref:`query-sort`.
 
 Features:
-* :doc:`/plugins/info`: Files can be specified through library queries.
+* :doc:`/plugins/info`: Files can be specified through library queries
+  and the `--library` option prints library fields instead of tags.
 
 Fixes:
 
diff --git a/test/helper.py b/test/helper.py
index 2e7732ede..d8b86b8c0 100644
--- a/test/helper.py
+++ b/test/helper.py
@@ -237,8 +237,8 @@ class TestHelper(object):
         path = os.path.join(_common.RSRC, 'full.' + ext)
         for i in range(count):
             item = Item.from_path(str(path))
-            item.album = u'\xc3\xa4lbum {0}'.format(i)  # Check unicode paths
-            item.title = u't\xc3\x8ftle {0}'.format(i)
+            item.album = u'\u00e4lbum {0}'.format(i)  # Check unicode paths
+            item.title = u't\u00eftle {0}'.format(i)
             item.add(self.lib)
             item.move(copy=True)
             item.store()

From 9d0156e0da5fdc9346f438dd641c563da2033082 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Tue, 26 Aug 2014 11:54:12 +0200
Subject: [PATCH 55/63] info: Check if normalized path is file

---
 beetsplug/info.py | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/beetsplug/info.py b/beetsplug/info.py
index b6876a0a1..da201a023 100644
--- a/beetsplug/info.py
+++ b/beetsplug/info.py
@@ -20,8 +20,7 @@ import os
 from beets.plugins import BeetsPlugin
 from beets import ui
 from beets import mediafile
-from beets import util
-from beets.util import displayable_path
+from beets.util import displayable_path, syspath, normpath
 
 
 def run(lib, opts, args):
@@ -48,8 +47,9 @@ def print_tag_info(lib, args):
     paths = []
     query = []
     for arg in args:
-        if os.sep in arg and os.path.exists(arg):
-            paths.append(util.normpath(arg))
+        path = normpath(arg)
+        if os.path.isfile(path):
+            paths.append(path)
         else:
             query.append(arg)
 
@@ -64,9 +64,7 @@ def print_tag_info(lib, args):
         try:
             data = tag_data(path)
         except mediafile.UnreadableFileError:
-            ui.print_('cannot read file: {0}'.format(
-                util.displayable_path(path)
-            ))
+            ui.print_('cannot read file: {0}'.format(displayable_path(path)))
         else:
             print_data(path, data)
         first = False

From 9774692dc808955bf311f7c1ec6c5b0ad0193a54 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Tue, 26 Aug 2014 11:55:36 +0200
Subject: [PATCH 56/63] PEP8 fixes

---
 beetsplug/info.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/beetsplug/info.py b/beetsplug/info.py
index da201a023..be26e5c4d 100644
--- a/beetsplug/info.py
+++ b/beetsplug/info.py
@@ -20,7 +20,7 @@ import os
 from beets.plugins import BeetsPlugin
 from beets import ui
 from beets import mediafile
-from beets.util import displayable_path, syspath, normpath
+from beets.util import displayable_path, normpath
 
 
 def run(lib, opts, args):
@@ -33,6 +33,7 @@ def run(lib, opts, args):
     else:
         print_tag_info(lib, args)
 
+
 def print_tag_info(lib, args):
     """Print tag info for each file referenced by args.
 

From 5e4600afd8a733cd168da0e9d12c1e59520d6ca9 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Tue, 26 Aug 2014 12:01:53 +0200
Subject: [PATCH 57/63] info: add tests and omit None values

---
 beetsplug/info.py | 13 ++++++--
 test/test_info.py | 80 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+), 3 deletions(-)
 create mode 100644 test/test_info.py

diff --git a/beetsplug/info.py b/beetsplug/info.py
index be26e5c4d..9ece52d7f 100644
--- a/beetsplug/info.py
+++ b/beetsplug/info.py
@@ -98,13 +98,20 @@ def library_data(item):
 
 
 def print_data(path, data):
-    maxwidth = max(len(key) for key in data)
+    formatted = {}
+    for key, value in data.iteritems():
+        if isinstance(value, list):
+            formatted[key] = u'; '.join(value)
+        if value is not None:
+            formatted[key] = value
+
+    maxwidth = max(len(key) for key in formatted)
     lineformat = u'{{0:>{0}}}: {{1}}'.format(maxwidth)
 
     ui.print_(displayable_path(path))
 
-    for field in sorted(data):
-        value = data[field]
+    for field in sorted(formatted):
+        value = formatted[field]
         if isinstance(value, list):
             value = u'; '.join(value)
         ui.print_(lineformat.format(field, value))
diff --git a/test/test_info.py b/test/test_info.py
new file mode 100644
index 000000000..6f3be8294
--- /dev/null
+++ b/test/test_info.py
@@ -0,0 +1,80 @@
+# This file is part of beets.
+# Copyright 2014, Thomas Scholtes.
+#
+# Permission is hereby granted, free of charge, to any person obtaining
+# a copy of this software and associated documentation files (the
+# "Software"), to deal in the Software without restriction, including
+# without limitation the rights to use, copy, modify, merge, publish,
+# distribute, sublicense, and/or sell copies of the Software, and to
+# permit persons to whom the Software is furnished to do so, subject to
+# the following conditions:
+#
+# The above copyright notice and this permission notice shall be
+# included in all copies or substantial portions of the Software.
+
+import os
+import os.path
+from _common import unittest
+from helper import TestHelper
+
+from beets.mediafile import MediaFile
+
+
+class InfoTest(unittest.TestCase, TestHelper):
+
+    def setUp(self):
+        self.setup_beets()
+        self.load_plugins('info')
+
+    def tearDown(self):
+        self.unload_plugins()
+        self.teardown_beets()
+
+    def run_command(self, *args):
+        super(InfoTest, self).run_command('info', *args)
+
+    def test_path(self):
+        path = self.create_mediafile_fixture()
+
+        mediafile = MediaFile(path)
+        mediafile.albumartist = 'AAA'
+        mediafile.disctitle = 'DDD'
+        mediafile.genres = ['a', 'b', 'c']
+        mediafile.composer = None
+        mediafile.save()
+
+        out = self.run_with_output(path)
+        self.assertIn(path, out)
+        self.assertIn('albumartist: AAA', out)
+        self.assertIn('disctitle: DDD', out)
+        self.assertIn('genres: a; b; c', out)
+        self.assertNotIn('composer:', out)
+
+    def test_item_query(self):
+        items = self.add_item_fixtures(count=2)
+        items[0].album = 'xxxx'
+        items[0].write()
+        items[0].album = 'yyyy'
+        items[0].store()
+
+        out = self.run_with_output('album:yyyy')
+        self.assertIn(items[0].path, out)
+        self.assertIn('album: xxxx', out)
+
+        self.assertNotIn(items[1].path, out)
+
+    def test_item_library_query(self):
+        item, = self.add_item_fixtures()
+        item.album = 'xxxx'
+        item.store()
+
+        out = self.run_with_output('--library', 'album:xxxx')
+        self.assertIn(item.path, out)
+        self.assertIn('album: xxxx', out)
+
+
+def suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)
+
+if __name__ == '__main__':
+    unittest.main(defaultTest='suite')

From fe0a2482f300c1e961189b18f01e3594a512a8ca Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Tue, 26 Aug 2014 12:32:26 +0200
Subject: [PATCH 58/63] info: add --summarize option

Closes #409.
---
 beetsplug/info.py  | 61 ++++++++++++++++++++++++++++++++++------------
 docs/changelog.rst |  1 +
 test/test_info.py  | 19 +++++++++++++++
 3 files changed, 65 insertions(+), 16 deletions(-)

diff --git a/beetsplug/info.py b/beetsplug/info.py
index 9ece52d7f..be6020b63 100644
--- a/beetsplug/info.py
+++ b/beetsplug/info.py
@@ -29,18 +29,22 @@ def run(lib, opts, args):
     Main entry point for the `beet info ARGS...` command.
     """
     if opts.library:
-        print_library_info(lib, args)
+        print_library_info(lib, args, opts.summarize)
     else:
-        print_tag_info(lib, args)
+        print_tag_info(lib, args, opts.summarize)
 
 
-def print_tag_info(lib, args):
+def print_tag_info(lib, args, summarize=False):
     """Print tag info for each file referenced by args.
 
     If an argument is a path pointing to an existing file, then the tags
     of that file are printed. All other arguments are considered
     queries, and for each item matching all those queries the tags from
     the file are printed.
+
+    If `summarize` is true, the function merges all tags into one
+    dictionary and only prints that. If two files have different values
+    for the same tag, the value is set to '[various]'
     """
     if not args:
         raise ui.UserError('no file specified')
@@ -59,27 +63,41 @@ def print_tag_info(lib, args):
             paths.append(item.path)
 
     first = True
+    # FIXME the summary thing is horrible and duplicates code from print
+    # library info.
+    summary = {}
     for path in paths:
-        if not first:
-            ui.print_()
-        try:
-            data = tag_data(path)
-        except mediafile.UnreadableFileError:
-            ui.print_('cannot read file: {0}'.format(displayable_path(path)))
+        if summarize:
+            update_summary(summary, tag_data(path))
         else:
-            print_data(path, data)
-        first = False
+            if not first:
+                ui.print_()
+            try:
+                data = tag_data(path)
+            except mediafile.UnreadableFileError:
+                ui.print_('cannot read file: {0}'.format(displayable_path(path)))
+            else:
+                print_data(path, data)
+            first = False
+    if summarize:
+        print_data(None, summary)
 
 
-def print_library_info(lib, queries):
+def print_library_info(lib, queries, summarize=False):
     """Print library data for each item matching all queries
     """
     first = True
+    summary = {}
     for item in lib.items(queries):
-        if not first:
-            ui.print_()
-        print_data(item.path, library_data(item))
-        first = False
+        if summarize:
+            update_summary(summary, library_data(item))
+        else:
+            if not first:
+                ui.print_()
+            print_data(item.path, library_data(item))
+            first = False
+    if summarize:
+        print_data(None, summary)
 
 
 def tag_data(path):
@@ -97,6 +115,15 @@ def library_data(item):
     return dict(item.formatted())
 
 
+def update_summary(summary, tags):
+    for key, value in tags.iteritems():
+        if key not in summary:
+            summary[key] = value
+        elif summary[key] != value:
+            summary[key] = '[various]'
+    return summary
+
+
 def print_data(path, data):
     formatted = {}
     for key, value in data.iteritems():
@@ -124,4 +151,6 @@ class InfoPlugin(BeetsPlugin):
         cmd.func = run
         cmd.parser.add_option('-l', '--library', action='store_true',
                               help='show library fields instead of tags')
+        cmd.parser.add_option('-s', '--summarize', action='store_true',
+                              help='summarize the tags of all files')
         return [cmd]
diff --git a/docs/changelog.rst b/docs/changelog.rst
index dd0958476..84a644c13 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -9,6 +9,7 @@ This release adds **sorting** to beets queries. See :ref:`query-sort`.
 Features:
 * :doc:`/plugins/info`: Files can be specified through library queries
   and the `--library` option prints library fields instead of tags.
+  Tags and library fields can be summarized with `--summarize` option.
 
 Fixes:
 
diff --git a/test/test_info.py b/test/test_info.py
index 6f3be8294..e9db2c689 100644
--- a/test/test_info.py
+++ b/test/test_info.py
@@ -72,6 +72,25 @@ class InfoTest(unittest.TestCase, TestHelper):
         self.assertIn(item.path, out)
         self.assertIn('album: xxxx', out)
 
+    def test_collect_item_and_path(self):
+        path = self.create_mediafile_fixture()
+        mediafile = MediaFile(path)
+        item, = self.add_item_fixtures()
+
+        item.album = mediafile.album = 'AAA'
+        item.tracktotal = mediafile.tracktotal = 5
+        item.title = 'TTT'
+        mediafile.title = 'SSS'
+
+        item.write()
+        item.store()
+        mediafile.save()
+
+        out = self.run_with_output('--summarize', 'album:AAA', path)
+        self.assertIn('album: AAA', out)
+        self.assertIn('tracktotal: 5', out)
+        self.assertIn('title: [various]', out)
+
 
 def suite():
     return unittest.TestLoader().loadTestsFromName(__name__)

From 3554c0925c7267fe9ece9f9f495fa6bee0fab87d Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Tue, 26 Aug 2014 12:44:23 +0200
Subject: [PATCH 59/63] Fix doc and flake8 build (again)

---
 beetsplug/info.py  | 3 ++-
 docs/changelog.rst | 1 +
 test/test_info.py  | 2 --
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/beetsplug/info.py b/beetsplug/info.py
index be6020b63..590512f4a 100644
--- a/beetsplug/info.py
+++ b/beetsplug/info.py
@@ -75,7 +75,8 @@ def print_tag_info(lib, args, summarize=False):
             try:
                 data = tag_data(path)
             except mediafile.UnreadableFileError:
-                ui.print_('cannot read file: {0}'.format(displayable_path(path)))
+                ui.print_('cannot read file: {0}'
+                          .format(displayable_path(path)))
             else:
                 print_data(path, data)
             first = False
diff --git a/docs/changelog.rst b/docs/changelog.rst
index 84a644c13..c2629d2f1 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -7,6 +7,7 @@ Changelog
 This release adds **sorting** to beets queries. See :ref:`query-sort`.
 
 Features:
+
 * :doc:`/plugins/info`: Files can be specified through library queries
   and the `--library` option prints library fields instead of tags.
   Tags and library fields can be summarized with `--summarize` option.
diff --git a/test/test_info.py b/test/test_info.py
index e9db2c689..9c641e501 100644
--- a/test/test_info.py
+++ b/test/test_info.py
@@ -12,8 +12,6 @@
 # The above copyright notice and this permission notice shall be
 # included in all copies or substantial portions of the Software.
 
-import os
-import os.path
 from _common import unittest
 from helper import TestHelper
 

From b5239e626dc6c0b2370d56b0dd8acc09dd577eb6 Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Tue, 26 Aug 2014 09:15:04 -0700
Subject: [PATCH 60/63] info: Add documentation for recent features

---
 docs/changelog.rst    |  5 +++--
 docs/plugins/info.rst | 13 +++++++++++++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/docs/changelog.rst b/docs/changelog.rst
index c2629d2f1..5acd2cce2 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -9,8 +9,9 @@ This release adds **sorting** to beets queries. See :ref:`query-sort`.
 Features:
 
 * :doc:`/plugins/info`: Files can be specified through library queries
-  and the `--library` option prints library fields instead of tags.
-  Tags and library fields can be summarized with `--summarize` option.
+  and the ``--library`` option prints library fields instead of tags.
+  Tags and library fields for multiple files can be summarized with the
+  ``--summarize`` option.
 
 Fixes:
 
diff --git a/docs/plugins/info.rst b/docs/plugins/info.rst
index 38ee40085..7830d61c7 100644
--- a/docs/plugins/info.rst
+++ b/docs/plugins/info.rst
@@ -12,5 +12,18 @@ Enable the plugin and then type::
 and the plugin will enumerate all the tags in the specified file. It also
 accepts multiple filenames in a single command-line.
 
+You can also enter a :doc:`query </reference/query>` to inspect music from
+your library::
+
+    $ beet info beatles
+
+Command-line options include:
+
+* ``--library`` or ``-l``: Show data from the library database instead of the
+  files' tags.
+* ``--summarize`` or ``-s``: Merge all the information from multiple files
+  into a single list of values. If the tags differ across the files, print
+  ``[various]``.
+
 .. _id3v2: http://id3v2.sourceforge.net
 .. _mp3info: http://www.ibiblio.org/mp3info/

From 2518f654bf40ecaf65cc9cb5751bbab319f64b83 Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Tue, 26 Aug 2014 09:16:27 -0700
Subject: [PATCH 61/63] info: Two minor fixes

* Decode query arguments before constructing query.
* Don't print "None" for the path in summary mode.
---
 beetsplug/info.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/beetsplug/info.py b/beetsplug/info.py
index 590512f4a..4ffe83ff3 100644
--- a/beetsplug/info.py
+++ b/beetsplug/info.py
@@ -59,7 +59,7 @@ def print_tag_info(lib, args, summarize=False):
             query.append(arg)
 
     if query:
-        for item in lib.items(query):
+        for item in lib.items(ui.decargs(query)):
             paths.append(item.path)
 
     first = True
@@ -136,7 +136,8 @@ def print_data(path, data):
     maxwidth = max(len(key) for key in formatted)
     lineformat = u'{{0:>{0}}}: {{1}}'.format(maxwidth)
 
-    ui.print_(displayable_path(path))
+    if path:
+        ui.print_(displayable_path(path))
 
     for field in sorted(formatted):
         value = formatted[field]

From 71060f145366eec9d159db92bfa14bb1d6fe5427 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Tue, 26 Aug 2014 13:13:50 +0200
Subject: [PATCH 62/63] info refcator

---
 beetsplug/info.py | 122 ++++++++++++++++++++++------------------------
 1 file changed, 58 insertions(+), 64 deletions(-)

diff --git a/beetsplug/info.py b/beetsplug/info.py
index 4ffe83ff3..891930bcc 100644
--- a/beetsplug/info.py
+++ b/beetsplug/info.py
@@ -16,6 +16,7 @@
 """
 
 import os
+import logging
 
 from beets.plugins import BeetsPlugin
 from beets import ui
@@ -23,97 +24,89 @@ from beets import mediafile
 from beets.util import displayable_path, normpath
 
 
+log = logging.getLogger('beets')
+
+
 def run(lib, opts, args):
     """Print tag info or library data for each file referenced by args.
 
     Main entry point for the `beet info ARGS...` command.
-    """
-    if opts.library:
-        print_library_info(lib, args, opts.summarize)
-    else:
-        print_tag_info(lib, args, opts.summarize)
-
-
-def print_tag_info(lib, args, summarize=False):
-    """Print tag info for each file referenced by args.
 
     If an argument is a path pointing to an existing file, then the tags
     of that file are printed. All other arguments are considered
     queries, and for each item matching all those queries the tags from
     the file are printed.
 
-    If `summarize` is true, the function merges all tags into one
+    If `opts.summarize` is true, the function merges all tags into one
     dictionary and only prints that. If two files have different values
     for the same tag, the value is set to '[various]'
     """
-    if not args:
-        raise ui.UserError('no file specified')
+    if opts.library:
+        data_collector = library_data
+    else:
+        data_collector = tag_data
 
-    paths = []
+    first = True
+    summary = {}
+    for data_emitter in data_collector(lib, ui.decargs(args)):
+        try:
+            data = data_emitter()
+        except mediafile.UnreadableFileError as ex:
+            log.error('cannot read file: {0}'.format(ex.message))
+            continue
+
+        if opts.summarize:
+            update_summary(summary, data)
+        else:
+            if not first:
+                ui.print_()
+            else:
+                print_data(data)
+            first = False
+
+    if opts.summarize:
+        print_data(summary)
+
+
+def tag_data(lib, args):
     query = []
     for arg in args:
         path = normpath(arg)
         if os.path.isfile(path):
-            paths.append(path)
+            yield tag_data_emitter(path)
         else:
             query.append(arg)
 
     if query:
-        for item in lib.items(ui.decargs(query)):
-            paths.append(item.path)
-
-    first = True
-    # FIXME the summary thing is horrible and duplicates code from print
-    # library info.
-    summary = {}
-    for path in paths:
-        if summarize:
-            update_summary(summary, tag_data(path))
-        else:
-            if not first:
-                ui.print_()
-            try:
-                data = tag_data(path)
-            except mediafile.UnreadableFileError:
-                ui.print_('cannot read file: {0}'
-                          .format(displayable_path(path)))
-            else:
-                print_data(path, data)
-            first = False
-    if summarize:
-        print_data(None, summary)
+        for item in lib.items(query):
+            yield tag_data_emitter(item.path)
 
 
-def print_library_info(lib, queries, summarize=False):
-    """Print library data for each item matching all queries
-    """
-    first = True
-    summary = {}
-    for item in lib.items(queries):
-        if summarize:
-            update_summary(summary, library_data(item))
-        else:
-            if not first:
-                ui.print_()
-            print_data(item.path, library_data(item))
-            first = False
-    if summarize:
-        print_data(None, summary)
+def tag_data_emitter(path):
+    def emitter():
+        fields = list(mediafile.MediaFile.readable_fields())
+        fields.remove('images')
+        mf = mediafile.MediaFile(path)
+        tags = {}
+        for field in fields:
+            tags[field] = getattr(mf, field)
+        tags['art'] = mf.art is not None
+        tags['path'] = displayable_path(path)
+        return tags
+    return emitter
 
 
-def tag_data(path):
-    fields = list(mediafile.MediaFile.readable_fields())
-    fields.remove('images')
-    mf = mediafile.MediaFile(path)
-    tags = {}
-    for field in fields:
-        tags[field] = getattr(mf, field)
-    tags['art'] = mf.art is not None
-    return tags
+def library_data(lib, args):
+    for item in lib.items(args):
+        yield library_data_emitter(item)
 
 
-def library_data(item):
-    return dict(item.formatted())
+def library_data_emitter(item):
+    def emitter():
+        data = dict(item.formatted())
+        data['path'] = displayable_path(item.path)
+        return data
+    return emitter
 
 
 def update_summary(summary, tags):
@@ -125,7 +118,8 @@ def update_summary(summary, tags):
     return summary
 
 
-def print_data(path, data):
+def print_data(data):
+    path = data.pop('path')
     formatted = {}
     for key, value in data.iteritems():
         if isinstance(value, list):

From b869dbed0e321c76f57425ae255e4d9af81cb557 Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Tue, 26 Aug 2014 10:28:32 -0700
Subject: [PATCH 63/63] info: Add `syspath` calls

Sorry for interposing, @geigerzaehler. :smiley:
---
 beetsplug/info.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/beetsplug/info.py b/beetsplug/info.py
index 891930bcc..9fd6f4f12 100644
--- a/beetsplug/info.py
+++ b/beetsplug/info.py
@@ -21,7 +21,7 @@ import logging
 from beets.plugins import BeetsPlugin
 from beets import ui
 from beets import mediafile
-from beets.util import displayable_path, normpath
+from beets.util import displayable_path, normpath, syspath
 
 
 log = logging.getLogger('beets')
@@ -72,7 +72,7 @@ def tag_data(lib, args):
     query = []
     for arg in args:
         path = normpath(arg)
-        if os.path.isfile(path):
+        if os.path.isfile(syspath(path)):
             yield tag_data_emitter(path)
         else:
             query.append(arg)
@@ -86,7 +86,7 @@ def tag_data_emitter(path):
     def emitter():
         fields = list(mediafile.MediaFile.readable_fields())
         fields.remove('images')
-        mf = mediafile.MediaFile(path)
+        mf = mediafile.MediaFile(syspath(path))
         tags = {}
         for field in fields:
             tags[field] = getattr(mf, field)