Review Request 111804: Fix a crash on startup

[Edward Hades Toroshchin]

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


Thank you very much for the patch.

I understand the problem, however the solution could be improved a bit.

Please see the code comments, address them, and we'll push your patch.


src/playlist/PlaylistSortWidget.cpp
<http://git.reviewboard.kde.org/r/111804/#comment27177>

    If the sort token has neither "asc" nor "des" suffix, it won't be added to the sort at all.
    
    I suggest you make the line 73 something like that:
    
    if( levelParts.count() < 2 || levelParts.at( 1 ) == QString( "asc" ) )
    
    This way it will assume ascending order by default, if no specifier is present.
    
    Please check, if this indeed compiles and works. Please also check that the "Shuffle" playlist sort works after Amarok restart. (I believe it isn't in the current solution.)


- Edward Hades Toroshchin


On July 30, 2013, 8:16 p.m., Fabian Kosmale wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111804/
> -----------------------------------------------------------
> 
> (Updated July 30, 2013, 8:16 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> -------
> 
> Without this patch, Amarok would crash with 
> ASSERT failure in QList<T>::at: "index out of range"
> when amarokrc contained
> [Playlist Sorting]
> SortPath=Shuffle
> 
> 
> Diffs
> -----
> 
>   src/playlist/PlaylistSortWidget.cpp 384e4a0eaa28f82eb27665a1a0d497e018a61161 
> 
> Diff: http://git.reviewboard.kde.org/r/111804/diff/
> 
> 
> Testing
> -------
> 
> Compiled and started Amarok. It doesn't crash anymore.
> 
> 
> Thanks,
> 
> Fabian Kosmale
> 
>

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