From 4bfa439ee14decfb9d690260f1de435508b63711 Mon Sep 17 00:00:00 2001
From: Bruno Cauet <brunocauet@gmail.com>
Date: Sun, 15 Mar 2015 11:58:53 +0100
Subject: [PATCH 01/22] database_change: send model that changed

---
 beets/library.py           | 6 +++---
 beetsplug/mpdupdate.py     | 2 +-
 beetsplug/plexupdate.py    | 2 +-
 beetsplug/smartplaylist.py | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/beets/library.py b/beets/library.py
index 139cdfec0..132df4f52 100644
--- a/beets/library.py
+++ b/beets/library.py
@@ -270,15 +270,15 @@ class LibModel(dbcore.Model):
 
     def store(self):
         super(LibModel, self).store()
-        plugins.send('database_change', lib=self._db)
+        plugins.send('database_change', lib=self._db, model=self)
 
     def remove(self):
         super(LibModel, self).remove()
-        plugins.send('database_change', lib=self._db)
+        plugins.send('database_change', lib=self._db, model=self)
 
     def add(self, lib=None):
         super(LibModel, self).add(lib)
-        plugins.send('database_change', lib=self._db)
+        plugins.send('database_change', lib=self._db, model=self)
 
     def __format__(self, spec):
         if not spec:
diff --git a/beetsplug/mpdupdate.py b/beetsplug/mpdupdate.py
index 96141c567..dfe402497 100644
--- a/beetsplug/mpdupdate.py
+++ b/beetsplug/mpdupdate.py
@@ -80,7 +80,7 @@ class MPDUpdatePlugin(BeetsPlugin):
 
         self.register_listener('database_change', self.db_change)
 
-    def db_change(self, lib):
+    def db_change(self, lib, model):
         self.register_listener('cli_exit', self.update)
 
     def update(self, lib):
diff --git a/beetsplug/plexupdate.py b/beetsplug/plexupdate.py
index 9781e300e..5aa096486 100644
--- a/beetsplug/plexupdate.py
+++ b/beetsplug/plexupdate.py
@@ -55,7 +55,7 @@ class PlexUpdate(BeetsPlugin):
 
         self.register_listener('database_change', self.listen_for_db_change)
 
-    def listen_for_db_change(self, lib):
+    def listen_for_db_change(self, lib, model):
         """Listens for beets db change and register the update for the end"""
         self.register_listener('cli_exit', self.update)
 
diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py
index e2c256b2b..a9c0a25aa 100644
--- a/beetsplug/smartplaylist.py
+++ b/beetsplug/smartplaylist.py
@@ -65,7 +65,7 @@ class SmartPlaylistPlugin(BeetsPlugin):
         spl_update.func = update
         return [spl_update]
 
-    def db_change(self, lib):
+    def db_change(self, lib, model):
         self.register_listener('cli_exit', self.update_playlists)
 
     def update_playlists(self, lib):

From f06c33cb71f800ed60ffed41ef282a8413ddca4d Mon Sep 17 00:00:00 2001
From: Bruno Cauet <brunocauet@gmail.com>
Date: Sun, 15 Mar 2015 12:02:21 +0100
Subject: [PATCH 02/22] Smartplaylist: update only if item changed

---
 beetsplug/smartplaylist.py | 101 +++++++++++++++++++++++++------------
 test/test_smartplaylist.py |  41 +++++++++++++++
 2 files changed, 110 insertions(+), 32 deletions(-)
 create mode 100644 test/test_smartplaylist.py

diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py
index a9c0a25aa..ab63c9de8 100644
--- a/beetsplug/smartplaylist.py
+++ b/beetsplug/smartplaylist.py
@@ -21,30 +21,13 @@ from __future__ import (division, absolute_import, print_function,
 from beets.plugins import BeetsPlugin
 from beets import ui
 from beets.util import mkdirall, normpath, syspath
+from beets.library import Item, Album, parse_query_parts
+from beets.dbcore import OrQuery
 import os
 
 
-def _items_for_query(lib, queries, album):
-    """Get the matching items for a list of queries.
-
-    `queries` can either be a single string or a list of strings. In the
-    latter case, the results from each query are concatenated. `album`
-    indicates whether the queries are item-level or album-level.
-    """
-    if isinstance(queries, basestring):
-        queries = [queries]
-    if album:
-        for query in queries:
-            for album in lib.albums(query):
-                for item in album.items():
-                    yield item
-    else:
-        for query in queries:
-            for item in lib.items(query):
-                yield item
-
-
 class SmartPlaylistPlugin(BeetsPlugin):
+
     def __init__(self):
         super(SmartPlaylistPlugin, self).__init__()
         self.config.add({
@@ -54,42 +37,96 @@ class SmartPlaylistPlugin(BeetsPlugin):
             'playlists': []
         })
 
+        self._matched_playlists = None
+        self._unmatched_playlists = None
+
         if self.config['auto']:
             self.register_listener('database_change', self.db_change)
 
     def commands(self):
         def update(lib, opts, args):
+            self.build_queries()
+            self._matched_playlists = self._unmatched_playlists
             self.update_playlists(lib)
         spl_update = ui.Subcommand('splupdate',
                                    help='update the smart playlists')
         spl_update.func = update
         return [spl_update]
 
+    def build_queries(self):
+        """
+        Instanciate queries for the playlists.
+
+        Each playlist has 2 queries: one or items one for albums. We must also
+        remember its name. _unmatched_playlists is a set of tuples
+        (name, q, album_q).
+        """
+        self._unmatched_playlists = set()
+        self._matched_playlists = set()
+
+        for playlist in self.config['playlists'].get(list):
+            playlist_data = (playlist['name'],)
+            for key, Model in (('query', Item), ('album_query', Album)):
+                qs = playlist.get(key)
+                # FIXME sort mgmt
+                if qs is None:
+                    query = None
+                    sort = None
+                elif isinstance(qs, basestring):
+                    query, sort = parse_query_parts(qs, Model)
+                else:
+                    query = OrQuery([parse_query_parts(q, Model)[0]
+                                     for q in qs])
+                    sort = None
+                playlist_data += (query,)
+
+            self._unmatched_playlists.add(playlist_data)
+
     def db_change(self, lib, model):
-        self.register_listener('cli_exit', self.update_playlists)
+        if self._unmatched_playlists is None:
+            self.build_queries()
+
+        for playlist in self._unmatched_playlists:
+            n, q, a_q = playlist
+            if a_q and isinstance(model, Album):
+                matches = a_q.match(model)
+            elif q and isinstance(model, Item):
+                matches = q.match(model) or q.match(model.get_album())
+            else:
+                matches = False
+
+            if matches:
+                self._log.debug("{0} will be updated because of {1}", n, model)
+                self._matched_playlists.add(playlist)
+                self.register_listener('cli_exit', self.update_playlists)
+
+        self._unmatched_playlists -= self._matched_playlists
 
     def update_playlists(self, lib):
-        self._log.info("Updating smart playlists...")
-        playlists = self.config['playlists'].get(list)
+        self._log.info("Updating {0} smart playlists...",
+                       len(self._matched_playlists))
+
         playlist_dir = self.config['playlist_dir'].as_filename()
         relative_to = self.config['relative_to'].get()
         if relative_to:
             relative_to = normpath(relative_to)
 
-        for playlist in playlists:
-            self._log.debug(u"Creating playlist {0[name]}", playlist)
+        for playlist in self._matched_playlists:
+            name, query, album_query = playlist
+            self._log.debug(u"Creating playlist {0}", name)
             items = []
-            if 'album_query' in playlist:
-                items.extend(_items_for_query(lib, playlist['album_query'],
-                                              True))
-            if 'query' in playlist:
-                items.extend(_items_for_query(lib, playlist['query'], False))
+
+            if query:
+                items.extend(lib.items(query))
+            if album_query:
+                for album in lib.albums(album_query):
+                    items.extend(album.items())
 
             m3us = {}
             # As we allow tags in the m3u names, we'll need to iterate through
             # the items and generate the correct m3u file names.
             for item in items:
-                m3u_name = item.evaluate_template(playlist['name'], True)
+                m3u_name = item.evaluate_template(name, True)
                 if m3u_name not in m3us:
                     m3us[m3u_name] = []
                 item_path = item.path
@@ -104,4 +141,4 @@ class SmartPlaylistPlugin(BeetsPlugin):
                 with open(syspath(m3u_path), 'w') as f:
                     for path in m3us[m3u]:
                         f.write(path + b'\n')
-        self._log.info("{0} playlists updated", len(playlists))
+        self._log.info("{0} playlists updated", len(self._matched_playlists))
diff --git a/test/test_smartplaylist.py b/test/test_smartplaylist.py
new file mode 100644
index 000000000..90aca4e9b
--- /dev/null
+++ b/test/test_smartplaylist.py
@@ -0,0 +1,41 @@
+# This file is part of beets.
+# Copyright 2015, Bruno Cauet.
+#
+# 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.
+
+from __future__ import (division, absolute_import, print_function,
+                        unicode_literals)
+
+from test._common import unittest
+from beetsplug import smartplaylist
+from beets import config, ui
+
+from test.helper import TestHelper
+
+
+class SmartPlaylistTest(unittest.TestCase):
+    def test_build_queries(self):
+        pass
+
+    def test_db_changes(self):
+        pass
+
+    def test_playlist_update(self):
+        pass
+
+
+class SmartPlaylistCLITest(unittest.TestCase, TestHelper):
+    def test_import(self):
+        pass
+
+    def test_splupdate(self):
+        pass

From 2d9f665848b726589e93cd7bca3785a30d442652 Mon Sep 17 00:00:00 2001
From: Bruno Cauet <brunocauet@gmail.com>
Date: Mon, 16 Mar 2015 16:03:14 +0100
Subject: [PATCH 03/22] Smartplaylist: offer "splupdate <playlist names>"

splupdate command of the SmartPlaylistPlugin looks in "args" for matches
of playlist names.
---
 beetsplug/smartplaylist.py | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py
index ab63c9de8..197257459 100644
--- a/beetsplug/smartplaylist.py
+++ b/beetsplug/smartplaylist.py
@@ -44,15 +44,33 @@ class SmartPlaylistPlugin(BeetsPlugin):
             self.register_listener('database_change', self.db_change)
 
     def commands(self):
-        def update(lib, opts, args):
-            self.build_queries()
-            self._matched_playlists = self._unmatched_playlists
-            self.update_playlists(lib)
         spl_update = ui.Subcommand('splupdate',
-                                   help='update the smart playlists')
-        spl_update.func = update
+                                   help='update the smart playlists. Playlist '
+                                        'names may be passed as arguments.')
+        spl_update.func = self.update_cmd
         return [spl_update]
 
