Fix 'load' so one can append a playlist to the current playlist

docs/changes.rst

@@ -50,6 +50,7 @@ greatly improved MPD client support.
   - Implement ``seek`` and ``seekid``.
   - Fix ``playlistfind`` output so the correct song is played when playing
     songs directly from search results in GMPC.
+  - Fix ``load`` so that one can append a playlist to the current playlist.
 
 - Backends:
 
@@ -73,7 +74,10 @@ greatly improved MPD client support.
   - :meth:`mopidy.backends.base.BaseBackend()` now accepts an
     ``output_queue`` which it can use to send messages (i.e. audio data)
     to the output process.
-
+  - :meth:`mopidy.backends.base.BaseCurrentPlaylistController.load()` now
+    appends to the existing playlist. Use
+    :meth:`mopidy.backends.base.BaseCurrentPlaylistController.clear()` if you
+    want to clear it first.
 
 
 0.1.0a3 (2010-08-03)

mopidy/backends/base/current_playlist.py

@@ -107,16 +107,15 @@ class BaseCurrentPlaylistController(object):
 
     def load(self, tracks):
         """
-        Replace the tracks in the current playlist with the given tracks.
+        Append the given tracks to the current playlist.
 
         :param tracks: tracks to load
         :type tracks: list of :class:`mopidy.models.Track`
         """
-        self._cp_tracks = []
         self.version += 1
         for track in tracks:
             self.add(track)
-        self.backend.playback.new_playlist_loaded_callback()
+        self.backend.playback.on_current_playlist_change()
 
     def move(self, start, end, to_position):
         """

mopidy/backends/base/playback.py

@@ -253,23 +253,21 @@ class BasePlaybackController(object):
             self.stop()
             self.current_cp_track = None
 
-    def new_playlist_loaded_callback(self):
+    def on_current_playlist_change(self):
         """
-        Tell the playback controller that a new playlist has been loaded.
+        Tell the playback controller that the current playlist has changed.
 
-        Typically called by :class:`mopidy.process.CoreProcess` after a message
-        from a library thread is received.
+        Used by :class:`mopidy.backends.base.BaseCurrentPlaylistController`.
         """
-        self.current_cp_track = None
         self._first_shuffle = True
         self._shuffled = []
 
-        if self.state == self.PLAYING:
-            if len(self.backend.current_playlist.tracks) > 0:
-                self.play()
-            else:
-                self.stop()
-        elif self.state == self.PAUSED:
+        if not self.backend.current_playlist.cp_tracks:
+            self.stop()
+            self.current_cp_track = None
+        elif (self.current_cp_track not in
+                self.backend.current_playlist.cp_tracks):
+            self.current_cp_track = None
             self.stop()
 
     def next(self):

mopidy/frontends/mpd/protocol/current_playlist.py

@@ -341,6 +341,7 @@ def swap(frontend, songpos1, songpos2):
     tracks.insert(songpos1, song2)
     del tracks[songpos2]
     tracks.insert(songpos2, song1)
+    frontend.backend.current_playlist.clear()
     frontend.backend.current_playlist.load(tracks)
 
 @handle_pattern(r'^swapid "(?P<cpid1>\d+)" "(?P<cpid2>\d+)"$')

mopidy/frontends/mpd/protocol/stored_playlists.py

@@ -86,6 +86,10 @@ def load(frontend, name):
         ``load {NAME}``
 
         Loads the playlist ``NAME.m3u`` from the playlist directory.
+
+    *Clarifications:*
+
+    - ``load`` appends the given playlist to the current playlist.
     """
     matches = frontend.backend.stored_playlists.search(name)
     if matches:
@@ -139,9 +143,9 @@ def playlistmove(frontend, name, from_pos, to_pos):
 
     *Clarifications:*
 
