<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/105586/">http://git.reviewboard.kde.org/r/105586/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Adding global search is awesome \o/. Well done, it's much needed.
Unfortunately I have some gripes about how you're breaking model separation.
I think I would:
- not touch entity-model
- have a QList<Tp::SearchHit> in the LogViewer class.
- in LogViewer::globalSearchFinished update the LogViewer search hits. This updates the entityFilter just like you wrote it.
- in LogViewer::onEntitySelected() if m_searchHits.size().. search for the appropriate SearchHit in m_searchHits, then set the datepicker dates here.
To make the API simpler, you could even consider passing the SearchHits directly to the EntityFilterModel and the DatePicker and it's up to these classes to search through them. As a QList is implicitly shared so it's arguably faster than copying into a different list. So it becomes up to the DatePicker::setEntity() to say "do I have any search hits? I'll go look through them and choose the right dates. If not I'll start my pendingDates.". </pre>
<br />
<div>
<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/105586/diff/1/?file=72994#file72994line122" style="color: black; font-weight: bold; text-decoration: underline;">logviewer/entity-model-item.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; ">void EntityModelItem::setData(const QVariant &data, int role)</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">122</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">m_filterDates</span> <span class="o">=</span> <span class="n">data</span><span class="p">.</span><span class="n">value</span><span class="o"><</span> <span class="n">QList</span><span class="o"><</span><span class="n">QDate</span><span class="o">></span> <span class="o">></span><span class="p">();</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Storing the list of dates that match a keyword unrelated to this model in this model makes no sense to me.
It's completely destroyed any data-view separation.</pre>
</div>
<br />
<div>
<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/105586/diff/1/?file=72995#file72995line78" style="color: black; font-weight: bold; text-decoration: underline;">logviewer/entity-model.h</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; ">class EntityModel : public QAbstractItemModel</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">78</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">EntityModelItem</span> <span class="o">*</span><span class="n">itemForEntity</span><span class="p">(</span><span class="k">const</span> <span class="n">Tp</span><span class="o">::</span><span class="n">AccountPtr</span> <span class="o">&</span><span class="n">account</span><span class="p">,</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I'd rather we kept EntityModelItem prviate.
We did this sort of thing in one of the other KTp models, and it's led to all sorts of problems.</pre>
</div>
<br />
<div>
<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/105586/diff/1/?file=73000#file73000line118" style="color: black; font-weight: bold; text-decoration: underline;">logviewer/log-viewer.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; ">void LogViewer::onEntitySelected(const QModelIndex ¤t, const QModelIndex &previous)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">108</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">ui</span><span class="o">-></span><span class="n">datePicker</span><span class="o">-></span><span class="n">setEntity</span><span class="p">(</span><span class="n">account</span><span class="p">,</span> <span class="n">entity</span><span class="p">);</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">118</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">ui</span><span class="o">-></span><span class="n">datePicker</span><span class="o">-></span><span class="n">setEntity</span><span class="p">(</span><span class="n">account</span><span class="p">,</span> <span class="n">entity</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">when you call setEntity() on the datePicker it starts a query for all dates for this contact..destroying what you pass it.</pre>
</div>
<br />
<p>- David</p>
<br />
<p>On July 15th, 2012, 9:50 p.m., Dan Vratil wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/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 Dan Vratil.</div>
<p style="color: grey;"><i>Updated July 15, 2012, 9:50 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 a KLineEdit to the bottom part of the window. Hitting enter starts global search for given term. Only entities with at least one matching log are displayed. Only dates with matching logs are displayed. Matching terms are highlighted in the MessageView. I am cool.</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="http://bugs.kde.org/show_bug.cgi?id=294652">294652</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>logviewer/conversation-date-picker.h <span style="color: grey">(868eb43)</span></li>
<li>logviewer/conversation-date-picker.cpp <span style="color: grey">(3817083)</span></li>
<li>logviewer/entity-model-item.h <span style="color: grey">(e740d25)</span></li>
<li>logviewer/entity-model-item.cpp <span style="color: grey">(c7f2249)</span></li>
<li>logviewer/entity-model.h <span style="color: grey">(205c132)</span></li>
<li>logviewer/entity-model.cpp <span style="color: grey">(cbee00a)</span></li>
<li>logviewer/entity-proxy-model.h <span style="color: grey">(0548081)</span></li>
<li>logviewer/entity-proxy-model.cpp <span style="color: grey">(ac1bbf5)</span></li>
<li>logviewer/log-viewer.h <span style="color: grey">(2b13bc1)</span></li>
<li>logviewer/log-viewer.cpp <span style="color: grey">(04fba29)</span></li>
<li>logviewer/log-viewer.ui <span style="color: grey">(7e4097f)</span></li>
<li>logviewer/message-view.h <span style="color: grey">(9e01260)</span></li>
<li>logviewer/message-view.cpp <span style="color: grey">(1f51a83)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105586/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>