+    def update_cmd(self, lib, opts, args):
+        self.build_queries()
+        if args:
+            args = set(ui.decargs(args))
+            for a in list(args):
+                if not a.endswith(".m3u"):
+                    args.add("{0}.m3u".format(a))
+
+            playlists = {(name, q, a_q)
+                         for name, q, a_q in self._unmatched_playlists
+                         if name in args}
+            if not playlists:
+                # raise UserError
+                pass
+            self._matched_playlists = playlists
+            self._unmatched_playlists -= playlists
+        else:
+            self._matched_playlists = self._unmatched_playlists
+
+        self.update_playlists(lib)
+
     def build_queries(self):
         """
         Instanciate queries for the playlists.

From 774decda7d120e8ad83c69c5f70ae66ccfaf7925 Mon Sep 17 00:00:00 2001
From: Bruno Cauet <brunocauet@gmail.com>
Date: Mon, 16 Mar 2015 16:21:49 +0100
Subject: [PATCH 04/22] =?UTF-8?q?Smartplayist:=20parse=5Fquery=5Fparts()?=
 =?UTF-8?q?=20=E2=86=92=20...ry=5Fstring()?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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

diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py
index 197257459..12e9c3fcd 100644
--- a/beetsplug/smartplaylist.py
+++ b/beetsplug/smartplaylist.py
@@ -21,7 +21,7 @@ from __future__ import (division, absolute_import, print_function,
 from beets.plugins import BeetsPlugin
 from beets import ui
 from beets.util import mkdirall, normpath, syspath
-from beets.library import Item, Album, parse_query_parts
+from beets.library import Item, Album, parse_query_string
 from beets.dbcore import OrQuery
 import os
 
@@ -91,9 +91,9 @@ class SmartPlaylistPlugin(BeetsPlugin):
                     query = None
                     sort = None
                 elif isinstance(qs, basestring):
-                    query, sort = parse_query_parts(qs, Model)
+                    query, sort = parse_query_string(qs, Model)
                 else:
-                    query = OrQuery([parse_query_parts(q, Model)[0]
+                    query = OrQuery([parse_query_string(q, Model)[0]
                                      for q in qs])
                     sort = None
                 playlist_data += (query,)

From 40e793cdb1e4d2081eab636fae728893befeefae Mon Sep 17 00:00:00 2001
From: Bruno Cauet <brunocauet@gmail.com>
Date: Mon, 16 Mar 2015 16:25:43 +0100
Subject: [PATCH 05/22] Fix flake8 errors

---
 beetsplug/smartplaylist.py | 1 +
 test/test_smartplaylist.py | 3 ---
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py
index 12e9c3fcd..e89a43cf7 100644
--- a/beetsplug/smartplaylist.py
+++ b/beetsplug/smartplaylist.py
@@ -96,6 +96,7 @@ class SmartPlaylistPlugin(BeetsPlugin):
                     query = OrQuery([parse_query_string(q, Model)[0]
                                      for q in qs])
                     sort = None
+                del sort  # FIXME
                 playlist_data += (query,)
 
             self._unmatched_playlists.add(playlist_data)
diff --git a/test/test_smartplaylist.py b/test/test_smartplaylist.py
index 90aca4e9b..5d1f894a9 100644
--- a/test/test_smartplaylist.py
+++ b/test/test_smartplaylist.py
@@ -16,9 +16,6 @@ from __future__ import (division, absolute_import, print_function,
                         unicode_literals)
 
 from test._common import unittest
-from beetsplug import smartplaylist
-from beets import config, ui
-
 from test.helper import TestHelper
 
 

From a1b048f5de5ce7cad89aea524245dfc1876867ef Mon Sep 17 00:00:00 2001
From: Bruno Cauet <brunocauet@gmail.com>
Date: Mon, 16 Mar 2015 16:45:09 +0100
Subject: [PATCH 06/22] =?UTF-8?q?RegexpQuery:=20fix=20typo:=20false=20?=
 =?UTF-8?q?=E2=86=92=20fast?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 beets/dbcore/query.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py
index cd891148e..24020e94c 100644
--- a/beets/dbcore/query.py
+++ b/beets/dbcore/query.py
@@ -177,8 +177,8 @@ class RegexpQuery(StringFieldQuery):
     Raises InvalidQueryError when the pattern is not a valid regular
     expression.
     """
-    def __init__(self, field, pattern, false=True):
-        super(RegexpQuery, self).__init__(field, pattern, false)
+    def __init__(self, field, pattern, fast=True):
+        super(RegexpQuery, self).__init__(field, pattern, fast)
         try:
             self.pattern = re.compile(self.pattern)
         except re.error as exc:

From 4151e819691555a4240b007b170b211a57d89b73 Mon Sep 17 00:00:00 2001
From: Bruno Cauet <brunocauet@gmail.com>
Date: Mon, 16 Mar 2015 16:56:25 +0100
Subject: [PATCH 07/22] Implement __eq__ for all Query subclasses

Tests are a bit light.
---
 beets/dbcore/query.py | 18 ++++++++++++++++--
 test/test_query.py    | 20 ++++++++++++++++++++
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py
index 24020e94c..b1314d1f8 100644
--- a/beets/dbcore/query.py
+++ b/beets/dbcore/query.py
@@ -73,6 +73,9 @@ class Query(object):
         """
         raise NotImplementedError
 
+    def __eq__(self, other):
+        return type(self) == type(other)
+
 
 class FieldQuery(Query):
     """An abstract query that searches in a specific field for a
@@ -106,6 +109,10 @@ class FieldQuery(Query):
     def match(self, item):
         return self.value_match(self.pattern, item.get(self.field))
 
+    def __eq__(self, other):
+        return super(FieldQuery, self).__eq__(other) and \
+               self.field == other.field and self.pattern == other.pattern
+
 
 class MatchQuery(FieldQuery):
     """A query that looks for exact matches in an item field."""
@@ -120,8 +127,7 @@ class MatchQuery(FieldQuery):
 class NoneQuery(FieldQuery):
 
     def __init__(self, field, fast=True):
-        self.field = field
-        self.fast = fast
+        super(NoneQuery, self).__init__(field, None, fast)
 
     def col_clause(self):
         return self.field + " IS NULL", ()
@@ -337,6 +343,10 @@ class CollectionQuery(Query):
         clause = (' ' + joiner + ' ').join(clause_parts)
         return clause, subvals
 
+    def __eq__(self, other):
+        return super(CollectionQuery, self).__eq__(other) and \
+               self.subqueries == other.subqueries
+
 
 class AnyFieldQuery(CollectionQuery):
     """A query that matches if a given FieldQuery subclass matches in
@@ -362,6 +372,10 @@ class AnyFieldQuery(CollectionQuery):
                 return True
         return False
 
+    def __eq__(self, other):
+        return super(AnyFieldQuery, self).__eq__(other) and \
+               self.query_class == other.query_class
+
 
 class MutableCollectionQuery(CollectionQuery):
     """A collection query whose subqueries may be modified after the