-        - The second argument is not a ``SONGID`` as used elsewhere in the
-          protocol documentation, but just the ``SONGPOS`` to move *from*,
-          i.e. ``playlistmove {NAME} {FROM_SONGPOS} {TO_SONGPOS}``.
+    - The second argument is not a ``SONGID`` as used elsewhere in the protocol
+      documentation, but just the ``SONGPOS`` to move *from*, i.e.
+      ``playlistmove {NAME} {FROM_SONGPOS} {TO_SONGPOS}``.
     """
     raise MpdNotImplemented # TODO
 

tests/backends/base.py

@@ -91,12 +91,6 @@ class BaseCurrentPlaylistControllerTest(object):
         self.controller.clear()
         self.assertEqual(self.playback.state, self.playback.STOPPED)
 
-    def test_load(self):
-        tracks = []
-        self.assertNotEqual(id(tracks), id(self.controller.tracks))
-        self.controller.load(tracks)
-        self.assertEqual(tracks, self.controller.tracks)
-
     def test_get_by_uri_returns_unique_match(self):
         track = Track(uri='a')
         self.controller.load([Track(uri='z'), track, Track(uri='y')])
@@ -136,10 +130,15 @@ class BaseCurrentPlaylistControllerTest(object):
         self.controller.load([track1, track2, track3])
         self.assertEqual(track2, self.controller.get(uri='b')[1])
 
-    @populate_playlist
-    def test_load_replaces_playlist(self):
-        self.backend.current_playlist.load([])
-        self.assertEqual(len(self.backend.current_playlist.tracks), 0)
+    def test_load_appends_to_the_current_playlist(self):
+        self.controller.load([Track(uri='a'), Track(uri='b')])
+        self.assertEqual(len(self.controller.tracks), 2)
+        self.controller.load([Track(uri='c'), Track(uri='d')])
+        self.assertEqual(len(self.controller.tracks), 4)
+        self.assertEqual(self.controller.tracks[0].uri, 'a')
+        self.assertEqual(self.controller.tracks[1].uri, 'b')
+        self.assertEqual(self.controller.tracks[2].uri, 'c')
+        self.assertEqual(self.controller.tracks[3].uri, 'd')
 
     def test_load_does_not_reset_version(self):
         version = self.controller.version
@@ -148,22 +147,17 @@ class BaseCurrentPlaylistControllerTest(object):
 
     @populate_playlist
     def test_load_preserves_playing_state(self):
-        tracks = self.controller.tracks
-        playback = self.playback
-
         self.playback.play()
-        self.controller.load([tracks[1]])
-        self.assertEqual(playback.state, playback.PLAYING)
-        self.assertEqual(tracks[1], self.playback.current_track)
+        track = self.playback.current_track
+        self.controller.load(self.controller.tracks[1:2])
+        self.assertEqual(self.playback.state, self.playback.PLAYING)
+        self.assertEqual(self.playback.current_track, track)
 
     @populate_playlist
     def test_load_preserves_stopped_state(self):
-        tracks = self.controller.tracks
-        playback = self.playback
-
-        self.controller.load([tracks[2]])
-        self.assertEqual(playback.state, playback.STOPPED)
-        self.assertEqual(None, self.playback.current_track)
+        self.controller.load(self.controller.tracks[1:2])
+        self.assertEqual(self.playback.state, self.playback.STOPPED)
+        self.assertEqual(self.playback.current_track, None)
 
     @populate_playlist
     def test_move_single(self):
@@ -575,15 +569,15 @@ class BasePlaybackControllerTest(object):
         self.playback.end_of_track_callback()
         self.assertEqual(self.playback.current_playlist_position, None)
 
-    def test_new_playlist_loaded_callback_gets_called(self):
-        callback = self.playback.new_playlist_loaded_callback
+    def test_on_current_playlist_change_gets_called(self):
+        callback = self.playback.on_current_playlist_change
 
         def wrapper():
             wrapper.called = True
             return callback()
         wrapper.called = False
 
-        self.playback.new_playlist_loaded_callback = wrapper
+        self.playback.on_current_playlist_change = wrapper
         self.backend.current_playlist.load([])
 
         self.assert_(wrapper.called)
@@ -608,27 +602,28 @@ class BasePlaybackControllerTest(object):
         self.assert_(event.is_set())
 
     @populate_playlist
-    def test_new_playlist_loaded_callback_when_playing(self):
+    def test_on_current_playlist_change_when_playing(self):
         self.playback.play()
+        current_track = self.playback.current_track
         self.backend.current_playlist.load([self.tracks[2]])
         self.assertEqual(self.playback.state, self.playback.PLAYING)
-        self.assertEqual(self.playback.current_track, self.tracks[2])
+        self.assertEqual(self.playback.current_track, current_track)
 
     @populate_playlist
-    def test_new_playlist_loaded_callback_when_stopped(self):
+    def test_on_current_playlist_change_when_stopped(self):
+        current_track = self.playback.current_track
         self.backend.current_playlist.load([self.tracks[2]])
         self.assertEqual(self.playback.state, self.playback.STOPPED)
         self.assertEqual(self.playback.current_track, None)
-        self.assertEqual(self.playback.next_track, self.tracks[2])
 
     @populate_playlist
-    def test_new_playlist_loaded_callback_when_paused(self):
+    def test_on_current_playlist_change_when_paused(self):
         self.playback.play()
         self.playback.pause()
+        current_track = self.playback.current_track
         self.backend.current_playlist.load([self.tracks[2]])
-        self.assertEqual(self.playback.state, self.playback.STOPPED)
-        self.assertEqual(self.playback.current_track, None)
-        self.assertEqual(self.playback.next_track, self.tracks[2])
+        self.assertEqual(self.playback.state, self.backend.playback.PAUSED)
+        self.assertEqual(self.playback.current_track, current_track)
 
     @populate_playlist
     def test_pause_when_stopped(self):
@@ -899,7 +894,7 @@ class BasePlaybackControllerTest(object):
         self.playback.random = True
         self.assertEqual(self.playback.next_track, self.tracks[2])
         self.backend.current_playlist.load(self.tracks[:1])
-        self.assertEqual(self.playback.next_track, self.tracks[0])
+        self.assertEqual(self.playback.next_track, self.tracks[1])
 
     @populate_playlist
     def test_played_track_during_random_not_played_again(self):