<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/106333/">http://git.reviewboard.kde.org/r/106333/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 6th, 2012, 12:47 p.m., <b>David Faure</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/106333/diff/1/?file=83835#file83835line718" style="color: black; font-weight: bold; text-decoration: underline;">dolphin/src/dolphinpart.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="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">DolphinPartListingNotificationExtension::DolphinPartListingNotificationExtension(DolphinPart* part)</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">718</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">KFileItemModel</span><span class="o">*</span> <span class="n">model</span> <span class="o">=</span> <span class="p">(</span><span class="n">view</span> <span class="o">?</span> <span class="n">qFindChild</span><span class="o"><</span><span class="n">KFileItemModel</span><span class="o">*></span><span class="p">(</span><span class="n">view</span><span class="p">)</span> <span class="o">:</span> <span class="mi">0</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;">IMHO this is worse than a DolphinView::fileItemModel() method, because it will break at runtime rather than at compile time.</pre>
</blockquote>
<p>On September 6th, 2012, 1:21 p.m., <b>Dawit Alemayehu</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;">Well, I doubt the Dolphin guys would allow the addition of such accessor function to the public API of DolphinView. The reason why I am taking this approach in the first place is because they did not like the idea of having new signals added to KFileItemModel and DolphinView. See the discussion @ https://git.reviewboard.kde.org/r/106289/.</pre>
</blockquote>
<p>On September 6th, 2012, 1:38 p.m., <b>David Faure</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 know, and I'm telling them (and you) that either they'll have to accept an accessor (or signals), or you'll have to find another solution. A dynamic object retrieval is a *worse* solution than an accessor, I'm completely against it, it just makes things more fragile.
The equivalent, less fragile, solution would be a private accessor and making DolphinPart a friend. This way it's not public API that will get used elsewhere, but at least the compiler will tell us when it breaks again.</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 against signals and public accessors, but the 'friend' solution looks OK. A private accessor is not really needed because DolphinPart can just access m_view->m_model directly then.
Please add a comment that explains why it needs to be a friend (for examples, do 'grep "friend class" * -r' in Dolphin's source).</pre>
<br />
<p>- Frank</p>
<br />
<p>On September 5th, 2012, 4:33 p.m., Dawit Alemayehu 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 Dolphin and KDE Base Apps.</div>
<div>By Dawit Alemayehu.</div>
<p style="color: grey;"><i>Updated Sept. 5, 2012, 4:33 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;">The following patch implements the new KParts listing notification extension in Dolphin. This implementation does not add new signals to KFileItemModel and DolphinView classes as the previous solution. Instead it relies on the QObject's introspection to look for KDirLister and connect to the desired signals.
This solution is based on the "hack" that was originally implemented in the directory listern plugin. However, unlike that implementation, this one will not suffer from sudden and unexpected changes in Dolphin's code changes because it is closer to the source and it looks for the directory lister directly in KFileItemModel. This is something that cannot be done at the plugin level because it would require the plugin to link against dolphin.</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>dolphin/src/dolphinpart.h <span style="color: grey">(f9c0bbf)</span></li>
<li>dolphin/src/dolphinpart.cpp <span style="color: grey">(bf3d2a5)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/106333/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>