----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107473/#review29971 -----------------------------------------------------------
Hmmm, I'm apparently able to update the diff, please disregard the attached file. Please have a look a differences between v11 and v12, most of them are style fixes or cleanups, but it also boasts fixes for a couple of subtle bugs (like mismatched calls to XSPFPlaylist constructor). The patch v12 works rather well here, just one minor problem I've faced - if I drag a playlist from File Browser to play queue (the "main" playlist), the track order is not preserved. Could you please have a look at it? Perhaps it is better to query all tracks at once after trackLoaded() is called instead of using trackAdded() method. - Matěj Laitl On March 27, 2013, 7:35 p.m., Tatjana Gornak wrote: > > ----------------------------------------------------------- > 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. > > > 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 > ----- > > 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 > ---------------- > > 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 > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
