> On March 28, 2013, 12:16 a.m., Matěj Laitl wrote:
> > 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.
>
> Tatjana Gornak wrote:
> Yes, sure I'll try to fix it.
>
> But there is a problem with v12. When I tried to build it with
> -DKDE4_BUILD_TESTS=ON, build failed because of undefined reference to symbol
> 'ThreadWeaver::Weaver::instance()' in testmetamultitrack. It happened because
> ${KDE4_THREADWEAVER_LIBRARIES} was not added as link library for
> testmetamultitrack.
>
> Matěj Laitl wrote:
> Oh, please fix that link error. It is strange but known that something on
> my box links ThreadWeaver even though I don't explicitly mention it, which
> then causes build failures on other boxes.
I want to ask for an advice about sorting. The current DirectoryLoader behaves
like this: if you provide it with a playlist it just loads playlist without
sorting its content, if you provide it with a directory, then it loads
directory and sorts content of directory, so if there was a playlist its
content will appear sorted.
New DirectoryLoader sorts content in all cases, that as you pointed is wrong in
case if just one playlist was loaded.
But maybe it is better to preserve order of tracks in playlist in all cases?
- Tatjana
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107473/#review29971
-----------------------------------------------------------
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
