<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/100838/">http://git.reviewboard.kde.org/r/100838/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 25th, 2011, 1:30 a.m., <b>David Edmundson</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/100838/diff/4/?file=12746#file12746line377" style="color: black; font-weight: bold; text-decoration: underline;">main-widget.cpp</a>
<span style="font-weight: normal;">
(Diff revision 4)
</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; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">239</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">m_modelFilter</span><span class="o">-></span><span class="n">refilter</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 you need to manually invalidate your filter you have something wrong with either your model or the filter code.</pre>
</blockquote>
<p>On March 25th, 2011, 11:06 a.m., <b>Martin Klapetek</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;">This is the case I described above - this is a slot called when the account connection state changes. When it gets disconnected, the filtering needs to be invalidated to 'refilter' the items, therefore this is called here. See the docs for QSortFilterProxyModel::invalidateFilter() - "This function should be called if you are implementing custom filtering (e.g. filterAcceptsRow()), and your filter parameters have changed." - This is exactly the case.</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'm not convinced. You haven't changed the filter - you're still filtering out offline accounts, therefore the filter parameters haven't changed.
When the model emits dataChanged() it should automatically call filterAcceptsRow();
If you need something outside the model to monitor for changes and poke the filter, it seems to defeat the point of it.
I'll try taking a look. If I can make it work without it, then we remove it. Otherwise it clearly needs to stay.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 25th, 2011, 1:30 a.m., <b>David Edmundson</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/100838/diff/4/?file=12746#file12746line379" style="color: black; font-weight: bold; text-decoration: underline;">main-widget.cpp</a>
<span style="font-weight: normal;">
(Diff revision 4)
</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; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">241</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c1">//Fall through</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;">I think the comment "fall Through" is in the wrong place.
I initially read this as expecting ConnectionStatusDisconnected to fall through, which clearly it doesn't.</pre>
</blockquote>
<p>On March 25th, 2011, 11:09 a.m., <b>Martin Klapetek</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;">So it should be one line down? I'm used to write comments above the lines, so that's why it might be off.</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;">It could just be a personal style thing.
I'm used to writing
switch(foo) {
case A:
break;
case B: /*Fall Through*/
case C:
break;
}
so it's clear that B is meant to fall through and doesn't accidentally having a missing break.
I'm not too fussed</pre>
<br />
<p>- David</p>
<br />
<p>On March 23rd, 2011, 9:32 p.m., Martin Klapetek 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 Telepathy.</div>
<div>By Martin Klapetek.</div>
<p style="color: grey;"><i>Updated March 23, 2011, 9:32 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;">This is the work that begun in my clone repo and has matured enough now to be merged back to 'upstream', where the work should continue now.</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;">We did a lot of thourough testing on #kde-telepathy, also some people emailed their reports which all has been fixed.</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>CMakeLists.txt <span style="color: grey">(2c62ea10556cdba38d1bca0fe63603df97414336)</span></li>
<li>account-button.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>account-button.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>account-filter-model.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>account-filter-model.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>accounts-model-item.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>accounts-model-item.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>accounts-model.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>accounts-model.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>contact-delegate-overlay.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>contact-delegate-overlay.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>contact-delegate.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>contact-delegate.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>contact-model-item.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>contact-model-item.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>contact-overlays.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>contact-overlays.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>contact-view-hover-button.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>contact-view-hover-button.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>filter-bar.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>filter-bar.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>main-widget.h <span style="color: grey">(aed6f7c8336321bc8a6ffb3b9af6b1d493f85790)</span></li>
<li>main-widget.cpp <span style="color: grey">(6146b62eec17b54be63b594200613931673ac5fe)</span></li>
<li>main-widget.ui <span style="color: grey">(5b0d5aaaf3a4e2f49eb030b98b15fcae3a86170c)</span></li>
<li>main.cpp <span style="color: grey">(bba0e4175e8853afe603f26ea4707f4974192d0f)</span></li>
<li>ontologies/CMakeLists.txt <span style="color: grey">(3e0bbe8c634908f689dcd360409aeddcc6fc0d23)</span></li>
<li>tree-node.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>tree-node.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/100838/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>