Review Request 107473: Changes in processing playlist files

[Matěj Laitl]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107473/
-----------------------------------------------------------

(Updated March 27, 2013, 7:35 p.m.)


Review request for Amarok.


Changes
-------

Added patch to apply on top of v11 (which should be itself rebased on top of current master first) since I cannot apparently update the diff directly.

Tatjana, please post the above as v12, I'll comment then.


Description
-------


I've started my changes with an implementation of a ASXPlaylist class,
due this work I've found that implementation of different playlist file
types has different logic, as result I end up with a rewriting
playlist's implementations.

As example of difference:
  -- Constructor M3UPlaylist::M3UPlaylist( const KUrl &url ) sets a url, but
     does not load playlist, therefore loading of playlist are
posponed till data from playlist are actualy needed (lazy loading)
     On the other hand constructor XSPFPlaylist::XSPFPlaylist( const
KUrl &url, bool autoAppend )  loads playlist.

The main idea of proposed changes is to create a unify code for
processing playlist files:
  -- lazy loading through LazyLoadPlaylist
  -- common functionality was moved to PlaylistFile.

It is worth to be noticed that this patch changes structure of
playlist's classes, bit does not changes their behavior, even if this behavior
is inconsistent in some cases.

In following patch-requests I plan to submit ASX Playlist parser and
organize playlists processing in more consistent way.


This addresses bug 291934.
    https://bugs.kde.org/show_bug.cgi?id=291934


Diffs (updated)
-----

  src/browsers/playlistbrowser/PlaylistBrowserModel.cpp 213af3b 
  src/CMakeLists.txt 4cdee5e 
  src/DirectoryLoader.h e53a30d 
  src/DirectoryLoader.cpp bd7fa34 
  src/browsers/playlistbrowser/PlaylistBrowserModel.h 1b7a44d 
  src/context/applets/info/InfoApplet.cpp f215885 
  src/core-impl/collections/ipodcollection/IpodPlaylistProvider.h 4dd79f3 
  src/core-impl/collections/umscollection/podcasts/UmsPodcastProvider.cpp 1a4a934 
  src/core-impl/playlists/types/file/PlaylistFile.h 43dfdeb 
  src/core-impl/playlists/types/file/PlaylistFile.cpp 2730354 
  src/core-impl/playlists/types/file/PlaylistFileLoaderJob.h PRE-CREATION 
  src/core-impl/playlists/types/file/PlaylistFileLoaderJob.cpp PRE-CREATION 
  src/core-impl/playlists/types/file/PlaylistFileSupport.h b1a1c26 
  src/core-impl/playlists/types/file/PlaylistFileSupport.cpp 2dcc0cd 
  src/core-impl/playlists/types/file/m3u/M3UPlaylist.h 7b982fe 
  src/core-impl/playlists/types/file/m3u/M3UPlaylist.cpp 2395260 
  src/core-impl/playlists/types/file/pls/PLSPlaylist.h e69e133 
  src/core-impl/playlists/types/file/pls/PLSPlaylist.cpp 8fe4b51 
  src/core-impl/playlists/types/file/xspf/XSPFPlaylist.h 6cf33e9 
  src/core-impl/playlists/types/file/xspf/XSPFPlaylist.cpp bbcecc3 
  src/core-impl/podcasts/sql/SqlPodcastMeta.cpp 1a025ce 
  src/core-impl/podcasts/sql/SqlPodcastProvider.cpp 0da9b51 
  src/core/playlists/Playlist.h 018d2af 
  src/core/playlists/Playlist.cpp 36879a1 
  src/core/playlists/PlaylistProvider.h 0fb5818 
  src/playlist/PlaylistActions.cpp 00bf13a 
  src/playlist/PlaylistController.cpp e5bde8b 
  src/playlist/PlaylistRestorer.h PRE-CREATION 
  src/playlist/PlaylistRestorer.cpp PRE-CREATION 
  src/playlistmanager/PlaylistManager.h cbf65b0 
  src/playlistmanager/PlaylistManager.cpp 89c754b 
  src/playlistmanager/SyncedPlaylist.h fce3d14 
  src/playlistmanager/SyncedPlaylist.cpp 4f7c0b7 
  src/playlistmanager/file/PlaylistFileProvider.cpp e4e7284 
  src/playlistmanager/sql/SqlPlaylist.cpp 88e685b 
  src/playlistmanager/sql/SqlUserPlaylistProvider.cpp 5eb6987 
  src/services/gpodder/GpodderProvider.h 8e856e3 
  tests/TestDirectoryLoader.h 0583a9e 
  tests/TestDirectoryLoader.cpp b98247d 
  tests/core-impl/meta/multi/TestMetaMultiTrack.h 8379bf4 
  tests/core-impl/meta/multi/TestMetaMultiTrack.cpp 8e9e47d 
  tests/core-impl/playlists/types/file/m3u/TestM3UPlaylist.cpp b25d059 
  tests/core-impl/playlists/types/file/pls/TestPLSPlaylist.h 1183599 
  tests/core-impl/playlists/types/file/pls/TestPLSPlaylist.cpp 5eab2f3 
  tests/core-impl/playlists/types/file/xspf/TestXSPFPlaylist.cpp 5ea3a41 
  tests/core/playlists/CMakeLists.txt e2e0ce4 
  tests/core/playlists/TestPlaylistObserver.h PRE-CREATION 
  tests/core/playlists/TestPlaylistObserver.cpp PRE-CREATION 
  tests/playlistmanager/file/TestPlaylistFileProvider.cpp 7d0971a 

Diff: http://git.reviewboard.kde.org/r/107473/diff/


Testing
-------

1) All unit-tests were passed.
2) For all playlists I've also checked loading and
   saving through gui.


File Attachments (updated)
----------------

Cleanups and fixes; please rebase v11 on top of current master and apply this
  http://git.reviewboard.kde.org/media/uploaded/files/2013/03/27/0001-Cleanups-and-fixes-for-Tatjana-s-changes_1.patch


Thanks,

Tatjana Gornak

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20130327/2e9f7b1e/attachment-0001.html>