diff --git a/test/test_query.py b/test/test_query.py
index d512e02b8..6d8d744fe 100644
--- a/test/test_query.py
+++ b/test/test_query.py
@@ -60,6 +60,16 @@ class AnyFieldQueryTest(_common.LibTestCase):
                                        dbcore.query.SubstringQuery)
         self.assertEqual(self.lib.items(q).get(), None)
 
+    def test_eq(self):
+        q1 = dbcore.query.AnyFieldQuery('foo', ['bar'],
+                                        dbcore.query.SubstringQuery)
+        q2 = dbcore.query.AnyFieldQuery('foo', ['bar'],
+                                        dbcore.query.SubstringQuery)
+        self.assertEqual(q1, q2)
+
+        q2.query_class = None
+        self.assertNotEqual(q1, q2)
+
 
 class AssertsMixin(object):
     def assert_items_matched(self, results, titles):
@@ -344,6 +354,16 @@ class MatchTest(_common.TestCase):
     def test_open_range(self):
         dbcore.query.NumericQuery('bitrate', '100000..')
 
+    def test_eq(self):
+        q1 = dbcore.query.MatchQuery('foo', 'bar')
+        q2 = dbcore.query.MatchQuery('foo', 'bar')
+        q3 = dbcore.query.MatchQuery('foo', 'baz')
+        q4 = dbcore.query.StringFieldQuery('foo', 'bar')
+        self.assertEqual(q1, q2)
+        self.assertNotEqual(q1, q3)
+        self.assertNotEqual(q1, q4)
+        self.assertNotEqual(q3, q4)
+
 
 class PathQueryTest(_common.LibTestCase, TestHelper, AssertsMixin):
     def setUp(self):

From 64614ff57904bd4c10b0c422be2b1ef0c4295c23 Mon Sep 17 00:00:00 2001
From: Bruno Cauet <brunocauet@gmail.com>
Date: Mon, 16 Mar 2015 17:27:42 +0100
Subject: [PATCH 08/22] Query.__eq__: fix indents

---
 beets/dbcore/query.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py
index b1314d1f8..3a6f03041 100644
--- a/beets/dbcore/query.py
+++ b/beets/dbcore/query.py
@@ -111,7 +111,7 @@ class FieldQuery(Query):
 
     def __eq__(self, other):
         return super(FieldQuery, self).__eq__(other) and \
-               self.field == other.field and self.pattern == other.pattern
+            self.field == other.field and self.pattern == other.pattern
 
 
 class MatchQuery(FieldQuery):
@@ -345,7 +345,7 @@ class CollectionQuery(Query):
 
     def __eq__(self, other):
         return super(CollectionQuery, self).__eq__(other) and \
-               self.subqueries == other.subqueries
+            self.subqueries == other.subqueries
 
 
 class AnyFieldQuery(CollectionQuery):
@@ -374,7 +374,7 @@ class AnyFieldQuery(CollectionQuery):
 
     def __eq__(self, other):
         return super(AnyFieldQuery, self).__eq__(other) and \
-               self.query_class == other.query_class
+            self.query_class == other.query_class
 
 
 class MutableCollectionQuery(CollectionQuery):

From e00282b4fe74e3a83f16739a0709a23b227d9961 Mon Sep 17 00:00:00 2001
From: Bruno Cauet <brunocauet@gmail.com>
Date: Mon, 16 Mar 2015 17:25:09 +0100
Subject: [PATCH 09/22] Implement hash() for query subclasses

It is implemented on mutable queries as well, which is bad. IMPORTANT:
don't mutate your queries if you want to put them in a set or in a dict
(as keys).
---
 beets/dbcore/query.py | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py
index 3a6f03041..ba7125634 100644
--- a/beets/dbcore/query.py
+++ b/beets/dbcore/query.py
@@ -18,7 +18,7 @@ from __future__ import (division, absolute_import, print_function,
                         unicode_literals)
 
 import re
-from operator import attrgetter
+from operator import attrgetter, mul
 from beets import util
 from datetime import datetime, timedelta
 
@@ -76,6 +76,9 @@ class Query(object):
     def __eq__(self, other):
         return type(self) == type(other)
 
+    def __hash__(self):
+        return 0
+
 
 class FieldQuery(Query):
     """An abstract query that searches in a specific field for a
@@ -113,6 +116,9 @@ class FieldQuery(Query):
         return super(FieldQuery, self).__eq__(other) and \
             self.field == other.field and self.pattern == other.pattern
 
+    def __hash__(self):
+        return hash((self.field, hash(self.pattern)))
+
 
 class MatchQuery(FieldQuery):
     """A query that looks for exact matches in an item field."""
@@ -347,6 +353,12 @@ class CollectionQuery(Query):
         return super(CollectionQuery, self).__eq__(other) and \
             self.subqueries == other.subqueries
 
+    def __hash__(self):
+        """Since subqueries are mutable, this object should not be hashable.
+        However and for conveniencies purposes, it can be hashed.
+        """
+        return reduce(mul, map(hash, self.subqueries), 1)
+
 
 class AnyFieldQuery(CollectionQuery):
     """A query that matches if a given FieldQuery subclass matches in
@@ -376,6 +388,9 @@ class AnyFieldQuery(CollectionQuery):
         return super(AnyFieldQuery, self).__eq__(other) and \
             self.query_class == other.query_class
 
