<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/100003/">http://git.reviewboard.kde.org/r/100003/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Looks good to me, though I am not an expert in this part of Amarok. Still, you can commit for more testing and feedback, 2.4 is a while away :)</pre>
<br />
<p>- Leo</p>
<br />
<p>On September 27th, 2010, 11:30 a.m., Nikhil Shantanu Marathe wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for amarok.</div>
<div>By Nikhil Shantanu Marathe.</div>
<p style="color: grey;"><i>Updated 2010-09-27 11:30:13</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;">Just another heads up of something I sent earlier but didn't get replies to. Markey suggested this might be a good place.
This is a fix regarding the exact conditions under which a
setRequiresUpdate(false) is done on a CollectionTreeItem, the parent
of a "Various Artists" item.
Consider the case when there are no entries tagged "Various Artists"
returned by a QueryMaker. In that case, the
CollectionTreeItemModelBase::handleSpecialQueryResult() function
receives a dataList of size == 0. If dataList is empty() we remove the
special node from the tree (or it may not exist previously). This
means we are no longer looking for a various artists type result.
Now consider the middle of :handleSpecialQueryResult
//populate children will call setRequiresUpdate on vaNode
//but as the special query is based on specialNode's parent,
//we have to call setRequiresUpdate on the parent too
//yes, this will mean we will call setRequiresUpdate twice
parent->setRequiresUpdate( false );
where we have actually FOUND entries, and so we stop trying to update
the parent node again.
My question is, shouldn't this be the case when dataList is empty too?
Which means shouldn't we call parent->setRequiresUpdate( false ); even
when we get empty results. The attached patch fixes what I think is a bug.</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;">I use this fix all the time in my local build and it doesn't seem to cause problems, and it definitely prevents my upnp-collection from being polled again and again because it doesn't return any Various Artists entries.</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/browsers/CollectionTreeItemModelBase.cpp <span style="color: grey">(c25549b)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/100003/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>