From b34a8c1f738db684a75eb90a5ca0f5dd73a77e07 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Sun, 19 Jan 2014 13:47:09 +0100
Subject: [PATCH 01/35] mpd: Add MPD tokenizer

- Adds tests for correctness of tokenizer (which also would have shown shlex
  wouldn't have worked).
- Should conform with the original's behavior, though we won't be able to match
  the error messages without a lot of extra work as a non-regexp version is
  likely a no go on python due to speed.
---
 mopidy/mpd/protocol/__init__.py      |  40 +++++++++
 tests/mpd/protocol/test_tokenizer.py | 129 +++++++++++++++++++++++++++
 2 files changed, 169 insertions(+)
 create mode 100644 tests/mpd/protocol/test_tokenizer.py

diff --git a/mopidy/mpd/protocol/__init__.py b/mopidy/mpd/protocol/__init__.py
index 8a0993d8..5504086a 100644
--- a/mopidy/mpd/protocol/__init__.py
+++ b/mopidy/mpd/protocol/__init__.py
@@ -93,3 +93,43 @@ def load_protocol_modules():
         audio_output, channels, command_list, connection, current_playlist,
         empty, music_db, playback, reflection, status, stickers,
         stored_playlists)
+
+
+WORD_RE = re.compile(r"""
+    ^                 # Leading whitespace is not allowed
+    ([a-z][a-z0-9_]*) # A command name
+    (?:\s+|$)         # trailing whitespace or EOS
+    (.*)              # Possibly a remainder to be parsed
+    """, re.VERBOSE)
+
+# Quotes matching is an unrolled version of "(?:[^"\\]|\\.)*"
+PARAM_RE = re.compile(r"""
+    ^                               # Leading whitespace is not allowed
+    (?:
+        ([^%(unprintable)s"\\]+)    # ord(char) < 0x20, not ", not backslash
+        |                           # or
+        "([^"\\]*(?:\\.[^"\\]*)*)"  # anything surrounded by quotes
+    )
+    (?:\s+|$)                       # trailing whitespace or EOS
+    (.*)                            # Possibly a remainder to be parsed
+    """ % {'unprintable': ''.join(map(chr, range(0x21)))}, re.VERBOSE)
+
+UNESCAPE_RE = re.compile(r'\\(.)')  # Backslash escapes any following char.
+
+
+# TODO: update exception usage and messages
+def tokenize(line):
+    match = WORD_RE.match(line)
+    if not match:
+        raise Exception('Invalid command')
+    command, remainder = match.groups()
+    result = [command]
+
+    while remainder:
+        match = PARAM_RE.match(remainder)
+        if not match:
+            raise Exception('Invalid parameter')
+        unquoted, quoted, remainder = match.groups()
+        result.append(unquoted or UNESCAPE_RE.sub(r'\g<1>', quoted))
+
+    return result
diff --git a/tests/mpd/protocol/test_tokenizer.py b/tests/mpd/protocol/test_tokenizer.py
new file mode 100644
index 00000000..27c9ca2d
--- /dev/null
+++ b/tests/mpd/protocol/test_tokenizer.py
@@ -0,0 +1,129 @@
+#encoding: utf-8
+
+from __future__ import unicode_literals
+
+import unittest
+
+from mopidy.mpd import protocol
+
+
+class TestTokenizer(unittest.TestCase):
+    def assertTokenizeEquals(self, expected, line):
+        self.assertEqual(expected, protocol.tokenize(line))
+
+    def assertTokenizeRaises(self, exception, line):
+        with self.assertRaises(exception):
+            protocol.tokenize(line)
+
+    def test_empty_string(self):
+        self.assertTokenizeRaises(Exception, '')
+
+    def test_whitespace(self):
+        self.assertTokenizeRaises(Exception, '   ')
+        self.assertTokenizeRaises(Exception, '\t\t\t')
+
+    def test_command(self):
+        self.assertTokenizeEquals(['test'], 'test')
+        self.assertTokenizeEquals(['test123'], 'test123')
+        self.assertTokenizeEquals(['foo_bar'], 'foo_bar')
+
+    def test_command_trailing_whitespace(self):
+        self.assertTokenizeEquals(['test'], 'test   ')
+        self.assertTokenizeEquals(['test'], 'test\t\t\t')
+
+    def test_command_leading_whitespace(self):
+        self.assertTokenizeRaises(Exception, ' test')
+        self.assertTokenizeRaises(Exception, '\ttest')
+
+    def test_invalid_command(self):
+        self.assertTokenizeRaises(Exception, 'foo/bar')
+        self.assertTokenizeRaises(Exception, 'æøå')
+        self.assertTokenizeRaises(Exception, 'test?')
+        self.assertTokenizeRaises(Exception, 'te"st')
+
+    def test_unquoted_param(self):
+        self.assertTokenizeEquals(['test', 'param'], 'test param')
+        self.assertTokenizeEquals(['test', 'param'], 'test\tparam')
+
+    def test_unquoted_param_leading_whitespace(self):
+        self.assertTokenizeEquals(['test', 'param'], 'test  param')
+        self.assertTokenizeEquals(['test', 'param'], 'test\t\tparam')
+
+    def test_unquoted_param_trailing_whitespace(self):
+        self.assertTokenizeEquals(['test', 'param'], 'test param  ')
+        self.assertTokenizeEquals(['test', 'param'], 'test param\t\t')
+
+    def test_unquoted_param_invalid_chars(self):
+        self.assertTokenizeRaises(Exception, 'test par"m')
+        self.assertTokenizeRaises(Exception, 'test foo\\bar')
+        self.assertTokenizeRaises(Exception, 'test foo\bbar')
+        self.assertTokenizeRaises(Exception, 'test "foo"bar')
+        self.assertTokenizeRaises(Exception, 'test foo"bar"baz')
+
+    def test_unquoted_param_numbers(self):
+        self.assertTokenizeEquals(['test', '123'], 'test 123')
+        self.assertTokenizeEquals(['test', '+123'], 'test +123')
+        self.assertTokenizeEquals(['test', '-123'], 'test -123')
+        self.assertTokenizeEquals(['test', '3.14'], 'test 3.14')
+
+    def test_unquoted_param_extended_chars(self):
+        self.assertTokenizeEquals(['test', 'æøå'], 'test æøå')
+        self.assertTokenizeEquals(['test', '?#\'$'], 'test ?#\'$')
+        self.assertTokenizeEquals(['test', '/foo/bar/'], 'test /foo/bar/')
+
+    def test_unquoted_params(self):
+        self.assertTokenizeEquals(['test', 'foo', 'bar'], 'test foo bar')
+
+    def test_quoted_param(self):
+        self.assertTokenizeEquals(['test', 'param'], 'test "param"')
+        self.assertTokenizeEquals(['test', 'param'], 'test\t"param"')
+
+    def test_quoted_param_leading_whitespace(self):
+        self.assertTokenizeEquals(['test', 'param'], 'test  "param"')
+        self.assertTokenizeEquals(['test', 'param'], 'test\t\t"param"')
+
+    def test_quoted_param_trailing_whitespace(self):
+        self.assertTokenizeEquals(['test', 'param'], 'test "param"  ')
+        self.assertTokenizeEquals(['test', 'param'], 'test "param"\t\t')
+
+    def test_quoted_param_invalid_chars(self):
+        self.assertTokenizeRaises(Exception, 'test "par"m"')
+
+    def test_quoted_param_numbers(self):
+        self.assertTokenizeEquals(['test', '123'], 'test "123"')
+        self.assertTokenizeEquals(['test', '+123'], 'test "+123"')
+        self.assertTokenizeEquals(['test', '-123'], 'test "-123"')
+        self.assertTokenizeEquals(['test', '3.14'], 'test "3.14"')
+
+    def test_quoted_param_spaces(self):
+        self.assertTokenizeEquals(['test', 'foo bar'], 'test "foo bar"')
+        self.assertTokenizeEquals(['test', 'foo bar'], 'test "foo bar"')
+        self.assertTokenizeEquals(['test', ' param\t'], 'test " param\t"')
+
+    def test_quoted_param_extended_chars(self):
+        self.assertTokenizeEquals(['test', 'æøå'], 'test "æøå"')
+        self.assertTokenizeEquals(['test', '?#$'], 'test "?#$"')
+        self.assertTokenizeEquals(['test', '/foo/bar/'], 'test "/foo/bar/"')
+
+    def test_quoted_param_escaping(self):
+        self.assertTokenizeEquals(['test', '\\'], r'test "\\"')
+        self.assertTokenizeEquals(['test', '"'], r'test "\""')
+        self.assertTokenizeEquals(['test', ' '], r'test "\ "')
+        self.assertTokenizeEquals(['test', '\\n'], r'test "\\\n"')
+
+    def test_quoted_params(self):
+        self.assertTokenizeEquals(['test', 'foo', 'bar'], 'test "foo" "bar"')
+
+    def test_mixed_params(self):
+        self.assertTokenizeEquals(['test', 'foo', 'bar'], 'test foo "bar"')
+        self.assertTokenizeEquals(['test', 'foo', 'bar'], 'test "foo" bar')
+        self.assertTokenizeEquals(['test', '1', '2'], 'test 1 "2"')
+        self.assertTokenizeEquals(['test', '1', '2'], 'test "1" 2')
+
+        self.assertTokenizeEquals(['test', 'foo bar', 'baz', '123'],
+                                  'test "foo bar" baz 123')
+        self.assertTokenizeEquals(['test', 'foo"bar', 'baz', '123'],
+                                  r'test "foo\"bar" baz 123')
+
+    def test_unbalanced_quotes(self):
+        self.assertTokenizeRaises(Exception, 'test "foo bar" baz"')

From 455f3dd4031c9a7cb1f26ab3126026c38e2e147e Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Sun, 19 Jan 2014 19:59:07 +0100
Subject: [PATCH 02/35] mpd: Write commands helper for tokenized command
 handling

This provides a class which works as a command registry. Functions are added to
the registry through a method on the class `add`.  Add acts as a decorator
taking in the name of the command and optionally type converters for arguments.

When handling requests, we can now tokenize the line, and then just pass the
arguments to our commands helper. It will lookup the correct handler and apply
any validation before calling the original function.

For the sake of testing the original function is not wrapped. This the
functions, and anything testing them directly can simply assume pre-converted
data as long as type annotations are in place.

A sample usage would be:

    @protocol.commands.add('addid', position=protocol.integer)
    def addid(context, uri, position=None)
        pass

    def handle_request(line):
        tokens = protocol.tokenizer(line)
        context = ...
        result = protocol.commands.call(tokens, context=context)
---
 mopidy/mpd/protocol/__init__.py               |  65 +++++-
 tests/mpd/protocol/test_commands_decorator.py | 187 ++++++++++++++++++
 2 files changed, 250 insertions(+), 2 deletions(-)
 create mode 100644 tests/mpd/protocol/test_commands_decorator.py

diff --git a/mopidy/mpd/protocol/__init__.py b/mopidy/mpd/protocol/__init__.py
index 5504086a..e7ffb32c 100644
--- a/mopidy/mpd/protocol/__init__.py
+++ b/mopidy/mpd/protocol/__init__.py
@@ -12,7 +12,8 @@ implement our own MPD server which is compatible with the numerous existing
 
 from __future__ import unicode_literals
 
-from collections import namedtuple
+import collections
+import inspect
 import re
 
 from mopidy.utils import formatting
@@ -26,7 +27,7 @@ LINE_TERMINATOR = '\n'
 #: The MPD protocol version is 0.17.0.
 VERSION = '0.17.0'
 
-MpdCommand = namedtuple('MpdCommand', ['name', 'auth_required'])
+MpdCommand = collections.namedtuple('MpdCommand', ['name', 'auth_required'])
 
 #: Set of all available commands, represented as :class:`MpdCommand` objects.
 mpd_commands = set()
@@ -133,3 +134,63 @@ def tokenize(line):
         result.append(unquoted or UNESCAPE_RE.sub(r'\g<1>', quoted))
 
     return result
+
+
+def integer(value):
+    if value is None:
+        raise ValueError('None is not a valid integer')
+    return int(value)
+
+
+def boolean(value):
+    if value in ('1', '0'):
+        return bool(int(value))
+    raise ValueError('%r is not 0 or 1' % value)
+
+
+class Commands(object):
+    def __init__(self):
+        self.handlers = {}
+
+    def add(self, command, **validators):
+        def wrapper(func):
+            if command in self.handlers:
+                raise Exception('%s already registered' % command)
+
+            args, varargs, keywords, defaults = inspect.getargspec(func)
+            defaults = dict(zip(args[-len(defaults or []):], defaults or []))
+
+            if not args and not varargs:
+                raise TypeError('Handler must accept at least one argument.')
+
+            if len(args) > 1 and varargs:
+                raise TypeError(
+                    '*args may not be combined with regular argmuments')
+
+            if not set(validators.keys()).issubset(args):
+                raise TypeError('Validator for non-existent arg passed')
+
+            if keywords:
+                raise TypeError('**kwargs are not permitted')
+
+            def validate(*args, **kwargs):
+                if varargs:
+                    return func(*args, **kwargs)
+                callargs = inspect.getcallargs(func, *args, **kwargs)
+                for key, value in callargs.items():
+                    default = defaults.get(key, object())
+                    if key in validators and value != default:
+                        callargs[key] = validators[key](value)
+                return func(**callargs)
+
+            self.handlers[command] = validate
+            return func
+        return wrapper
+
+    def call(self, args, context=None):
+        if not args:
+            raise TypeError('No args provided')
+        command = args.pop(0)
+        if command not in self.handlers:
+            raise LookupError('Unknown command')
+        return self.handlers[command](context, *args)
diff --git a/tests/mpd/protocol/test_commands_decorator.py b/tests/mpd/protocol/test_commands_decorator.py
new file mode 100644
index 00000000..0d2d2ad3
--- /dev/null
+++ b/tests/mpd/protocol/test_commands_decorator.py
@@ -0,0 +1,187 @@
+#encoding: utf-8
+
+from __future__ import unicode_literals
+
+import unittest
+
+from mopidy.mpd import protocol
+
+
+class TestConverts(unittest.TestCase):
+    def test_integer(self):
+        self.assertEqual(123, protocol.integer('123'))
+        self.assertEqual(-123, protocol.integer('-123'))
+        self.assertEqual(123, protocol.integer('+123'))
+        self.assertRaises(ValueError, protocol.integer, '3.14')
+        self.assertRaises(ValueError, protocol.integer, '')
+        self.assertRaises(ValueError, protocol.integer, 'abc')
+        self.assertRaises(ValueError, protocol.integer, '12 34')
+
+    def test_boolean(self):
+        self.assertEqual(True, protocol.boolean('1'))
+        self.assertEqual(False, protocol.boolean('0'))
+        self.assertRaises(ValueError, protocol.boolean, '3.14')
+        self.assertRaises(ValueError, protocol.boolean, '')
+        self.assertRaises(ValueError, protocol.boolean, 'true')
+        self.assertRaises(ValueError, protocol.boolean, 'false')
+        self.assertRaises(ValueError, protocol.boolean, 'abc')
+        self.assertRaises(ValueError, protocol.boolean, '12 34')
+
+
+class TestCommands(unittest.TestCase):
+    def setUp(self):
+        self.commands = protocol.Commands()
+
+    def test_add_as_a_decorator(self):
+        @self.commands.add('test')
+        def test(context):
+            pass
+
+    def test_register_second_command_to_same_name_fails(self):
+        func = lambda context: True
+
+        self.commands.add('foo')(func)
+        with self.assertRaises(Exception):
+            self.commands.add('foo')(func)
+
+    def test_function_only_takes_context_succeeds(self):
+        sentinel = object()
+        self.commands.add('bar')(lambda context: sentinel)
+        self.assertEqual(sentinel, self.commands.call(['bar']))
+
+    def test_function_has_required_arg_succeeds(self):
+        sentinel = object()
+        self.commands.add('bar')(lambda context, required: sentinel)
+        self.assertEqual(sentinel, self.commands.call(['bar', 'arg']))
+
+    def test_function_has_optional_args_succeeds(self):
+        sentinel = object()
+        self.commands.add('bar')(lambda context, optional=None: sentinel)
+        self.assertEqual(sentinel, self.commands.call(['bar']))
+        self.assertEqual(sentinel, self.commands.call(['bar', 'arg']))
+
+    def test_function_has_required_and_optional_args_succeeds(self):
+        sentinel = object()
+        func = lambda context, required, optional=None: sentinel
+        self.commands.add('bar')(func)
+        self.assertEqual(sentinel, self.commands.call(['bar', 'arg']))
+        self.assertEqual(sentinel, self.commands.call(['bar', 'arg', 'arg']))
+
+    def test_function_has_varargs_succeeds(self):
+        sentinel, args = object(), []
+        self.commands.add('bar')(lambda context, *args: sentinel)
+        for i in range(10):
+            self.assertEqual(sentinel, self.commands.call(['bar'] + args))
+            args.append('test')
+
+    def test_function_has_only_varags_succeeds(self):
+        sentinel = object()
+        self.commands.add('baz')(lambda *args: sentinel)
+        self.assertEqual(sentinel, self.commands.call(['baz']))
+
+    def test_function_has_no_arguments_fails(self):
+        with self.assertRaises(TypeError):
+            self.commands.add('test')(lambda: True)
+
+    def test_function_has_required_and_varargs_fails(self):
+        with self.assertRaises(TypeError):
+            func = lambda context, required, *args: True
+            self.commands.add('test')(func)
+
+    def test_function_has_optional_and_varargs_fails(self):
+        with self.assertRaises(TypeError):
+            func = lambda context, optional=None, *args: True
+            self.commands.add('test')(func)
+
+    def test_function_hash_keywordargs_fails(self):
+        with self.assertRaises(TypeError):
+            self.commands.add('test')(lambda context, **kwargs: True)
+
+    def test_call_chooses_correct_handler(self):
+        sentinel1, sentinel2, sentinel3 = object(), object(), object()
+        self.commands.add('foo')(lambda context: sentinel1)
+        self.commands.add('bar')(lambda context: sentinel2)
+        self.commands.add('baz')(lambda context: sentinel3)
+
+        self.assertEqual(sentinel1, self.commands.call(['foo']))
+        self.assertEqual(sentinel2, self.commands.call(['bar']))
+        self.assertEqual(sentinel3, self.commands.call(['baz']))
+
+    def test_call_with_nonexistent_handler(self):
+        with self.assertRaises(LookupError):
+            self.commands.call(['bar'])
+
+    def test_call_passes_context(self):
+        sentinel = object()
+        self.commands.add('foo')(lambda context: context)
+        self.assertEqual(
+            sentinel, self.commands.call(['foo'], context=sentinel))
+
+    def test_call_without_args_fails(self):
+        with self.assertRaises(TypeError):
+            self.commands.call([])
+
+    def test_call_passes_required_argument(self):
+        self.commands.add('foo')(lambda context, required: required)
+        self.assertEqual('test123', self.commands.call(['foo', 'test123']))
+
+    def test_call_passes_optional_argument(self):
+        sentinel = object()
+        self.commands.add('foo')(lambda context, optional=sentinel: optional)
+        self.assertEqual(sentinel, self.commands.call(['foo']))
+        self.assertEqual('test', self.commands.call(['foo', 'test']))
+
+    def test_call_passes_required_and_optional_argument(self):
+        func = lambda context, required, optional=None: (required, optional)
+        self.commands.add('foo')(func)
+        self.assertEqual(('arg', None), self.commands.call(['foo', 'arg']))
+        self.assertEqual(
+            ('arg', 'kwarg'), self.commands.call(['foo', 'arg', 'kwarg']))
+
+    def test_call_passes_varargs(self):
+        self.commands.add('foo')(lambda context, *args: args)
+
+    def test_call_incorrect_args(self):
+        self.commands.add('foo')(lambda context: context)
+        with self.assertRaises(TypeError):
+            self.commands.call(['foo', 'bar'])
+
+        self.commands.add('bar')(lambda context, required: context)
+        with self.assertRaises(TypeError):
+            self.commands.call(['bar', 'bar', 'baz'])
+
+        self.commands.add('baz')(lambda context, optional=None: context)
+        with self.assertRaises(TypeError):
+            self.commands.call(['baz', 'bar', 'baz'])
+
+    def test_validator_gets_applied_to_required_arg(self):
+        sentinel = object()
+        func = lambda context, required: required
+        self.commands.add('test', required=lambda v: sentinel)(func)
+        self.assertEqual(sentinel, self.commands.call(['test', 'foo']))
+
+    def test_validator_gets_applied_to_optional_arg(self):
+        sentinel = object()
+        func = lambda context, optional=None: optional
+        self.commands.add('foo', optional=lambda v: sentinel)(func)
+
+        self.assertEqual(sentinel, self.commands.call(['foo', '123']))
+
+    def test_validator_skips_optional_default(self):
+        sentinel = object()
+        func = lambda context, optional=sentinel: optional
+        self.commands.add('foo', optional=lambda v: None)(func)
+
+        self.assertEqual(sentinel, self.commands.call(['foo']))
+
+    def test_validator_applied_to_non_existent_arg_fails(self):
+        self.commands.add('foo')(lambda context, arg: arg)
+        with self.assertRaises(TypeError):
+            func = lambda context, wrong_arg: wrong_arg
+            self.commands.add('bar', arg=lambda v: v)(func)
+
+    def test_validator_called_context_fails(self):
+        return  # TODO: how to handle this
+        with self.assertRaises(TypeError):
+            func = lambda context: True
+            self.commands.add('bar', context=lambda v: v)(func)

From 335cf4e61246c706aabb2720e4dc64c223039039 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Tue, 21 Jan 2014 20:40:49 +0100
Subject: [PATCH 03/35] mpd: Return command name in find handler.

---
 mopidy/mpd/dispatcher.py     | 10 +++++-----
 tests/mpd/test_dispatcher.py |  5 +++--
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/mopidy/mpd/dispatcher.py b/mopidy/mpd/dispatcher.py
index 6aeace9d..69b5e839 100644
--- a/mopidy/mpd/dispatcher.py
+++ b/mopidy/mpd/dispatcher.py
@@ -164,21 +164,21 @@ class MpdDispatcher(object):
             raise exceptions.MpdSystemError(e)
 
     def _call_handler(self, request):
-        (handler, kwargs) = self._find_handler(request)
+        (command_name, handler, kwargs) = self._find_handler(request)
         try:
             return handler(self.context, **kwargs)
         except exceptions.MpdAckError as exc:
             if exc.command is None:
-                exc.command = handler.__name__.split('__', 1)[0]
+                exc.command = command_name
             raise
 
     def _find_handler(self, request):
+        command_name = request.split(' ')[0]
         for pattern in protocol.request_handlers:
             matches = re.match(pattern, request)
             if matches is not None:
-                return (
-                    protocol.request_handlers[pattern], matches.groupdict())
-        command_name = request.split(' ')[0]
+                handler = protocol.request_handlers[pattern]
+                return (command_name, handler, matches.groupdict())
         if command_name in [command.name for command in protocol.mpd_commands]:
             raise exceptions.MpdArgError(
                 'incorrect arguments', command=command_name)
diff --git a/tests/mpd/test_dispatcher.py b/tests/mpd/test_dispatcher.py
index c4da1714..7ef4c13b 100644
--- a/tests/mpd/test_dispatcher.py
+++ b/tests/mpd/test_dispatcher.py
@@ -43,13 +43,14 @@ class MpdDispatcherTest(unittest.TestCase):
                 e.get_mpd_ack(),
                 'ACK [5@0] {} unknown command "an_unknown_command"')
 
-    def test_find_handler_for_known_command_returns_handler_and_kwargs(self):
+    def test_find_handler_for_known_command_return_name_handler_and_args(self):
         expected_handler = lambda x: None
         request_handlers['known_command (?P<arg1>.+)'] = \
             expected_handler
-        (handler, kwargs) = self.dispatcher._find_handler(
+        (name, handler, kwargs) = self.dispatcher._find_handler(
             'known_command an_arg')
         self.assertEqual(handler, expected_handler)
+        self.assertEqual('known_command', name)
         self.assertIn('arg1', kwargs)
         self.assertEqual(kwargs['arg1'], 'an_arg')
 

From d4457403186ccc9dc2b0eb1a16532ce043f2c2a3 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Tue, 21 Jan 2014 21:10:55 +0100
Subject: [PATCH 04/35] mpd: Split out tokenizer and add proper errors.

---
 mopidy/mpd/protocol/__init__.py            | 40 -------------------
 mopidy/mpd/tokenize.py                     | 46 ++++++++++++++++++++++
 tests/mpd/{protocol => }/test_tokenizer.py | 39 +++++++++---------
 3 files changed, 66 insertions(+), 59 deletions(-)
 create mode 100644 mopidy/mpd/tokenize.py
 rename tests/mpd/{protocol => }/test_tokenizer.py (77%)

diff --git a/mopidy/mpd/protocol/__init__.py b/mopidy/mpd/protocol/__init__.py
index e7ffb32c..8aa5f3d1 100644
--- a/mopidy/mpd/protocol/__init__.py
+++ b/mopidy/mpd/protocol/__init__.py
@@ -96,46 +96,6 @@ def load_protocol_modules():
         stored_playlists)
 
 
-WORD_RE = re.compile(r"""
-    ^                 # Leading whitespace is not allowed
-    ([a-z][a-z0-9_]*) # A command name
-    (?:\s+|$)         # trailing whitespace or EOS
-    (.*)              # Possibly a remainder to be parsed
-    """, re.VERBOSE)
-
-# Quotes matching is an unrolled version of "(?:[^"\\]|\\.)*"
-PARAM_RE = re.compile(r"""
-    ^                               # Leading whitespace is not allowed
-    (?:
-        ([^%(unprintable)s"\\]+)    # ord(char) < 0x20, not ", not backslash
-        |                           # or
-        "([^"\\]*(?:\\.[^"\\]*)*)"  # anything surrounded by quotes
-    )
-    (?:\s+|$)                       # trailing whitespace or EOS
-    (.*)                            # Possibly a remainder to be parsed
-    """ % {'unprintable': ''.join(map(chr, range(0x21)))}, re.VERBOSE)
-
-UNESCAPE_RE = re.compile(r'\\(.)')  # Backslash escapes any following char.
-
-
-# TODO: update exception usage and messages
-def tokenize(line):
-    match = WORD_RE.match(line)
-    if not match:
-        raise Exception('Invalid command')
-    command, remainder = match.groups()
-    result = [command]
-
-    while remainder:
-        match = PARAM_RE.match(remainder)
-        if not match:
-            raise Exception('Invalid parameter')
-        unquoted, quoted, remainder = match.groups()
-        result.append(unquoted or UNESCAPE_RE.sub(r'\g<1>', quoted))
-
-    return result
-
-
 def integer(value):
     if value is None:
         raise ValueError('None is not a valid integer')
