From f3f140c19f3f15422a6ab6b80a9568f4e38eec00 Mon Sep 17 00:00:00 2001
From: Mark Greenwood <fatgerman@gmail.com>
Date: Mon, 6 Jul 2015 09:26:52 +0100
Subject: [PATCH] mpd:Update protocol to 0.19

Address issues raised in review:
Fix formatting by shortening function name to concat_multi_values
Change comments and variable names to reflect generic nature of function
Fix typos in tests
Default to single quotes for strings
---
 mopidy/mpd/translator.py     | 39 ++++++++++++++----------------------
 tests/mpd/test_translator.py | 15 +++++++-------
 2 files changed, 22 insertions(+), 32 deletions(-)

diff --git a/mopidy/mpd/translator.py b/mopidy/mpd/translator.py
index 49af8e5e..2cd9e3ad 100644
--- a/mopidy/mpd/translator.py
+++ b/mopidy/mpd/translator.py
@@ -37,7 +37,7 @@ def track_to_mpd_format(track, position=None, stream_title=None):
         # TODO: only show length if not none, see:
         # https://github.com/mopidy/mopidy/issues/923#issuecomment-79584110
         ('Time', track.length and (track.length // 1000) or 0),
-        ('Artist', concatenate_multiple_values(track.artists, 'name')),
+        ('Artist', concat_multi_values(track.artists, 'name')),
         ('Title', track.name or ''),
         ('Album', track.album and track.album.name or ''),
     ]
@@ -61,34 +61,24 @@ def track_to_mpd_format(track, position=None, stream_title=None):
 
     if track.album is not None and track.album.artists:
         result.append(
-            ('AlbumArtist',
-                concatenate_multiple_values(track.album.artists, 'name')))
-        musicbrainz_ids = concatenate_multiple_values(
+            ('AlbumArtist', concat_multi_values(track.album.artists, 'name')))
+        musicbrainz_ids = concat_multi_values(
             track.album.artists, 'musicbrainz_id')
         if musicbrainz_ids:
             result.append(('MUSICBRAINZ_ALBUMARTISTID', musicbrainz_ids))
 
     if track.artists:
-        musicbrainz_ids = concatenate_multiple_values(
-            track.artists, 'musicbrainz_id')
+        musicbrainz_ids = concat_multi_values(track.artists, 'musicbrainz_id')
         if musicbrainz_ids:
             result.append(('MUSICBRAINZ_ARTISTID', musicbrainz_ids))
 
     if track.composers:
         result.append(
-            (
-                'Composer',
-                concatenate_multiple_values(track.composers, 'name')
-            )
-        )
+            ('Composer', concat_multi_values(track.composers, 'name')))
 
     if track.performers:
         result.append(
-            (
-                'Performer',
-                concatenate_multiple_values(track.performers, 'name')
-            )
-        )
+            ('Performer', concat_multi_values(track.performers, 'name')))
 
     if track.genre:
         result.append(('Genre', track.genre))
@@ -101,22 +91,23 @@ def track_to_mpd_format(track, position=None, stream_title=None):
     return result
 
 
-def concatenate_multiple_values(artists, attribute):
+def concat_multi_values(models, attribute):
     """
-    Format track artist values for output to MPD client.
+    Format mopidy model values for output to MPD client.
 
-    :param artists: the artists
-    :type track: array of :class:`mopidy.models.Artist`
-    :param attribute: the artist attribute to use
-    :type string
+    :param models: the models
+    :type models: array of :class:`mopidy.models.Artist`,
+        :class:`mopidy.models.Album` or :class:`mopidy.models.Track`
+    :param attribute: the attribute to use
+    :type attribute: string
     :rtype: string
     """
     # Don't sort the values. MPD doesn't appear to (or if it does it's not
     # strict alphabetical). If we just use them in the order in which they come
     # in then the musicbrainz ids have a higher chance of staying in sync
     return ';'.join(
-        getattr(a, attribute)
-        for a in artists if getattr(a, attribute, None) is not None
+        getattr(m, attribute)
+        for m in models if getattr(m, attribute, None) is not None
     )
 
 
diff --git a/tests/mpd/test_translator.py b/tests/mpd/test_translator.py
index 4127eaa2..99c87dad 100644
--- a/tests/mpd/test_translator.py
+++ b/tests/mpd/test_translator.py
@@ -103,20 +103,19 @@ class TrackMpdFormatTest(unittest.TestCase):
         result = translator.track_to_mpd_format(track)
         self.assertIn(('MUSICBRAINZ_ARTISTID', 'foo'), result)
 
-    def test_concatenate_multiple_values(self):
+    def test_concat_multi_values(self):
         artists = [Artist(name='ABBA'), Artist(name='Beatles')]
-        translated = translator.concatenate_multiple_values(artists, 'name')
+        translated = translator.concat_multi_values(artists, 'name')
         self.assertEqual(translated, 'ABBA;Beatles')
 
-    def test_concatenate_muultiple_values_artist_with_no_name(self):
+    def test_concat_multi_values_artist_with_no_name(self):
         artists = [Artist(name=None)]
-        translated = translator.concatenate_multiple_values(artists, 'name')
+        translated = translator.concat_multi_values(artists, 'name')
         self.assertEqual(translated, '')
 
-    def test_concatenate_muultiple_values_artist_with_no_musicbrainz_id(self):
-        artists = [Artist(name="Jah Wobble")]
-        translated = translator.concatenate_multiple_values(
-            artists, 'musicbrainz_id')
+    def test_concat_multi_values_artist_with_no_musicbrainz_id(self):
+        artists = [Artist(name='Jah Wobble')]
+        translated = translator.concat_multi_values(artists, 'musicbrainz_id')
         self.assertEqual(translated, '')