+    def __hash__(self):
+        return hash((self.pattern, tuple(self.fields), self.query_class))
+
 
 class MutableCollectionQuery(CollectionQuery):
     """A collection query whose subqueries may be modified after the

From 8b6b938a37bcd096c5c247c88bed8ef1ed9a9eb6 Mon Sep 17 00:00:00 2001
From: Bruno Cauet <brunocauet@gmail.com>
Date: Mon, 16 Mar 2015 16:56:49 +0100
Subject: [PATCH 10/22] SmartPlaylistPlugin: add unit tests

They're not exhaustive but still quite heavy.
---
 test/test_smartplaylist.py | 115 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 112 insertions(+), 3 deletions(-)

diff --git a/test/test_smartplaylist.py b/test/test_smartplaylist.py
index 5d1f894a9..83bb4543f 100644
--- a/test/test_smartplaylist.py
+++ b/test/test_smartplaylist.py
@@ -15,19 +15,128 @@
 from __future__ import (division, absolute_import, print_function,
                         unicode_literals)
 
+from os import path
+from tempfile import mkdtemp
+from shutil import rmtree
+
+from mock import Mock, MagicMock
+
+from beetsplug.smartplaylist import SmartPlaylistPlugin
+from beets.library import Item, Album, parse_query_string
+from beets.dbcore import OrQuery
+from beets.util import syspath
+from beets import config
+
 from test._common import unittest
 from test.helper import TestHelper
 
 
 class SmartPlaylistTest(unittest.TestCase):
     def test_build_queries(self):
-        pass
+        spl = SmartPlaylistPlugin()
+        self.assertEqual(spl._matched_playlists, None)
+        self.assertEqual(spl._unmatched_playlists, None)
+
+        config['smartplaylist']['playlists'].set([])
+        spl.build_queries()
+        self.assertEqual(spl._matched_playlists, set())
+        self.assertEqual(spl._unmatched_playlists, set())
+
+        config['smartplaylist']['playlists'].set([
+            {'name': 'foo',
+             'query': 'FOO foo'},
+            {'name': 'bar',
+             'album_query': ['BAR bar1', 'BAR bar2']},
+            {'name': 'baz',
+             'query': 'BAZ baz',
+             'album_query': 'BAZ baz'}
+        ])
+        spl.build_queries()
+        self.assertEqual(spl._matched_playlists, set())
+        foo_foo, _ = parse_query_string('FOO foo', Item)
+        bar_bar = OrQuery([parse_query_string('BAR bar1', Album)[0],
+                           parse_query_string('BAR bar2', Album)[0]])
+        baz_baz, _ = parse_query_string('BAZ baz', Item)
+        baz_baz2, _ = parse_query_string('BAZ baz', Album)
+        self.assertEqual(spl._unmatched_playlists, {
+            ('foo', foo_foo, None),
+            ('bar', None, bar_bar),
+            ('baz', baz_baz, baz_baz2)
+        })
 
     def test_db_changes(self):
-        pass
+        spl = SmartPlaylistPlugin()
+
+        i1 = MagicMock(Item)
+        i2 = MagicMock(Item)
+        a = MagicMock(Album)
+        i1.get_album.return_value = a
+
+        q1 = Mock()
+        q1.matches.side_effect = {i1: False, i2: False}.__getitem__
+        a_q1 = Mock()
+        a_q1.matches.side_effect = {a: True}.__getitem__
+        q2 = Mock()
+        q2.matches.side_effect = {i1: False, i2: True}.__getitem__
+
+        pl1 = ('1', q1, a_q1)
+        pl2 = ('2', None, a_q1)
+        pl3 = ('3', q2, None)
+
+        spl._unmatched_playlists = {pl1, pl2, pl3}
+        spl._matched_playlists = set()
+        spl.db_change(None, i1)
+        self.assertEqual(spl._unmatched_playlists, {pl2})
+        self.assertEqual(spl._matched_playlists, {pl1, pl3})
+
+        spl._unmatched_playlists = {pl1, pl2, pl3}
+        spl._matched_playlists = set()
+        spl.db_change(None, i2)
+        self.assertEqual(spl._unmatched_playlists, {pl2})
+        self.assertEqual(spl._matched_playlists, {pl1, pl3})
+
+        spl._unmatched_playlists = {pl1, pl2, pl3}
+        spl._matched_playlists = set()
+        spl.db_change(None, a)
+        self.assertEqual(spl._unmatched_playlists, {pl3})
+        self.assertEqual(spl._matched_playlists, {pl1, pl2})
+        spl.db_change(None, i2)
+        self.assertEqual(spl._unmatched_playlists, set())
+        self.assertEqual(spl._matched_playlists, {pl1, pl2, pl3})
 
     def test_playlist_update(self):
-        pass
+        spl = SmartPlaylistPlugin()
+
+        i = Mock(path='/tagada.mp3')
+        i.evaluate_template.side_effect = lambda x, _: x
+        q = Mock()
+        a_q = Mock()
+        lib = Mock()
+        lib.items.return_value = [i]
+        lib.albums.return_value = []
+        pl = 'my_playlist.m3u', q, a_q
+        spl._matched_playlists = {pl}
+
+        dir = mkdtemp()
+        config['smartplaylist']['relative_to'] = False
+        config['smartplaylist']['playlist_dir'] = dir
+        try:
+            spl.update_playlists(lib)
+        except Exception:
+            rmtree(dir)
+            raise
+
+        lib.items.assert_called_once_with(q)
+        lib.albums.assert_called_once_with(a_q)
+
+        m3u_filepath = path.join(dir, pl[0])
+        self.assertTrue(path.exists(m3u_filepath), m3u_filepath)
+
+        with open(syspath(m3u_filepath), 'r') as f:
+            content = f.readlines()
+        rmtree(dir)
+
+        self.assertEqual(content, ["/tagada.mp3\n"])
 
 
 class SmartPlaylistCLITest(unittest.TestCase, TestHelper):

From b79c025142602e9be8d1d6682d4dde98326cc05f Mon Sep 17 00:00:00 2001
From: Bruno Cauet <brunocauet@gmail.com>
Date: Mon, 16 Mar 2015 18:36:08 +0100
Subject: [PATCH 11/22] CLI tests for smartplaylist plugin

No import CLI test.
---
 beetsplug/smartplaylist.py |  6 +++--
 test/test_smartplaylist.py | 48 +++++++++++++++++++++++++++++++-------
 2 files changed, 44 insertions(+), 10 deletions(-)

diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py
index e89a43cf7..3e23f8407 100644
--- a/beetsplug/smartplaylist.py
+++ b/beetsplug/smartplaylist.py
@@ -62,8 +62,10 @@ class SmartPlaylistPlugin(BeetsPlugin):
                          for name, q, a_q in self._unmatched_playlists
                          if name in args}
             if not playlists:
-                # raise UserError
-                pass
+                raise ui.UserError('No playlist matching any of {0} '
+                                   'found'.format([name for name, _, _ in
+                                                   self._unmatched_playlists]))
+
             self._matched_playlists = playlists
             self._unmatched_playlists -= playlists
         else:
diff --git a/test/test_smartplaylist.py b/test/test_smartplaylist.py
index 83bb4543f..ac6ff921a 100644
--- a/test/test_smartplaylist.py
+++ b/test/test_smartplaylist.py
@@ -15,7 +15,7 @@
 from __future__ import (division, absolute_import, print_function,
                         unicode_literals)
 
-from os import path
+from os import path, remove
 from tempfile import mkdtemp
 from shutil import rmtree
 
@@ -25,6 +25,7 @@ from beetsplug.smartplaylist import SmartPlaylistPlugin
 from beets.library import Item, Album, parse_query_string
 from beets.dbcore import OrQuery
 from beets.util import syspath
+from beets.ui import UserError
 from beets import config
 
 from test._common import unittest
@@ -130,18 +131,49 @@ class SmartPlaylistTest(unittest.TestCase):
         lib.albums.assert_called_once_with(a_q)
 
         m3u_filepath = path.join(dir, pl[0])
-        self.assertTrue(path.exists(m3u_filepath), m3u_filepath)
-
+        self.assertTrue(path.exists(m3u_filepath))
         with open(syspath(m3u_filepath), 'r') as f:
-            content = f.readlines()
+            content = f.read()
         rmtree(dir)
 
-        self.assertEqual(content, ["/tagada.mp3\n"])
+        self.assertEqual(content, "/tagada.mp3\n")
 
 
 class SmartPlaylistCLITest(unittest.TestCase, TestHelper):
-    def test_import(self):
-        pass
+    def setUp(self):
+        self.setup_beets()
+
+        self.item = self.add_item()
+        config['smartplaylist']['playlists'].set([
+            {'name': 'my_playlist.m3u',
+             'query': self.item.title},
+            {'name': 'all.m3u',
+             'query': ''}
+        ])
+        config['smartplaylist']['playlist_dir'].set(self.temp_dir)
+        self.load_plugins('smartplaylist')
+
+    def tearDown(self):
+        self.unload_plugins()
+        self.teardown_beets()
 
     def test_splupdate(self):
-        pass
+        with self.assertRaises(UserError):
+            self.run_with_output('splupdate', 'tagada')
+
+        self.run_with_output('splupdate', 'my_playlist')
+        m3u_path = path.join(self.temp_dir, 'my_playlist.m3u')
+        self.assertTrue(path.exists(m3u_path))
+        with open(m3u_path, 'r') as f:
+            self.assertEqual(f.read(), self.item.path + b"\n")
+        remove(m3u_path)
+
+        self.run_with_output('splupdate', 'my_playlist.m3u')
+        with open(m3u_path, 'r') as f:
+            self.assertEqual(f.read(), self.item.path + b"\n")
+        remove(m3u_path)
+
+        self.run_with_output('splupdate')
+        for name in ('my_playlist.m3u', 'all.m3u'):
+            with open(path.join(self.temp_dir, name), 'r') as f:
+                self.assertEqual(f.read(), self.item.path + b"\n")

From 191ff61c532b58a8721354a03454c7a52b4f0f10 Mon Sep 17 00:00:00 2001
From: Bruno Cauet <brunocauet@gmail.com>
Date: Mon, 16 Mar 2015 18:38:15 +0100
Subject: [PATCH 12/22] Document the database_change parameter update

---
 docs/changelog.rst   | 2 ++
 docs/dev/plugins.rst | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/docs/changelog.rst b/docs/changelog.rst
index b222e3ad8..57802841e 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -128,6 +128,8 @@ Fixes:
 
 For developers:
 
+* The ``database_change`` event now sends the item or album that is subject to
+  a change in the db.
 * the ``OptionParser`` is now a ``CommonOptionsParser`` that offers facilities
   for adding usual options (``--album``, ``--path`` and ``--format``). See
   :ref:`add_subcommands`. :bug:`1271`
diff --git a/docs/dev/plugins.rst b/docs/dev/plugins.rst
index 0c1f7017f..1d610f53b 100644
--- a/docs/dev/plugins.rst
+++ b/docs/dev/plugins.rst
@@ -203,7 +203,7 @@ The events currently available are:
   Library object. Parameter: ``lib``.
 
 * *database_change*: a modification has been made to the library database. The
-  change might not be committed yet. Parameter: ``lib``.
+  change might not be committed yet. Parameters: ``lib`` and ``model``.
 
 * *cli_exit*: called just before the ``beet`` command-line program exits.
   Parameter: ``lib``.

From 5d1ee0457fe99b9c3a66cd675ca0e1f54e70421f Mon Sep 17 00:00:00 2001
From: Bruno Cauet <brunocauet@gmail.com>
Date: Mon, 16 Mar 2015 18:47:38 +0100
Subject: [PATCH 13/22] Document the smartplaylist plugin updates

---
 docs/changelog.rst             |  5 +++++
 docs/plugins/smartplaylist.rst | 11 +++++++----
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/docs/changelog.rst b/docs/changelog.rst
index 57802841e..c2adb1126 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -6,6 +6,11 @@ Changelog
 
 Features:
 
+* :doc:`/plugins/smartplaylist`: detect for each playlist if it needs to be
+  regenated, instead of systematically regenerating all of them after a
+  database modification.
+* :doc:`/plugins/smartplaylist`: the ``splupdate`` command can now take
+  additinal parameters: names of the playlists to regenerate.
 * Beets now accept top-level options ``--format-item`` and ``--format-album``
   before any subcommand to control how items and albums are displayed.
   :bug:`1271`:
diff --git a/docs/plugins/smartplaylist.rst b/docs/plugins/smartplaylist.rst
index bc39e581e..b414e1eb0 100644
--- a/docs/plugins/smartplaylist.rst
+++ b/docs/plugins/smartplaylist.rst
@@ -53,13 +53,16 @@ to albums that have a ``for_travel`` extensible field set to 1::
       album_query: 'for_travel:1'
       query: 'for_travel:1'
 
-By default, all playlists are automatically regenerated at the end of the
-session if the library database was changed. To force regeneration, you can
-invoke it manually from the command line::
+By default, each playlist is automatically regenerated at the end of the
+session if an item or album it matches changed in the library database. To
+force regeneration, you can invoke it manually from the command line::
 
     $ beet splupdate
 
-which will generate your new smart playlists.
+This will regenerate all smart playlists. You can also specify which ones you
+want to regenerate::
+
+    $ beet splupdate BeatlesUniverse.m3u MyTravelPlaylist
 
 You can also use this plugin together with the :doc:`mpdupdate`, in order to
 automatically notify MPD of the playlist change, by adding ``mpdupdate`` to

From 7bee2f093b90a5fb454e0b6f9fedeeb3fffc592e Mon Sep 17 00:00:00 2001
From: Bruno Cauet <brunocauet@gmail.com>
Date: Mon, 16 Mar 2015 18:48:14 +0100
Subject: [PATCH 14/22] changelog: fix an extra ':' after a bug #

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

diff --git a/docs/changelog.rst b/docs/changelog.rst
index c2adb1126..f3e71e9dd 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -13,7 +13,7 @@ Features:
   additinal parameters: names of the playlists to regenerate.
 * Beets now accept top-level options ``--format-item`` and ``--format-album``
   before any subcommand to control how items and albums are displayed.
-  :bug:`1271`:
+  :bug:`1271`
 * :doc:`/plugins/replaygain`: There is a new backend for the `bs1770gain`_
   tool. Thanks to :user:`jmwatte`. :bug:`1343`
 * There are now multiple levels of verbosity. On the command line, you can

From 65b52b9c48a4fe63a97342c9d0e589a159432b17 Mon Sep 17 00:00:00 2001
From: Bruno Cauet <brunocauet@gmail.com>
Date: Mon, 16 Mar 2015 19:30:31 +0100
Subject: [PATCH 15/22] python 2.6 compat: don't use set literals

In smartplaylist and test_smartplaylist.
---
 beetsplug/smartplaylist.py |  6 +++---
 test/test_smartplaylist.py | 26 +++++++++++++-------------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py
index 3e23f8407..e7048a1cf 100644
--- a/beetsplug/smartplaylist.py
+++ b/beetsplug/smartplaylist.py
@@ -58,9 +58,9 @@ class SmartPlaylistPlugin(BeetsPlugin):
                 if not a.endswith(".m3u"):
                     args.add("{0}.m3u".format(a))
 
-            playlists = {(name, q, a_q)
-                         for name, q, a_q in self._unmatched_playlists
-                         if name in args}
+            playlists = set((name, q, a_q)
+                            for name, q, a_q in self._unmatched_playlists
+                            if name in args)
             if not playlists:
                 raise ui.UserError('No playlist matching any of {0} '
                                    'found'.format([name for name, _, _ in
diff --git a/test/test_smartplaylist.py b/test/test_smartplaylist.py
index ac6ff921a..fc9bba59a 100644
--- a/test/test_smartplaylist.py
+++ b/test/test_smartplaylist.py
@@ -59,11 +59,11 @@ class SmartPlaylistTest(unittest.TestCase):
                            parse_query_string('BAR bar2', Album)[0]])
         baz_baz, _ = parse_query_string('BAZ baz', Item)
         baz_baz2, _ = parse_query_string('BAZ baz', Album)
-        self.assertEqual(spl._unmatched_playlists, {
+        self.assertEqual(spl._unmatched_playlists, set([
             ('foo', foo_foo, None),
             ('bar', None, bar_bar),
             ('baz', baz_baz, baz_baz2)
-        })
+        ]))
 
     def test_db_changes(self):
         spl = SmartPlaylistPlugin()
@@ -84,26 +84,26 @@ class SmartPlaylistTest(unittest.TestCase):
         pl2 = ('2', None, a_q1)
         pl3 = ('3', q2, None)
 
-        spl._unmatched_playlists = {pl1, pl2, pl3}
+        spl._unmatched_playlists = set([pl1, pl2, pl3])
         spl._matched_playlists = set()
         spl.db_change(None, i1)
-        self.assertEqual(spl._unmatched_playlists, {pl2})
-        self.assertEqual(spl._matched_playlists, {pl1, pl3})
+        self.assertEqual(spl._unmatched_playlists, set([pl2]))
+        self.assertEqual(spl._matched_playlists, set([pl1, pl3]))
 
-        spl._unmatched_playlists = {pl1, pl2, pl3}
+        spl._unmatched_playlists = set([pl1, pl2, pl3])
         spl._matched_playlists = set()
         spl.db_change(None, i2)
-        self.assertEqual(spl._unmatched_playlists, {pl2})
-        self.assertEqual(spl._matched_playlists, {pl1, pl3})
+        self.assertEqual(spl._unmatched_playlists, set([pl2]))
+        self.assertEqual(spl._matched_playlists, set([pl1, pl3]))
 
-        spl._unmatched_playlists = {pl1, pl2, pl3}
+        spl._unmatched_playlists = set([pl1, pl2, pl3])
         spl._matched_playlists = set()
         spl.db_change(None, a)
-        self.assertEqual(spl._unmatched_playlists, {pl3})
-        self.assertEqual(spl._matched_playlists, {pl1, pl2})
+        self.assertEqual(spl._unmatched_playlists, set([pl3]))
+        self.assertEqual(spl._matched_playlists, set([pl1, pl2]))
         spl.db_change(None, i2)
         self.assertEqual(spl._unmatched_playlists, set())
-        self.assertEqual(spl._matched_playlists, {pl1, pl2, pl3})
+        self.assertEqual(spl._matched_playlists, set([pl1, pl2, pl3]))
 
     def test_playlist_update(self):
         spl = SmartPlaylistPlugin()
@@ -116,7 +116,7 @@ class SmartPlaylistTest(unittest.TestCase):
         lib.items.return_value = [i]
         lib.albums.return_value = []
         pl = 'my_playlist.m3u', q, a_q
-        spl._matched_playlists = {pl}
+        spl._matched_playlists = [pl]
 
         dir = mkdtemp()
         config['smartplaylist']['relative_to'] = False

From 45c0c9b3cbcf4689eb22555f97e53c362f94c4fe Mon Sep 17 00:00:00 2001
From: Bruno Cauet <brunocauet@gmail.com>
Date: Tue, 17 Mar 2015 18:43:55 +0100
Subject: [PATCH 16/22] Deal with sorting

Try to follow any sort found & manage absence of sort. When there are
multiple sort directives given, concatenate them.

Tests not extended yet.
---
 beets/dbcore/query.py      | 12 +++++++++
 beetsplug/smartplaylist.py | 50 ++++++++++++++++++++++++++------------
 test/test_smartplaylist.py | 37 +++++++++++++++++-----------
 3 files changed, 69 insertions(+), 30 deletions(-)

diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py
index ba7125634..a51805bed 100644
--- a/beets/dbcore/query.py
+++ b/beets/dbcore/query.py
@@ -730,3 +730,15 @@ class NullSort(Sort):
     """No sorting. Leave results unsorted."""
     def sort(items):
         return items
+
+    def __nonzero__(self):
+        return self.__bool__()
+
+    def __bool__(self):
+        return False
+
+    def __eq__(self, other):
+        return type(self) == type(other) or other is None
+
+    def __hash__(self):
+        return 0
diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py
index e7048a1cf..80e1faa89 100644
--- a/beetsplug/smartplaylist.py
+++ b/beetsplug/smartplaylist.py
@@ -23,6 +23,7 @@ from beets import ui
 from beets.util import mkdirall, normpath, syspath
 from beets.library import Item, Album, parse_query_string
 from beets.dbcore import OrQuery
+from beets.dbcore.query import MultipleSort
 import os
 
 
@@ -77,9 +78,16 @@ class SmartPlaylistPlugin(BeetsPlugin):
         """
         Instanciate queries for the playlists.
 