diff --git a/mopidy/mpd/tokenize.py b/mopidy/mpd/tokenize.py
new file mode 100644
index 00000000..2b7ab237
--- /dev/null
+++ b/mopidy/mpd/tokenize.py
@@ -0,0 +1,46 @@
+from __future__ import unicode_literals
+
+import re
+
+
+class TokenizeError(Exception):
+    pass
+
+
+WORD_RE = re.compile(r"""
+    ^                 # Leading whitespace is not allowed
+    ([a-z][a-z0-9_]*) # A command name
+    (?:\s+|$)         # trailing whitespace or EOS
+    (.*)              # Possibly a remainder to be parsed
+    """, re.VERBOSE)
+
+# Quotes matching is an unrolled version of "(?:[^"\\]|\\.)*"
+PARAM_RE = re.compile(r"""
+    ^                               # Leading whitespace is not allowed
+    (?:
+        ([^%(unprintable)s"\\]+)    # ord(char) < 0x20, not ", not backslash
+        |                           # or
+        "([^"\\]*(?:\\.[^"\\]*)*)"  # anything surrounded by quotes
+    )
+    (?:\s+|$)                       # trailing whitespace or EOS
+    (.*)                            # Possibly a remainder to be parsed
+    """ % {'unprintable': ''.join(map(chr, range(0x21)))}, re.VERBOSE)
+
+UNESCAPE_RE = re.compile(r'\\(.)')  # Backslash escapes any following char.
+
+
+def split(line):
+    match = WORD_RE.match(line)
+    if not match:
+        raise TokenizeError('Invalid word')
+    command, remainder = match.groups()
+    result = [command]
+
+    while remainder:
+        match = PARAM_RE.match(remainder)
+        if not match:
+            raise TokenizeError('Invalid parameter')
+        unquoted, quoted, remainder = match.groups()
+        result.append(unquoted or UNESCAPE_RE.sub(r'\g<1>', quoted))
+
+    return result
diff --git a/tests/mpd/protocol/test_tokenizer.py b/tests/mpd/test_tokenizer.py
similarity index 77%
rename from tests/mpd/protocol/test_tokenizer.py
rename to tests/mpd/test_tokenizer.py
index 27c9ca2d..3ed3eb02 100644
--- a/tests/mpd/protocol/test_tokenizer.py
+++ b/tests/mpd/test_tokenizer.py
@@ -4,23 +4,23 @@ from __future__ import unicode_literals
 
 import unittest
 
-from mopidy.mpd import protocol
+from mopidy.mpd import tokenize
 
 
 class TestTokenizer(unittest.TestCase):
     def assertTokenizeEquals(self, expected, line):
-        self.assertEqual(expected, protocol.tokenize(line))
+        self.assertEqual(expected, tokenize.split(line))
 
     def assertTokenizeRaises(self, exception, line):
         with self.assertRaises(exception):
-            protocol.tokenize(line)
+            tokenize.split(line)
 
     def test_empty_string(self):
-        self.assertTokenizeRaises(Exception, '')
+        self.assertTokenizeRaises(tokenize.TokenizeError, '')
 
     def test_whitespace(self):
-        self.assertTokenizeRaises(Exception, '   ')
-        self.assertTokenizeRaises(Exception, '\t\t\t')
+        self.assertTokenizeRaises(tokenize.TokenizeError, '   ')
+        self.assertTokenizeRaises(tokenize.TokenizeError, '\t\t\t')
 
     def test_command(self):
         self.assertTokenizeEquals(['test'], 'test')
@@ -32,14 +32,14 @@ class TestTokenizer(unittest.TestCase):
         self.assertTokenizeEquals(['test'], 'test\t\t\t')
 
     def test_command_leading_whitespace(self):
-        self.assertTokenizeRaises(Exception, ' test')
-        self.assertTokenizeRaises(Exception, '\ttest')
+        self.assertTokenizeRaises(tokenize.TokenizeError, ' test')
+        self.assertTokenizeRaises(tokenize.TokenizeError, '\ttest')
 
     def test_invalid_command(self):
-        self.assertTokenizeRaises(Exception, 'foo/bar')
-        self.assertTokenizeRaises(Exception, 'æøå')
-        self.assertTokenizeRaises(Exception, 'test?')
-        self.assertTokenizeRaises(Exception, 'te"st')
+        self.assertTokenizeRaises(tokenize.TokenizeError, 'foo/bar')
+        self.assertTokenizeRaises(tokenize.TokenizeError, 'æøå')
+        self.assertTokenizeRaises(tokenize.TokenizeError, 'test?')
+        self.assertTokenizeRaises(tokenize.TokenizeError, 'te"st')
 
     def test_unquoted_param(self):
         self.assertTokenizeEquals(['test', 'param'], 'test param')
@@ -54,11 +54,11 @@ class TestTokenizer(unittest.TestCase):
         self.assertTokenizeEquals(['test', 'param'], 'test param\t\t')
 
     def test_unquoted_param_invalid_chars(self):
-        self.assertTokenizeRaises(Exception, 'test par"m')
-        self.assertTokenizeRaises(Exception, 'test foo\\bar')
-        self.assertTokenizeRaises(Exception, 'test foo\bbar')
-        self.assertTokenizeRaises(Exception, 'test "foo"bar')
-        self.assertTokenizeRaises(Exception, 'test foo"bar"baz')
+        self.assertTokenizeRaises(tokenize.TokenizeError, 'test par"m')
+        self.assertTokenizeRaises(tokenize.TokenizeError, 'test foo\\bar')
+        self.assertTokenizeRaises(tokenize.TokenizeError, 'test foo\bbar')
+        self.assertTokenizeRaises(tokenize.TokenizeError, 'test "foo"bar')
+        self.assertTokenizeRaises(tokenize.TokenizeError, 'test fo"b"ar')
 
     def test_unquoted_param_numbers(self):
         self.assertTokenizeEquals(['test', '123'], 'test 123')
@@ -87,7 +87,7 @@ class TestTokenizer(unittest.TestCase):
         self.assertTokenizeEquals(['test', 'param'], 'test "param"\t\t')
 
     def test_quoted_param_invalid_chars(self):
-        self.assertTokenizeRaises(Exception, 'test "par"m"')
+        self.assertTokenizeRaises(tokenize.TokenizeError, 'test "par"m"')
 
     def test_quoted_param_numbers(self):
         self.assertTokenizeEquals(['test', '123'], 'test "123"')
@@ -126,4 +126,5 @@ class TestTokenizer(unittest.TestCase):
                                   r'test "foo\"bar" baz 123')
 
     def test_unbalanced_quotes(self):
-        self.assertTokenizeRaises(Exception, 'test "foo bar" baz"')
+        self.assertTokenizeRaises(tokenize.TokenizeError,
+                                  'test "foo bar" baz"')

From f7aff706a84fbf6b7781dabc4edad258d4202b55 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Tue, 21 Jan 2014 21:14:30 +0100
Subject: [PATCH 05/35] mpd: Move commands tests to mpd directory

---
 .../mpd/{protocol/test_commands_decorator.py => test_commands.py} | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename tests/mpd/{protocol/test_commands_decorator.py => test_commands.py} (100%)

diff --git a/tests/mpd/protocol/test_commands_decorator.py b/tests/mpd/test_commands.py
similarity index 100%
rename from tests/mpd/protocol/test_commands_decorator.py
rename to tests/mpd/test_commands.py

From f7ec1fba01744d7151f3dfe942ef45e5b602bc0d Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Tue, 21 Jan 2014 22:10:00 +0100
Subject: [PATCH 06/35] mpd: Fix tokenizer error messages to match original
 protocol

---
 mopidy/mpd/tokenize.py      | 17 +++++++++-----
 tests/mpd/test_tokenizer.py | 47 +++++++++++++++++++++----------------
 2 files changed, 38 insertions(+), 26 deletions(-)

diff --git a/mopidy/mpd/tokenize.py b/mopidy/mpd/tokenize.py
index 2b7ab237..f10e349f 100644
--- a/mopidy/mpd/tokenize.py
+++ b/mopidy/mpd/tokenize.py
@@ -3,12 +3,13 @@ from __future__ import unicode_literals
 import re
 
 
