<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/111655/">http://git.reviewboard.kde.org/r/111655/</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 23rd, 2013, 7:40 p.m. UTC, <b>Matěj Laitl</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/111655/diff/1-2/?file=173012#file173012line218" style="color: black; font-weight: bold; text-decoration: underline;">src/context/widgets/RecentlyPlayedListWidget.cpp</a>
<span style="font-weight: normal;">
(Diff revisions 1 - 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">RecentlyPlayedListWidget::trackChanged( const Meta::TrackPtr &track )</pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">RecentlyPlayedListWidget::trackChanged( Meta::TrackPtr track )</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">200</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">RecentlyPlayedListWidget</span><span class="o">::</span><span class="n">trackChanged</span><span class="p">(</span> <span class="k"><span class="hl">const</span></span><span class="hl"> </span><span class="n">Meta</span><span class="o">::</span><span class="n">TrackPtr</span> <span class="o"><span class="hl">&</span></span><span class="n">track</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">215</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">RecentlyPlayedListWidget</span><span class="o">::</span><span class="n">trackChanged</span><span class="p">(</span> <span class="n">Meta</span><span class="o">::</span><span class="n">TrackPtr</span> <span class="n">track</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;">Hmm, const-ref -> by-value change doesn't seem really needed, but this is sooo unimportant, feel free to leave it as it it.</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;">You're very much right here. I misremembered something from GoingNative 2012 conference about danger of passing shared pointers by const reference. I'm going to change it back, as it also gets rid of the .moc file include.</pre>
<br />
<p>- Konrad</p>
<br />
<p>On July 23rd, 2013, 7:21 p.m. UTC, Konrad Zemek 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 Konrad Zemek.</div>
<p style="color: grey;"><i>Updated July 23, 2013, 7:21 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;">Reimplement RecentlyPlayedListWidget
Also fix an issue where certain overloads of
Playlist::Controller::insertOptioned couldn't automatically start
playback when given StartPlay option.
Rewrote RecentlyPlayedListWidget from the basics. There was a major
inconsistency in this widget, where tracks were added to it if they were
recently played at all, but the time shown was the "Last Played"
statistics which is only updated after whole song is played. This
caused gravely incorrect data to be displayed (see bug 302485).
There were two different methods of solving the inconsistency: focusing
on the "Last Played" time, and adding songs to the widget only if the
"Last Played" changed; and focusing on recent plays completely
disregarding "Last Played". I opted for the latter as I felt it fits the
widget's nature better.
Because we can't rely on already available data, the widget needs to do
its own housekeeping. It saves its data on shutdown, to be restored
on next startup. One other major benefit of this approach is that
widget's data remains correct even if collections change, while
previously tracks from removed collections would disappear.
Finally, I added the feature to add tracks to playlist directly from the
widget, provided that the track exists. Adding to playlist works
consistently with other parts of Amarok, with standard double-click and
middle-click behavior.
BUG: 302485
FEATURE: 296090
FEATURE: 279263
REVIEW: 111655</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;">Manual.</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/context/widgets/RecentlyPlayedListWidget.h <span style="color: grey">(caa78039b891511ca36ada8f61cd4f776d1f3c6d)</span></li>
<li>src/context/widgets/RecentlyPlayedListWidget.cpp <span style="color: grey">(6ea501affcf6e61d237002e15f7ed4e26989b91b)</span></li>
<li>src/playlist/PlaylistController.cpp <span style="color: grey">(60c99e5a10459b84b9839f07aad0d1e2bfa5d259)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/111655/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>