-        Each playlist has 2 queries: one or items one for albums. We must also
-        remember its name. _unmatched_playlists is a set of tuples
-        (name, q, album_q).
+        Each playlist has 2 queries: one or items one for albums, each with a
+        sort. We must also remember its name. _unmatched_playlists is a set of
+        tuples (name, (q, q_sort), (album_q, album_q_sort)).
+
+        sort may be any sort, or NullSort, or None. None and NullSort are
+        equivalent and both eval to False.
+        More precisely
+        - it will be NullSort when a playlist query ('query' or 'album_query')
+          is a single item or a list with 1 element
+        - it will be None when there are multiple items i a query
         """
         self._unmatched_playlists = set()
         self._matched_playlists = set()
@@ -88,18 +96,28 @@ class SmartPlaylistPlugin(BeetsPlugin):
             playlist_data = (playlist['name'],)
             for key, Model in (('query', Item), ('album_query', Album)):
                 qs = playlist.get(key)
-                # FIXME sort mgmt
                 if qs is None:
-                    query = None
-                    sort = None
+                    query_and_sort = None, None
                 elif isinstance(qs, basestring):
-                    query, sort = parse_query_string(qs, Model)
+                    query_and_sort = parse_query_string(qs, Model)
+                elif len(qs) == 1:
+                    query_and_sort = parse_query_string(qs[0], Model)
                 else:
-                    query = OrQuery([parse_query_string(q, Model)[0]
-                                     for q in qs])
-                    sort = None
-                del sort  # FIXME
-                playlist_data += (query,)
+                    # multiple queries and sorts
+                    queries, sorts = zip(*(parse_query_string(q, Model)
+                                           for q in qs))
+                    query = OrQuery(queries)
+                    sort = MultipleSort()
+                    for s in sorts:
+                        if s:
+                            sort.add_sort(s)
+                    if not sort.sorts:
+                        sort = None
+                    elif len(sort.sorts) == 1:
+                        sort = sort.sorts[0]
+                    query_and_sort = query, sort
+
+                playlist_data += (query_and_sort,)
 
             self._unmatched_playlists.add(playlist_data)
 
@@ -108,7 +126,7 @@ class SmartPlaylistPlugin(BeetsPlugin):
             self.build_queries()
 
         for playlist in self._unmatched_playlists:
-            n, q, a_q = playlist
+            n, (q, _), (a_q, _) = playlist
             if a_q and isinstance(model, Album):
                 matches = a_q.match(model)
             elif q and isinstance(model, Item):
@@ -133,14 +151,14 @@ class SmartPlaylistPlugin(BeetsPlugin):
             relative_to = normpath(relative_to)
 
         for playlist in self._matched_playlists:
-            name, query, album_query = playlist
+            name, (query, q_sort), (album_query, a_q_sort) = playlist
             self._log.debug(u"Creating playlist {0}", name)
             items = []
 
             if query:
-                items.extend(lib.items(query))
+                items.extend(lib.items(query, q_sort))
             if album_query:
-                for album in lib.albums(album_query):
+                for album in lib.albums(album_query, a_q_sort):
                     items.extend(album.items())
 
             m3us = {}
diff --git a/test/test_smartplaylist.py b/test/test_smartplaylist.py
index fc9bba59a..42bffc8a5 100644
--- a/test/test_smartplaylist.py
+++ b/test/test_smartplaylist.py
@@ -24,6 +24,7 @@ from mock import Mock, MagicMock
 from beetsplug.smartplaylist import SmartPlaylistPlugin
 from beets.library import Item, Album, parse_query_string
 from beets.dbcore import OrQuery
+from beets.dbcore.query import NullSort
 from beets.util import syspath
 from beets.ui import UserError
 from beets import config
@@ -54,15 +55,15 @@ class SmartPlaylistTest(unittest.TestCase):
         ])
         spl.build_queries()
         self.assertEqual(spl._matched_playlists, set())
-        foo_foo, _ = parse_query_string('FOO foo', Item)
-        bar_bar = OrQuery([parse_query_string('BAR bar1', Album)[0],
-                           parse_query_string('BAR bar2', Album)[0]])
-        baz_baz, _ = parse_query_string('BAZ baz', Item)
-        baz_baz2, _ = parse_query_string('BAZ baz', Album)
+        foo_foo = parse_query_string('FOO foo', Item)
+        baz_baz = parse_query_string('BAZ baz', Item)
+        baz_baz2 = parse_query_string('BAZ baz', Album)
+        bar_bar = OrQuery((parse_query_string('BAR bar1', Album)[0],
+                           parse_query_string('BAR bar2', Album)[0]))
         self.assertEqual(spl._unmatched_playlists, set([
-            ('foo', foo_foo, None),
-            ('bar', None, bar_bar),
-            ('baz', baz_baz, baz_baz2)
+            ('foo', foo_foo, (None, None)),
+            ('baz', baz_baz, baz_baz2),
+            ('bar', (None, None), (bar_bar, None)),
         ]))
 
     def test_db_changes(self):
@@ -80,9 +81,9 @@ class SmartPlaylistTest(unittest.TestCase):
         q2 = Mock()
         q2.matches.side_effect = {i1: False, i2: True}.__getitem__
 
-        pl1 = ('1', q1, a_q1)
-        pl2 = ('2', None, a_q1)
-        pl3 = ('3', q2, None)
+        pl1 = '1', (q1, None), (a_q1, None)
+        pl2 = '2', (None, None), (a_q1, None)
+        pl3 = '3', (q2, None), (None, None)
 
         spl._unmatched_playlists = set([pl1, pl2, pl3])
         spl._matched_playlists = set()
@@ -115,7 +116,7 @@ class SmartPlaylistTest(unittest.TestCase):
         lib = Mock()
         lib.items.return_value = [i]
         lib.albums.return_value = []
-        pl = 'my_playlist.m3u', q, a_q
+        pl = 'my_playlist.m3u', (q, None), (a_q, None)
         spl._matched_playlists = [pl]
 
         dir = mkdtemp()
@@ -127,8 +128,8 @@ class SmartPlaylistTest(unittest.TestCase):
             rmtree(dir)
             raise
 
-        lib.items.assert_called_once_with(q)
-        lib.albums.assert_called_once_with(a_q)
+        lib.items.assert_called_once_with(q, None)
+        lib.albums.assert_called_once_with(a_q, None)
 
         m3u_filepath = path.join(dir, pl[0])
         self.assertTrue(path.exists(m3u_filepath))
@@ -177,3 +178,11 @@ class SmartPlaylistCLITest(unittest.TestCase, TestHelper):
         for name in ('my_playlist.m3u', 'all.m3u'):
             with open(path.join(self.temp_dir, name), 'r') as f:
                 self.assertEqual(f.read(), self.item.path + b"\n")
+
+
+def suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)
+
+
+if __name__ == b'__main__':
+    unittest.main(defaultTest='suite')

From bcd57bd2b5f0c67b0ccfb650f96cbf60cf79328a Mon Sep 17 00:00:00 2001
From: Bruno Cauet <brunocauet@gmail.com>
Date: Wed, 18 Mar 2015 18:46:24 +0100
Subject: [PATCH 17/22] Test queries building sort management in smartplaylist

Slighly modify Sort parsing: avoid building MultiplSort() instances
comptised of a single sort, but return that sort instead, since it wraps
things with any gain.
---
 beets/dbcore/query.py      | 21 +++++++++++++++++++++
 beets/dbcore/queryparse.py |  6 ++++--
 beetsplug/smartplaylist.py | 15 ++++++++++-----
 test/test_dbcore.py        | 11 ++++++-----
 test/test_smartplaylist.py | 27 ++++++++++++++++++++++++++-
 5 files changed, 67 insertions(+), 13 deletions(-)

diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py
index a51805bed..965c13868 100644
--- a/beets/dbcore/query.py
+++ b/beets/dbcore/query.py
@@ -625,6 +625,12 @@ class Sort(object):
         """
         return False
 
