Review Request 107473: Changes in processing playlist files

[Tatjana Gornak]

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

(Updated Feb. 3, 2013, 6:39 p.m.)


Review request for Amarok.


Changes
-------

Refactoring playlists.

This patch also includes:
1) Bug in loading default playlist was fixed.
2) Logic for restoreDefaultPlaylist was rewritten.

Testing:
1) First start up loads default tune.
2) Default playlist is restored successfully.

Remark:
I do not know how to generate playlist which can contain other playlists,
so I was unable to test this situation.


Description
-------

IMPORTANT: I have found bug in last implementation. Please postpone review until I fix it. 


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/core-impl/playlists/types/file/PlaylistFile.h 4322da9 
  src/core-impl/playlists/types/file/PlaylistFile.cpp 2730354 
  src/core-impl/playlists/types/file/PlaylistFileSupport.cpp 2dcc0cd 
  src/core-impl/playlists/types/file/m3u/M3UPlaylist.h 3176c45 
  src/core-impl/playlists/types/file/m3u/M3UPlaylist.cpp 2395260 
  src/core-impl/playlists/types/file/pls/PLSPlaylist.h e974539 
  src/core-impl/playlists/types/file/pls/PLSPlaylist.cpp 3ba154a 
  src/core-impl/playlists/types/file/xspf/XSPFPlaylist.h ebf3235 
  src/core-impl/playlists/types/file/xspf/XSPFPlaylist.cpp bbcecc3 
  src/core/playlists/Playlist.h 8fd1ffb 
  src/playlist/PlaylistActions.h d57a5c9 
  src/playlist/PlaylistActions.cpp 13810cd 
  src/playlistmanager/PlaylistManager.h cbf65b0 
  src/playlistmanager/PlaylistManager.cpp 89c754b 
  src/playlistmanager/file/PlaylistFileProvider.cpp e4e7284 
  tests/core-impl/playlists/types/file/CMakeLists.txt ef69236 
  tests/core-impl/playlists/types/file/TestPlaylistObserver.h PRE-CREATION 
  tests/core-impl/playlists/types/file/TestPlaylistObserver.cpp PRE-CREATION 
  tests/core-impl/playlists/types/file/m3u/TestM3UPlaylist.cpp 0f19e1b 
  tests/core-impl/playlists/types/file/xspf/TestXSPFPlaylist.cpp f4643ca 

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.


Thanks,

Tatjana Gornak

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20130203/e22fe8fc/attachment.html>