Review Request 110070: Cleaned up playlist multilevel sorting algorithm. Added album artist special case to playlist sorting.

[Konrad Zemek]

> On April 19, 2013, 11:28 a.m., Matěj Laitl wrote:
> > src/playlist/proxymodels/SortScheme.h, line 66
> > <http://git.reviewboard.kde.org/r/110070/diff/4/?file=139677#file139677line66>
> >
> >     I think some compilers choke on "implicit template argument" like this, safer way would be: typedef QStack<SortLevel>::const_iterator ...; see http://quickgit.kde.org/?p=amarok.git&a=commitdiff&h=1140d3056af65ddf0f3999df846f1fd44970c303
> >     
> >     OTOH, this typedef seems to be needed just to make declarations of below methods shorter, right? In that case it may not be worth it.

Actually, this typedef is there to bring const_iterator to the public interface of SortScheme; since SortScheme privately inherits from QStack, QStack::const_iterator is by default inaccessible from the outside.


- Konrad


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


On April 19, 2013, 12:08 a.m., Konrad Zemek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110070/
> -----------------------------------------------------------
> 
> (Updated April 19, 2013, 12:08 a.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> -------
> 
> Cleaned up playlist multilevel sorting algorithm. Added album artist special case to playlist sorting.
> 
> It fixes an issue with two albums being merged on the playlist when they happen to have the same name, and sorting by album is enabled.
> Originally thought to fix bug 271105, it actually fixes some other, but connected, one (see my comment https://bugs.kde.org/show_bug.cgi?id=271105#c10 ).
> I'll search for a bug report and submit one if I find none.
> 
> 
> This addresses bug 271105.
>     https://bugs.kde.org/show_bug.cgi?id=271105
> 
> 
> Diffs
> -----
> 
>   src/playlist/proxymodels/SortAlgorithms.h da18fcc2107d83515ff176746e751fe00e786362 
>   src/playlist/proxymodels/SortAlgorithms.cpp c7d0df9f6b86318fd896a6de08fef6d85623215b 
>   src/playlist/proxymodels/SortScheme.h 0fcd3a580deff7f2b4e6e65d0551c7332e0b6c9e 
>   src/playlist/proxymodels/SortScheme.cpp de4f37adaeb6e8bb914eb7c91ff4803d01fc5623 
>   tests/mocks/MetaMock.h 03fea2aaf7f27236c43a8cb78c241078414dd39c 
>   tests/playlist/TestPlaylistModels.cpp 8964333517e431487b6ad4ba50d3e359f51bf3f6 
> 
> Diff: http://git.reviewboard.kde.org/r/110070/diff/
> 
> 
> Testing
> -------
> 
> Manual tests + relevant unittest added in testplaylistmodels
> 
> 
> Thanks,
> 
> Konrad Zemek
> 
>

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