+    def __hash__(self):
+        return 0
+
+    def __eq__(self, other):
+        return type(self) == type(other)
+
 
 class MultipleSort(Sort):
     """Sort that encapsulates multiple sub-sorts.
@@ -686,6 +692,13 @@ class MultipleSort(Sort):
     def __repr__(self):
         return u'MultipleSort({0})'.format(repr(self.sorts))
 
+    def __hash__(self):
+        return hash(tuple(self.sorts))
+
+    def __eq__(self, other):
+        return super(MultipleSort, self).__eq__(other) and \
+               self.sorts == other.sorts
+
 
 class FieldSort(Sort):
     """An abstract sort criterion that orders by a specific field (of
@@ -709,6 +722,14 @@ class FieldSort(Sort):
             '+' if self.ascending else '-',
         )
 
+    def __hash__(self):
+        return hash((self.field, self.ascending))
+
+    def __eq__(self, other):
+        return super(FieldSort, self).__eq__(other) and \
+               self.field == other.field and \
+               self.ascending == other.ascending
+
 
 class FixedFieldSort(FieldSort):
     """Sort object to sort on a fixed field.
diff --git a/beets/dbcore/queryparse.py b/beets/dbcore/queryparse.py
index 6628bebf0..1dcf9c4b3 100644
--- a/beets/dbcore/queryparse.py
+++ b/beets/dbcore/queryparse.py
@@ -152,12 +152,14 @@ def sort_from_strings(model_cls, sort_parts):
     """Create a `Sort` from a list of sort criteria (strings).
     """
     if not sort_parts:
-        return query.NullSort()
+        sort = query.NullSort()
+    elif len(sort_parts) == 1:
+        sort = construct_sort_part(model_cls, sort_parts[0])
     else:
         sort = query.MultipleSort()
         for part in sort_parts:
             sort.add_sort(construct_sort_part(model_cls, part))
-        return sort
+    return sort
 
 
 def parse_sorted_query(model_cls, parts, prefixes={},
diff --git a/beetsplug/smartplaylist.py b/beetsplug/smartplaylist.py
index 80e1faa89..34fa94979 100644
--- a/beetsplug/smartplaylist.py
+++ b/beetsplug/smartplaylist.py
@@ -107,14 +107,19 @@ class SmartPlaylistPlugin(BeetsPlugin):
                     queries, sorts = zip(*(parse_query_string(q, Model)
                                            for q in qs))
                     query = OrQuery(queries)
-                    sort = MultipleSort()
+                    final_sorts = []
                     for s in sorts:
                         if s:
-                            sort.add_sort(s)
-                    if not sort.sorts:
+                            if isinstance(s, MultipleSort):
+                                final_sorts += s.sorts
+                            else:
+                                final_sorts.append(s)
+                    if not final_sorts:
                         sort = None
-                    elif len(sort.sorts) == 1:
-                        sort = sort.sorts[0]
+                    elif len(final_sorts) == 1:
+                        sort, = final_sorts
+                    else:
+                        sort = MultipleSort(final_sorts)
                     query_and_sort = query, sort
 
                 playlist_data += (query_and_sort,)
diff --git a/test/test_dbcore.py b/test/test_dbcore.py
index dffe9ae75..39867ceb0 100644
--- a/test/test_dbcore.py
+++ b/test/test_dbcore.py
@@ -449,6 +449,7 @@ class SortFromStringsTest(unittest.TestCase):
     def test_zero_parts(self):
         s = self.sfs([])
         self.assertIsInstance(s, dbcore.query.NullSort)
+        self.assertEqual(s, dbcore.query.NullSort())
 
     def test_one_parts(self):
         s = self.sfs(['field+'])
@@ -461,17 +462,17 @@ class SortFromStringsTest(unittest.TestCase):
 
     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)
+        self.assertIsInstance(s, dbcore.query.FixedFieldSort)
+        self.assertEqual(s, dbcore.query.FixedFieldSort('field_one'))
 
     def test_flex_field_sort(self):
         s = self.sfs(['flex_field+'])
-        self.assertIsInstance(s, dbcore.query.MultipleSort)
-        self.assertIsInstance(s.sorts[0], dbcore.query.SlowFieldSort)
+        self.assertIsInstance(s, dbcore.query.SlowFieldSort)
+        self.assertEqual(s, dbcore.query.SlowFieldSort('flex_field'))
 
     def test_special_sort(self):
         s = self.sfs(['some_sort+'])
-        self.assertIsInstance(s.sorts[0], TestSort)
+        self.assertIsInstance(s, TestSort)
 
 
 class ResultsIteratorTest(unittest.TestCase):
diff --git a/test/test_smartplaylist.py b/test/test_smartplaylist.py
index 42bffc8a5..4ed2cb4ba 100644
--- a/test/test_smartplaylist.py
+++ b/test/test_smartplaylist.py
@@ -24,7 +24,7 @@ from mock import Mock, MagicMock
 from beetsplug.smartplaylist import SmartPlaylistPlugin
 from beets.library import Item, Album, parse_query_string
 from beets.dbcore import OrQuery
-from beets.dbcore.query import NullSort
+from beets.dbcore.query import NullSort, MultipleSort, FixedFieldSort
 from beets.util import syspath
 from beets.ui import UserError
 from beets import config
@@ -66,6 +66,31 @@ class SmartPlaylistTest(unittest.TestCase):
             ('bar', (None, None), (bar_bar, None)),
         ]))
 
+    def test_build_queries_with_sorts(self):
+        spl = SmartPlaylistPlugin()
+        config['smartplaylist']['playlists'].set([
+            {'name': 'no_sort', 'query': 'foo'},
+            {'name': 'one_sort', 'query': 'foo year+'},
+            {'name': 'only_empty_sorts', 'query': ['foo', 'bar']},
+            {'name': 'one_non_empty_sort', 'query': ['foo year+', 'bar']},
+            {'name': 'multiple_sorts', 'query': ['foo year+', 'bar genre-']},
+            {'name': 'mixed', 'query': ['foo year+', 'bar', 'baz genre+ id-']}
+        ])
+
+        spl.build_queries()
+        sorts = {name: sort for name, (_, sort), _ in spl._unmatched_playlists}
+
+        asseq = self.assertEqual  # less cluttered code
+        S = FixedFieldSort  # short cut since we're only dealing with this
+        asseq(sorts["no_sort"], NullSort())
+        asseq(sorts["one_sort"], S('year'))
+        asseq(sorts["only_empty_sorts"], None)
+        asseq(sorts["one_non_empty_sort"], S('year'))
+        asseq(sorts["multiple_sorts"],
+              MultipleSort([S('year'), S('genre', False)]))
+        asseq(sorts["mixed"],
+              MultipleSort([S('year'), S('genre'), S('id', False)]))
+
     def test_db_changes(self):
         spl = SmartPlaylistPlugin()
 

From 86443c076dd52b55a75411090ff84a668448d714 Mon Sep 17 00:00:00 2001
From: Bruno Cauet <brunocauet@gmail.com>
Date: Wed, 18 Mar 2015 19:00:44 +0100
Subject: [PATCH 18/22] Document smartplaylist sorting behavior.

---
 docs/conf.py                   |  2 +-
 docs/plugins/smartplaylist.rst | 12 ++++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/docs/conf.py b/docs/conf.py
index 4aeb66d33..82fc15da8 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -23,7 +23,7 @@ extlinks = {
 }
 
 # Options for HTML output
-html_theme = 'classic'
+html_theme = 'default'
 htmlhelp_basename = 'beetsdoc'
 
 # Options for LaTeX output
diff --git a/docs/plugins/smartplaylist.rst b/docs/plugins/smartplaylist.rst
index b414e1eb0..2f691c4fe 100644
--- a/docs/plugins/smartplaylist.rst
+++ b/docs/plugins/smartplaylist.rst
@@ -44,6 +44,18 @@ You can also gather the results of several queries by putting them in a list.
     - name: 'BeatlesUniverse.m3u'
       query: ['artist:beatles', 'genre:"beatles cover"']
 
+Note that since beets query syntax is in effect, you can also use sorting
+directives::
+
+    - name: 'Chronological Beatles'
+      query: 'artist:Beatles year+'
+    - name: 'Mixed Rock'
+      query: ['artist:Beatles year+', 'artist:"Led Zeppelin" bitrate+']
+
+The former case behaves as expected, however please note that in the latter the
+sorts will be merged: ``year+ bitrate+`` will apply to both the Beatles and Led
+Zeppelin. If that bothers you, please get in touch.
+
 For querying albums instead of items (mainly useful with extensible fields),
 use the ``album_query`` field. ``query`` and ``album_query`` can be used at the
 same time. The following example gathers single items but also items belonging

From 12c2511b1fe72a29ab7bfe47ff509897dfd6780b Mon Sep 17 00:00:00 2001
From: Bruno Cauet <brunocauet@gmail.com>
Date: Thu, 19 Mar 2015 13:37:29 +0100
Subject: [PATCH 19/22] Smartplaylist tests: don't use a dict literal

fucking py26!
---
 test/test_smartplaylist.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/test/test_smartplaylist.py b/test/test_smartplaylist.py
index 4ed2cb4ba..4af1cfeb6 100644
--- a/test/test_smartplaylist.py
+++ b/test/test_smartplaylist.py
@@ -78,7 +78,8 @@ class SmartPlaylistTest(unittest.TestCase):
         ])
 
         spl.build_queries()
-        sorts = {name: sort for name, (_, sort), _ in spl._unmatched_playlists}
+        sorts = dict((name, sort)
+                     for name, (_, sort), _ in spl._unmatched_playlists)
 
         asseq = self.assertEqual  # less cluttered code
         S = FixedFieldSort  # short cut since we're only dealing with this

From 7f34c101d70126497afbf448c9d7ffb6faf6655a Mon Sep 17 00:00:00 2001
From: Bruno Cauet <brunocauet@gmail.com>
Date: Thu, 19 Mar 2015 13:37:53 +0100
Subject: [PATCH 20/22] Plugin events: restore backwards compatibility

An event listener that expects too few arguments won't crash, arguments
will be cut off instead. This restores a backwards-compatibility hack
that was removed in commit 327b62b6.
---
 beets/plugins.py     | 28 +++++++++----
 test/test_plugins.py | 94 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 112 insertions(+), 10 deletions(-)

diff --git a/beets/plugins.py b/beets/plugins.py
index bee4d9f32..c642e9318 100755
--- a/beets/plugins.py
+++ b/beets/plugins.py
@@ -17,6 +17,7 @@
 from __future__ import (division, absolute_import, print_function,
                         unicode_literals)
 
+import inspect
 import traceback
 import re
 from collections import defaultdict
@@ -100,26 +101,37 @@ class BeetsPlugin(object):
         `self.import_stages`. Wrapping provides some bookkeeping for the
         plugin: specifically, the logging level is adjusted to WARNING.
         """