-class TokenizeError(Exception):
+class Error(Exception):
     pass
 
 
 WORD_RE = re.compile(r"""
-    ^                 # Leading whitespace is not allowed
+    ^
+    (\s*)             # Leading whitespace not allowed, capture it to report.
     ([a-z][a-z0-9_]*) # A command name
     (?:\s+|$)         # trailing whitespace or EOS
     (.*)              # Possibly a remainder to be parsed
@@ -18,7 +19,7 @@ WORD_RE = re.compile(r"""
 PARAM_RE = re.compile(r"""
     ^                               # Leading whitespace is not allowed
     (?:
-        ([^%(unprintable)s"\\]+)    # ord(char) < 0x20, not ", not backslash
+        ([^%(unprintable)s"']+)     # ord(char) < 0x20, not ", not '
         |                           # or
         "([^"\\]*(?:\\.[^"\\]*)*)"  # anything surrounded by quotes
     )
@@ -30,16 +31,20 @@ UNESCAPE_RE = re.compile(r'\\(.)')  # Backslash escapes any following char.
 
 
 def split(line):
+    if not line.strip():
+        raise Error('No command given')
     match = WORD_RE.match(line)
     if not match:
-        raise TokenizeError('Invalid word')
-    command, remainder = match.groups()
+        raise Error('Invalid word character')
+    whitespace, command, remainder = match.groups()
+    if whitespace:
+        raise Error('Letter expected')
     result = [command]
 
     while remainder:
         match = PARAM_RE.match(remainder)
         if not match:
-            raise TokenizeError('Invalid parameter')
+            raise Error('Invalid unquoted character')
         unquoted, quoted, remainder = match.groups()
         result.append(unquoted or UNESCAPE_RE.sub(r'\g<1>', quoted))
 
diff --git a/tests/mpd/test_tokenizer.py b/tests/mpd/test_tokenizer.py
index 3ed3eb02..29e25cae 100644
--- a/tests/mpd/test_tokenizer.py
+++ b/tests/mpd/test_tokenizer.py
@@ -11,16 +11,18 @@ class TestTokenizer(unittest.TestCase):
     def assertTokenizeEquals(self, expected, line):
         self.assertEqual(expected, tokenize.split(line))
 
-    def assertTokenizeRaises(self, exception, line):
-        with self.assertRaises(exception):
+    def assertTokenizeRaisesError(self, line, message=None):
+        with self.assertRaises(tokenize.Error) as cm:
             tokenize.split(line)
+        if message:
+            self.assertEqual(cm.exception.message, message)
 
     def test_empty_string(self):
-        self.assertTokenizeRaises(tokenize.TokenizeError, '')
+        self.assertTokenizeRaisesError('', 'No command given')
 
     def test_whitespace(self):
-        self.assertTokenizeRaises(tokenize.TokenizeError, '   ')
-        self.assertTokenizeRaises(tokenize.TokenizeError, '\t\t\t')
+        self.assertTokenizeRaisesError('      ', 'No command given')
+        self.assertTokenizeRaisesError('\t\t\t', 'No command given')
 
     def test_command(self):
         self.assertTokenizeEquals(['test'], 'test')
@@ -32,14 +34,14 @@ class TestTokenizer(unittest.TestCase):
         self.assertTokenizeEquals(['test'], 'test\t\t\t')
 
     def test_command_leading_whitespace(self):
-        self.assertTokenizeRaises(tokenize.TokenizeError, ' test')
-        self.assertTokenizeRaises(tokenize.TokenizeError, '\ttest')
+        self.assertTokenizeRaisesError('  test', 'Letter expected')
+        self.assertTokenizeRaisesError('\ttest', 'Letter expected')
 
     def test_invalid_command(self):
-        self.assertTokenizeRaises(tokenize.TokenizeError, 'foo/bar')
-        self.assertTokenizeRaises(tokenize.TokenizeError, 'æøå')
-        self.assertTokenizeRaises(tokenize.TokenizeError, 'test?')
-        self.assertTokenizeRaises(tokenize.TokenizeError, 'te"st')
+        self.assertTokenizeRaisesError('foo/bar', 'Invalid word character')
+        self.assertTokenizeRaisesError('æøå', 'Invalid word character')
+        self.assertTokenizeRaisesError('test?', 'Invalid word character')
+        self.assertTokenizeRaisesError('te"st', 'Invalid word character')
 
     def test_unquoted_param(self):
         self.assertTokenizeEquals(['test', 'param'], 'test param')
@@ -54,11 +56,12 @@ class TestTokenizer(unittest.TestCase):
         self.assertTokenizeEquals(['test', 'param'], 'test param\t\t')
 
     def test_unquoted_param_invalid_chars(self):
-        self.assertTokenizeRaises(tokenize.TokenizeError, 'test par"m')
-        self.assertTokenizeRaises(tokenize.TokenizeError, 'test foo\\bar')
-        self.assertTokenizeRaises(tokenize.TokenizeError, 'test foo\bbar')
-        self.assertTokenizeRaises(tokenize.TokenizeError, 'test "foo"bar')
-        self.assertTokenizeRaises(tokenize.TokenizeError, 'test fo"b"ar')
+        msg = 'Invalid unquoted character'
+        self.assertTokenizeRaisesError('test par"m', msg)
+        self.assertTokenizeRaisesError('test foo\bbar', msg)
+        self.assertTokenizeRaisesError('test "foo"bar', msg)
+        self.assertTokenizeRaisesError('test foo"bar"baz', msg)
+        self.assertTokenizeRaisesError('test foo\'bar', msg)
 
     def test_unquoted_param_numbers(self):
         self.assertTokenizeEquals(['test', '123'], 'test 123')
@@ -68,8 +71,9 @@ class TestTokenizer(unittest.TestCase):
 
     def test_unquoted_param_extended_chars(self):
         self.assertTokenizeEquals(['test', 'æøå'], 'test æøå')
-        self.assertTokenizeEquals(['test', '?#\'$'], 'test ?#\'$')
+        self.assertTokenizeEquals(['test', '?#$'], 'test ?#$')
         self.assertTokenizeEquals(['test', '/foo/bar/'], 'test /foo/bar/')
+        self.assertTokenizeEquals(['test', 'foo\\bar'], 'test foo\\bar')
 
     def test_unquoted_params(self):
         self.assertTokenizeEquals(['test', 'foo', 'bar'], 'test foo bar')
@@ -87,7 +91,10 @@ class TestTokenizer(unittest.TestCase):
         self.assertTokenizeEquals(['test', 'param'], 'test "param"\t\t')
 
     def test_quoted_param_invalid_chars(self):
-        self.assertTokenizeRaises(tokenize.TokenizeError, 'test "par"m"')
+        # TODO: Figure out how to check for " without space behind it.
+        #msg = """Space expected after closing '"'"""
+        msg = 'Invalid unquoted character'
+        self.assertTokenizeRaisesError('test "par"m"', msg)
 
     def test_quoted_param_numbers(self):
         self.assertTokenizeEquals(['test', '123'], 'test "123"')
@@ -126,5 +133,5 @@ class TestTokenizer(unittest.TestCase):
                                   r'test "foo\"bar" baz 123')
 
     def test_unbalanced_quotes(self):
-        self.assertTokenizeRaises(tokenize.TokenizeError,
-                                  'test "foo bar" baz"')
+        msg = 'Invalid unquoted character'
+        self.assertTokenizeRaisesError('test "foo bar" baz"', msg)

From 756cf1518bf899b7517914df409b6f41acc3a731 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Tue, 21 Jan 2014 23:20:24 +0100
Subject: [PATCH 07/35] mpd: Add last missing error cases for tokenizer.

If we decide that exact error message parity is not needed these can and should
be removed as they complicate the tokenizer.
---
 mopidy/mpd/tokenize.py      | 27 ++++++++++++++++++++++-----
 tests/mpd/test_tokenizer.py | 14 +++++++++-----
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/mopidy/mpd/tokenize.py b/mopidy/mpd/tokenize.py
index f10e349f..554df9e6 100644
--- a/mopidy/mpd/tokenize.py
+++ b/mopidy/mpd/tokenize.py
@@ -27,24 +27,41 @@ PARAM_RE = re.compile(r"""
     (.*)                            # Possibly a remainder to be parsed
     """ % {'unprintable': ''.join(map(chr, range(0x21)))}, re.VERBOSE)
 
+BAD_QUOTED_PARAM_RE = re.compile(r"""
+    ^
+    "[^"\\]*(?:\\.[^"\\]*)*  # start of a quoted value
+    (?:                      # followed by:
+        ("[^\s])             # non-escaped quote, followed by non-whitespace
+        |                    # or
+        ([^"])               # anything that is not a quote
+    )
+    """, re.VERBOSE)
+
 UNESCAPE_RE = re.compile(r'\\(.)')  # Backslash escapes any following char.
 
 
 def split(line):
     if not line.strip():
-        raise Error('No command given')
+        raise Error('No command given')  # 5@0
     match = WORD_RE.match(line)
     if not match:
-        raise Error('Invalid word character')
+        raise Error('Invalid word character')  # 5@0
     whitespace, command, remainder = match.groups()
     if whitespace:
-        raise Error('Letter expected')
-    result = [command]
+        raise Error('Letter expected')  # 5@0
 
+    result = [command]
     while remainder:
         match = PARAM_RE.match(remainder)
         if not match:
-            raise Error('Invalid unquoted character')
+            # Following checks are simply to match MPD error messages:
+            match = BAD_QUOTED_PARAM_RE.match(remainder)
+            if match:
+                if match.group(1):
+                    raise Error('Space expected after closing \'"\'')  # 2@0
+                else:
+                    raise Error('Missing closing \'"\'')  # 2@0
+            raise Error('Invalid unquoted character')  # 2@0
         unquoted, quoted, remainder = match.groups()
         result.append(unquoted or UNESCAPE_RE.sub(r'\g<1>', quoted))
 
diff --git a/tests/mpd/test_tokenizer.py b/tests/mpd/test_tokenizer.py
index 29e25cae..7c014c84 100644
--- a/tests/mpd/test_tokenizer.py
+++ b/tests/mpd/test_tokenizer.py
@@ -59,7 +59,6 @@ class TestTokenizer(unittest.TestCase):
         msg = 'Invalid unquoted character'
         self.assertTokenizeRaisesError('test par"m', msg)
         self.assertTokenizeRaisesError('test foo\bbar', msg)
-        self.assertTokenizeRaisesError('test "foo"bar', msg)
         self.assertTokenizeRaisesError('test foo"bar"baz', msg)
         self.assertTokenizeRaisesError('test foo\'bar', msg)
 
@@ -91,10 +90,11 @@ class TestTokenizer(unittest.TestCase):
         self.assertTokenizeEquals(['test', 'param'], 'test "param"\t\t')
 
     def test_quoted_param_invalid_chars(self):
-        # TODO: Figure out how to check for " without space behind it.
-        #msg = """Space expected after closing '"'"""
-        msg = 'Invalid unquoted character'
-        self.assertTokenizeRaisesError('test "par"m"', msg)
+        msg = 'Space expected after closing \'"\''
+        self.assertTokenizeRaisesError('test "foo"bar"', msg)
+        self.assertTokenizeRaisesError('test "foo"bar" ', msg)
+        self.assertTokenizeRaisesError('test "foo"bar', msg)
+        self.assertTokenizeRaisesError('test "foo"bar ', msg)
 
     def test_quoted_param_numbers(self):
         self.assertTokenizeEquals(['test', '123'], 'test "123"')
@@ -135,3 +135,7 @@ class TestTokenizer(unittest.TestCase):
     def test_unbalanced_quotes(self):
         msg = 'Invalid unquoted character'
         self.assertTokenizeRaisesError('test "foo bar" baz"', msg)
+
+    def test_missing_closing_quote(self):
+        self.assertTokenizeRaisesError('test "foo', 'Missing closing \'"\'')
+        self.assertTokenizeRaisesError('test "foo a ', 'Missing closing \'"\'')

From 066fed15220ea21ac102359283ec0dafc53b69f1 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Wed, 22 Jan 2014 00:10:30 +0100
Subject: [PATCH 08/35] mpd: Update tokenizer to use mopidy.mpd.exceptions

---
 mopidy/mpd/exceptions.py    |  4 ++-
 mopidy/mpd/tokenize.py      | 30 +++++++++++-------
 tests/mpd/test_tokenizer.py | 62 +++++++++++++++++++++----------------
 3 files changed, 56 insertions(+), 40 deletions(-)

diff --git a/mopidy/mpd/exceptions.py b/mopidy/mpd/exceptions.py
index ec874553..6738b4c9 100644
--- a/mopidy/mpd/exceptions.py
+++ b/mopidy/mpd/exceptions.py
@@ -54,9 +54,11 @@ class MpdPermissionError(MpdAckError):
         self.message = 'you don\'t have permission for "%s"' % self.command
 
 
-class MpdUnknownCommand(MpdAckError):
+class MpdUnknownError(MpdAckError):
     error_code = MpdAckError.ACK_ERROR_UNKNOWN
 
+
+class MpdUnknownCommand(MpdUnknownError):
     def __init__(self, *args, **kwargs):
         super(MpdUnknownCommand, self).__init__(*args, **kwargs)
         assert self.command is not None, 'command must be given explicitly'
diff --git a/mopidy/mpd/tokenize.py b/mopidy/mpd/tokenize.py
index 554df9e6..04799719 100644
--- a/mopidy/mpd/tokenize.py
+++ b/mopidy/mpd/tokenize.py
@@ -2,6 +2,8 @@ from __future__ import unicode_literals
 
 import re
 
+from mopidy.mpd import exceptions
+
 
 class Error(Exception):
     pass
@@ -42,27 +44,31 @@ UNESCAPE_RE = re.compile(r'\\(.)')  # Backslash escapes any following char.
 
 def split(line):
     if not line.strip():
-        raise Error('No command given')  # 5@0
+        raise exceptions.MpdNoCommand('No command given')
     match = WORD_RE.match(line)
     if not match:
-        raise Error('Invalid word character')  # 5@0
+        raise exceptions.MpdUnknownError('Invalid word character')
     whitespace, command, remainder = match.groups()
     if whitespace:
-        raise Error('Letter expected')  # 5@0
+        raise exceptions.MpdUnknownError('Letter expected')
 
     result = [command]
     while remainder:
         match = PARAM_RE.match(remainder)
         if not match:
-            # Following checks are simply to match MPD error messages:
-            match = BAD_QUOTED_PARAM_RE.match(remainder)
-            if match:
-                if match.group(1):
-                    raise Error('Space expected after closing \'"\'')  # 2@0
-                else:
-                    raise Error('Missing closing \'"\'')  # 2@0
-            raise Error('Invalid unquoted character')  # 2@0
+            msg = _determine_error_message(remainder)
+            raise exceptions.MpdArgError(msg, command=command)
         unquoted, quoted, remainder = match.groups()
         result.append(unquoted or UNESCAPE_RE.sub(r'\g<1>', quoted))
-
     return result
+
+
+def _determine_error_message(remainder):
+    # Following checks are simply to match MPD error messages:
+    match = BAD_QUOTED_PARAM_RE.match(remainder)
+    if match:
+        if match.group(1):
+            return 'Space expected after closing \'"\''
+        else:
+            return 'Missing closing \'"\''
+    return 'Invalid unquoted character'
diff --git a/tests/mpd/test_tokenizer.py b/tests/mpd/test_tokenizer.py
index 7c014c84..546df847 100644
--- a/tests/mpd/test_tokenizer.py
+++ b/tests/mpd/test_tokenizer.py
@@ -4,25 +4,24 @@ from __future__ import unicode_literals
 
 import unittest
 
-from mopidy.mpd import tokenize
+from mopidy.mpd import exceptions, tokenize
 
 
 class TestTokenizer(unittest.TestCase):
     def assertTokenizeEquals(self, expected, line):
         self.assertEqual(expected, tokenize.split(line))
 
-    def assertTokenizeRaisesError(self, line, message=None):
-        with self.assertRaises(tokenize.Error) as cm:
+    def assertTokenizeRaises(self, exception, message, line):
+        with self.assertRaises(exception) as cm:
             tokenize.split(line)
-        if message:
-            self.assertEqual(cm.exception.message, message)
+        self.assertEqual(cm.exception.message, message)
 
     def test_empty_string(self):
-        self.assertTokenizeRaisesError('', 'No command given')
-
-    def test_whitespace(self):
-        self.assertTokenizeRaisesError('      ', 'No command given')
-        self.assertTokenizeRaisesError('\t\t\t', 'No command given')
+        ex = exceptions.MpdNoCommand
+        msg = 'No command given'
+        self.assertTokenizeRaises(ex, msg, '')
+        self.assertTokenizeRaises(ex, msg, '      ')
+        self.assertTokenizeRaises(ex, msg, '\t\t\t')
 
     def test_command(self):
         self.assertTokenizeEquals(['test'], 'test')
@@ -34,14 +33,18 @@ class TestTokenizer(unittest.TestCase):
         self.assertTokenizeEquals(['test'], 'test\t\t\t')
 
     def test_command_leading_whitespace(self):
-        self.assertTokenizeRaisesError('  test', 'Letter expected')
-        self.assertTokenizeRaisesError('\ttest', 'Letter expected')
+        ex = exceptions.MpdUnknownError
+        msg = 'Letter expected'
+        self.assertTokenizeRaises(ex, msg, '  test')
+        self.assertTokenizeRaises(ex, msg, '\ttest')
 
     def test_invalid_command(self):
-        self.assertTokenizeRaisesError('foo/bar', 'Invalid word character')
-        self.assertTokenizeRaisesError('æøå', 'Invalid word character')
-        self.assertTokenizeRaisesError('test?', 'Invalid word character')
-        self.assertTokenizeRaisesError('te"st', 'Invalid word character')
+        ex = exceptions.MpdUnknownError
+        msg = 'Invalid word character'
+        self.assertTokenizeRaises(ex, msg, 'foo/bar')
+        self.assertTokenizeRaises(ex, msg, 'æøå')
+        self.assertTokenizeRaises(ex, msg, 'test?')
+        self.assertTokenizeRaises(ex, msg, 'te"st')
 
     def test_unquoted_param(self):
         self.assertTokenizeEquals(['test', 'param'], 'test param')
@@ -56,11 +59,12 @@ class TestTokenizer(unittest.TestCase):
         self.assertTokenizeEquals(['test', 'param'], 'test param\t\t')
 
     def test_unquoted_param_invalid_chars(self):
+        ex = exceptions.MpdArgError
         msg = 'Invalid unquoted character'
-        self.assertTokenizeRaisesError('test par"m', msg)
-        self.assertTokenizeRaisesError('test foo\bbar', msg)
-        self.assertTokenizeRaisesError('test foo"bar"baz', msg)
-        self.assertTokenizeRaisesError('test foo\'bar', msg)
+        self.assertTokenizeRaises(ex, msg, 'test par"m')
+        self.assertTokenizeRaises(ex, msg, 'test foo\bbar')
+        self.assertTokenizeRaises(ex, msg, 'test foo"bar"baz')
+        self.assertTokenizeRaises(ex, msg, 'test foo\'bar')
 
     def test_unquoted_param_numbers(self):
         self.assertTokenizeEquals(['test', '123'], 'test 123')
@@ -90,11 +94,12 @@ class TestTokenizer(unittest.TestCase):
         self.assertTokenizeEquals(['test', 'param'], 'test "param"\t\t')
 
     def test_quoted_param_invalid_chars(self):
+        ex = exceptions.MpdArgError
         msg = 'Space expected after closing \'"\''
-        self.assertTokenizeRaisesError('test "foo"bar"', msg)
-        self.assertTokenizeRaisesError('test "foo"bar" ', msg)
-        self.assertTokenizeRaisesError('test "foo"bar', msg)
-        self.assertTokenizeRaisesError('test "foo"bar ', msg)
+        self.assertTokenizeRaises(ex, msg, 'test "foo"bar"')
+        self.assertTokenizeRaises(ex, msg, 'test "foo"bar" ')
+        self.assertTokenizeRaises(ex, msg, 'test "foo"bar')
+        self.assertTokenizeRaises(ex, msg, 'test "foo"bar ')
 
     def test_quoted_param_numbers(self):
         self.assertTokenizeEquals(['test', '123'], 'test "123"')
@@ -133,9 +138,12 @@ class TestTokenizer(unittest.TestCase):
                                   r'test "foo\"bar" baz 123')
 
     def test_unbalanced_quotes(self):
+        ex = exceptions.MpdArgError
         msg = 'Invalid unquoted character'
-        self.assertTokenizeRaisesError('test "foo bar" baz"', msg)
+        self.assertTokenizeRaises(ex, msg, 'test "foo bar" baz"')
 
     def test_missing_closing_quote(self):
-        self.assertTokenizeRaisesError('test "foo', 'Missing closing \'"\'')
-        self.assertTokenizeRaisesError('test "foo a ', 'Missing closing \'"\'')
+        ex = exceptions.MpdArgError
+        msg = 'Missing closing \'"\''
+        self.assertTokenizeRaises(ex, msg, 'test "foo')
+        self.assertTokenizeRaises(ex, msg, 'test "foo a ')

From d3db5c4fe1eeee88edbf7d1bbf998ea684bfe12e Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Wed, 22 Jan 2014 22:44:40 +0100
Subject: [PATCH 09/35] mpd: Install new commands helpers in _call_handler

This means we now tokenize every request, and then try in call the appropriate
handler. If none is found we simply fall back to the old handlers.
---
 mopidy/mpd/dispatcher.py        | 11 ++++++++++-
 mopidy/mpd/protocol/__init__.py |  9 ++++++---
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/mopidy/mpd/dispatcher.py b/mopidy/mpd/dispatcher.py
index 69b5e839..8a3602d4 100644
--- a/mopidy/mpd/dispatcher.py
+++ b/mopidy/mpd/dispatcher.py
@@ -5,7 +5,7 @@ import re
 
 import pykka
 
-from mopidy.mpd import exceptions, protocol
+from mopidy.mpd import exceptions, protocol, tokenize
 
 logger = logging.getLogger(__name__)
 
@@ -164,6 +164,15 @@ class MpdDispatcher(object):
             raise exceptions.MpdSystemError(e)
 
     def _call_handler(self, request):
+        tokens = tokenize.split(request)
+        try:
+            return protocol.commands.call(tokens, context=self.context)
+        except exceptions.MpdAckError as exc:
+            if exc.command is None:
+                exc.command = tokens[0]
+            raise
+        except LookupError:
+            pass  # Command has not been converted, i.e. fallback...
         (command_name, handler, kwargs) = self._find_handler(request)
         try:
             return handler(self.context, **kwargs)
diff --git a/mopidy/mpd/protocol/__init__.py b/mopidy/mpd/protocol/__init__.py
index 8aa5f3d1..03ac6dda 100644
--- a/mopidy/mpd/protocol/__init__.py
+++ b/mopidy/mpd/protocol/__init__.py
@@ -150,7 +150,10 @@ class Commands(object):
     def call(self, args, context=None):
         if not args:
             raise TypeError('No args provided')
-        command = args.pop(0)
-        if command not in self.handlers:
+        if args[0] not in self.handlers:
             raise LookupError('Unknown command')
-        return self.handlers[command](context, *args)
+        return self.handlers[args[0]](context, *args[1:])
+
+
+#: Global instance to install commands into
+commands = Commands()

From eb85f92d96bdc653a8964b335764dc5165e9b5d5 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Wed, 22 Jan 2014 23:05:13 +0100
Subject: [PATCH 10/35] mpd: Store auth required and if command should be
 listed

---
 mopidy/mpd/protocol/__init__.py | 10 ++++++----
 tests/mpd/test_commands.py      | 18 ++++++++++++++++++
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/mopidy/mpd/protocol/__init__.py b/mopidy/mpd/protocol/__init__.py
index 03ac6dda..06a1b3ab 100644
--- a/mopidy/mpd/protocol/__init__.py
+++ b/mopidy/mpd/protocol/__init__.py
@@ -112,10 +112,10 @@ class Commands(object):
     def __init__(self):
         self.handlers = {}
 
-    def add(self, command, **validators):
+    def add(self, name, auth_required=True, list_command=True, **validators):
         def wrapper(func):
-            if command in self.handlers:
-                raise Exception('%s already registered' % command)
+            if name in self.handlers:
+                raise Exception('%s already registered' % name)
 
             args, varargs, keywords, defaults = inspect.getargspec(func)
             defaults = dict(zip(args[-len(defaults or []):], defaults or []))
@@ -143,7 +143,9 @@ class Commands(object):
                         callargs[key] = validators[key](value)
                 return func(**callargs)
 
-            self.handlers[command] = validate
+            validate.auth_required = auth_required
+            validate.list_command = list_command
+            self.handlers[name] = validate
             return func
         return wrapper
 
diff --git a/tests/mpd/test_commands.py b/tests/mpd/test_commands.py
index 0d2d2ad3..f0f04bb7 100644
--- a/tests/mpd/test_commands.py
+++ b/tests/mpd/test_commands.py
@@ -185,3 +185,21 @@ class TestCommands(unittest.TestCase):
         with self.assertRaises(TypeError):
             func = lambda context: True
             self.commands.add('bar', context=lambda v: v)(func)
+
+    def test_auth_required_gets_stored(self):
+        func1 = lambda context: context
+        func2 = lambda context: context
+        self.commands.add('foo')(func1)
+        self.commands.add('bar', auth_required=False)(func2)
+
+        self.assertTrue(self.commands.handlers['foo'].auth_required)
+        self.assertFalse(self.commands.handlers['bar'].auth_required)
+
+    def test_list_command_gets_stored(self):
+        func1 = lambda context: context
+        func2 = lambda context: context
+        self.commands.add('foo')(func1)
+        self.commands.add('bar', list_command=False)(func2)
+
+        self.assertTrue(self.commands.handlers['foo'].list_command)
+        self.assertFalse(self.commands.handlers['bar'].list_command)

From 55a46c31d7e8429829b65fc7607887e9379c3282 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Wed, 22 Jan 2014 23:11:21 +0100
Subject: [PATCH 11/35] mpd: Cleanup imports in reflection

---
 mopidy/mpd/protocol/reflection.py | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/mopidy/mpd/protocol/reflection.py b/mopidy/mpd/protocol/reflection.py
index 79aa1247..de8e4b57 100644
--- a/mopidy/mpd/protocol/reflection.py
+++ b/mopidy/mpd/protocol/reflection.py
@@ -1,10 +1,9 @@
 from __future__ import unicode_literals
 
-from mopidy.mpd.exceptions import MpdPermissionError
-from mopidy.mpd.protocol import handle_request, mpd_commands
+from mopidy.mpd import exceptions, protocol
 
 
-@handle_request(r'config$', auth_required=False)
+@protocol.handle_request(r'config$', auth_required=False)
 def config(context):
     """
     *musicpd.org, reflection section:*
@@ -15,10 +14,10 @@ def config(context):
         command is only permitted to "local" clients (connected via UNIX domain
         socket).
     """
-    raise MpdPermissionError(command='config')
+    raise exceptions.MpdPermissionError(command='config')
 
 
-@handle_request(r'commands$', auth_required=False)
+@protocol.handle_request(r'commands$', auth_required=False)
 def commands(context):
     """
     *musicpd.org, reflection section:*
@@ -28,11 +27,11 @@ def commands(context):
         Shows which commands the current user has access to.
     """
     if context.dispatcher.authenticated:
-        command_names = set([command.name for command in mpd_commands])
+        command_names = set(command.name for command in protocol.mpd_commands)
     else:
-        command_names = set([
-            command.name for command in mpd_commands
-            if not command.auth_required])
+        command_names = set(
+            command.name for command in protocol.mpd_commands
+            if not command.auth_required)
 
     # No one is permited to use 'config' or 'kill', rest of commands are not
     # listed by MPD, so we shouldn't either.
@@ -45,7 +44,7 @@ def commands(context):
         ('command', command_name) for command_name in sorted(command_names)]
 
 
-@handle_request(r'decoders$')
+@protocol.handle_request(r'decoders$')
 def decoders(context):
     """
     *musicpd.org, reflection section:*
@@ -72,7 +71,7 @@ def decoders(context):
     return  # TODO
 
 
-@handle_request(r'notcommands$', auth_required=False)
+@protocol.handle_request(r'notcommands$', auth_required=False)
 def notcommands(context):
     """
     *musicpd.org, reflection section:*
@@ -84,8 +83,8 @@ def notcommands(context):
     if context.dispatcher.authenticated:
         command_names = []
     else:
-        command_names = [
-            command.name for command in mpd_commands if command.auth_required]
+        command_names = [command.name for command in protocol.mpd_commands
+                         if command.auth_required]
 
     # No permission to use
     command_names.append('config')
@@ -95,7 +94,7 @@ def notcommands(context):
         ('command', command_name) for command_name in sorted(command_names)]
 
 
-@handle_request(r'tagtypes$')
+@protocol.handle_request(r'tagtypes$')
 def tagtypes(context):
     """
     *musicpd.org, reflection section:*
@@ -107,7 +106,7 @@ def tagtypes(context):
     pass  # TODO
 
 
-@handle_request(r'urlhandlers$')
+@protocol.handle_request(r'urlhandlers$')
 def urlhandlers(context):
     """
     *musicpd.org, reflection section:*

From 9df2eebfe2138267fbaa9bea3da1c81021f0e8e6 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Wed, 22 Jan 2014 23:26:31 +0100
Subject: [PATCH 12/35] mpd: Upate command reflection to handle new commands
 helper

---
 mopidy/mpd/protocol/reflection.py | 38 +++++++++++++++++++------------
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/mopidy/mpd/protocol/reflection.py b/mopidy/mpd/protocol/reflection.py
index de8e4b57..2f7d606e 100644
--- a/mopidy/mpd/protocol/reflection.py
+++ b/mopidy/mpd/protocol/reflection.py
@@ -26,13 +26,21 @@ def commands(context):
 
         Shows which commands the current user has access to.
     """
-    if context.dispatcher.authenticated:
-        command_names = set(command.name for command in protocol.mpd_commands)
-    else:
-        command_names = set(
-            command.name for command in protocol.mpd_commands
-            if not command.auth_required)
+    command_names = set()
+    for name, handler in protocol.commands.handlers.items():
+        if not handler.list_command:
+            continue
+        if context.dispatcher.authenticated or not handler.auth_required:
+            command_names.add(name)
 
+    # TODO: remove
+    if context.dispatcher.authenticated:
+        command_names.update(c.name for c in protocol.mpd_commands)
+    else:
+        command_names.update(c.name for c in protocol.mpd_commands
+                             if not c.auth_required)
+
+    # TODO: remove once we've marked all of these as list_command=False
     # No one is permited to use 'config' or 'kill', rest of commands are not
     # listed by MPD, so we shouldn't either.
     command_names = command_names - set([
@@ -80,15 +88,17 @@ def notcommands(context):
 
         Shows which commands the current user does not have access to.
     """
-    if context.dispatcher.authenticated:
-        command_names = []
-    else:
-        command_names = [command.name for command in protocol.mpd_commands
-                         if command.auth_required]
+    command_names = set(['config', 'kill'])  # No permission to use
+    for name, handler in protocol.commands.handlers.items():
+        if not handler.list_command:
+            continue
+        if not context.dispatcher.authenticated and handler.auth_required:
+            command_names.add(name)
 
-    # No permission to use
-    command_names.append('config')
-    command_names.append('kill')
+    # TODO: remove
+    if not context.dispatcher.authenticated:
+        command_names.update(command.name for command in protocol.mpd_commands
+                             if command.auth_required)
 
     return [
         ('command', command_name) for command_name in sorted(command_names)]

From 538e46e0e74d6ab46036ce3ce53754d720dd0d0b Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Wed, 22 Jan 2014 23:26:53 +0100
Subject: [PATCH 13/35] mpd: Convert protocol.status to new helper

---
 mopidy/mpd/protocol/status.py | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/mopidy/mpd/protocol/status.py b/mopidy/mpd/protocol/status.py
index 96bca6d6..15e46002 100644
--- a/mopidy/mpd/protocol/status.py
+++ b/mopidy/mpd/protocol/status.py
@@ -3,9 +3,7 @@ from __future__ import unicode_literals
 import pykka
 
 from mopidy.core import PlaybackState
-from mopidy.mpd.exceptions import MpdNotImplemented
-from mopidy.mpd.protocol import handle_request
-from mopidy.mpd.translator import track_to_mpd_format
+from mopidy.mpd import exceptions, protocol, translator
 
 #: Subsystems that can be registered with idle command.
 SUBSYSTEMS = [
@@ -13,7 +11,7 @@ SUBSYSTEMS = [
     'stored_playlist', 'update']
 
 
-@handle_request(r'clearerror$')
+@protocol.commands.add('clearerror')
 def clearerror(context):
     """
     *musicpd.org, status section:*
@@ -23,10 +21,10 @@ def clearerror(context):
         Clears the current error message in status (this is also
         accomplished by any command that starts playback).
     """
-    raise MpdNotImplemented  # TODO
+    raise exceptions.MpdNotImplemented  # TODO
 
 
-@handle_request(r'currentsong$')
+@protocol.commands.add('currentsong')
 def currentsong(context):
     """
     *musicpd.org, status section:*
@@ -39,12 +37,11 @@ def currentsong(context):
     tl_track = context.core.playback.current_tl_track.get()
     if tl_track is not None:
         position = context.core.tracklist.index(tl_track).get()
-        return track_to_mpd_format(tl_track, position=position)
+        return translator.track_to_mpd_format(tl_track, position=position)
 
 
-@handle_request(r'idle$')
-@handle_request(r'idle\ (?P<subsystems>.+)$')
-def idle(context, subsystems=None):
+@protocol.commands.add('idle')
+def idle(context, *subsystems):
     """
     *musicpd.org, status section:*
 
@@ -77,10 +74,9 @@ def idle(context, subsystems=None):
         notifications when something changed in one of the specified
         subsystems.
     """
+    # TODO: test against valid subsystems
 
-    if subsystems:
-        subsystems = subsystems.split()
-    else:
+    if not subsystems:
         subsystems = SUBSYSTEMS
 
     for subsystem in subsystems:
@@ -100,7 +96,7 @@ def idle(context, subsystems=None):
     return response
 
 
-@handle_request(r'noidle$')
+@protocol.commands.add('noidle')
 def noidle(context):
     """See :meth:`_status_idle`."""
     if not context.subscriptions:
@@ -110,7 +106,7 @@ def noidle(context):
     context.session.prevent_timeout = False
 
 
-@handle_request(r'stats$')
+@protocol.commands.add('stats')
 def stats(context):
     """
     *musicpd.org, status section:*
@@ -137,7 +133,7 @@ def stats(context):
     }
 
 
-@handle_request(r'status$')
+@protocol.commands.add('status')
 def status(context):
     """
     *musicpd.org, status section:*

From b9b5a7893825942b43158fa2088a9a1c5990ddf4 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Wed, 22 Jan 2014 23:58:44 +0100
Subject: [PATCH 14/35] mpd: Convert protocol.playback

---
 mopidy/mpd/protocol/playback.py | 173 ++++++++++++++------------------
 1 file changed, 78 insertions(+), 95 deletions(-)

diff --git a/mopidy/mpd/protocol/playback.py b/mopidy/mpd/protocol/playback.py
index 4f8ae73a..d5100ee1 100644
--- a/mopidy/mpd/protocol/playback.py
+++ b/mopidy/mpd/protocol/playback.py
@@ -1,12 +1,10 @@
 from __future__ import unicode_literals
 
 from mopidy.core import PlaybackState
-from mopidy.mpd.protocol import handle_request
-from mopidy.mpd.exceptions import (
-    MpdArgError, MpdNoExistError, MpdNotImplemented)
+from mopidy.mpd import exceptions, protocol
 
 
-@handle_request(r'consume\ ("?)(?P<state>[01])\1$')
+@protocol.commands.add('consume', state=protocol.boolean)
 def consume(context, state):
     """
     *musicpd.org, playback section:*
@@ -17,13 +15,10 @@ def consume(context, state):
         1. When consume is activated, each song played is removed from
         playlist.
     """
-    if int(state):
-        context.core.tracklist.consume = True
-    else:
-        context.core.tracklist.consume = False
+    context.core.tracklist.consume = state
 
 
-@handle_request(r'crossfade\ "(?P<seconds>\d+)"$')
+@protocol.commands.add('crossfade', seconds=protocol.integer)
 def crossfade(context, seconds):
     """
     *musicpd.org, playback section:*
@@ -32,11 +27,11 @@ def crossfade(context, seconds):
 
         Sets crossfading between songs.
     """
-    seconds = int(seconds)
-    raise MpdNotImplemented  # TODO
+    # TODO: check that seconds is positive
+    raise exceptions.MpdNotImplemented  # TODO
 
 
-@handle_request(r'next$')
+@protocol.commands.add('next')
 def next_(context):
     """
     *musicpd.org, playback section:*
@@ -94,8 +89,7 @@ def next_(context):
     return context.core.playback.next().get()
 
 
-@handle_request(r'pause$')
-@handle_request(r'pause\ "(?P<state>[01])"$')
+@protocol.commands.add('pause', state=protocol.boolean)
 def pause(context, state=None):
     """
     *musicpd.org, playback section:*
@@ -109,54 +103,19 @@ def pause(context, state=None):
     - Calls ``pause`` without any arguments to toogle pause.
     """
     if state is None:
+        # TODO: emit warning about this being deprecated
         if (context.core.playback.state.get() == PlaybackState.PLAYING):
             context.core.playback.pause()
         elif (context.core.playback.state.get() == PlaybackState.PAUSED):
             context.core.playback.resume()
-    elif int(state):
+    elif state:
         context.core.playback.pause()
     else:
         context.core.playback.resume()
 
 
-@handle_request(r'play$')
-def play(context):
-    """
-    The original MPD server resumes from the paused state on ``play``
-    without arguments.
-    """
-    return context.core.playback.play().get()
-
-
-@handle_request(r'playid\ ("?)(?P<tlid>-?\d+)\1$')
-def playid(context, tlid):
-    """
-    *musicpd.org, playback section:*
-
-        ``playid [SONGID]``
-
-        Begins playing the playlist at song ``SONGID``.
-
-    *Clarifications:*
-
-    - ``playid "-1"`` when playing is ignored.
-    - ``playid "-1"`` when paused resumes playback.
-    - ``playid "-1"`` when stopped with a current track starts playback at the
-      current track.
-    - ``playid "-1"`` when stopped without a current track, e.g. after playlist
-      replacement, starts playback at the first track.
-    """
-    tlid = int(tlid)
-    if tlid == -1:
-        return _play_minus_one(context)
-    tl_tracks = context.core.tracklist.filter(tlid=[tlid]).get()
-    if not tl_tracks:
-        raise MpdNoExistError('No such song')
-    return context.core.playback.play(tl_tracks[0]).get()
-
-
-@handle_request(r'play\ ("?)(?P<songpos>-?\d+)\1$')
-def play__pos(context, songpos):
+@protocol.commands.add('play', tlid=protocol.integer)
+def play(context, tlid=None):
     """
     *musicpd.org, playback section:*
 
@@ -164,6 +123,9 @@ def play__pos(context, songpos):
 
         Begins playing the playlist at song number ``SONGPOS``.
 
+    The original MPD server resumes from the paused state on ``play``
+    without arguments.
+
     *Clarifications:*
 
     - ``play "-1"`` when playing is ignored.
@@ -177,14 +139,16 @@ def play__pos(context, songpos):
 
     - issues ``play 6`` without quotes around the argument.
     """
-    songpos = int(songpos)
-    if songpos == -1:
+    if tlid is None:
+        return context.core.playback.play().get()
+    elif tlid == -1:
         return _play_minus_one(context)
+
     try:
-        tl_track = context.core.tracklist.slice(songpos, songpos + 1).get()[0]
+        tl_track = context.core.tracklist.slice(tlid, tlid + 1).get()[0]
         return context.core.playback.play(tl_track).get()
     except IndexError:
-        raise MpdArgError('Bad song index')
+        raise exceptions.MpdArgError('Bad song index')
 
 
 def _play_minus_one(context):
@@ -202,7 +166,33 @@ def _play_minus_one(context):
         return  # Fail silently
 
 
-@handle_request(r'previous$')
+@protocol.commands.add('playid', tlid=protocol.integer)
+def playid(context, tlid):
+    """
+    *musicpd.org, playback section:*
+
+        ``playid [SONGID]``
+
+        Begins playing the playlist at song ``SONGID``.
+
+    *Clarifications:*
+
+    - ``playid "-1"`` when playing is ignored.
+    - ``playid "-1"`` when paused resumes playback.
+    - ``playid "-1"`` when stopped with a current track starts playback at the
+      current track.
+    - ``playid "-1"`` when stopped without a current track, e.g. after playlist
+      replacement, starts playback at the first track.
+    """
+    if tlid == -1:
+        return _play_minus_one(context)
+    tl_tracks = context.core.tracklist.filter(tlid=[tlid]).get()
+    if not tl_tracks:
+        raise exceptions.MpdNoExistError('No such song')
+    return context.core.playback.play(tl_tracks[0]).get()
+
+
+@protocol.commands.add('previous')
 def previous(context):
     """
     *musicpd.org, playback section:*
@@ -249,7 +239,7 @@ def previous(context):
     return context.core.playback.previous().get()
 
 
-@handle_request(r'random\ ("?)(?P<state>[01])\1$')
+@protocol.commands.add('random', state=protocol.boolean)
 def random(context, state):
     """
     *musicpd.org, playback section:*
@@ -258,13 +248,10 @@ def random(context, state):
 
         Sets random state to ``STATE``, ``STATE`` should be 0 or 1.
     """
-    if int(state):
-        context.core.tracklist.random = True
-    else:
-        context.core.tracklist.random = False
+    context.core.tracklist.random = state
 
 
-@handle_request(r'repeat\ ("?)(?P<state>[01])\1$')
+@protocol.commands.add('repeat', state=protocol.boolean)
 def repeat(context, state):
     """
     *musicpd.org, playback section:*
@@ -273,13 +260,10 @@ def repeat(context, state):
 
         Sets repeat state to ``STATE``, ``STATE`` should be 0 or 1.
     """
-    if int(state):
-        context.core.tracklist.repeat = True
-    else:
-        context.core.tracklist.repeat = False
+    context.core.tracklist.repeat = state
 
 
-@handle_request(r'replay_gain_mode\ "(?P<mode>(off|track|album))"$')
+@protocol.commands.add('replay_gain_mode')
 def replay_gain_mode(context, mode):
     """
     *musicpd.org, playback section:*
@@ -293,10 +277,10 @@ def replay_gain_mode(context, mode):
 
         This command triggers the options idle event.
     """
-    raise MpdNotImplemented  # TODO
+    raise exceptions.MpdNotImplemented  # TODO
 
 
-@handle_request(r'replay_gain_status$')
+@protocol.commands.add('replay_gain_status')
 def replay_gain_status(context):
     """
     *musicpd.org, playback section:*
@@ -309,8 +293,9 @@ def replay_gain_status(context):
     return 'off'  # TODO
 
 
-@handle_request(r'seek\ ("?)(?P<songpos>\d+)\1\ ("?)(?P<seconds>\d+)\3$')
-def seek(context, songpos, seconds):
+@protocol.commands.add(
+    'seek', tlid=protocol.integer, seconds=protocol.integer)
+def seek(context, tlid, seconds):
     """
     *musicpd.org, playback section:*
 
@@ -323,13 +308,15 @@ def seek(context, songpos, seconds):
 
     - issues ``seek 1 120`` without quotes around the arguments.
     """
+    # TODO: check tlid is positive
     tl_track = context.core.playback.current_tl_track.get()
-    if context.core.tracklist.index(tl_track).get() != int(songpos):
-        play__pos(context, songpos)
-    context.core.playback.seek(int(seconds) * 1000).get()
+    if context.core.tracklist.index(tl_track).get() != tlid:
+        play(context, tlid)
+    context.core.playback.seek(seconds * 1000).get()
 
 
-@handle_request(r'seekid\ "(?P<tlid>\d+)"\ "(?P<seconds>\d+)"$')
+@protocol.commands.add(
+    'seekid', tlid=protocol.integer, seconds=protocol.integer)
 def seekid(context, tlid, seconds):
     """
     *musicpd.org, playback section:*
@@ -338,15 +325,15 @@ def seekid(context, tlid, seconds):
 
         Seeks to the position ``TIME`` (in seconds) of song ``SONGID``.
     """
+    # TODO: check that songid and time is positive
     tl_track = context.core.playback.current_tl_track.get()
-    if not tl_track or tl_track.tlid != int(tlid):
+    if not tl_track or tl_track.tlid != tlid:
         playid(context, tlid)
-    context.core.playback.seek(int(seconds) * 1000).get()
+    context.core.playback.seek(seconds * 1000).get()
 
 
-@handle_request(r'seekcur\ "(?P<position>\d+)"$')
-@handle_request(r'seekcur\ "(?P<diff>[-+]\d+)"$')
-def seekcur(context, position=None, diff=None):
+@protocol.commands.add('seekcur')
+def seekcur(context, time):
     """
     *musicpd.org, playback section:*
 
@@ -355,16 +342,16 @@ def seekcur(context, position=None, diff=None):
         Seeks to the position ``TIME`` within the current song. If prefixed by
         '+' or '-', then the time is relative to the current playing position.
     """
-    if position is not None:
-        position = int(position) * 1000
-        context.core.playback.seek(position).get()
-    elif diff is not None:
+    if time.startswith(('+', '-')):
         position = context.core.playback.time_position.get()
-        position += int(diff) * 1000
+        position += int(time) * 1000
+        context.core.playback.seek(position).get()
+    else:
+        position = int(time) * 1000
         context.core.playback.seek(position).get()
 
 
-@handle_request(r'setvol\ ("?)(?P<volume>[-+]*\d+)\1$')
+@protocol.commands.add('setvol', volume=protocol.integer)
 def setvol(context, volume):
     """
     *musicpd.org, playback section:*
@@ -377,7 +364,6 @@ def setvol(context, volume):
 
     - issues ``setvol 50`` without quotes around the argument.
     """
-    volume = int(volume)
     if volume < 0:
         volume = 0
     if volume > 100:
@@ -385,7 +371,7 @@ def setvol(context, volume):
     context.core.playback.volume = volume
 
 
-@handle_request(r'single\ ("?)(?P<state>[01])\1$')
+@protocol.commands.add('single', state=protocol.boolean)
 def single(context, state):
     """
     *musicpd.org, playback section:*
@@ -396,13 +382,10 @@ def single(context, state):
         single is activated, playback is stopped after current song, or
         song is repeated if the ``repeat`` mode is enabled.
     """
-    if int(state):
-        context.core.tracklist.single = True
-    else:
-        context.core.tracklist.single = False
+    context.core.tracklist.single = state
 
 
-@handle_request(r'stop$')
+@protocol.commands.add('stop')
 def stop(context):
     """
     *musicpd.org, playback section:*

From e7017f3ccb907e3219c0bbfe488d86021298b6aa Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Thu, 23 Jan 2014 00:40:18 +0100
Subject: [PATCH 15/35] mpd: Add protocol.position_or_range converter

---
 mopidy/mpd/protocol/__init__.py | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/mopidy/mpd/protocol/__init__.py b/mopidy/mpd/protocol/__init__.py
index 06a1b3ab..9e85d058 100644
--- a/mopidy/mpd/protocol/__init__.py
+++ b/mopidy/mpd/protocol/__init__.py
@@ -108,6 +108,21 @@ def boolean(value):
     raise ValueError('%r is not 0 or 1' % value)
 
 
+def position_or_range(value):
+    # TODO: test and check that values are positive
+    if ':' in value:
+        start, stop = value.split(':', 1)
+        start = int(start)
+        if stop.strip():
+            stop = int(stop)
+        else:
+            stop = None
+    else:
+        start = int(value)
+        stop = start + 1
+    return slice(start, stop)
+
+
 class Commands(object):
     def __init__(self):
         self.handlers = {}

From 442d9d4b1edcdbe860cf4f99ed0613a2b28ee3b8 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Thu, 23 Jan 2014 00:40:46 +0100
Subject: [PATCH 16/35] mpd: Convert almost all of protocol.current_playlist

---
 mopidy/mpd/protocol/current_playlist.py | 141 ++++++++++--------------
 1 file changed, 57 insertions(+), 84 deletions(-)

diff --git a/mopidy/mpd/protocol/current_playlist.py b/mopidy/mpd/protocol/current_playlist.py
index de8721d3..41a4782b 100644
--- a/mopidy/mpd/protocol/current_playlist.py
+++ b/mopidy/mpd/protocol/current_playlist.py
@@ -1,12 +1,9 @@
 from __future__ import unicode_literals
 
-from mopidy.mpd import translator
-from mopidy.mpd.exceptions import (
-    MpdArgError, MpdNoExistError, MpdNotImplemented)
-from mopidy.mpd.protocol import handle_request
+from mopidy.mpd import exceptions, protocol, translator
 
 
-@handle_request(r'add\ "(?P<uri>[^"]*)"$')
+@protocol.commands.add('add')
 def add(context, uri):
     """
     *musicpd.org, current playlist section:*
@@ -29,8 +26,7 @@ def add(context, uri):
 
     try:
         uri = context.directory_path_to_uri(translator.normalize_path(uri))
-    except MpdNoExistError as e:
-        e.command = 'add'
+    except exceptions.MpdNoExistError as e:
         e.message = 'directory or file not found'
         raise
 
@@ -48,12 +44,12 @@ def add(context, uri):
         tracks.extend(future.get())
 
     if not tracks:
-        raise MpdNoExistError('directory or file not found')
+        raise exceptions.MpdNoExistError('directory or file not found')
 
     context.core.tracklist.add(tracks=tracks)
 
 
-@handle_request(r'addid\ "(?P<uri>[^"]*)"(\ "(?P<songpos>\d+)")*$')
+@protocol.commands.add('addid', songpos=protocol.integer)
 def addid(context, uri, songpos=None):
     """
     *musicpd.org, current playlist section:*
@@ -72,20 +68,19 @@ def addid(context, uri, songpos=None):
 
     - ``addid ""`` should return an error.
     """
+    # TODO: check that songpos is positive
     if not uri:
-        raise MpdNoExistError('No such song')
-    if songpos is not None:
-        songpos = int(songpos)
-    if songpos and songpos > context.core.tracklist.length.get():
-        raise MpdArgError('Bad song index')
+        raise exceptions.MpdNoExistError('No such song')
+    if songpos is not None and songpos > context.core.tracklist.length.get():
+        raise exceptions.MpdArgError('Bad song index')
     tl_tracks = context.core.tracklist.add(uri=uri, at_position=songpos).get()
     if not tl_tracks:
-        raise MpdNoExistError('No such song')
+        raise exceptions.MpdNoExistError('No such song')
     return ('Id', tl_tracks[0].tlid)
 
 
-@handle_request(r'delete\ "(?P<start>\d+):(?P<end>\d+)*"$')
-def delete_range(context, start, end=None):
+@protocol.commands.add('delete', position=protocol.position_or_range)
+def delete(context, position):
     """
     *musicpd.org, current playlist section:*
 
@@ -93,31 +88,18 @@ def delete_range(context, start, end=None):
 
         Deletes a song from the playlist.
     """
-    start = int(start)
-    if end is not None:
-        end = int(end)
-    else:
+    start = position.start
+    end = position.stop
+    if end is None:
         end = context.core.tracklist.length.get()
     tl_tracks = context.core.tracklist.slice(start, end).get()
     if not tl_tracks:
-        raise MpdArgError('Bad song index', command='delete')
+        raise exceptions.MpdArgError('Bad song index', command='delete')
     for (tlid, _) in tl_tracks:
         context.core.tracklist.remove(tlid=[tlid])
 
 
-@handle_request(r'delete\ "(?P<songpos>\d+)"$')
-def delete_songpos(context, songpos):
-    """See :meth:`delete_range`"""
-    try:
-        songpos = int(songpos)
-        (tlid, _) = context.core.tracklist.slice(
-            songpos, songpos + 1).get()[0]
-        context.core.tracklist.remove(tlid=[tlid])
-    except IndexError:
-        raise MpdArgError('Bad song index', command='delete')
-
-
-@handle_request(r'deleteid\ "(?P<tlid>\d+)"$')
+@protocol.commands.add('deleteid', tlid=protocol.integer)
 def deleteid(context, tlid):
     """
     *musicpd.org, current playlist section:*
@@ -126,13 +108,13 @@ def deleteid(context, tlid):
 
         Deletes the song ``SONGID`` from the playlist
     """
-    tlid = int(tlid)
+    # TODO: check that tlid is positive
     tl_tracks = context.core.tracklist.remove(tlid=[tlid]).get()
     if not tl_tracks:
-        raise MpdNoExistError('No such song')
+        raise exceptions.MpdNoExistError('No such song')
 
 
-@handle_request(r'clear$')
+@protocol.commands.add('clear')
 def clear(context):
     """
     *musicpd.org, current playlist section:*
@@ -144,8 +126,9 @@ def clear(context):
     context.core.tracklist.clear()
 
 
-@handle_request(r'move\ "(?P<start>\d+):(?P<end>\d+)*"\ "(?P<to>\d+)"$')
-def move_range(context, start, to, end=None):
+@protocol.commands.add(
+    'move', position=protocol.position_or_range, to=protocol.integer)
+def move_range(context, position, to):
     """
     *musicpd.org, current playlist section:*
 
@@ -154,23 +137,15 @@ def move_range(context, start, to, end=None):
         Moves the song at ``FROM`` or range of songs at ``START:END`` to
         ``TO`` in the playlist.
     """
+    # TODO: check that to is positive
+    start = position.start
+    end = position.stop
     if end is None:
         end = context.core.tracklist.length.get()
-    start = int(start)
-    end = int(end)
-    to = int(to)
     context.core.tracklist.move(start, end, to)
 
 
-@handle_request(r'move\ "(?P<songpos>\d+)"\ "(?P<to>\d+)"$')
-def move_songpos(context, songpos, to):
-    """See :meth:`move_range`."""
-    songpos = int(songpos)
-    to = int(to)
-    context.core.tracklist.move(songpos, songpos + 1, to)
-
-
-@handle_request(r'moveid\ "(?P<tlid>\d+)"\ "(?P<to>\d+)"$')
+@protocol.commands.add('moveid', tlid=protocol.integer, to=protocol.integer)
 def moveid(context, tlid, to):
     """
     *musicpd.org, current playlist section:*
@@ -181,16 +156,15 @@ def moveid(context, tlid, to):
         the playlist. If ``TO`` is negative, it is relative to the current
         song in the playlist (if there is one).
     """
-    tlid = int(tlid)
-    to = int(to)
+    # TODO: check that tlid and to are positive
     tl_tracks = context.core.tracklist.filter(tlid=[tlid]).get()
     if not tl_tracks:
-        raise MpdNoExistError('No such song')
+        raise exceptions.MpdNoExistError('No such song')
     position = context.core.tracklist.index(tl_tracks[0]).get()
     context.core.tracklist.move(position, position + 1, to)
 
 
-@handle_request(r'playlist$')
+@protocol.commands.add('playlist')
 def playlist(context):
     """
     *musicpd.org, current playlist section:*
@@ -206,7 +180,7 @@ def playlist(context):
     return playlistinfo(context)
 
 
-@handle_request(r'playlistfind\ ("?)(?P<tag>[^"]+)\1\ "(?P<needle>[^"]+)"$')
+@protocol.commands.add('playlistfind')
 def playlistfind(context, tag, needle):
     """
     *musicpd.org, current playlist section:*
@@ -225,11 +199,10 @@ def playlistfind(context, tag, needle):
             return None
         position = context.core.tracklist.index(tl_tracks[0]).get()
         return translator.track_to_mpd_format(tl_tracks[0], position=position)
-    raise MpdNotImplemented  # TODO
+    raise exceptions.MpdNotImplemented  # TODO
 
 
-@handle_request(r'playlistid$')
-@handle_request(r'playlistid\ "(?P<tlid>\d+)"$')
+@protocol.commands.add('playlistid', tlid=protocol.integer)
 def playlistid(context, tlid=None):
     """
     *musicpd.org, current playlist section:*
@@ -239,11 +212,11 @@ def playlistid(context, tlid=None):
         Displays a list of songs in the playlist. ``SONGID`` is optional
         and specifies a single song to display info for.
     """
+    # TODO: check that tlid is positive if not none
     if tlid is not None:
-        tlid = int(tlid)
         tl_tracks = context.core.tracklist.filter(tlid=[tlid]).get()
         if not tl_tracks:
-            raise MpdNoExistError('No such song')
+            raise exceptions.MpdNoExistError('No such song')
         position = context.core.tracklist.index(tl_tracks[0]).get()
         return translator.track_to_mpd_format(tl_tracks[0], position=position)
     else:
@@ -251,9 +224,10 @@ def playlistid(context, tlid=None):
             context.core.tracklist.tl_tracks.get())
 
 
-@handle_request(r'playlistinfo$')
-@handle_request(r'playlistinfo\ "(?P<songpos>-?\d+)"$')
-@handle_request(r'playlistinfo\ "(?P<start>\d+):(?P<end>\d+)*"$')
+# TODO: convert
+@protocol.handle_request(r'playlistinfo$')
+@protocol.handle_request(r'playlistinfo\ "(?P<songpos>-?\d+)"$')
+@protocol.handle_request(r'playlistinfo\ "(?P<start>\d+):(?P<end>\d+)*"$')
 def playlistinfo(context, songpos=None, start=None, end=None):
     """
     *musicpd.org, current playlist section:*
@@ -280,7 +254,7 @@ def playlistinfo(context, songpos=None, start=None, end=None):
             start = 0
         start = int(start)
         if not (0 <= start <= context.core.tracklist.length.get()):
-            raise MpdArgError('Bad song index')
+            raise exceptions.MpdArgError('Bad song index')
         if end is not None:
             end = int(end)
             if end > context.core.tracklist.length.get():
@@ -289,7 +263,7 @@ def playlistinfo(context, songpos=None, start=None, end=None):
         return translator.tracks_to_mpd_format(tl_tracks, start, end)
 
 
-@handle_request(r'playlistsearch\ ("?)(?P<tag>\w+)\1\ "(?P<needle>[^"]+)"$')
+@protocol.commands.add('playlistsearch')
 def playlistsearch(context, tag, needle):
     """
     *musicpd.org, current playlist section:*
@@ -304,10 +278,10 @@ def playlistsearch(context, tag, needle):
     - does not add quotes around the tag
     - uses ``filename`` and ``any`` as tags
     """
-    raise MpdNotImplemented  # TODO
+    raise exceptions.MpdNotImplemented  # TODO
 
 
-@handle_request(r'plchanges\ ("?)(?P<version>-?\d+)\1$')
+@protocol.commands.add('plchanges', version=protocol.integer)
 def plchanges(context, version):
     """
     *musicpd.org, current playlist section:*
@@ -329,7 +303,7 @@ def plchanges(context, version):
             context.core.tracklist.tl_tracks.get())
 
 
-@handle_request(r'plchangesposid\ "(?P<version>\d+)"$')
+@protocol.commands.add('plchangesposid', version=protocol.integer)
 def plchangesposid(context, version):
     """
     *musicpd.org, current playlist section:*
@@ -353,9 +327,8 @@ def plchangesposid(context, version):
         return result
 
 
-@handle_request(r'shuffle$')
-@handle_request(r'shuffle\ "(?P<start>\d+):(?P<end>\d+)*"$')
-def shuffle(context, start=None, end=None):
+@protocol.commands.add('shuffle', position=protocol.position_or_range)
+def shuffle(context, position=None):
     """
     *musicpd.org, current playlist section:*
 
@@ -364,14 +337,15 @@ def shuffle(context, start=None, end=None):
         Shuffles the current playlist. ``START:END`` is optional and
         specifies a range of songs.
     """
-    if start is not None:
-        start = int(start)
-    if end is not None:
-        end = int(end)
+    if position is None:
+        start, end = None, None
+    else:
+        start, end = position.start, position.stop
     context.core.tracklist.shuffle(start, end)
 
 
-@handle_request(r'swap\ "(?P<songpos1>\d+)"\ "(?P<songpos2>\d+)"$')
+@protocol.commands.add(
+    'swap', songpos1=protocol.integer, songpos2=protocol.integer)
 def swap(context, songpos1, songpos2):
     """
     *musicpd.org, current playlist section:*
@@ -380,8 +354,7 @@ def swap(context, songpos1, songpos2):
 
         Swaps the positions of ``SONG1`` and ``SONG2``.
     """
-    songpos1 = int(songpos1)
-    songpos2 = int(songpos2)
+    # TODO: check that songpos is positive
     tracks = context.core.tracklist.tracks.get()
     song1 = tracks[songpos1]
     song2 = tracks[songpos2]
@@ -393,7 +366,8 @@ def swap(context, songpos1, songpos2):
     context.core.tracklist.add(tracks)
 
 
-@handle_request(r'swapid\ "(?P<tlid1>\d+)"\ "(?P<tlid2>\d+)"$')
+@protocol.commands.add(
+    'swapid', tlid1=protocol.integer, tlid2=protocol.integer)
 def swapid(context, tlid1, tlid2):
     """
     *musicpd.org, current playlist section:*
@@ -402,12 +376,11 @@ def swapid(context, tlid1, tlid2):
 
         Swaps the positions of ``SONG1`` and ``SONG2`` (both song ids).
     """
-    tlid1 = int(tlid1)
-    tlid2 = int(tlid2)
+    # TODO: check that tlid is positive
     tl_tracks1 = context.core.tracklist.filter(tlid=[tlid1]).get()
     tl_tracks2 = context.core.tracklist.filter(tlid=[tlid2]).get()
     if not tl_tracks1 or not tl_tracks2:
-        raise MpdNoExistError('No such song')
+        raise exceptions.MpdNoExistError('No such song')
     position1 = context.core.tracklist.index(tl_tracks1[0]).get()
     position2 = context.core.tracklist.index(tl_tracks2[0]).get()
     swap(context, position1, position2)

From fddb299ed96ed811173510678014f3cf3ae49c46 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Thu, 23 Jan 2014 21:31:18 +0100
Subject: [PATCH 17/35] mpd: Update type converters naming and add UINT

Also adds tests for RANGE helper.
---
 mopidy/mpd/protocol/__init__.py         | 23 +++++++---
 mopidy/mpd/protocol/current_playlist.py | 33 ++++++---------
 mopidy/mpd/protocol/playback.py         | 34 ++++++---------
 tests/mpd/test_commands.py              | 56 ++++++++++++++++++-------
 4 files changed, 83 insertions(+), 63 deletions(-)

diff --git a/mopidy/mpd/protocol/__init__.py b/mopidy/mpd/protocol/__init__.py
index 9e85d058..2b0d2cbe 100644
--- a/mopidy/mpd/protocol/__init__.py
+++ b/mopidy/mpd/protocol/__init__.py
@@ -96,29 +96,40 @@ def load_protocol_modules():
         stored_playlists)
 
 
-def integer(value):
+def INT(value):
     if value is None:
         raise ValueError('None is not a valid integer')
+    # TODO: check for whitespace via value != value.strip()?
     return int(value)
 
 
-def boolean(value):
+def UINT(value):
+    if value is None:
+        raise ValueError('None is not a valid integer')
+    if not value.isdigit():
+        raise ValueError('Only positive numbers are allowed')
+    return int(value)
+
+
+def BOOL(value):
     if value in ('1', '0'):
         return bool(int(value))
     raise ValueError('%r is not 0 or 1' % value)
 
 
-def position_or_range(value):
+def RANGE(value):
     # TODO: test and check that values are positive
     if ':' in value:
         start, stop = value.split(':', 1)
-        start = int(start)
+        start = UINT(start)
         if stop.strip():
-            stop = int(stop)
+            stop = UINT(stop)
+            if start >= stop:
+                raise ValueError('End must be larger than start')
         else:
             stop = None
     else:
-        start = int(value)
+        start = UINT(value)
         stop = start + 1
     return slice(start, stop)
 
diff --git a/mopidy/mpd/protocol/current_playlist.py b/mopidy/mpd/protocol/current_playlist.py
index 41a4782b..662cac8b 100644
--- a/mopidy/mpd/protocol/current_playlist.py
+++ b/mopidy/mpd/protocol/current_playlist.py
@@ -49,7 +49,7 @@ def add(context, uri):
     context.core.tracklist.add(tracks=tracks)
 
 
-@protocol.commands.add('addid', songpos=protocol.integer)
+@protocol.commands.add('addid', songpos=protocol.UINT)
 def addid(context, uri, songpos=None):
     """
     *musicpd.org, current playlist section:*
@@ -68,7 +68,6 @@ def addid(context, uri, songpos=None):
 
     - ``addid ""`` should return an error.
     """
-    # TODO: check that songpos is positive
     if not uri:
         raise exceptions.MpdNoExistError('No such song')
     if songpos is not None and songpos > context.core.tracklist.length.get():
@@ -79,7 +78,7 @@ def addid(context, uri, songpos=None):
     return ('Id', tl_tracks[0].tlid)
 
 
-@protocol.commands.add('delete', position=protocol.position_or_range)
+@protocol.commands.add('delete', position=protocol.RANGE)
 def delete(context, position):
     """
     *musicpd.org, current playlist section:*
@@ -99,7 +98,7 @@ def delete(context, position):
         context.core.tracklist.remove(tlid=[tlid])
 
 
-@protocol.commands.add('deleteid', tlid=protocol.integer)
+@protocol.commands.add('deleteid', tlid=protocol.UINT)
 def deleteid(context, tlid):
     """
     *musicpd.org, current playlist section:*
@@ -108,7 +107,6 @@ def deleteid(context, tlid):
 
         Deletes the song ``SONGID`` from the playlist
     """
-    # TODO: check that tlid is positive
     tl_tracks = context.core.tracklist.remove(tlid=[tlid]).get()
     if not tl_tracks:
         raise exceptions.MpdNoExistError('No such song')
@@ -126,8 +124,7 @@ def clear(context):
     context.core.tracklist.clear()
 
 
-@protocol.commands.add(
-    'move', position=protocol.position_or_range, to=protocol.integer)
+@protocol.commands.add('move', position=protocol.RANGE, to=protocol.UINT)
 def move_range(context, position, to):
     """
     *musicpd.org, current playlist section:*
@@ -137,7 +134,6 @@ def move_range(context, position, to):
         Moves the song at ``FROM`` or range of songs at ``START:END`` to
         ``TO`` in the playlist.
     """
-    # TODO: check that to is positive
     start = position.start
     end = position.stop
     if end is None:
@@ -145,7 +141,7 @@ def move_range(context, position, to):
     context.core.tracklist.move(start, end, to)
 
 
-@protocol.commands.add('moveid', tlid=protocol.integer, to=protocol.integer)
+@protocol.commands.add('moveid', tlid=protocol.UINT, to=protocol.UINT)
 def moveid(context, tlid, to):
     """
     *musicpd.org, current playlist section:*
@@ -156,7 +152,6 @@ def moveid(context, tlid, to):
         the playlist. If ``TO`` is negative, it is relative to the current
         song in the playlist (if there is one).
     """
-    # TODO: check that tlid and to are positive
     tl_tracks = context.core.tracklist.filter(tlid=[tlid]).get()
     if not tl_tracks:
         raise exceptions.MpdNoExistError('No such song')
@@ -177,6 +172,7 @@ def playlist(context):
 
             Do not use this, instead use ``playlistinfo``.
     """
+    # TODO: warn about this being deprecated
     return playlistinfo(context)
 
 
@@ -202,7 +198,7 @@ def playlistfind(context, tag, needle):
     raise exceptions.MpdNotImplemented  # TODO
 
 
-@protocol.commands.add('playlistid', tlid=protocol.integer)
+@protocol.commands.add('playlistid', tlid=protocol.UINT)
 def playlistid(context, tlid=None):
     """
     *musicpd.org, current playlist section:*
@@ -212,7 +208,6 @@ def playlistid(context, tlid=None):
         Displays a list of songs in the playlist. ``SONGID`` is optional
         and specifies a single song to display info for.
     """
-    # TODO: check that tlid is positive if not none
     if tlid is not None:
         tl_tracks = context.core.tracklist.filter(tlid=[tlid]).get()
         if not tl_tracks:
@@ -281,7 +276,7 @@ def playlistsearch(context, tag, needle):
     raise exceptions.MpdNotImplemented  # TODO
 
 
-@protocol.commands.add('plchanges', version=protocol.integer)
+@protocol.commands.add('plchanges', version=protocol.INT)
 def plchanges(context, version):
     """
     *musicpd.org, current playlist section:*
@@ -303,7 +298,7 @@ def plchanges(context, version):
             context.core.tracklist.tl_tracks.get())
 
 
-@protocol.commands.add('plchangesposid', version=protocol.integer)
+@protocol.commands.add('plchangesposid', version=protocol.INT)
 def plchangesposid(context, version):
     """
     *musicpd.org, current playlist section:*
@@ -327,7 +322,7 @@ def plchangesposid(context, version):
         return result
 
 
-@protocol.commands.add('shuffle', position=protocol.position_or_range)
+@protocol.commands.add('shuffle', position=protocol.RANGE)
 def shuffle(context, position=None):
     """
     *musicpd.org, current playlist section:*
@@ -344,8 +339,7 @@ def shuffle(context, position=None):
     context.core.tracklist.shuffle(start, end)
 
 
-@protocol.commands.add(
-    'swap', songpos1=protocol.integer, songpos2=protocol.integer)
+@protocol.commands.add('swap', songpos1=protocol.UINT, songpos2=protocol.UINT)
 def swap(context, songpos1, songpos2):
     """
     *musicpd.org, current playlist section:*
@@ -354,7 +348,6 @@ def swap(context, songpos1, songpos2):
 
         Swaps the positions of ``SONG1`` and ``SONG2``.
     """
-    # TODO: check that songpos is positive
     tracks = context.core.tracklist.tracks.get()
     song1 = tracks[songpos1]
     song2 = tracks[songpos2]
@@ -366,8 +359,7 @@ def swap(context, songpos1, songpos2):
     context.core.tracklist.add(tracks)
 
 
-@protocol.commands.add(
-    'swapid', tlid1=protocol.integer, tlid2=protocol.integer)
+@protocol.commands.add('swapid', tlid1=protocol.UINT, tlid2=protocol.UINT)
 def swapid(context, tlid1, tlid2):
     """
     *musicpd.org, current playlist section:*
@@ -376,7 +368,6 @@ def swapid(context, tlid1, tlid2):
 
         Swaps the positions of ``SONG1`` and ``SONG2`` (both song ids).
     """
-    # TODO: check that tlid is positive
     tl_tracks1 = context.core.tracklist.filter(tlid=[tlid1]).get()
     tl_tracks2 = context.core.tracklist.filter(tlid=[tlid2]).get()
     if not tl_tracks1 or not tl_tracks2:
diff --git a/mopidy/mpd/protocol/playback.py b/mopidy/mpd/protocol/playback.py
index d5100ee1..d18aff50 100644
--- a/mopidy/mpd/protocol/playback.py
+++ b/mopidy/mpd/protocol/playback.py
@@ -4,7 +4,7 @@ from mopidy.core import PlaybackState
 from mopidy.mpd import exceptions, protocol
 
 
-@protocol.commands.add('consume', state=protocol.boolean)
+@protocol.commands.add('consume', state=protocol.BOOL)
 def consume(context, state):
     """
     *musicpd.org, playback section:*
@@ -18,7 +18,7 @@ def consume(context, state):
     context.core.tracklist.consume = state
 
 
-@protocol.commands.add('crossfade', seconds=protocol.integer)
+@protocol.commands.add('crossfade', seconds=protocol.UINT)
 def crossfade(context, seconds):
     """
     *musicpd.org, playback section:*
@@ -27,7 +27,6 @@ def crossfade(context, seconds):
 
         Sets crossfading between songs.
     """
-    # TODO: check that seconds is positive
     raise exceptions.MpdNotImplemented  # TODO
 
 
@@ -89,7 +88,7 @@ def next_(context):
     return context.core.playback.next().get()
 
 
-@protocol.commands.add('pause', state=protocol.boolean)
+@protocol.commands.add('pause', state=protocol.BOOL)
 def pause(context, state=None):
     """
     *musicpd.org, playback section:*
@@ -114,7 +113,7 @@ def pause(context, state=None):
         context.core.playback.resume()
 
 
-@protocol.commands.add('play', tlid=protocol.integer)
+@protocol.commands.add('play', tlid=protocol.INT)
 def play(context, tlid=None):
     """
     *musicpd.org, playback section:*
@@ -166,7 +165,7 @@ def _play_minus_one(context):
         return  # Fail silently
 
 
-@protocol.commands.add('playid', tlid=protocol.integer)
+@protocol.commands.add('playid', tlid=protocol.INT)
 def playid(context, tlid):
     """
     *musicpd.org, playback section:*
@@ -239,7 +238,7 @@ def previous(context):
     return context.core.playback.previous().get()
 
 
-@protocol.commands.add('random', state=protocol.boolean)
+@protocol.commands.add('random', state=protocol.BOOL)
 def random(context, state):
     """
     *musicpd.org, playback section:*
@@ -251,7 +250,7 @@ def random(context, state):
     context.core.tracklist.random = state
 
 
-@protocol.commands.add('repeat', state=protocol.boolean)
+@protocol.commands.add('repeat', state=protocol.BOOL)
 def repeat(context, state):
     """
     *musicpd.org, playback section:*
@@ -293,8 +292,7 @@ def replay_gain_status(context):
     return 'off'  # TODO
 
 
-@protocol.commands.add(
-    'seek', tlid=protocol.integer, seconds=protocol.integer)
+@protocol.commands.add('seek', tlid=protocol.UINT, seconds=protocol.UINT)
 def seek(context, tlid, seconds):
     """
     *musicpd.org, playback section:*
@@ -308,15 +306,13 @@ def seek(context, tlid, seconds):
 
     - issues ``seek 1 120`` without quotes around the arguments.
     """
-    # TODO: check tlid is positive
     tl_track = context.core.playback.current_tl_track.get()
     if context.core.tracklist.index(tl_track).get() != tlid:
         play(context, tlid)
     context.core.playback.seek(seconds * 1000).get()
 
 
-@protocol.commands.add(
-    'seekid', tlid=protocol.integer, seconds=protocol.integer)
+@protocol.commands.add('seekid', tlid=protocol.UINT, seconds=protocol.UINT)
 def seekid(context, tlid, seconds):
     """
     *musicpd.org, playback section:*
@@ -325,7 +321,6 @@ def seekid(context, tlid, seconds):
 
         Seeks to the position ``TIME`` (in seconds) of song ``SONGID``.
     """
-    # TODO: check that songid and time is positive
     tl_track = context.core.playback.current_tl_track.get()
     if not tl_track or tl_track.tlid != tlid:
         playid(context, tlid)
@@ -351,7 +346,7 @@ def seekcur(context, time):
         context.core.playback.seek(position).get()
 
 
-@protocol.commands.add('setvol', volume=protocol.integer)
+@protocol.commands.add('setvol', volume=protocol.INT)
 def setvol(context, volume):
     """
     *musicpd.org, playback section:*
@@ -364,14 +359,11 @@ def setvol(context, volume):
 
     - issues ``setvol 50`` without quotes around the argument.
     """
-    if volume < 0:
-        volume = 0
-    if volume > 100:
-        volume = 100
-    context.core.playback.volume = volume
+    # NOTE: we use INT as clients can pass in +N etc.
+    context.core.playback.volume = min(max(0, volume), 100)
 
 
-@protocol.commands.add('single', state=protocol.boolean)
+@protocol.commands.add('single', state=protocol.BOOL)
 def single(context, state):
     """
     *musicpd.org, playback section:*
diff --git a/tests/mpd/test_commands.py b/tests/mpd/test_commands.py
index f0f04bb7..845796af 100644
--- a/tests/mpd/test_commands.py
+++ b/tests/mpd/test_commands.py
@@ -9,23 +9,49 @@ from mopidy.mpd import protocol
 
 class TestConverts(unittest.TestCase):
     def test_integer(self):
-        self.assertEqual(123, protocol.integer('123'))
-        self.assertEqual(-123, protocol.integer('-123'))
-        self.assertEqual(123, protocol.integer('+123'))
-        self.assertRaises(ValueError, protocol.integer, '3.14')
-        self.assertRaises(ValueError, protocol.integer, '')
-        self.assertRaises(ValueError, protocol.integer, 'abc')
-        self.assertRaises(ValueError, protocol.integer, '12 34')
+        self.assertEqual(123, protocol.INT('123'))
+        self.assertEqual(-123, protocol.INT('-123'))
+        self.assertEqual(123, protocol.INT('+123'))
+        self.assertRaises(ValueError, protocol.INT, '3.14')
+        self.assertRaises(ValueError, protocol.INT, '')
+        self.assertRaises(ValueError, protocol.INT, 'abc')
+        self.assertRaises(ValueError, protocol.INT, '12 34')
+
+    def test_unsigned_integer(self):
+        self.assertEqual(123, protocol.UINT('123'))
+        self.assertRaises(ValueError, protocol.UINT, '-123')
+        self.assertRaises(ValueError, protocol.UINT, '+123')
+        self.assertRaises(ValueError, protocol.UINT, '3.14')
+        self.assertRaises(ValueError, protocol.UINT, '')
+        self.assertRaises(ValueError, protocol.UINT, 'abc')
+        self.assertRaises(ValueError, protocol.UINT, '12 34')
 
     def test_boolean(self):
-        self.assertEqual(True, protocol.boolean('1'))
-        self.assertEqual(False, protocol.boolean('0'))
-        self.assertRaises(ValueError, protocol.boolean, '3.14')
-        self.assertRaises(ValueError, protocol.boolean, '')
-        self.assertRaises(ValueError, protocol.boolean, 'true')
-        self.assertRaises(ValueError, protocol.boolean, 'false')
-        self.assertRaises(ValueError, protocol.boolean, 'abc')
-        self.assertRaises(ValueError, protocol.boolean, '12 34')
+        self.assertEqual(True, protocol.BOOL('1'))
+        self.assertEqual(False, protocol.BOOL('0'))
+        self.assertRaises(ValueError, protocol.BOOL, '3.14')
+        self.assertRaises(ValueError, protocol.BOOL, '')
+        self.assertRaises(ValueError, protocol.BOOL, 'true')
+        self.assertRaises(ValueError, protocol.BOOL, 'false')
+        self.assertRaises(ValueError, protocol.BOOL, 'abc')
+        self.assertRaises(ValueError, protocol.BOOL, '12 34')
+
+    def test_range(self):
+        self.assertEqual(slice(1, 2), protocol.RANGE('1'))
+        self.assertEqual(slice(0, 1), protocol.RANGE('0'))
+        self.assertEqual(slice(0, None), protocol.RANGE('0:'))
+        self.assertEqual(slice(1, 3), protocol.RANGE('1:3'))
+        self.assertRaises(ValueError, protocol.RANGE, '3.14')
+        self.assertRaises(ValueError, protocol.RANGE, '1:abc')
+        self.assertRaises(ValueError, protocol.RANGE, 'abc:1')
+        self.assertRaises(ValueError, protocol.RANGE, '2:1')
+        self.assertRaises(ValueError, protocol.RANGE, '-1:2')
+        self.assertRaises(ValueError, protocol.RANGE, '1 : 2')
+        self.assertRaises(ValueError, protocol.RANGE, '')
+        self.assertRaises(ValueError, protocol.RANGE, 'true')
+        self.assertRaises(ValueError, protocol.RANGE, 'false')
+        self.assertRaises(ValueError, protocol.RANGE, 'abc')
+        self.assertRaises(ValueError, protocol.RANGE, '12 34')
 
 
 class TestCommands(unittest.TestCase):

From 1dc35c2bf7448659d831a0c0c7b2fb4b4840a4d5 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Thu, 23 Jan 2014 21:41:06 +0100
Subject: [PATCH 18/35] mpd: Convert channels and stickers and delete empty.

---
 mopidy/mpd/protocol/channels.py | 27 +++++------
 mopidy/mpd/protocol/empty.py    | 10 -----
 mopidy/mpd/protocol/stickers.py | 79 +++++++++------------------------
 3 files changed, 35 insertions(+), 81 deletions(-)
 delete mode 100644 mopidy/mpd/protocol/empty.py

diff --git a/mopidy/mpd/protocol/channels.py b/mopidy/mpd/protocol/channels.py
index e8efd2a0..4ae00622 100644
--- a/mopidy/mpd/protocol/channels.py
+++ b/mopidy/mpd/protocol/channels.py
@@ -1,10 +1,9 @@
 from __future__ import unicode_literals
 
-from mopidy.mpd.protocol import handle_request
-from mopidy.mpd.exceptions import MpdNotImplemented
+from mopidy.mpd import exceptions, protocol
 
 
-@handle_request(r'subscribe\ "(?P<channel>[A-Za-z0-9:._-]+)"$')
+@protocol.commands.add('subscribe')
 def subscribe(context, channel):
     """
     *musicpd.org, client to client section:*
@@ -15,10 +14,11 @@ def subscribe(context, channel):
         already. The name may consist of alphanumeric ASCII characters plus
         underscore, dash, dot and colon.
     """
-    raise MpdNotImplemented  # TODO
+    # TODO: match channel against [A-Za-z0-9:._-]+
+    raise exceptions.MpdNotImplemented  # TODO
 
 
-@handle_request(r'unsubscribe\ "(?P<channel>[A-Za-z0-9:._-]+)"$')
+@protocol.commands.add('unsubscribe')
 def unsubscribe(context, channel):
     """
     *musicpd.org, client to client section:*
@@ -27,10 +27,11 @@ def unsubscribe(context, channel):
 
         Unsubscribe from a channel.
     """
-    raise MpdNotImplemented  # TODO
+    # TODO: match channel against [A-Za-z0-9:._-]+
+    raise exceptions.MpdNotImplemented  # TODO
 
 
-@handle_request(r'channels$')
+@protocol.commands.add('channels')
 def channels(context):
     """
     *musicpd.org, client to client section:*
@@ -40,10 +41,10 @@ def channels(context):
         Obtain a list of all channels. The response is a list of "channel:"
         lines.
     """
-    raise MpdNotImplemented  # TODO
+    raise exceptions.MpdNotImplemented  # TODO
 
 
-@handle_request(r'readmessages$')
+@protocol.commands.add('readmessages')
 def readmessages(context):
     """
     *musicpd.org, client to client section:*
@@ -53,11 +54,10 @@ def readmessages(context):
         Reads messages for this client. The response is a list of "channel:"
         and "message:" lines.
     """
-    raise MpdNotImplemented  # TODO
+    raise exceptions.MpdNotImplemented  # TODO
 
 
-@handle_request(
-    r'sendmessage\ "(?P<channel>[A-Za-z0-9:._-]+)"\ "(?P<text>[^"]*)"$')
+@protocol.commands.add('sendmessage')
 def sendmessage(context, channel, text):
     """
     *musicpd.org, client to client section:*
@@ -66,4 +66,5 @@ def sendmessage(context, channel, text):
 
         Send a message to the specified channel.
     """
-    raise MpdNotImplemented  # TODO
+    # TODO: match channel against [A-Za-z0-9:._-]+
+    raise exceptions.MpdNotImplemented  # TODO
diff --git a/mopidy/mpd/protocol/empty.py b/mopidy/mpd/protocol/empty.py
deleted file mode 100644
index 64cfc1fb..00000000
--- a/mopidy/mpd/protocol/empty.py
+++ /dev/null
@@ -1,10 +0,0 @@
-from __future__ import unicode_literals
-
-from mopidy.mpd.protocol import handle_request
-from mopidy.mpd.exceptions import MpdNoCommand
-
-
-@handle_request(r'[\ ]*$')
-def empty(context):
-    """The original MPD server returns an error on an empty request."""
-    raise MpdNoCommand()
diff --git a/mopidy/mpd/protocol/stickers.py b/mopidy/mpd/protocol/stickers.py
index 17798523..53ce0caa 100644
--- a/mopidy/mpd/protocol/stickers.py
+++ b/mopidy/mpd/protocol/stickers.py
@@ -1,76 +1,39 @@
 from __future__ import unicode_literals
 
-from mopidy.mpd.protocol import handle_request
-from mopidy.mpd.exceptions import MpdNotImplemented
+from mopidy.mpd import exceptions, protocol
 
 
-@handle_request(
-    r'sticker\ delete\ "(?P<field>[^"]+)"\ '
-    r'"(?P<uri>[^"]+)"(\ "(?P<name>[^"]+)")*$')
-def sticker__delete(context, field, uri, name=None):
-    """
-    *musicpd.org, sticker section:*
-
-        ``sticker delete {TYPE} {URI} [NAME]``
-
-        Deletes a sticker value from the specified object. If you do not
-        specify a sticker name, all sticker values are deleted.
-    """
-    raise MpdNotImplemented  # TODO
-
-
-@handle_request(
-    r'sticker\ find\ "(?P<field>[^"]+)"\ "(?P<uri>[^"]+)"\ '
-    r'"(?P<name>[^"]+)"$')
-def sticker__find(context, field, uri, name):
-    """
-    *musicpd.org, sticker section:*
-
-        ``sticker find {TYPE} {URI} {NAME}``
-
-        Searches the sticker database for stickers with the specified name,
-        below the specified directory (``URI``). For each matching song, it
-        prints the ``URI`` and that one sticker's value.
-    """
-    raise MpdNotImplemented  # TODO
-
-
-@handle_request(
-    r'sticker\ get\ "(?P<field>[^"]+)"\ "(?P<uri>[^"]+)"\ '
-    r'"(?P<name>[^"]+)"$')
-def sticker__get(context, field, uri, name):
-    """
-    *musicpd.org, sticker section:*
-
-        ``sticker get {TYPE} {URI} {NAME}``
-
-        Reads a sticker value for the specified object.
-    """
-    raise MpdNotImplemented  # TODO
-
-
-@handle_request(r'sticker\ list\ "(?P<field>[^"]+)"\ "(?P<uri>[^"]+)"$')
-def sticker__list(context, field, uri):
+@protocol.commands.add('sticker')
+def sticker(context, action, field, uri, name=None, value=None):
     """
     *musicpd.org, sticker section:*
 
         ``sticker list {TYPE} {URI}``
 
         Lists the stickers for the specified object.
-    """
-    raise MpdNotImplemented  # TODO
 
+        ``sticker find {TYPE} {URI} {NAME}``
+
+        Searches the sticker database for stickers with the specified name,
+        below the specified directory (``URI``). For each matching song, it
+        prints the ``URI`` and that one sticker's value.
+
+        ``sticker get {TYPE} {URI} {NAME}``
+
+        Reads a sticker value for the specified object.
 
-@handle_request(
-    r'sticker\ set\ "(?P<field>[^"]+)"\ "(?P<uri>[^"]+)"\ '
-    r'"(?P<name>[^"]+)"\ "(?P<value>[^"]+)"$')
-def sticker__set(context, field, uri, name, value):
-    """
-    *musicpd.org, sticker section:*
 
         ``sticker set {TYPE} {URI} {NAME} {VALUE}``
 
         Adds a sticker value to the specified object. If a sticker item
         with that name already exists, it is replaced.
+
+        ``sticker delete {TYPE} {URI} [NAME]``
+
+        Deletes a sticker value from the specified object. If you do not
+        specify a sticker name, all sticker values are deleted.
+
     """
-    raise MpdNotImplemented  # TODO
+    # TODO: check that action in ('list', 'find', 'get', 'set', 'delete')
+    # TODO: check name/value matches with action
+    raise exceptions.MpdNotImplemented  # TODO

From a2ae51ff6503da7e4c2644645e77e714f22c4f8b Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Thu, 23 Jan 2014 22:02:23 +0100
Subject: [PATCH 19/35] mpd: Update auth filter and convert audio_output and
 connection

---
 mopidy/mpd/dispatcher.py            | 13 +++++++++++--
 mopidy/mpd/protocol/audio_output.py | 17 ++++++++---------
 mopidy/mpd/protocol/connection.py   | 16 +++++++---------
 3 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/mopidy/mpd/dispatcher.py b/mopidy/mpd/dispatcher.py
index 8a3602d4..5194885f 100644
--- a/mopidy/mpd/dispatcher.py
+++ b/mopidy/mpd/dispatcher.py
@@ -37,6 +37,7 @@ class MpdDispatcher(object):
         response = []
         filter_chain = [
             self._catch_mpd_ack_errors_filter,
+            # TODO: tokenize filter
             self._authenticate_filter,
             self._command_list_filter,
             self._idle_filter,
@@ -88,9 +89,17 @@ class MpdDispatcher(object):
             return self._call_next_filter(request, response, filter_chain)
         else:
             command_name = request.split(' ')[0]
+            command = protocol.commands.handlers.get(command_name)
+            if command:
+                if command.auth_required:
+                    raise exceptions.MpdPermissionError(command=command_name)
+                else:
+                    return self._call_next_filter(
+                        request, response, filter_chain)
+
+            # TODO: remove
             command_names_not_requiring_auth = [
-                command.name for command in protocol.mpd_commands
-                if not command.auth_required]
+                c.name for c in protocol.mpd_commands if not c.auth_required]
             if command_name in command_names_not_requiring_auth:
                 return self._call_next_filter(request, response, filter_chain)
             else:
diff --git a/mopidy/mpd/protocol/audio_output.py b/mopidy/mpd/protocol/audio_output.py
index 802be6c0..981cdd55 100644
--- a/mopidy/mpd/protocol/audio_output.py
+++ b/mopidy/mpd/protocol/audio_output.py
@@ -1,10 +1,9 @@
 from __future__ import unicode_literals
 
-from mopidy.mpd.exceptions import MpdNoExistError
-from mopidy.mpd.protocol import handle_request
+from mopidy.mpd import exceptions, protocol
 
 
-@handle_request(r'disableoutput\ "(?P<outputid>\d+)"$')
+@protocol.commands.add('disableoutput', outputid=protocol.UINT)
 def disableoutput(context, outputid):
     """
     *musicpd.org, audio output section:*
@@ -13,13 +12,13 @@ def disableoutput(context, outputid):
 
         Turns an output off.
     """
-    if int(outputid) == 0:
+    if outputid == 0:
         context.core.playback.set_mute(False)
     else:
-        raise MpdNoExistError('No such audio output')
+        raise exceptions.MpdNoExistError('No such audio output')
 
 
-@handle_request(r'enableoutput\ "(?P<outputid>\d+)"$')
+@protocol.commands.add('enableoutput', outputid=protocol.UINT)
 def enableoutput(context, outputid):
     """
     *musicpd.org, audio output section:*
@@ -28,13 +27,13 @@ def enableoutput(context, outputid):
 
         Turns an output on.
     """
-    if int(outputid) == 0:
+    if outputid == 0:
         context.core.playback.set_mute(True)
     else:
-        raise MpdNoExistError('No such audio output')
+        raise exceptions.MpdNoExistError('No such audio output')
 
 
-@handle_request(r'outputs$')
+@protocol.commands.add('outputs')
 def outputs(context):
     """
     *musicpd.org, audio output section:*
diff --git a/mopidy/mpd/protocol/connection.py b/mopidy/mpd/protocol/connection.py
index a6f9ffcb..a1a643c3 100644
--- a/mopidy/mpd/protocol/connection.py
+++ b/mopidy/mpd/protocol/connection.py
@@ -1,11 +1,9 @@
 from __future__ import unicode_literals
 
-from mopidy.mpd.protocol import handle_request
-from mopidy.mpd.exceptions import (
-    MpdPasswordError, MpdPermissionError)
+from mopidy.mpd import exceptions, protocol
 
 
-@handle_request(r'close$', auth_required=False)
+@protocol.commands.add('close', auth_required=False)
 def close(context):
     """
     *musicpd.org, connection section:*
@@ -17,7 +15,7 @@ def close(context):
     context.session.close()
 
 
-@handle_request(r'kill$')
+@protocol.commands.add('kill')
 def kill(context):
     """
     *musicpd.org, connection section:*
@@ -26,10 +24,10 @@ def kill(context):
 
         Kills MPD.
     """
-    raise MpdPermissionError(command='kill')
+    raise exceptions.MpdPermissionError(command='kill')
 
 
-@handle_request(r'password\ "(?P<password>[^"]+)"$', auth_required=False)
+@protocol.commands.add('password', auth_required=False)
 def password(context, password):
     """
     *musicpd.org, connection section:*
@@ -42,10 +40,10 @@ def password(context, password):
     if password == context.config['mpd']['password']:
         context.dispatcher.authenticated = True
     else:
-        raise MpdPasswordError('incorrect password')
+        raise exceptions.MpdPasswordError('incorrect password')
 
 
-@handle_request(r'ping$', auth_required=False)
+@protocol.commands.add('ping', auth_required=False)
 def ping(context):
     """
     *musicpd.org, connection section:*

From 047ed40ccc8bc3bdab100281fb7d4a019319da5a Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Thu, 23 Jan 2014 22:28:37 +0100
Subject: [PATCH 20/35] mpd: Stop string escaping all input as we have a proper
 tokenizer

---
 mopidy/mpd/dispatcher.py | 2 ++
 mopidy/mpd/session.py    | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/mopidy/mpd/dispatcher.py b/mopidy/mpd/dispatcher.py
index 5194885f..849d3821 100644
--- a/mopidy/mpd/dispatcher.py
+++ b/mopidy/mpd/dispatcher.py
@@ -182,6 +182,8 @@ class MpdDispatcher(object):
             raise
         except LookupError:
             pass  # Command has not been converted, i.e. fallback...
+
+        request = request.decode('string_escape')
         (command_name, handler, kwargs) = self._find_handler(request)
         try:
             return handler(self.context, **kwargs)
diff --git a/mopidy/mpd/session.py b/mopidy/mpd/session.py
index 2c0bd840..f0317ede 100644
--- a/mopidy/mpd/session.py
+++ b/mopidy/mpd/session.py
@@ -45,7 +45,7 @@ class MpdSession(network.LineProtocol):
 
     def decode(self, line):
         try:
-            return super(MpdSession, self).decode(line.decode('string_escape'))
+            return super(MpdSession, self).decode(line)
         except ValueError:
             logger.warning(
                 'Stopping actor due to unescaping error, data '

From 01a62d3ada6413b666ac9d2de7f9eafcb1c815a8 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Thu, 23 Jan 2014 22:28:53 +0100
Subject: [PATCH 21/35] mpd: Convert command lists and stored playlists

---
 mopidy/mpd/protocol/command_list.py     | 12 ++---
 mopidy/mpd/protocol/stored_playlists.py | 64 +++++++++++--------------
 2 files changed, 34 insertions(+), 42 deletions(-)

diff --git a/mopidy/mpd/protocol/command_list.py b/mopidy/mpd/protocol/command_list.py
index 8268c55d..d800ab72 100644
--- a/mopidy/mpd/protocol/command_list.py
+++ b/mopidy/mpd/protocol/command_list.py
@@ -1,10 +1,9 @@
 from __future__ import unicode_literals
 
-from mopidy.mpd.protocol import handle_request
-from mopidy.mpd.exceptions import MpdUnknownCommand
+from mopidy.mpd import exceptions, protocol
 
 
-@handle_request(r'command_list_begin$')
+@protocol.commands.add('command_list_begin')
 def command_list_begin(context):
     """
     *musicpd.org, command list section:*
@@ -26,11 +25,12 @@ def command_list_begin(context):
     context.dispatcher.command_list = []
 
 
-@handle_request(r'command_list_end$')
+@protocol.commands.add('command_list_end')
 def command_list_end(context):
     """See :meth:`command_list_begin()`."""
+    # TODO: batch consecutive add commands
     if not context.dispatcher.command_list_receiving:
-        raise MpdUnknownCommand(command='command_list_end')
+        raise exceptions.MpdUnknownCommand(command='command_list_end')
     context.dispatcher.command_list_receiving = False
     (command_list, context.dispatcher.command_list) = (
         context.dispatcher.command_list, [])
@@ -49,7 +49,7 @@ def command_list_end(context):
     return command_list_response
 
 
-@handle_request(r'command_list_ok_begin$')
+@protocol.commands.add('command_list_ok_begin')
 def command_list_ok_begin(context):
     """See :meth:`command_list_begin()`."""
     context.dispatcher.command_list_receiving = True
diff --git a/mopidy/mpd/protocol/stored_playlists.py b/mopidy/mpd/protocol/stored_playlists.py
index a852d795..706e8df2 100644
--- a/mopidy/mpd/protocol/stored_playlists.py
+++ b/mopidy/mpd/protocol/stored_playlists.py
@@ -1,13 +1,11 @@
 from __future__ import unicode_literals
 
-import datetime as dt
+import datetime
 
-from mopidy.mpd.exceptions import MpdNoExistError, MpdNotImplemented
-from mopidy.mpd.protocol import handle_request
-from mopidy.mpd.translator import playlist_to_mpd_format
+from mopidy.mpd import exceptions, protocol, translator
 
 
-@handle_request(r'listplaylist\ ("?)(?P<name>[^"]+)\1$')
+@protocol.commands.add('listplaylist')
 def listplaylist(context, name):
     """
     *musicpd.org, stored playlists section:*
@@ -24,11 +22,11 @@ def listplaylist(context, name):
     """
     playlist = context.lookup_playlist_from_name(name)
     if not playlist:
-        raise MpdNoExistError('No such playlist')
+        raise exceptions.MpdNoExistError('No such playlist')
     return ['file: %s' % t.uri for t in playlist.tracks]
 
 
-@handle_request(r'listplaylistinfo\ ("?)(?P<name>[^"]+)\1$')
+@protocol.commands.add('listplaylistinfo')
 def listplaylistinfo(context, name):
     """
     *musicpd.org, stored playlists section:*
@@ -44,11 +42,11 @@ def listplaylistinfo(context, name):
     """
     playlist = context.lookup_playlist_from_name(name)
     if not playlist:
-        raise MpdNoExistError('No such playlist')
-    return playlist_to_mpd_format(playlist)
+        raise exceptions.MpdNoExistError('No such playlist')
+    return translator.playlist_to_mpd_format(playlist)
 
 
-@handle_request(r'listplaylists$')
+@protocol.commands.add('listplaylists')
 def listplaylists(context):
     """
     *musicpd.org, stored playlists section:*
@@ -81,7 +79,7 @@ def listplaylists(context):
         name = context.lookup_playlist_name_from_uri(playlist.uri)
         result.append(('playlist', name))
         last_modified = (
-            playlist.last_modified or dt.datetime.utcnow()).isoformat()
+            playlist.last_modified or datetime.datetime.utcnow()).isoformat()
         # Remove microseconds
         last_modified = last_modified.split('.')[0]
         # Add time zone information
@@ -90,9 +88,8 @@ def listplaylists(context):
     return result
 
 
-@handle_request(
-    r'load\ "(?P<name>[^"]+)"(\ "(?P<start>\d+):(?P<end>\d+)*")*$')
-def load(context, name, start=None, end=None):
+@protocol.commands.add('load', playlist_slice=protocol.RANGE)
+def load(context, name, playlist_slice=slice(0, None)):
     """
     *musicpd.org, stored playlists section:*
 
@@ -115,15 +112,11 @@ def load(context, name, start=None, end=None):
     """
     playlist = context.lookup_playlist_from_name(name)
     if not playlist:
-        raise MpdNoExistError('No such playlist')
-    if start is not None:
-        start = int(start)
-    if end is not None:
-        end = int(end)
-    context.core.tracklist.add(playlist.tracks[start:end])
+        raise exceptions.MpdNoExistError('No such playlist')
+    context.core.tracklist.add(playlist.tracks[playlist_slice])
 
 
-@handle_request(r'playlistadd\ "(?P<name>[^"]+)"\ "(?P<uri>[^"]+)"$')
+@protocol.commands.add('playlistadd')
 def playlistadd(context, name, uri):
     """
     *musicpd.org, stored playlists section:*
@@ -134,10 +127,10 @@ def playlistadd(context, name, uri):
 
         ``NAME.m3u`` will be created if it does not exist.
     """
-    raise MpdNotImplemented  # TODO
+    raise exceptions.MpdNotImplemented  # TODO
 
 
-@handle_request(r'playlistclear\ "(?P<name>[^"]+)"$')
+@protocol.commands.add('playlistclear')
 def playlistclear(context, name):
     """
     *musicpd.org, stored playlists section:*
@@ -146,10 +139,10 @@ def playlistclear(context, name):
 
         Clears the playlist ``NAME.m3u``.
     """
-    raise MpdNotImplemented  # TODO
+    raise exceptions.MpdNotImplemented  # TODO
 
 
-@handle_request(r'playlistdelete\ "(?P<name>[^"]+)"\ "(?P<songpos>\d+)"$')
+@protocol.commands.add('playlistdelete', songpos=protocol.UINT)
 def playlistdelete(context, name, songpos):
     """
     *musicpd.org, stored playlists section:*
@@ -158,12 +151,11 @@ def playlistdelete(context, name, songpos):
 
         Deletes ``SONGPOS`` from the playlist ``NAME.m3u``.
     """
-    raise MpdNotImplemented  # TODO
+    raise exceptions.MpdNotImplemented  # TODO
 
 
-@handle_request(
-    r'playlistmove\ "(?P<name>[^"]+)"\ '
-    r'"(?P<from_pos>\d+)"\ "(?P<to_pos>\d+)"$')
+@protocol.commands.add(
+    'playlistmove', from_pos=protocol.UINT, to_pos=protocol.UINT)
 def playlistmove(context, name, from_pos, to_pos):
     """
     *musicpd.org, stored playlists section:*
@@ -179,10 +171,10 @@ def playlistmove(context, name, from_pos, to_pos):
       documentation, but just the ``SONGPOS`` to move *from*, i.e.
       ``playlistmove {NAME} {FROM_SONGPOS} {TO_SONGPOS}``.
     """
-    raise MpdNotImplemented  # TODO
+    raise exceptions.MpdNotImplemented  # TODO
 
 
-@handle_request(r'rename\ "(?P<old_name>[^"]+)"\ "(?P<new_name>[^"]+)"$')
+@protocol.commands.add('rename')
 def rename(context, old_name, new_name):
     """
     *musicpd.org, stored playlists section:*
@@ -191,10 +183,10 @@ def rename(context, old_name, new_name):
 
         Renames the playlist ``NAME.m3u`` to ``NEW_NAME.m3u``.
     """
-    raise MpdNotImplemented  # TODO
+    raise exceptions.MpdNotImplemented  # TODO
 
 
-@handle_request(r'rm\ "(?P<name>[^"]+)"$')
+@protocol.commands.add('rm')
 def rm(context, name):
     """
     *musicpd.org, stored playlists section:*
@@ -203,10 +195,10 @@ def rm(context, name):
 
         Removes the playlist ``NAME.m3u`` from the playlist directory.
     """
-    raise MpdNotImplemented  # TODO
+    raise exceptions.MpdNotImplemented  # TODO
 
 
-@handle_request(r'save\ "(?P<name>[^"]+)"$')
+@protocol.commands.add('save')
 def save(context, name):
     """
     *musicpd.org, stored playlists section:*
@@ -216,4 +208,4 @@ def save(context, name):
         Saves the current playlist to ``NAME.m3u`` in the playlist
         directory.
     """
-    raise MpdNotImplemented  # TODO
+    raise exceptions.MpdNotImplemented  # TODO

From 45a0a9233cfc5c2044b661ec04d92e3ddd5c9008 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Thu, 23 Jan 2014 22:38:47 +0100
Subject: [PATCH 22/35] mpd: Convert reflection and mark non listed commands

---
 mopidy/mpd/protocol/command_list.py |  6 +++---
 mopidy/mpd/protocol/connection.py   |  2 +-
 mopidy/mpd/protocol/reflection.py   | 20 ++++++--------------
 mopidy/mpd/protocol/status.py       |  4 ++--
 mopidy/mpd/protocol/stickers.py     |  2 +-
 5 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/mopidy/mpd/protocol/command_list.py b/mopidy/mpd/protocol/command_list.py
index d800ab72..d8551105 100644
--- a/mopidy/mpd/protocol/command_list.py
+++ b/mopidy/mpd/protocol/command_list.py
@@ -3,7 +3,7 @@ from __future__ import unicode_literals
 from mopidy.mpd import exceptions, protocol
 
 
-@protocol.commands.add('command_list_begin')
+@protocol.commands.add('command_list_begin', list_command=False)
 def command_list_begin(context):
     """
     *musicpd.org, command list section:*
@@ -25,7 +25,7 @@ def command_list_begin(context):
     context.dispatcher.command_list = []
 
 
-@protocol.commands.add('command_list_end')
+@protocol.commands.add('command_list_end', list_command=False)
 def command_list_end(context):
     """See :meth:`command_list_begin()`."""
     # TODO: batch consecutive add commands
@@ -49,7 +49,7 @@ def command_list_end(context):
     return command_list_response
 
 
-@protocol.commands.add('command_list_ok_begin')
+@protocol.commands.add('command_list_ok_begin', list_command=False)
 def command_list_ok_begin(context):
     """See :meth:`command_list_begin()`."""
     context.dispatcher.command_list_receiving = True
diff --git a/mopidy/mpd/protocol/connection.py b/mopidy/mpd/protocol/connection.py
index a1a643c3..b324add8 100644
--- a/mopidy/mpd/protocol/connection.py
+++ b/mopidy/mpd/protocol/connection.py
@@ -15,7 +15,7 @@ def close(context):
     context.session.close()
 
 
-@protocol.commands.add('kill')
+@protocol.commands.add('kill', list_command=False)
 def kill(context):
     """
     *musicpd.org, connection section:*
diff --git a/mopidy/mpd/protocol/reflection.py b/mopidy/mpd/protocol/reflection.py
index 2f7d606e..227a17a3 100644
--- a/mopidy/mpd/protocol/reflection.py
+++ b/mopidy/mpd/protocol/reflection.py
@@ -3,7 +3,7 @@ from __future__ import unicode_literals
 from mopidy.mpd import exceptions, protocol
 
 
-@protocol.handle_request(r'config$', auth_required=False)
+@protocol.commands.add('config', list_command=False)
 def config(context):
     """
     *musicpd.org, reflection section:*
@@ -17,7 +17,7 @@ def config(context):
     raise exceptions.MpdPermissionError(command='config')
 
 
-@protocol.handle_request(r'commands$', auth_required=False)
+@protocol.commands.add('commands', auth_required=False)
 def commands(context):
     """
     *musicpd.org, reflection section:*
@@ -40,19 +40,11 @@ def commands(context):
         command_names.update(c.name for c in protocol.mpd_commands
                              if not c.auth_required)
 
-    # TODO: remove once we've marked all of these as list_command=False
-    # No one is permited to use 'config' or 'kill', rest of commands are not
-    # listed by MPD, so we shouldn't either.
-    command_names = command_names - set([
-        'config', 'kill', 'command_list_begin', 'command_list_ok_begin',
-        'command_list_ok_begin', 'command_list_end', 'idle', 'noidle',
-        'sticker'])
-
     return [
         ('command', command_name) for command_name in sorted(command_names)]
 
 
-@protocol.handle_request(r'decoders$')
+@protocol.commands.add('decoders')
 def decoders(context):
     """
     *musicpd.org, reflection section:*
@@ -79,7 +71,7 @@ def decoders(context):
     return  # TODO
 
 
-@protocol.handle_request(r'notcommands$', auth_required=False)
+@protocol.commands.add('notcommands', auth_required=False)
 def notcommands(context):
     """
     *musicpd.org, reflection section:*
@@ -104,7 +96,7 @@ def notcommands(context):
         ('command', command_name) for command_name in sorted(command_names)]
 
 
-@protocol.handle_request(r'tagtypes$')
+@protocol.commands.add('tagtypes')
 def tagtypes(context):
     """
     *musicpd.org, reflection section:*
@@ -116,7 +108,7 @@ def tagtypes(context):
     pass  # TODO
 
 
-@protocol.handle_request(r'urlhandlers$')
+@protocol.commands.add('urlhandlers')
 def urlhandlers(context):
     """
     *musicpd.org, reflection section:*
diff --git a/mopidy/mpd/protocol/status.py b/mopidy/mpd/protocol/status.py
index 15e46002..8f97c2e4 100644
--- a/mopidy/mpd/protocol/status.py
+++ b/mopidy/mpd/protocol/status.py
@@ -40,7 +40,7 @@ def currentsong(context):
         return translator.track_to_mpd_format(tl_track, position=position)
 
 
-@protocol.commands.add('idle')
+@protocol.commands.add('idle', list_command=False)
 def idle(context, *subsystems):
     """
     *musicpd.org, status section:*
@@ -96,7 +96,7 @@ def idle(context, *subsystems):
     return response
 
 
-@protocol.commands.add('noidle')
+@protocol.commands.add('noidle', list_command=False)
 def noidle(context):
     """See :meth:`_status_idle`."""
     if not context.subscriptions:
diff --git a/mopidy/mpd/protocol/stickers.py b/mopidy/mpd/protocol/stickers.py
index 53ce0caa..e8718f1d 100644
--- a/mopidy/mpd/protocol/stickers.py
+++ b/mopidy/mpd/protocol/stickers.py
@@ -3,7 +3,7 @@ from __future__ import unicode_literals
 from mopidy.mpd import exceptions, protocol
 
 
-@protocol.commands.add('sticker')
+@protocol.commands.add('sticker', list_command=False)
 def sticker(context, action, field, uri, name=None, value=None):
     """
     *musicpd.org, sticker section:*

From 8f8bed5b878b0e3c6186567df4a28e3b3211158e Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Thu, 23 Jan 2014 23:12:25 +0100
Subject: [PATCH 23/35] mpd: Implement playlistinfo from current playlist

---
 mopidy/mpd/protocol/current_playlist.py     | 35 ++++++++-------------
 tests/mpd/protocol/test_current_playlist.py |  4 +++
 2 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/mopidy/mpd/protocol/current_playlist.py b/mopidy/mpd/protocol/current_playlist.py
index 662cac8b..39ba9486 100644
--- a/mopidy/mpd/protocol/current_playlist.py
+++ b/mopidy/mpd/protocol/current_playlist.py
@@ -219,11 +219,8 @@ def playlistid(context, tlid=None):
             context.core.tracklist.tl_tracks.get())
 
 
-# TODO: convert
-@protocol.handle_request(r'playlistinfo$')
-@protocol.handle_request(r'playlistinfo\ "(?P<songpos>-?\d+)"$')
-@protocol.handle_request(r'playlistinfo\ "(?P<start>\d+):(?P<end>\d+)*"$')
-def playlistinfo(context, songpos=None, start=None, end=None):
+@protocol.commands.add('playlistinfo')
+def playlistinfo(context, parameter=None):
     """
     *musicpd.org, current playlist section:*
 
@@ -238,24 +235,18 @@ def playlistinfo(context, songpos=None, start=None, end=None):
     - uses negative indexes, like ``playlistinfo "-1"``, to request
       the entire playlist
     """
-    if songpos == '-1':
-        songpos = None
-    if songpos is not None:
-        songpos = int(songpos)
-        tl_track = context.core.tracklist.tl_tracks.get()[songpos]
-        return translator.track_to_mpd_format(tl_track, position=songpos)
+    if parameter is None or parameter == '-1':
+        start, end = 0, None
     else:
-        if start is None:
-            start = 0
-        start = int(start)
-        if not (0 <= start <= context.core.tracklist.length.get()):
-            raise exceptions.MpdArgError('Bad song index')
-        if end is not None:
-            end = int(end)
-            if end > context.core.tracklist.length.get():
-                end = None
-        tl_tracks = context.core.tracklist.tl_tracks.get()
-        return translator.tracks_to_mpd_format(tl_tracks, start, end)
+        tracklist_slice = protocol.RANGE(parameter)
+        start, end = tracklist_slice.start, tracklist_slice.stop
+
+    tl_tracks = context.core.tracklist.tl_tracks.get()
+    if start and start > len(tl_tracks):
+        raise exceptions.MpdArgError('Bad song index')
+    if end and end > len(tl_tracks):
+        end = None
+    return translator.tracks_to_mpd_format(tl_tracks, start, end)
 
 
 @protocol.commands.add('playlistsearch')
diff --git a/tests/mpd/protocol/test_current_playlist.py b/tests/mpd/protocol/test_current_playlist.py
index e2db8b05..e9898dd9 100644
--- a/tests/mpd/protocol/test_current_playlist.py
+++ b/tests/mpd/protocol/test_current_playlist.py
@@ -389,6 +389,10 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase):
         self.sendRequest('playlistinfo "0:20"')
         self.assertInResponse('OK')
 
+    def test_playlistinfo_with_zero_returns_ok(self):
+        self.sendRequest('playlistinfo "0"')
+        self.assertInResponse('OK')
+
     def test_playlistsearch(self):
         self.sendRequest('playlistsearch "any" "needle"')
         self.assertEqualResponse('ACK [0@0] {playlistsearch} Not implemented')

From 4c57184f438533d10b1613dd2ce50aa53bf0d792 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Thu, 23 Jan 2014 23:22:38 +0100
Subject: [PATCH 24/35] mpd: Convert non-search bits of music db

---
 mopidy/mpd/protocol/music_db.py | 56 ++++++++++++++-------------------
 1 file changed, 24 insertions(+), 32 deletions(-)

diff --git a/mopidy/mpd/protocol/music_db.py b/mopidy/mpd/protocol/music_db.py
index 58681557..e9b16861 100644
--- a/mopidy/mpd/protocol/music_db.py
+++ b/mopidy/mpd/protocol/music_db.py
@@ -5,9 +5,7 @@ import itertools
 import re
 
 from mopidy.models import Ref, Track
-from mopidy.mpd import translator
-from mopidy.mpd.exceptions import MpdArgError, MpdNoExistError
-from mopidy.mpd.protocol import handle_request, stored_playlists
+from mopidy.mpd import exceptions, protocol, translator
 
 
 LIST_QUERY = r"""
@@ -145,7 +143,7 @@ def _artist_as_track(artist):
         artists=[artist])
 
 
-@handle_request(r'count\ ' + SEARCH_QUERY)
+@protocol.handle_request(r'count\ ' + SEARCH_QUERY)
 def count(context, mpd_query):
     """
     *musicpd.org, music database section:*
@@ -163,7 +161,7 @@ def count(context, mpd_query):
     try:
         query = _query_from_mpd_search_format(mpd_query)
     except ValueError:
-        raise MpdArgError('incorrect arguments')
+        raise exceptions.MpdArgError('incorrect arguments')
     results = context.core.library.find_exact(**query).get()
     result_tracks = _get_tracks(results)
     return [
@@ -172,7 +170,7 @@ def count(context, mpd_query):
     ]
 
 
-@handle_request(r'find\ ' + SEARCH_QUERY)
+@protocol.handle_request(r'find\ ' + SEARCH_QUERY)
 def find(context, mpd_query):
     """
     *musicpd.org, music database section:*
@@ -217,7 +215,7 @@ def find(context, mpd_query):
     return translator.tracks_to_mpd_format(result_tracks)
 
 
-@handle_request(r'findadd\ ' + SEARCH_QUERY)
+@protocol.handle_request(r'findadd\ ' + SEARCH_QUERY)
 def findadd(context, mpd_query):
     """
     *musicpd.org, music database section:*
@@ -235,7 +233,7 @@ def findadd(context, mpd_query):
     context.core.tracklist.add(_get_tracks(results))
 
 
-@handle_request(r'list\ ' + LIST_QUERY)
+@protocol.handle_request(r'list\ ' + LIST_QUERY)
 def list_(context, field, mpd_query=None):
     """
     *musicpd.org, music database section:*
@@ -407,8 +405,8 @@ def _list_genre(context, query):
     return genres
 
 
-@handle_request(r'listall$')
-@handle_request(r'listall\ "(?P<uri>[^"]+)"$')
+# TODO: see if we can combine listall, listallinfo and lsinfo to one helper
+@protocol.commands.add('listall')
 def listall(context, uri=None):
     """
     *musicpd.org, music database section:*
@@ -419,12 +417,9 @@ def listall(context, uri=None):
     """
     result = []
     root_path = translator.normalize_path(uri)
-    # TODO: doesn't the dispatcher._call_handler have enough info to catch
-    # the error this can produce, set the command and then 'raise'?
     try:
         uri = context.directory_path_to_uri(root_path)
-    except MpdNoExistError as e:
-        e.command = 'listall'
+    except exceptions.MpdNoExistError as e:
         e.message = 'Not found'
         raise
     browse_futures = [(root_path, context.core.library.browse(uri))]
@@ -441,13 +436,12 @@ def listall(context, uri=None):
                 result.append(('file', ref.uri))
 
     if not result:
-        raise MpdNoExistError('Not found')
+        raise exceptions.MpdNoExistError('Not found')
 
     return [('directory', root_path)] + result
 
 
-@handle_request(r'listallinfo$')
-@handle_request(r'listallinfo\ "(?P<uri>[^"]+)"$')
+@protocol.commands.add('listallinfo')
 def listallinfo(context, uri=None):
     """
     *musicpd.org, music database section:*
@@ -462,7 +456,7 @@ def listallinfo(context, uri=None):
     root_path = translator.normalize_path(uri)
     try:
         uri = context.directory_path_to_uri(root_path)
-    except MpdNoExistError as e:
+    except exceptions.MpdNoExistError as e:
         e.command = 'listallinfo'
         e.message = 'Not found'
         raise
@@ -489,13 +483,12 @@ def listallinfo(context, uri=None):
             result.append(obj)
 
     if not result:
-        raise MpdNoExistError('Not found')
+        raise exceptions.MpdNoExistError('Not found')
 
     return [('directory', root_path)] + result
 
 
-@handle_request(r'lsinfo$')
-@handle_request(r'lsinfo\ "(?P<uri>[^"]*)"$')
+@protocol.commands.add('lsinfo')
 def lsinfo(context, uri=None):
     """
     *musicpd.org, music database section:*
@@ -516,13 +509,13 @@ def lsinfo(context, uri=None):
     root_path = translator.normalize_path(uri, relative=True)
     try:
         uri = context.directory_path_to_uri(root_path)
-    except MpdNoExistError as e:
+    except exceptions.MpdNoExistError as e:
         e.command = 'lsinfo'
         e.message = 'Not found'
         raise
 
     if uri is None:
-        result.extend(stored_playlists.listplaylists(context))
+        result.extend(protocol.stored_playlists.listplaylists(context))
 
     for ref in context.core.library.browse(uri).get():
         if ref.type == Ref.DIRECTORY:
@@ -536,8 +529,7 @@ def lsinfo(context, uri=None):
     return result
 
 
-@handle_request(r'rescan$')
-@handle_request(r'rescan\ "(?P<uri>[^"]+)"$')
+@protocol.commands.add('rescan')
 def rescan(context, uri=None):
     """
     *musicpd.org, music database section:*
@@ -546,10 +538,10 @@ def rescan(context, uri=None):
 
         Same as ``update``, but also rescans unmodified files.
     """
-    return update(context, uri, rescan_unmodified_files=True)
+    return {'updating_db': 0}  # TODO
 
 
-@handle_request(r'search\ ' + SEARCH_QUERY)
+@protocol.handle_request(r'search\ ' + SEARCH_QUERY)
 def search(context, mpd_query):
     """
     *musicpd.org, music database section:*
@@ -588,7 +580,7 @@ def search(context, mpd_query):
     return translator.tracks_to_mpd_format(artists + albums + tracks)
 
 
-@handle_request(r'searchadd\ ' + SEARCH_QUERY)
+@protocol.handle_request(r'searchadd\ ' + SEARCH_QUERY)
 def searchadd(context, mpd_query):
     """
     *musicpd.org, music database section:*
@@ -609,7 +601,8 @@ def searchadd(context, mpd_query):
     context.core.tracklist.add(_get_tracks(results))
 
 
-@handle_request(r'searchaddpl\ "(?P<playlist_name>[^"]+)"\ ' + SEARCH_QUERY)
+@protocol.handle_request(
+    r'searchaddpl\ "(?P<playlist_name>[^"]+)"\ ' + SEARCH_QUERY)
 def searchaddpl(context, playlist_name, mpd_query):
     """
     *musicpd.org, music database section:*
@@ -638,9 +631,8 @@ def searchaddpl(context, playlist_name, mpd_query):
     context.core.playlists.save(playlist)
 
 
-@handle_request(r'update$')
-@handle_request(r'update\ "(?P<uri>[^"]+)"$')
-def update(context, uri=None, rescan_unmodified_files=False):
+@protocol.commands.add('update')
+def update(context, uri=None):
     """
     *musicpd.org, music database section:*
 

From 68aa0b556c49babfa0dc22eb50a43a36904222bd Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Thu, 23 Jan 2014 23:58:42 +0100
Subject: [PATCH 25/35] mpd: Convert search functions in music db

---
 mopidy/mpd/protocol/__init__.py     |   1 +
 mopidy/mpd/protocol/music_db.py     | 150 +++++++++-------------------
 tests/mpd/protocol/test_music_db.py |   4 +-
 3 files changed, 49 insertions(+), 106 deletions(-)

diff --git a/mopidy/mpd/protocol/__init__.py b/mopidy/mpd/protocol/__init__.py
index 2b0d2cbe..04193c58 100644
--- a/mopidy/mpd/protocol/__init__.py
+++ b/mopidy/mpd/protocol/__init__.py
@@ -176,6 +176,7 @@ class Commands(object):
         return wrapper
 
     def call(self, args, context=None):
+        # TODO: raise mopidy.mpd.exceptions
         if not args:
             raise TypeError('No args provided')
         if args[0] not in self.handlers:
diff --git a/mopidy/mpd/protocol/music_db.py b/mopidy/mpd/protocol/music_db.py
index e9b16861..6f4c0fc6 100644
--- a/mopidy/mpd/protocol/music_db.py
+++ b/mopidy/mpd/protocol/music_db.py
@@ -2,7 +2,6 @@ from __future__ import unicode_literals
 
 import functools
 import itertools
-import re
 
 from mopidy.models import Ref, Track
 from mopidy.mpd import exceptions, protocol, translator
@@ -27,94 +26,33 @@ LIST_QUERY = r"""
   $
 """
 
-SEARCH_FIELDS = r"""
-    [Aa]lbum
-  | [Aa]lbumartist
-  | [Aa]ny
-  | [Aa]rtist
-  | [Cc]omment
-  | [Cc]omposer
-  | [Dd]ate
-  | [Ff]ile
-  | [Ff]ilename
-  | [Gg]enre
-  | [Pp]erformer
-  | [Tt]itle
-  | [Tt]rack
-"""
-
-# TODO Would be nice to get ("?)...\1 working for the quotes here
-SEARCH_QUERY = r"""
-  (?P<mpd_query>
-    (?:                 # Non-capturing group for repeating query pairs
-      "?                # Optional quote around the field type
-      (?:
-""" + SEARCH_FIELDS + r"""
-      )
-      "?                # End of optional quote around the field type
-      \                 # A single space
-      "[^"]*"           # Matching a quoted search string
-      \s?
-    )+
-  )
-  $
-"""
-
-# TODO Would be nice to get ("?)...\1 working for the quotes here
-SEARCH_PAIR_WITHOUT_GROUPS = r"""
-  \b                  # Only begin matching at word bundaries
-  "?                  # Optional quote around the field type
-  (?:                 # A non-capturing group for the field type
-""" + SEARCH_FIELDS + """
-  )
-  "?                  # End of optional quote around the field type
-  \                   # A single space
-  "[^"]+"             # Matching a quoted search string
-"""
-SEARCH_PAIR_WITHOUT_GROUPS_RE = re.compile(
-    SEARCH_PAIR_WITHOUT_GROUPS, flags=(re.UNICODE | re.VERBOSE))
-
-# TODO Would be nice to get ("?)...\1 working for the quotes here
-SEARCH_PAIR_WITH_GROUPS = r"""
-  \b                  # Only begin matching at word bundaries
-  "?                  # Optional quote around the field type
-  (?P<field>(         # A capturing group for the field type
-""" + SEARCH_FIELDS + """
-  ))
-  "?                  # End of optional quote around the field type
-  \                   # A single space
-  "(?P<what>[^"]+)"   # Capturing a quoted search string
-"""
-SEARCH_PAIR_WITH_GROUPS_RE = re.compile(
-    SEARCH_PAIR_WITH_GROUPS, flags=(re.UNICODE | re.VERBOSE))
+_SEARCH_FIELD_MAPPING = {
+    'album': 'album',
+    'albumartist': 'albumartist',
+    'any': 'any',
+    'artist': 'artist',
+    'comment': 'comment',
+    'composer': 'composer',
+    'date': 'date',
+    'file': 'uri',
+    'filename': 'uri',
+    'genre': 'genre',
+    'performer': 'performer',
+    'title': 'track_name',
+    'track': 'track_no'}
 
 
-def _query_from_mpd_search_format(mpd_query):
-    """
-    Parses an MPD ``search`` or ``find`` query and converts it to the Mopidy
-    query format.
-
-    :param mpd_query: the MPD search query
-    :type mpd_query: string
-    """
-    pairs = SEARCH_PAIR_WITHOUT_GROUPS_RE.findall(mpd_query)
+def _query_from_mpd_search_parameters(parameters):
     query = {}
-    for pair in pairs:
-        m = SEARCH_PAIR_WITH_GROUPS_RE.match(pair)
-        field = m.groupdict()['field'].lower()
-        if field == 'title':
-            field = 'track_name'
-        elif field == 'track':
-            field = 'track_no'
-        elif field in ('file', 'filename'):
-            field = 'uri'
-        what = m.groupdict()['what']
-        if not what:
+    parameters = list(parameters)
+    while parameters:
+        # TODO: does it matter that this is now case insensitive
+        field = _SEARCH_FIELD_MAPPING.get(parameters.pop(0).lower())
+        if not field:
+            raise exceptions.MpdArgError('incorrect arguments')
+        if not parameters:
             raise ValueError
-        if field in query:
-            query[field].append(what)
-        else:
-            query[field] = [what]
+        query.setdefault(field, []).append(parameters.pop(0))
     return query
 
 
@@ -143,8 +81,8 @@ def _artist_as_track(artist):
         artists=[artist])
 
 
-@protocol.handle_request(r'count\ ' + SEARCH_QUERY)
-def count(context, mpd_query):
+@protocol.commands.add('count')
+def count(context, *args):
     """
     *musicpd.org, music database section:*
 
@@ -159,7 +97,7 @@ def count(context, mpd_query):
     - use multiple tag-needle pairs to make more specific searches.
     """
     try:
-        query = _query_from_mpd_search_format(mpd_query)
+        query = _query_from_mpd_search_parameters(args)
     except ValueError:
         raise exceptions.MpdArgError('incorrect arguments')
     results = context.core.library.find_exact(**query).get()
@@ -170,8 +108,8 @@ def count(context, mpd_query):
     ]
 
 
-@protocol.handle_request(r'find\ ' + SEARCH_QUERY)
-def find(context, mpd_query):
+@protocol.commands.add('find')
+def find(context, *args):
     """
     *musicpd.org, music database section:*
 
@@ -199,9 +137,10 @@ def find(context, mpd_query):
     - uses "file" instead of "filename".
     """
     try:
-        query = _query_from_mpd_search_format(mpd_query)
+        query = _query_from_mpd_search_parameters(args)
     except ValueError:
         return
+
     results = context.core.library.find_exact(**query).get()
     result_tracks = []
     if ('artist' not in query and
@@ -215,8 +154,8 @@ def find(context, mpd_query):
     return translator.tracks_to_mpd_format(result_tracks)
 
 
-@protocol.handle_request(r'findadd\ ' + SEARCH_QUERY)
-def findadd(context, mpd_query):
+@protocol.commands.add('findadd')
+def findadd(context, *args):
     """
     *musicpd.org, music database section:*
 
@@ -226,7 +165,7 @@ def findadd(context, mpd_query):
         current playlist. Parameters have the same meaning as for ``find``.
     """
     try:
-        query = _query_from_mpd_search_format(mpd_query)
+        query = _query_from_mpd_search_parameters(args)
     except ValueError:
         return
     results = context.core.library.find_exact(**query).get()
@@ -541,8 +480,8 @@ def rescan(context, uri=None):
     return {'updating_db': 0}  # TODO
 
 
-@protocol.handle_request(r'search\ ' + SEARCH_QUERY)
-def search(context, mpd_query):
+@protocol.commands.add('search')
+def search(context, *args):
     """
     *musicpd.org, music database section:*
 
@@ -570,7 +509,7 @@ def search(context, mpd_query):
     - uses "file" instead of "filename".
     """
     try:
-        query = _query_from_mpd_search_format(mpd_query)
+        query = _query_from_mpd_search_parameters(args)
     except ValueError:
         return
     results = context.core.library.search(**query).get()
@@ -580,8 +519,8 @@ def search(context, mpd_query):
     return translator.tracks_to_mpd_format(artists + albums + tracks)
 
 
-@protocol.handle_request(r'searchadd\ ' + SEARCH_QUERY)
-def searchadd(context, mpd_query):
+@protocol.commands.add('searchadd')
+def searchadd(context, *args):
     """
     *musicpd.org, music database section:*
 
@@ -594,16 +533,15 @@ def searchadd(context, mpd_query):
         not case sensitive.
     """
     try:
-        query = _query_from_mpd_search_format(mpd_query)
+        query = _query_from_mpd_search_parameters(args)
     except ValueError:
         return
     results = context.core.library.search(**query).get()
     context.core.tracklist.add(_get_tracks(results))
 
 
-@protocol.handle_request(
-    r'searchaddpl\ "(?P<playlist_name>[^"]+)"\ ' + SEARCH_QUERY)
-def searchaddpl(context, playlist_name, mpd_query):
+@protocol.commands.add('searchaddpl')
+def searchaddpl(context, *args):
     """
     *musicpd.org, music database section:*
 
@@ -617,8 +555,12 @@ def searchaddpl(context, playlist_name, mpd_query):
         Parameters have the same meaning as for ``find``, except that search is
         not case sensitive.
     """
+    parameters = list(args)
+    if not parameters:
+        raise exceptions.MpdArgError('incorrect arguments')
+    playlist_name = parameters.pop(0)
     try:
-        query = _query_from_mpd_search_format(mpd_query)
+        query = _query_from_mpd_search_parameters(parameters)
     except ValueError:
         return
     results = context.core.library.search(**query).get()
diff --git a/tests/mpd/protocol/test_music_db.py b/tests/mpd/protocol/test_music_db.py
index 8d74fb95..6e00b384 100644
--- a/tests/mpd/protocol/test_music_db.py
+++ b/tests/mpd/protocol/test_music_db.py
@@ -11,8 +11,8 @@ from tests.mpd import protocol
 
 class QueryFromMpdSearchFormatTest(unittest.TestCase):
     def test_dates_are_extracted(self):
-        result = music_db._query_from_mpd_search_format(
-            'Date "1974-01-02" Date "1975"')
+        result = music_db._query_from_mpd_search_parameters(
+            ['Date', '1974-01-02', 'Date', '1975'])
         self.assertEqual(result['date'][0], '1974-01-02')
         self.assertEqual(result['date'][1], '1975')
 

From dc8d311bc31509f0f877a49a4a7e105095ad5a5a Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Fri, 24 Jan 2014 00:22:51 +0100
Subject: [PATCH 26/35] mpd: Complete music db conversion with list

---
 mopidy/mpd/protocol/music_db.py     | 71 +++++++++++++++--------------
 mopidy/mpd/translator.py            | 45 ------------------
 tests/mpd/protocol/test_music_db.py |  2 +-
 3 files changed, 39 insertions(+), 79 deletions(-)

diff --git a/mopidy/mpd/protocol/music_db.py b/mopidy/mpd/protocol/music_db.py
index 6f4c0fc6..2472e1d9 100644
--- a/mopidy/mpd/protocol/music_db.py
+++ b/mopidy/mpd/protocol/music_db.py
@@ -6,27 +6,7 @@ import itertools
 from mopidy.models import Ref, Track
 from mopidy.mpd import exceptions, protocol, translator
 
-
-LIST_QUERY = r"""
-  ("?)                  # Optional quote around the field type
-  (?P<field>(           # Field to list in the response
-      [Aa]lbum
-    | [Aa]lbumartist
-    | [Aa]rtist
-    | [Cc]omposer
-    | [Dd]ate
-    | [Gg]enre
-    | [Pp]erformer
-  ))
-  \1                    # End of optional quote around the field type
-  (?:                   # Non-capturing group for optional search query
-    \                   # A single space
-    (?P<mpd_query>.*)
-  )?
-  $
-"""
-
-_SEARCH_FIELD_MAPPING = {
+_SEARCH_MAPPING = {
     'album': 'album',
     'albumartist': 'albumartist',
     'any': 'any',
@@ -41,13 +21,22 @@ _SEARCH_FIELD_MAPPING = {
     'title': 'track_name',
     'track': 'track_no'}
 
+_LIST_MAPPING = {
+    'album': 'album',
+    'albumartist': 'albumartist',
+    'artist': 'artist',
+    'composer': 'composer',
+    'date': 'date',
+    'genre': 'genre',
+    'performer': 'performer'}
 
-def _query_from_mpd_search_parameters(parameters):
+
+def _query_from_mpd_search_parameters(parameters, mapping):
     query = {}
     parameters = list(parameters)
     while parameters:
         # TODO: does it matter that this is now case insensitive
-        field = _SEARCH_FIELD_MAPPING.get(parameters.pop(0).lower())
+        field = mapping.get(parameters.pop(0).lower())
         if not field:
             raise exceptions.MpdArgError('incorrect arguments')
         if not parameters:
@@ -97,7 +86,7 @@ def count(context, *args):
     - use multiple tag-needle pairs to make more specific searches.
     """
     try:
-        query = _query_from_mpd_search_parameters(args)
+        query = _query_from_mpd_search_parameters(args, _SEARCH_MAPPING)
     except ValueError:
         raise exceptions.MpdArgError('incorrect arguments')
     results = context.core.library.find_exact(**query).get()
@@ -137,7 +126,7 @@ def find(context, *args):
     - uses "file" instead of "filename".
     """
     try:
-        query = _query_from_mpd_search_parameters(args)
+        query = _query_from_mpd_search_parameters(args, _SEARCH_MAPPING)
     except ValueError:
         return
 
@@ -165,15 +154,15 @@ def findadd(context, *args):
         current playlist. Parameters have the same meaning as for ``find``.
     """
     try:
-        query = _query_from_mpd_search_parameters(args)
+        query = _query_from_mpd_search_parameters(args, _SEARCH_MAPPING)
     except ValueError:
         return
     results = context.core.library.find_exact(**query).get()
     context.core.tracklist.add(_get_tracks(results))
 
 
-@protocol.handle_request(r'list\ ' + LIST_QUERY)
-def list_(context, field, mpd_query=None):
+@protocol.commands.add('list')
+def list_(context, *args):
     """
     *musicpd.org, music database section:*
 
@@ -255,11 +244,27 @@ def list_(context, field, mpd_query=None):
     - does not add quotes around the field argument.
     - capitalizes the field argument.
     """
-    field = field.lower()
+    parameters = list(args)
+    if not parameters:
+        raise exceptions.MpdArgError('incorrect arguments')
+    field = parameters.pop(0).lower()
+
+    if field not in _LIST_MAPPING:
+        raise exceptions.MpdArgError('incorrect arguments')
+
+    if len(parameters) == 1:
+        if field != 'album':
+            raise exceptions.MpdArgError('should be "Album" for 3 arguments')
+        return _list_artist(context, {'artist': parameters})
+
     try:
-        query = translator.query_from_mpd_list_format(field, mpd_query)
+        query = _query_from_mpd_search_parameters(parameters, _LIST_MAPPING)
+    except exceptions.MpdArgError as e:
+        e.message = 'not able to parse args'
+        raise
     except ValueError:
         return
+
     if field == 'artist':
         return _list_artist(context, query)
     if field == 'albumartist':
@@ -509,7 +514,7 @@ def search(context, *args):
     - uses "file" instead of "filename".
     """
     try:
-        query = _query_from_mpd_search_parameters(args)
+        query = _query_from_mpd_search_parameters(args, _SEARCH_MAPPING)
     except ValueError:
         return
     results = context.core.library.search(**query).get()
@@ -533,7 +538,7 @@ def searchadd(context, *args):
         not case sensitive.
     """
     try:
-        query = _query_from_mpd_search_parameters(args)
+        query = _query_from_mpd_search_parameters(args, _SEARCH_MAPPING)
     except ValueError:
         return
     results = context.core.library.search(**query).get()
@@ -560,7 +565,7 @@ def searchaddpl(context, *args):
         raise exceptions.MpdArgError('incorrect arguments')
     playlist_name = parameters.pop(0)
     try:
-        query = _query_from_mpd_search_parameters(parameters)
+        query = _query_from_mpd_search_parameters(parameters, _SEARCH_MAPPING)
     except ValueError:
         return
     results = context.core.library.search(**query).get()
diff --git a/mopidy/mpd/translator.py b/mopidy/mpd/translator.py
index 520e9ac8..252725ee 100644
--- a/mopidy/mpd/translator.py
+++ b/mopidy/mpd/translator.py
@@ -1,9 +1,7 @@
 from __future__ import unicode_literals
 
 import re
-import shlex
 
-from mopidy.mpd.exceptions import MpdArgError
 from mopidy.models import TlTrack
 
 # TODO: special handling of local:// uri scheme
@@ -137,46 +135,3 @@ def playlist_to_mpd_format(playlist, *args, **kwargs):
     Arguments as for :func:`tracks_to_mpd_format`, except the first one.
     """
     return tracks_to_mpd_format(playlist.tracks, *args, **kwargs)
-
-
-def query_from_mpd_list_format(field, mpd_query):
-    """
-    Converts an MPD ``list`` query to a Mopidy query.
-    """
-    if mpd_query is None:
-        return {}
-    try:
-        # shlex does not seem to be friends with unicode objects
-        tokens = shlex.split(mpd_query.encode('utf-8'))
-    except ValueError as error:
-        if str(error) == 'No closing quotation':
-            raise MpdArgError('Invalid unquoted character', command='list')
-        else:
-            raise
-    tokens = [t.decode('utf-8') for t in tokens]
-    if len(tokens) == 1:
-        if field == 'album':
-            if not tokens[0]:
-                raise ValueError
-            return {'artist': [tokens[0]]}
-        else:
-            raise MpdArgError(
-                'should be "Album" for 3 arguments', command='list')
-    elif len(tokens) % 2 == 0:
-        query = {}
-        while tokens:
-            key = tokens[0].lower()
-            value = tokens[1]
-            tokens = tokens[2:]
-            if key not in ('artist', 'album', 'albumartist', 'composer',
-                           'date', 'genre', 'performer'):
-                raise MpdArgError('not able to parse args', command='list')
-            if not value:
-                raise ValueError
-            if key in query:
-                query[key].append(value)
-            else:
-                query[key] = [value]
-        return query
-    else:
-        raise MpdArgError('not able to parse args', command='list')
diff --git a/tests/mpd/protocol/test_music_db.py b/tests/mpd/protocol/test_music_db.py
index 6e00b384..d41c05b2 100644
--- a/tests/mpd/protocol/test_music_db.py
+++ b/tests/mpd/protocol/test_music_db.py
@@ -12,7 +12,7 @@ from tests.mpd import protocol
 class QueryFromMpdSearchFormatTest(unittest.TestCase):
     def test_dates_are_extracted(self):
         result = music_db._query_from_mpd_search_parameters(
-            ['Date', '1974-01-02', 'Date', '1975'])
+            ['Date', '1974-01-02', 'Date', '1975'], music_db._SEARCH_MAPPING)
         self.assertEqual(result['date'][0], '1974-01-02')
         self.assertEqual(result['date'][1], '1975')
 

From 86f5602023171940fe4f9d0264fc936d76fcf975 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Fri, 24 Jan 2014 00:35:10 +0100
Subject: [PATCH 27/35] mpd: Remove old command handlers

---
 mopidy/mpd/dispatcher.py          | 35 +----------------
 mopidy/mpd/protocol/__init__.py   | 64 +------------------------------
 mopidy/mpd/protocol/reflection.py | 12 ------
 tests/mpd/test_dispatcher.py      | 32 +---------------
 4 files changed, 5 insertions(+), 138 deletions(-)

diff --git a/mopidy/mpd/dispatcher.py b/mopidy/mpd/dispatcher.py
index 849d3821..d91c8a66 100644
--- a/mopidy/mpd/dispatcher.py
+++ b/mopidy/mpd/dispatcher.py
@@ -90,17 +90,7 @@ class MpdDispatcher(object):
         else:
             command_name = request.split(' ')[0]
             command = protocol.commands.handlers.get(command_name)
-            if command:
-                if command.auth_required:
-                    raise exceptions.MpdPermissionError(command=command_name)
-                else:
-                    return self._call_next_filter(
-                        request, response, filter_chain)
-
-            # TODO: remove
-            command_names_not_requiring_auth = [
-                c.name for c in protocol.mpd_commands if not c.auth_required]
-            if command_name in command_names_not_requiring_auth:
+            if command and not command.auth_required:
                 return self._call_next_filter(request, response, filter_chain)
             else:
                 raise exceptions.MpdPermissionError(command=command_name)
@@ -181,28 +171,7 @@ class MpdDispatcher(object):
                 exc.command = tokens[0]
             raise
         except LookupError:
-            pass  # Command has not been converted, i.e. fallback...
-
-        request = request.decode('string_escape')
-        (command_name, handler, kwargs) = self._find_handler(request)
-        try:
-            return handler(self.context, **kwargs)
-        except exceptions.MpdAckError as exc:
-            if exc.command is None:
-                exc.command = command_name
-            raise
-
-    def _find_handler(self, request):
-        command_name = request.split(' ')[0]
-        for pattern in protocol.request_handlers:
-            matches = re.match(pattern, request)
-            if matches is not None:
-                handler = protocol.request_handlers[pattern]
-                return (command_name, handler, matches.groupdict())
-        if command_name in [command.name for command in protocol.mpd_commands]:
-            raise exceptions.MpdArgError(
-                'incorrect arguments', command=command_name)
-        raise exceptions.MpdUnknownCommand(command=command_name)
+            raise exceptions.MpdUnknownCommand(command=tokens[0])
 
     def _format_response(self, response):
         formatted_response = []
diff --git a/mopidy/mpd/protocol/__init__.py b/mopidy/mpd/protocol/__init__.py
index 04193c58..c86a39e6 100644
--- a/mopidy/mpd/protocol/__init__.py
+++ b/mopidy/mpd/protocol/__init__.py
@@ -12,11 +12,7 @@ implement our own MPD server which is compatible with the numerous existing
 
 from __future__ import unicode_literals
 
-import collections
 import inspect
-import re
-
-from mopidy.utils import formatting
 
 #: The MPD protocol uses UTF-8 for encoding all data.
 ENCODING = 'UTF-8'
@@ -27,63 +23,6 @@ LINE_TERMINATOR = '\n'
 #: The MPD protocol version is 0.17.0.
 VERSION = '0.17.0'
 
-MpdCommand = collections.namedtuple('MpdCommand', ['name', 'auth_required'])
-
-#: Set of all available commands, represented as :class:`MpdCommand` objects.
-mpd_commands = set()
-
-#: Map between request matchers and request handler functions.
-request_handlers = {}
-
-
-def handle_request(pattern, auth_required=True):
-    """
-    Decorator for connecting command handlers to command requests.
-
-    If you use named groups in the pattern, the decorated method will get the
-    groups as keyword arguments. If the group is optional, remember to give the
-    argument a default value.
-
-    For example, if the command is ``do that thing`` the ``what`` argument will
-    be ``this thing``::
-
-        @handle_request('do\ (?P<what>.+)$')
-        def do(what):
-            ...
-
-    Note that the patterns are compiled with the :attr:`re.VERBOSE` flag. Thus,
-    you must escape any space characters you want to match, but you're also
-    free to add non-escaped whitespace to format the pattern for easier
-    reading.
-
-    :param pattern: regexp pattern for matching commands
-    :type pattern: string
-    """
-    def decorator(func):
-        match = re.search('([a-z_]+)', pattern)
-        if match is not None:
-            mpd_commands.add(
-                MpdCommand(name=match.group(), auth_required=auth_required))
-        compiled_pattern = re.compile(pattern, flags=(re.UNICODE | re.VERBOSE))
-        if compiled_pattern in request_handlers:
-            raise ValueError('Tried to redefine handler for %s with %s' % (
-                pattern, func))
-        request_handlers[compiled_pattern] = func
-        func.__doc__ = """
-    *Pattern:*
-
-    .. code-block:: text
-
-%(pattern)s
-
-%(docs)s
-        """ % {
-            'pattern': formatting.indent(pattern, places=8, singles=True),
-            'docs': func.__doc__ or '',
-        }
-        return func
-    return decorator
-
 
 def load_protocol_modules():
     """
@@ -92,8 +31,7 @@ def load_protocol_modules():
     """
     from . import (  # noqa
         audio_output, channels, command_list, connection, current_playlist,
-        empty, music_db, playback, reflection, status, stickers,
-        stored_playlists)
+        music_db, playback, reflection, status, stickers, stored_playlists)
 
 
 def INT(value):
diff --git a/mopidy/mpd/protocol/reflection.py b/mopidy/mpd/protocol/reflection.py
index 227a17a3..4308c560 100644
--- a/mopidy/mpd/protocol/reflection.py
+++ b/mopidy/mpd/protocol/reflection.py
@@ -33,13 +33,6 @@ def commands(context):
         if context.dispatcher.authenticated or not handler.auth_required:
             command_names.add(name)
 
-    # TODO: remove
-    if context.dispatcher.authenticated:
-        command_names.update(c.name for c in protocol.mpd_commands)
-    else:
-        command_names.update(c.name for c in protocol.mpd_commands
-                             if not c.auth_required)
-
     return [
         ('command', command_name) for command_name in sorted(command_names)]
 
@@ -87,11 +80,6 @@ def notcommands(context):
         if not context.dispatcher.authenticated and handler.auth_required:
             command_names.add(name)
 
-    # TODO: remove
-    if not context.dispatcher.authenticated:
-        command_names.update(command.name for command in protocol.mpd_commands
-                             if command.auth_required)
-
     return [
         ('command', command_name) for command_name in sorted(command_names)]
 
diff --git a/tests/mpd/test_dispatcher.py b/tests/mpd/test_dispatcher.py
index 7ef4c13b..cee4531a 100644
--- a/tests/mpd/test_dispatcher.py
+++ b/tests/mpd/test_dispatcher.py
@@ -8,7 +8,6 @@ from mopidy import core
 from mopidy.backend import dummy
 from mopidy.mpd.dispatcher import MpdDispatcher
 from mopidy.mpd.exceptions import MpdAckError
-from mopidy.mpd.protocol import request_handlers, handle_request
 
 
 class MpdDispatcherTest(unittest.TestCase):
@@ -25,42 +24,15 @@ class MpdDispatcherTest(unittest.TestCase):
     def tearDown(self):
         pykka.ActorRegistry.stop_all()
 
-    def test_register_same_pattern_twice_fails(self):
-        func = lambda: None
+    def test_call_handler_for_unknown_command_raises_exception(self):
         try:
-            handle_request('a pattern')(func)
-            handle_request('a pattern')(func)
-            self.fail('Registering a pattern twice shoulde raise ValueError')
-        except ValueError:
-            pass
-
-    def test_finding_handler_for_unknown_command_raises_exception(self):
-        try:
-            self.dispatcher._find_handler('an_unknown_command with args')
+            self.dispatcher._call_handler('an_unknown_command with args')
             self.fail('Should raise exception')
         except MpdAckError as e:
             self.assertEqual(
                 e.get_mpd_ack(),
                 'ACK [5@0] {} unknown command "an_unknown_command"')
 
-    def test_find_handler_for_known_command_return_name_handler_and_args(self):
-        expected_handler = lambda x: None
-        request_handlers['known_command (?P<arg1>.+)'] = \
-            expected_handler
-        (name, handler, kwargs) = self.dispatcher._find_handler(
-            'known_command an_arg')
-        self.assertEqual(handler, expected_handler)
-        self.assertEqual('known_command', name)
-        self.assertIn('arg1', kwargs)
-        self.assertEqual(kwargs['arg1'], 'an_arg')
-
     def test_handling_unknown_request_yields_error(self):
         result = self.dispatcher.handle_request('an unhandled request')
         self.assertEqual(result[0], 'ACK [5@0] {} unknown command "an"')
-
-    def test_handling_known_request(self):
-        expected = 'magic'
-        request_handlers['known request'] = lambda x: expected
-        result = self.dispatcher.handle_request('known request')
-        self.assertIn('OK', result)
-        self.assertIn(expected, result)

From b3a273110cf6af7533ca394a3f533f949bb99f15 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Fri, 24 Jan 2014 00:48:16 +0100
Subject: [PATCH 28/35] mpd: Remove unused exception type

---
 mopidy/mpd/tokenize.py | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/mopidy/mpd/tokenize.py b/mopidy/mpd/tokenize.py
index 04799719..195209e3 100644
--- a/mopidy/mpd/tokenize.py
+++ b/mopidy/mpd/tokenize.py
@@ -5,10 +5,6 @@ import re
 from mopidy.mpd import exceptions
 
 
-class Error(Exception):
-    pass
-
-
 WORD_RE = re.compile(r"""
     ^
     (\s*)             # Leading whitespace not allowed, capture it to report.

From e304b7fc2be85f74d61654085a4c3ec536ab96a6 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Fri, 24 Jan 2014 01:12:26 +0100
Subject: [PATCH 29/35] mpd: Use mopidy.mpd.exceptions in commands

---
 mopidy/mpd/protocol/__init__.py | 14 +++++++++-----
 tests/mpd/test_commands.py      | 16 +++++++++++++---
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/mopidy/mpd/protocol/__init__.py b/mopidy/mpd/protocol/__init__.py
index c86a39e6..1be42dcc 100644
--- a/mopidy/mpd/protocol/__init__.py
+++ b/mopidy/mpd/protocol/__init__.py
@@ -14,6 +14,8 @@ from __future__ import unicode_literals
 
 import inspect
 
+from mopidy.mpd import exceptions
+
 #: The MPD protocol uses UTF-8 for encoding all data.
 ENCODING = 'UTF-8'
 
@@ -79,7 +81,7 @@ class Commands(object):
     def add(self, name, auth_required=True, list_command=True, **validators):
         def wrapper(func):
             if name in self.handlers:
-                raise Exception('%s already registered' % name)
+                raise ValueError('%s already registered' % name)
 
             args, varargs, keywords, defaults = inspect.getargspec(func)
             defaults = dict(zip(args[-len(defaults or []):], defaults or []))
@@ -104,7 +106,10 @@ class Commands(object):
                 for key, value in callargs.items():
                     default = defaults.get(key, object())
                     if key in validators and value != default:
-                        callargs[key] = validators[key](value)
+                        try:
+                            callargs[key] = validators[key](value)
+                        except ValueError:
+                            raise exceptions.MpdArgError('incorrect arguments')
                 return func(**callargs)
 
             validate.auth_required = auth_required
@@ -114,11 +119,10 @@ class Commands(object):
         return wrapper
 
     def call(self, args, context=None):
-        # TODO: raise mopidy.mpd.exceptions
         if not args:
-            raise TypeError('No args provided')
+            raise exceptions.MpdNoCommand()
         if args[0] not in self.handlers:
-            raise LookupError('Unknown command')
+            raise exceptions.MpdUnknownCommand(command=args[0])
         return self.handlers[args[0]](context, *args[1:])
 
 
diff --git a/tests/mpd/test_commands.py b/tests/mpd/test_commands.py
index 845796af..91a9125a 100644
--- a/tests/mpd/test_commands.py
+++ b/tests/mpd/test_commands.py
@@ -4,7 +4,7 @@ from __future__ import unicode_literals
 
 import unittest
 
-from mopidy.mpd import protocol
+from mopidy.mpd import exceptions, protocol
 
 
 class TestConverts(unittest.TestCase):
@@ -134,7 +134,7 @@ class TestCommands(unittest.TestCase):
         self.assertEqual(sentinel3, self.commands.call(['baz']))
 
     def test_call_with_nonexistent_handler(self):
-        with self.assertRaises(LookupError):
+        with self.assertRaises(exceptions.MpdUnknownCommand):
             self.commands.call(['bar'])
 
     def test_call_passes_context(self):
@@ -144,7 +144,7 @@ class TestCommands(unittest.TestCase):
             sentinel, self.commands.call(['foo'], context=sentinel))
 
     def test_call_without_args_fails(self):
-        with self.assertRaises(TypeError):
+        with self.assertRaises(exceptions.MpdNoCommand):
             self.commands.call([])
 
     def test_call_passes_required_argument(self):
@@ -212,6 +212,16 @@ class TestCommands(unittest.TestCase):
             func = lambda context: True
             self.commands.add('bar', context=lambda v: v)(func)
 
+    def test_validator_value_error_is_converted(self):
+        def validdate(value):
+            raise ValueError
+
+        func = lambda context, arg: True
+        self.commands.add('bar', arg=validdate)(func)
+
+        with self.assertRaises(exceptions.MpdArgError):
+            self.commands.call(['bar', 'test'])
+
     def test_auth_required_gets_stored(self):
         func1 = lambda context: context
         func2 = lambda context: context

From f24ca36e5a5b72c886d6d8e8df88be79fb094dda Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Sat, 25 Jan 2014 00:42:32 +0100
Subject: [PATCH 30/35] mpd: Switch browse commands to common helper

---
 mopidy/mpd/dispatcher.py                | 35 +++++++---
 mopidy/mpd/protocol/current_playlist.py | 22 ++----
 mopidy/mpd/protocol/music_db.py         | 91 ++++++-------------------
 tests/mpd/protocol/test_music_db.py     | 23 ++++++-
 4 files changed, 74 insertions(+), 97 deletions(-)

diff --git a/mopidy/mpd/dispatcher.py b/mopidy/mpd/dispatcher.py
index 61a07fb2..a53d387b 100644
--- a/mopidy/mpd/dispatcher.py
+++ b/mopidy/mpd/dispatcher.py
@@ -288,18 +288,35 @@ class MpdContext(object):
             self.refresh_playlists_mapping()
         return self._playlist_name_from_uri[uri]
 
-    # TODO: consider making context.browse(path) which uses this internally.
-    # advantage would be that all browse requests then go through the same code
-    # and we could prebuild/cache path->uri relationships instead of having to
-    # look them up all the time.
-    def directory_path_to_uri(self, path):
-        parts = re.findall(r'[^/]+', path)
+    def browse(self, path, recursive=True, lookup=True):
+        # TODO: consider caching lookups for less work mapping path->uri
+        path_parts = re.findall(r'[^/]+', path or '')
+        root_path = '/'.join([''] + path_parts)
         uri = None
-        for part in parts:
+
+        for part in path_parts:
             for ref in self.core.library.browse(uri).get():
                 if ref.type == ref.DIRECTORY and ref.name == part:
                     uri = ref.uri
                     break
             else:
-                raise exceptions.MpdNoExistError()
-        return uri
+                raise exceptions.MpdNoExistError('Not found')
+
+        if recursive:
+            yield (root_path, None)
+
+        path_and_futures = [(root_path, self.core.library.browse(uri))]
+        while path_and_futures:
+            base_path, future = path_and_futures.pop()
+            for ref in future.get():
+                path = '/'.join([base_path, ref.name.replace('/', '')])
+                if ref.type == ref.DIRECTORY:
+                    yield (path, None)
+                    if recursive:
+                        path_and_futures.append(
+                            (path, self.core.library.browse(ref.uri)))
+                elif ref.type == ref.TRACK:
+                    if lookup:
+                        yield (path, self.core.library.lookup(ref.uri))
+                    else:
+                        yield (path, ref)
diff --git a/mopidy/mpd/protocol/current_playlist.py b/mopidy/mpd/protocol/current_playlist.py
index 39ba9486..e50c1bff 100644
--- a/mopidy/mpd/protocol/current_playlist.py
+++ b/mopidy/mpd/protocol/current_playlist.py
@@ -20,32 +20,20 @@ def add(context, uri):
     if not uri.strip('/'):
         return
 
-    tl_tracks = context.core.tracklist.add(uri=uri).get()
-    if tl_tracks:
+    if context.core.tracklist.add(uri=uri).get():
         return
 
     try:
-        uri = context.directory_path_to_uri(translator.normalize_path(uri))
+        tracks = []
+        for path, lookup_future in context.browse(uri):
+            if lookup_future:
+                tracks.extend(lookup_future.get())
     except exceptions.MpdNoExistError as e:
         e.message = 'directory or file not found'
         raise
 
-    browse_futures = [context.core.library.browse(uri)]
-    lookup_futures = []
-    while browse_futures:
-        for ref in browse_futures.pop().get():
-            if ref.type == ref.DIRECTORY:
-                browse_futures.append(context.core.library.browse(ref.uri))
-            else:
-                lookup_futures.append(context.core.library.lookup(ref.uri))
-
-    tracks = []
-    for future in lookup_futures:
-        tracks.extend(future.get())
-
     if not tracks:
         raise exceptions.MpdNoExistError('directory or file not found')
-
     context.core.tracklist.add(tracks=tracks)
 
 
diff --git a/mopidy/mpd/protocol/music_db.py b/mopidy/mpd/protocol/music_db.py
index 2472e1d9..2d895d67 100644
--- a/mopidy/mpd/protocol/music_db.py
+++ b/mopidy/mpd/protocol/music_db.py
@@ -3,7 +3,7 @@ from __future__ import unicode_literals
 import functools
 import itertools
 
-from mopidy.models import Ref, Track
+from mopidy.models import Track
 from mopidy.mpd import exceptions, protocol, translator
 
 _SEARCH_MAPPING = {
@@ -349,7 +349,6 @@ def _list_genre(context, query):
     return genres
 
 
-# TODO: see if we can combine listall, listallinfo and lsinfo to one helper
 @protocol.commands.add('listall')
 def listall(context, uri=None):
     """
@@ -360,29 +359,15 @@ def listall(context, uri=None):
         Lists all songs and directories in ``URI``.
     """
     result = []
-    root_path = translator.normalize_path(uri)
-    try:
-        uri = context.directory_path_to_uri(root_path)
-    except exceptions.MpdNoExistError as e:
-        e.message = 'Not found'
-        raise
-    browse_futures = [(root_path, context.core.library.browse(uri))]
-
-    while browse_futures:
-        base_path, future = browse_futures.pop()
-        for ref in future.get():
-            if ref.type == Ref.DIRECTORY:
-                path = '/'.join([base_path, ref.name.replace('/', '')])
-                result.append(('directory', path))
-                browse_futures.append(
-                    (path, context.core.library.browse(ref.uri)))
-            elif ref.type == Ref.TRACK:
-                result.append(('file', ref.uri))
+    for path, track_ref in context.browse(uri, lookup=False):
+        if not track_ref:
+            result.append(('directory', path))
+        else:
+            result.append(('file', track_ref.uri))
 
     if not result:
         raise exceptions.MpdNoExistError('Not found')
-
-    return [('directory', root_path)] + result
+    return result
 
 
 @protocol.commands.add('listallinfo')
@@ -395,41 +380,14 @@ def listallinfo(context, uri=None):
         Same as ``listall``, except it also returns metadata info in the
         same format as ``lsinfo``.
     """
-    dirs_and_futures = []
     result = []
-    root_path = translator.normalize_path(uri)
-    try:
-        uri = context.directory_path_to_uri(root_path)
-    except exceptions.MpdNoExistError as e:
-        e.command = 'listallinfo'
-        e.message = 'Not found'
-        raise
-    browse_futures = [(root_path, context.core.library.browse(uri))]
-
-    while browse_futures:
-        base_path, future = browse_futures.pop()
-        for ref in future.get():
-            if ref.type == Ref.DIRECTORY:
-                path = '/'.join([base_path, ref.name.replace('/', '')])
-                future = context.core.library.browse(ref.uri)
-                browse_futures.append((path, future))
-                dirs_and_futures.append(('directory', path))
-            elif ref.type == Ref.TRACK:
-                # TODO Lookup tracks in batch for better performance
-                dirs_and_futures.append(context.core.library.lookup(ref.uri))
-
-    result = []
-    for obj in dirs_and_futures:
-        if hasattr(obj, 'get'):
-            for track in obj.get():
-                result.extend(translator.track_to_mpd_format(track))
+    for path, lookup_future in context.browse(uri):
+        if not lookup_future:
+            result.append(('directory', path))
         else:
-            result.append(obj)
-
-    if not result:
-        raise exceptions.MpdNoExistError('Not found')
-
-    return [('directory', root_path)] + result
+            for track in lookup_future.get():
+                result.extend(translator.track_to_mpd_format(track))
+    return result
 
 
 @protocol.commands.add('lsinfo')
@@ -450,26 +408,19 @@ def lsinfo(context, uri=None):
     ""``, and ``lsinfo "/"``.
     """
     result = []
-    root_path = translator.normalize_path(uri, relative=True)
-    try:
-        uri = context.directory_path_to_uri(root_path)
-    except exceptions.MpdNoExistError as e:
-        e.command = 'lsinfo'
-        e.message = 'Not found'
-        raise
-
-    if uri is None:
+    if uri in (None, '', '/'):
         result.extend(protocol.stored_playlists.listplaylists(context))
 
-    for ref in context.core.library.browse(uri).get():
-        if ref.type == Ref.DIRECTORY:
-            path = '/'.join([root_path, ref.name.replace('/', '')])
+    for path, lookup_future in context.browse(uri, recursive=False):
+        if not lookup_future:
             result.append(('directory', path.lstrip('/')))
-        elif ref.type == Ref.TRACK:
-            # TODO Lookup tracks in batch for better performance
-            tracks = context.core.library.lookup(ref.uri).get()
+        else:
+            tracks = lookup_future.get()
             if tracks:
                 result.extend(translator.track_to_mpd_format(tracks[0]))
+
+    if not result:
+        raise exceptions.MpdNoExistError('Not found')
     return result
 
 
diff --git a/tests/mpd/protocol/test_music_db.py b/tests/mpd/protocol/test_music_db.py
index d41c05b2..d60a485f 100644
--- a/tests/mpd/protocol/test_music_db.py
+++ b/tests/mpd/protocol/test_music_db.py
@@ -306,12 +306,33 @@ class MusicDatabaseHandlerTest(protocol.BaseTestCase):
 
     def test_lsinfo_for_dir_includes_subdirs(self):
         self.backend.library.dummy_browse_result = {
-            'dummy:/': [Ref.directory(uri='/foo', name='foo')]}
+            'dummy:/': [Ref.directory(uri='dummy:/foo', name='foo')]}
 
         self.sendRequest('lsinfo "/dummy"')
         self.assertInResponse('directory: dummy/foo')
         self.assertInResponse('OK')
 
+    def test_lsinfo_for_dir_does_not_recurse(self):
+        self.backend.library.dummy_library = [
+            Track(uri='dummy:/a', name='a'),
+        ]
+        self.backend.library.dummy_browse_result = {
+            'dummy:/': [Ref.directory(uri='dummy:/foo', name='foo')],
+            'dummy:/foo': [Ref.track(uri='dummy:/a', name='a')]}
+
+        self.sendRequest('lsinfo "/dummy"')
+        self.assertNotInResponse('file: dummy:/a')
+        self.assertInResponse('OK')
+
+    def test_lsinfo_for_dir_does_not_self(self):
+        self.backend.library.dummy_browse_result = {
+            'dummy:/': [Ref.directory(uri='dummy:/foo', name='foo')],
+            'dummy:/foo': [Ref.track(uri='dummy:/a', name='a')]}
+
+        self.sendRequest('lsinfo "/dummy"')
+        self.assertNotInResponse('directory: dummy')
+        self.assertInResponse('OK')
+
     def test_update_without_uri(self):
         self.sendRequest('update')
         self.assertInResponse('updating_db: 0')

From a7f4ffb124f231cdd3b625bf7d05f5150eba2a4d Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Thu, 30 Jan 2014 23:51:31 +0100
Subject: [PATCH 31/35] mpd: Update docs

---
 docs/modules/mpd.rst            |  7 ++++
 mopidy/mpd/protocol/__init__.py | 57 +++++++++++++++++++++++++++++----
 mopidy/mpd/tokenize.py          | 18 +++++++++++
 3 files changed, 76 insertions(+), 6 deletions(-)

diff --git a/docs/modules/mpd.rst b/docs/modules/mpd.rst
index 4a9eb7e8..1826e535 100644
--- a/docs/modules/mpd.rst
+++ b/docs/modules/mpd.rst
@@ -7,6 +7,13 @@ For details on how to use Mopidy's MPD server, see :ref:`ext-mpd`.
 .. automodule:: mopidy.mpd
     :synopsis: MPD server frontend
 
+MPD tokenizer
+=============
+
+.. automodule:: mopidy.mpd.tokenize
+    :synopsis: MPD request tokenizer
+    :members:
+
 
 MPD dispatcher
 ==============
diff --git a/mopidy/mpd/protocol/__init__.py b/mopidy/mpd/protocol/__init__.py
index 1be42dcc..7d009022 100644
--- a/mopidy/mpd/protocol/__init__.py
+++ b/mopidy/mpd/protocol/__init__.py
@@ -37,6 +37,7 @@ def load_protocol_modules():
 
 
 def INT(value):
+    """Converts a value that matches [+-]?\d+ into and integer."""
     if value is None:
         raise ValueError('None is not a valid integer')
     # TODO: check for whitespace via value != value.strip()?
@@ -44,6 +45,7 @@ def INT(value):
 
 
 def UINT(value):
+    """Converts a value that matches \d+ into and integer."""
     if value is None:
         raise ValueError('None is not a valid integer')
     if not value.isdigit():
@@ -52,13 +54,19 @@ def UINT(value):
 
 
 def BOOL(value):
+    """Convert the values 0 and 1 into booleans."""
     if value in ('1', '0'):
         return bool(int(value))
     raise ValueError('%r is not 0 or 1' % value)
 
 
 def RANGE(value):
-    # TODO: test and check that values are positive
+    """Convert a single integer or range spec into a slice
+
+    `n` should become `slice(n, n+1)`
+    `n:` should become `slice(n, None)`
+    `n:m` should become `slice(n, m)` and `m > n` must hold
+    """
     if ':' in value:
         start, stop = value.split(':', 1)
         start = UINT(start)
@@ -75,10 +83,38 @@ def RANGE(value):
 
 
 class Commands(object):
+    """Collection of MPD commands to expose to users.
+
+    Normally used through the global instance which command handlers have been
+    installed into.
+    """
+
     def __init__(self):
         self.handlers = {}
 
+    # TODO: consider removing auth_required and list_command in favour of
+    # additional command instances to register in?
     def add(self, name, auth_required=True, list_command=True, **validators):
+        """Create a decorator that registers a handler + validation rules.
+
+        Additional keyword arguments are treated as converts/validators to
+        apply to tokens converting them to proper python types.
+
+        Requirements for valid handlers:
+
+        - must accept a context argument as the first arg.
+        - may not use variable keyword arguments, ``**kwargs``.
+        - may use variable arguments ``*args`` *or* a mix of required and
+          optional arguments.
+
+        Decorator returns the unwrapped function so that tests etc can use the
+        functions with values with correct python types instead of strings.
+
+        :param string name: Name of the command being registered.
+        :param bool auth_required: If authorization is required.
+        :param bool list_command: If command should be listed in reflection.
+        """
+
         def wrapper(func):
             if name in self.handlers:
                 raise ValueError('%s already registered' % name)
@@ -118,12 +154,21 @@ class Commands(object):
             return func
         return wrapper
 
-    def call(self, args, context=None):
-        if not args:
+    def call(self, tokens, context=None):
+        """Find and run the handler registered for the given command.
+
+        If the handler was registered with any converters/validators there will
+        be run before calling the real handler.
+
+        :param list tokens: List of tokens to process
+        :param context: MPD context.
+        :type context: :class:`~mopidy.mpd.dispatcher.MpdContext`
+        """
+        if not tokens:
             raise exceptions.MpdNoCommand()
-        if args[0] not in self.handlers:
-            raise exceptions.MpdUnknownCommand(command=args[0])
-        return self.handlers[args[0]](context, *args[1:])
+        if tokens[0] not in self.handlers:
+            raise exceptions.MpdUnknownCommand(command=tokens[0])
+        return self.handlers[tokens[0]](context, *tokens[1:])
 
 
 #: Global instance to install commands into
diff --git a/mopidy/mpd/tokenize.py b/mopidy/mpd/tokenize.py
index 195209e3..bc0d6b3f 100644
--- a/mopidy/mpd/tokenize.py
+++ b/mopidy/mpd/tokenize.py
@@ -39,6 +39,23 @@ UNESCAPE_RE = re.compile(r'\\(.)')  # Backslash escapes any following char.
 
 
 def split(line):
+    """Splits a line into tokens using same rules as MPD.
+
+    - Lines may not start with whitespace
+    - Tokens are split by arbitrary amount of spaces or tabs
+    - First token must match `[a-z][a-z0-9_]*`
+    - Remaining tokens can be unquoted or quoted tokens.
+    - Unquoted tokens consist of all printable characters except double quotes,
+      single quotes, spaces and tabs.
+    - Quoted tokens are surrounded by a matching pair of double quotes.
+    - The closing quote must be followed by space, tab or end of line.
+    - Any value is allowed inside a quoted token. Including double quotes,
+      assuming it is correctly escaped.
+    - Backslash inside a quoted token is used to escape the following
+      character.
+
+    For examples see the tests for this function.
+    """
     if not line.strip():
         raise exceptions.MpdNoCommand('No command given')
     match = WORD_RE.match(line)
@@ -60,6 +77,7 @@ def split(line):
 
 
 def _determine_error_message(remainder):
+    """Helper to emulate MPD errors."""
     # Following checks are simply to match MPD error messages:
     match = BAD_QUOTED_PARAM_RE.match(remainder)
     if match:

From 2de897fb9c8943c5c9353ba388db1c0316b9633e Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Fri, 31 Jan 2014 00:37:27 +0100
Subject: [PATCH 32/35] mpd: Add warnings to deprecated commands

---
 mopidy/mpd/protocol/current_playlist.py |  5 ++++-
 mopidy/mpd/protocol/playback.py         | 11 ++++++++---
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/mopidy/mpd/protocol/current_playlist.py b/mopidy/mpd/protocol/current_playlist.py
index e50c1bff..45858f86 100644
--- a/mopidy/mpd/protocol/current_playlist.py
+++ b/mopidy/mpd/protocol/current_playlist.py
@@ -1,5 +1,7 @@
 from __future__ import unicode_literals
 
+import warnings
+
 from mopidy.mpd import exceptions, protocol, translator
 
 
@@ -160,7 +162,8 @@ def playlist(context):
 
             Do not use this, instead use ``playlistinfo``.
     """
-    # TODO: warn about this being deprecated
+    warnings.warn(
+        'Do not use this, instead use playlistinfo', DeprecationWarning)
     return playlistinfo(context)
 
 
diff --git a/mopidy/mpd/protocol/playback.py b/mopidy/mpd/protocol/playback.py
index d18aff50..c550a33a 100644
--- a/mopidy/mpd/protocol/playback.py
+++ b/mopidy/mpd/protocol/playback.py
@@ -1,5 +1,7 @@
 from __future__ import unicode_literals
 
+import warnings
+
 from mopidy.core import PlaybackState
 from mopidy.mpd import exceptions, protocol
 
@@ -102,7 +104,10 @@ def pause(context, state=None):
     - Calls ``pause`` without any arguments to toogle pause.
     """
     if state is None:
-        # TODO: emit warning about this being deprecated
+        warnings.warn(
+            'The use of pause command w/o the PAUSE argument is deprecated.',
+            DeprecationWarning)
+
         if (context.core.playback.state.get() == PlaybackState.PLAYING):
             context.core.playback.pause()
         elif (context.core.playback.state.get() == PlaybackState.PAUSED):
@@ -339,10 +344,10 @@ def seekcur(context, time):
     """
     if time.startswith(('+', '-')):
         position = context.core.playback.time_position.get()
-        position += int(time) * 1000
+        position += protocol.INT(time) * 1000
         context.core.playback.seek(position).get()
     else:
-        position = int(time) * 1000
+        position = protocol.UINT(time) * 1000
         context.core.playback.seek(position).get()
 
 

From 34997a46a49b3e1b2f3cc974404213172adabfd6 Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Fri, 31 Jan 2014 00:37:42 +0100
Subject: [PATCH 33/35] mpd: Add placeholders for missing commands

---
 mopidy/mpd/protocol/audio_output.py     | 13 ++++++
 mopidy/mpd/protocol/current_playlist.py | 62 +++++++++++++++++++++++++
 mopidy/mpd/protocol/music_db.py         | 24 ++++++++++
 mopidy/mpd/protocol/playback.py         | 32 +++++++++++++
 4 files changed, 131 insertions(+)

diff --git a/mopidy/mpd/protocol/audio_output.py b/mopidy/mpd/protocol/audio_output.py
index 981cdd55..868edc49 100644
--- a/mopidy/mpd/protocol/audio_output.py
+++ b/mopidy/mpd/protocol/audio_output.py
@@ -33,6 +33,19 @@ def enableoutput(context, outputid):
         raise exceptions.MpdNoExistError('No such audio output')
 
 
+# TODO: implement and test
+#@protocol.commands.add('toggleoutput', outputid=protocol.UINT)
+def toggleoutput(context, outputid):
+    """
+    *musicpd.org, audio output section:*
+
+        ``toggleoutput {ID}``
+
+        Turns an output on or off, depending on the current state.
+    """
+    pass
+
+
 @protocol.commands.add('outputs')
 def outputs(context):
     """
diff --git a/mopidy/mpd/protocol/current_playlist.py b/mopidy/mpd/protocol/current_playlist.py
index 45858f86..c2e54490 100644
--- a/mopidy/mpd/protocol/current_playlist.py
+++ b/mopidy/mpd/protocol/current_playlist.py
@@ -357,3 +357,65 @@ def swapid(context, tlid1, tlid2):
     position1 = context.core.tracklist.index(tl_tracks1[0]).get()
     position2 = context.core.tracklist.index(tl_tracks2[0]).get()
     swap(context, position1, position2)
+
+
+# TODO: add at least reflection tests before adding NotImplemented version
+#@protocol.commands.add(
+#    'prio', priority=protocol.UINT, position=protocol.RANGE)
+def prio(context, priority, position):
+    """
+    *musicpd.org, current playlist section:*
+
+        ``prio {PRIORITY} {START:END...}``
+
+        Set the priority of the specified songs. A higher priority means that
+        it will be played first when "random" mode is enabled.
+
+        A priority is an integer between 0 and 255. The default priority of new
+        songs is 0.
+    """
+    pass
+
+
+# TODO: add at least reflection tests before adding NotImplemented version
+#@protocol.commands.add('prioid')
+def prioid(context, *args):
+    """
+    *musicpd.org, current playlist section:*
+
+        ``prioid {PRIORITY} {ID...}``
+
+        Same as prio, but address the songs with their id.
+    """
+    pass
+
+
+# TODO: add at least reflection tests before adding NotImplemented version
+#@protocol.commands.add('addtagid', tlid=protocol.UINT)
+def addtagid(context, tlid, tag, value):
+    """
+    *musicpd.org, current playlist section:*
+
+        ``addtagid {SONGID} {TAG} {VALUE}``
+
+        Adds a tag to the specified song. Editing song tags is only possible
+        for remote songs. This change is volatile: it may be overwritten by
+        tags received from the server, and the data is gone when the song gets
+        removed from the queue.
+    """
+    pass
+
+
+# TODO: add at least reflection tests before adding NotImplemented version
+#@protocol.commands.add('cleartagid', tlid=protocol.UINT)
+def cleartagid(context, tlid, tag):
+    """
+    *musicpd.org, current playlist section:*
+
+        ``cleartagid {SONGID} [TAG]``
+
+        Removes tags from the specified song. If TAG is not specified, then all
+        tag values will be removed. Editing song tags is only possible for
+        remote songs.
+    """
+    pass
diff --git a/mopidy/mpd/protocol/music_db.py b/mopidy/mpd/protocol/music_db.py
index 2d895d67..3da9c5aa 100644
--- a/mopidy/mpd/protocol/music_db.py
+++ b/mopidy/mpd/protocol/music_db.py
@@ -547,3 +547,27 @@ def update(context, uri=None):
         ``status`` response.
     """
     return {'updating_db': 0}  # TODO
+
+
+# TODO: add at least reflection tests before adding NotImplemented version
+#@protocol.commands.add('readcomments')
+def readcomments(context, uri):
+    """
+    *musicpd.org, music database section:*
+
+        ``readcomments [URI]``
+
+        Read "comments" (i.e. key-value pairs) from the file specified by
+        "URI". This "URI" can be a path relative to the music directory or a
+        URL in the form "file:///foo/bar.ogg".
+
+        This command may be used to list metadata of remote files (e.g. URI
+        beginning with "http://" or "smb://").
+
+        The response consists of lines in the form "KEY: VALUE". Comments with
+        suspicious characters (e.g. newlines) are ignored silently.
+
+        The meaning of these depends on the codec, and not all decoder plugins
+        support it. For example, on Ogg files, this lists the Vorbis comments.
+    """
+    pass
diff --git a/mopidy/mpd/protocol/playback.py b/mopidy/mpd/protocol/playback.py
index c550a33a..a3de1891 100644
--- a/mopidy/mpd/protocol/playback.py
+++ b/mopidy/mpd/protocol/playback.py
@@ -32,6 +32,38 @@ def crossfade(context, seconds):
     raise exceptions.MpdNotImplemented  # TODO
 
 
+# TODO: add at least reflection tests before adding NotImplemented version
+#@protocol.commands.add('mixrampdb')
+def mixrampdb(context, decibels):
+    """
+    *musicpd.org, playback section:*
+
+        ``mixrampdb {deciBels}``
+
+    Sets the threshold at which songs will be overlapped. Like crossfading but
+    doesn't fade the track volume, just overlaps. The songs need to have
+    MixRamp tags added by an external tool. 0dB is the normalized maximum
+    volume so use negative values, I prefer -17dB. In the absence of mixramp
+    tags crossfading will be used. See http://sourceforge.net/projects/mixramp
+    """
+    pass
+
+
+# TODO: add at least reflection tests before adding NotImplemented version
+#@protocol.commands.add('mixrampdelay', seconds=protocol.UINT)
+def mixrampdelay(context, seconds):
+    """
+    *musicpd.org, playback section:*
+
+        ``mixrampdelay {SECONDS}``
+
+        Additional time subtracted from the overlap calculated by mixrampdb. A
+        value of "nan" disables MixRamp overlapping and falls back to
+        crossfading.
+    """
+    pass
+
+
 @protocol.commands.add('next')
 def next_(context):
     """

From 4e89ce7c067f6562f9db131ed2a3848243fd1d4c Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Fri, 31 Jan 2014 00:51:33 +0100
Subject: [PATCH 34/35] docs: Add changelog for MPD tokenizer

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

diff --git a/docs/changelog.rst b/docs/changelog.rst
index e6082f90..05becbe8 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -20,6 +20,21 @@ v0.19.0 (unreleased)
 - Minor refactor of context such that it stores password instead of config.
   (Fixes: :issue:`646`)
 
+- Proper command tokenization for MPD requests. This replaces the old regex
+  based system with an MPD protocol specific tokenizer responsible for breaking
+  requests into pieces before the handlers have at them.
+  (Fixes: :issue:`591` and :issue:`592`)
+
+- Updated commands handler system. As part of the tokenizer cleanup we've updated
+  how commands are registered and making it simpler to create new handlers.
+
+- Simplifies a bunch of handlers. All the "browse" type commands now use a
+  common browse helper under the hood for less repetition. Likewise the query
+  handling of "search" commands has been somewhat simplified.
+
+- Adds placeholders for missing MPD commands, preparing the way for bumping the
+  protocol version once they have been added.
+
 **Windows**
 
 - Network and signal handling has been updated to play nice on windows systems.

From 8756c2bff550c0c8f3b4625ee98c6e0c70b8640d Mon Sep 17 00:00:00 2001
From: Thomas Adamcik <thomas@adamcik.no>
Date: Sun, 16 Feb 2014 23:17:46 +0100
Subject: [PATCH 35/35] review: Type fixing

---
 mopidy/mpd/protocol/__init__.py     | 20 ++++++++++----------
 mopidy/mpd/protocol/stickers.py     |  1 -
 tests/mpd/protocol/test_music_db.py |  2 +-
 3 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/mopidy/mpd/protocol/__init__.py b/mopidy/mpd/protocol/__init__.py
index 7d009022..e24af565 100644
--- a/mopidy/mpd/protocol/__init__.py
+++ b/mopidy/mpd/protocol/__init__.py
@@ -29,7 +29,7 @@ VERSION = '0.17.0'
 def load_protocol_modules():
     """
     The protocol modules must be imported to get them registered in
-    :attr:`request_handlers` and :attr:`mpd_commands`.
+    :attr:`commands`.
     """
     from . import (  # noqa
         audio_output, channels, command_list, connection, current_playlist,
@@ -45,7 +45,7 @@ def INT(value):
 
 
 def UINT(value):
-    """Converts a value that matches \d+ into and integer."""
+    """Converts a value that matches \d+ into an integer."""
     if value is None:
         raise ValueError('None is not a valid integer')
     if not value.isdigit():
@@ -63,9 +63,9 @@ def BOOL(value):
 def RANGE(value):
     """Convert a single integer or range spec into a slice
 
-    `n` should become `slice(n, n+1)`
-    `n:` should become `slice(n, None)`
-    `n:m` should become `slice(n, m)` and `m > n` must hold
+    ``n`` should become ``slice(n, n+1)``
+    ``n:`` should become ``slice(n, None)``
+    ``n:m`` should become ``slice(n, m)`` and ``m > n`` must hold
     """
     if ':' in value:
         start, stop = value.split(':', 1)
@@ -95,10 +95,10 @@ class Commands(object):
     # TODO: consider removing auth_required and list_command in favour of
     # additional command instances to register in?
     def add(self, name, auth_required=True, list_command=True, **validators):
-        """Create a decorator that registers a handler + validation rules.
+        """Create a decorator that registers a handler and validation rules.
 
-        Additional keyword arguments are treated as converts/validators to
-        apply to tokens converting them to proper python types.
+        Additional keyword arguments are treated as converters/validators to
+        apply to tokens converting them to proper Python type`s.
 
         Requirements for valid handlers:
 
@@ -127,7 +127,7 @@ class Commands(object):
 
             if len(args) > 1 and varargs:
                 raise TypeError(
-                    '*args may not be combined with regular argmuments')
+                    '*args may not be combined with regular arguments')
 
             if not set(validators.keys()).issubset(args):
                 raise TypeError('Validator for non-existent arg passed')
@@ -157,7 +157,7 @@ class Commands(object):
     def call(self, tokens, context=None):
         """Find and run the handler registered for the given command.
 
-        If the handler was registered with any converters/validators there will
+        If the handler was registered with any converters/validators they will
         be run before calling the real handler.
 
         :param list tokens: List of tokens to process
diff --git a/mopidy/mpd/protocol/stickers.py b/mopidy/mpd/protocol/stickers.py
index e8718f1d..4d535423 100644
--- a/mopidy/mpd/protocol/stickers.py
+++ b/mopidy/mpd/protocol/stickers.py
@@ -22,7 +22,6 @@ def sticker(context, action, field, uri, name=None, value=None):
 
         Reads a sticker value for the specified object.
 
-
         ``sticker set {TYPE} {URI} {NAME} {VALUE}``
 
         Adds a sticker value to the specified object. If a sticker item
diff --git a/tests/mpd/protocol/test_music_db.py b/tests/mpd/protocol/test_music_db.py
index 6ea18caf..93e45a2e 100644
--- a/tests/mpd/protocol/test_music_db.py
+++ b/tests/mpd/protocol/test_music_db.py
@@ -323,7 +323,7 @@ class MusicDatabaseHandlerTest(protocol.BaseTestCase):
         self.assertNotInResponse('file: dummy:/a')
         self.assertInResponse('OK')
 
-    def test_lsinfo_for_dir_does_not_self(self):
+    def test_lsinfo_for_dir_does_not_include_self(self):
         self.backend.library.dummy_browse_result = {
             'dummy:/': [Ref.directory(uri='dummy:/foo', name='foo')],
             'dummy:/foo': [Ref.track(uri='dummy:/a', name='a')]}