<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/111804/">http://git.reviewboard.kde.org/r/111804/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 30th, 2013, 10:04 p.m. UTC, <b>Edward Hades Toroshchin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/111804/diff/1/?file=175527#file175527line73" style="color: black; font-weight: bold; text-decoration: underline;">src/playlist/PlaylistSortWidget.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">SortWidget::SortWidget( QWidget *parent )</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">73</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span><span class="p">(</span> <span class="n">levelParts</span><span class="p">.</span><span class="n"><span class="hl">at</span></span><span class="p">(</span> <span class="mi">1</span> <span class="p">)</span> <span class="o">==</span> <span class="n">QString</span><span class="p">(</span> <span class="s">"asc"</span> <span class="p">)</span> <span class="p">)</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">73</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span><span class="p">(</span> <span class="n">levelParts</span><span class="p">.</span><span class="n"><span class="hl">value</span></span><span class="p">(</span> <span class="mi">1</span> <span class="p">)</span> <span class="o">==</span> <span class="n">QString</span><span class="p">(</span> <span class="s">"asc"</span> <span class="p">)</span> <span class="p">)</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.)</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I think that we can safely file tokens without either "asc" or "des" as ill-formed, as with removal of the "Shuffle" special case everything is either one or the other.</pre>
<br />
<p>- Konrad</p>
<br />
<p>On July 30th, 2013, 8:16 p.m. UTC, Fabian Kosmale wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Amarok.</div>
<div>By Fabian Kosmale.</div>
<p style="color: grey;"><i>Updated July 30, 2013, 8:16 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Without this patch, Amarok would crash with
ASSERT failure in QList<T>::at: "index out of range"
when amarokrc contained
[Playlist Sorting]
SortPath=Shuffle
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Compiled and started Amarok. It doesn't crash anymore.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/playlist/PlaylistSortWidget.cpp <span style="color: grey">(384e4a0eaa28f82eb27665a1a0d497e018a61161)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/111804/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>