-        return [self._set_log_level(logging.WARNING, import_stage)
+        return [self._set_log_level_and_params(logging.WARNING, import_stage)
                 for import_stage in self.import_stages]
 
-    def _set_log_level(self, base_log_level, func):
+    def _set_log_level_and_params(self, base_log_level, func):
         """Wrap `func` to temporarily set this plugin's logger level to
         `base_log_level` + config options (and restore it to its previous
-        value after the function returns).
+        value after the function returns). Also determines which params may not
+        be sent for backwards-compatibility.
 
-        Note that that value may not be NOTSET, e.g. if a plugin import stage
-        triggers an event that is listened this very same plugin
+        Note that the log level value may not be NOTSET, e.g. if a plugin
+        import stage triggers an event that is listened this very same plugin.
         """
+        argspec = inspect.getargspec(func)
+
         @wraps(func)
         def wrapper(*args, **kwargs):
             old_log_level = self._log.level
             verbosity = beets.config['verbose'].get(int)
             log_level = max(logging.DEBUG, base_log_level - 10 * verbosity)
             self._log.setLevel(log_level)
-
             try:
-                return func(*args, **kwargs)
+                try:
+                    return func(*args, **kwargs)
+                except TypeError as exc:
+                    if exc.args[0].startswith(func.__name__):
+                        # caused by 'func' and not stuff internal to 'func'
+                        kwargs = dict((arg, val) for arg, val in kwargs.items()
+                                      if arg in argspec.args)
+                        return func(*args, **kwargs)
+                    else:
+                        raise
             finally:
                 self._log.setLevel(old_log_level)
         return wrapper
@@ -186,7 +198,7 @@ class BeetsPlugin(object):
     def register_listener(self, event, func):
         """Add a function as a listener for the specified event.
         """
-        wrapped_func = self._set_log_level(logging.WARNING, func)
+        wrapped_func = self._set_log_level_and_params(logging.WARNING, func)
 
         cls = self.__class__
         if cls.listeners is None or cls._raw_listeners is None:
diff --git a/test/test_plugins.py b/test/test_plugins.py
index c9c5be502..2e8bedca1 100644
--- a/test/test_plugins.py
+++ b/test/test_plugins.py
@@ -16,8 +16,9 @@ from __future__ import (division, absolute_import, print_function,
                         unicode_literals)
 
 import os
-from mock import patch
+from mock import patch, Mock
 import shutil
+import itertools
 
 from beets.importer import SingletonImportTask, SentinelImportTask, \
     ArchiveImportTask
@@ -57,7 +58,6 @@ class ItemTypesTest(unittest.TestCase, TestHelper):
 
     def setUp(self):
         self.setup_plugin_loader()
-        self.setup_beets()
 
     def tearDown(self):
         self.teardown_plugin_loader()
@@ -309,6 +309,96 @@ class ListenersTest(unittest.TestCase, TestHelper):
         self.assertEqual(DummyPlugin._raw_listeners['cli_exit'],
                          [d.dummy, d2.dummy])
 
+    @patch('beets.plugins.find_plugins')
+    @patch('beets.plugins.inspect')
+    def test_events_called(self, mock_inspect, mock_find_plugins):
+        mock_inspect.getargspec.return_value = None
+
+        class DummyPlugin(plugins.BeetsPlugin):
+            def __init__(self):
+                super(DummyPlugin, self).__init__()
+                self.foo = Mock(__name__=b'foo')
+                self.register_listener('event_foo', self.foo)
+                self.bar = Mock(__name__=b'bar')
+                self.register_listener('event_bar', self.bar)
+
+        d = DummyPlugin()
+        mock_find_plugins.return_value = d,
+
+        plugins.send('event')
+        d.foo.assert_has_calls([])
+        d.bar.assert_has_calls([])
+
+        plugins.send('event_foo', var="tagada")
+        d.foo.assert_called_once_with(var="tagada")
+        d.bar.assert_has_calls([])
+
+    @patch('beets.plugins.find_plugins')
+    def test_listener_params(self, mock_find_plugins):
+        test = self
+
+        class DummyPlugin(plugins.BeetsPlugin):
+            def __init__(self):
+                super(DummyPlugin, self).__init__()
+                for i in itertools.count(1):
+                    try:
+                        meth = getattr(self, 'dummy{0}'.format(i))
+                    except AttributeError:
+                        break
+                    self.register_listener('event{0}'.format(i), meth)
+
+            def dummy1(self, foo):
+                test.assertEqual(foo, 5)
+
+            def dummy2(self, foo=None):
+                test.assertEqual(foo, 5)
+
+            def dummy3(self):
+                # argument cut off
+                pass
+
+            def dummy4(self, bar=None):
+                # argument cut off
+                pass
+
+            def dummy5(self, bar):
+                test.assertFalse(True)
+
+            # more complex exmaples
+
+            def dummy6(self, foo, bar=None):
+                test.assertEqual(foo, 5)
+                test.assertEqual(bar, None)
+
+            def dummy7(self, foo, **kwargs):
+                test.assertEqual(foo, 5)
+                test.assertEqual(kwargs, {})
+
+            def dummy8(self, foo, bar, **kwargs):
+                test.assertFalse(True)
+
+            def dummy9(self, **kwargs):
+                test.assertEqual(kwargs, {"foo": 5})
+
+        d = DummyPlugin()
+        mock_find_plugins.return_value = d,
+
+        plugins.send('event1', foo=5)
+        plugins.send('event2', foo=5)
+        plugins.send('event3', foo=5)
+        plugins.send('event4', foo=5)
+
+        with self.assertRaises(TypeError):
+            plugins.send('event5', foo=5)
+
+        plugins.send('event6', foo=5)
+        plugins.send('event7', foo=5)
+
+        with self.assertRaises(TypeError):
+            plugins.send('event8', foo=5)
+
+        plugins.send('event9', foo=5)
+
 
 def suite():
     return unittest.TestLoader().loadTestsFromName(__name__)

From 7be7a78d5a4d65566144a4d165ebc683e4b8cfdd Mon Sep 17 00:00:00 2001
From: Bruno Cauet <brunocauet@gmail.com>
Date: Thu, 19 Mar 2015 13:41:55 +0100
Subject: [PATCH 21/22] Restore recent html_theme value in docs/conf.py

The old value got restored in commit 86443c076 by mistake.
---
 docs/conf.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/conf.py b/docs/conf.py
index 82fc15da8..4aeb66d33 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -23,7 +23,7 @@ extlinks = {
 }
 
 # Options for HTML output
-html_theme = 'default'
+html_theme = 'classic'
 htmlhelp_basename = 'beetsdoc'
 
 # Options for LaTeX output

From a96f2fd3c940366b6d5c0f722bc9ecea3689f6dc Mon Sep 17 00:00:00 2001
From: Bruno Cauet <brunocauet@gmail.com>
Date: Thu, 19 Mar 2015 13:51:20 +0100
Subject: [PATCH 22/22] Fix pep8

---
 beets/dbcore/query.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/beets/dbcore/query.py b/beets/dbcore/query.py
index 965c13868..330913706 100644
--- a/beets/dbcore/query.py
+++ b/beets/dbcore/query.py
@@ -697,7 +697,7 @@ class MultipleSort(Sort):
 
     def __eq__(self, other):
         return super(MultipleSort, self).__eq__(other) and \
-               self.sorts == other.sorts
+            self.sorts == other.sorts
 
 
 class FieldSort(Sort):
@@ -727,8 +727,8 @@ class FieldSort(Sort):
 
     def __eq__(self, other):
         return super(FieldSort, self).__eq__(other) and \
-               self.field == other.field and \
-               self.ascending == other.ascending
+            self.field == other.field and \
+            self.ascending == other.ascending
 
 
 class FixedFieldSort(FieldSort):