<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/110362/">http://git.reviewboard.kde.org/r/110362/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 9th, 2013, 6:11 p.m. UTC, <b>Mark Kretschmann</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Sorry, what does this patch have to do with the bug report? It doesn't fix the bug at all.
The command line argument issues are just a symptom of the underlying bug, see my comments in the report. I'm debugging this currently.</pre>
</blockquote>
<p>On May 9th, 2013, 7:14 p.m. UTC, <b>Anmol Ahuja</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I'm sorry, it doesn't solve the complete problem.
According to the bug report:
"Expected Results:
1) look for the actual song/url on the current playlist, or appending that song to the playlist if not found; and
2) queue that song"
So I added a -unique argument, though it doesn't fix the broken append and play.</pre>
</blockquote>
<p>On May 9th, 2013, 10:18 p.m. UTC, <b>Matěj Laitl</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">> Sorry, what does this patch have to do with the bug report? It doesn't fix the bug at all.
I don't agree with this, please read the original description of the bug again, Mark. This patch indeed could be the solution, however perhaps we just want a different one. The problem described is that --queue just inserts the track after the currently playing one, while it should use the Queue feature of our playlist instead. Anmol, feel free to update the patch to solve this point.
The reporter also wanted that --queue wouldn't re-add (but just move) a track if it is already in the playlist - I don't think we want to implement this - it looks like the reporter wants to do fancy things with the command line where DBus interface or Scripting API should be used instead.</pre>
</blockquote>
<p>On May 9th, 2013, 10:20 p.m. UTC, <b>Matěj Laitl</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I don't see "Reopen for review" button, Marok of Anmol, please hit it, this review shouldn't have been marked as discarded.</pre>
</blockquote>
<p>On May 12th, 2013, 1:24 p.m. UTC, <b>Anmol Ahuja</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Okay.(Apparently, you're not notified of new comments on discarded requests.)
And should I keep the unique argument? There was already some unused code and an enum for it.
</pre>
</blockquote>
<p>On May 12th, 2013, 3:28 p.m. UTC, <b>Anmol Ahuja</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Mat?j: ^</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hmmm, I'd like it more if not, --unique looks as a too complicated operation for a command line argument.</pre>
<br />
<p>- Matěj</p>
<br />
<p>On May 12th, 2013, 1:24 p.m. UTC, Anmol Ahuja 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 Anmol Ahuja.</div>
<p style="color: grey;"><i>Updated May 12, 2013, 1:24 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;">Added -unique argument to Amarok command line options.
Bug Report: https://bugs.kde.org/show_bug.cgi?id=317385</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=317385">317385</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/App.h <span style="color: grey">(97dfdf2)</span></li>
<li>src/App.cpp <span style="color: grey">(858d76d)</span></li>
<li>src/playlist/PlaylistActions.h <span style="color: grey">(75b71fd)</span></li>
<li>src/playlist/PlaylistActions.cpp <span style="color: grey">(b36794d)</span></li>
<li>src/playlist/PlaylistModel.cpp <span style="color: grey">(c23de72)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/110362/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>