<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/100874/">http://git.reviewboard.kde.org/r/100874/</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;">seems to be very good work, please update the kdevelop patch and I'll do some actual testing instead of just looking at the code.
but I'll probably give my OK if I don't spot anything important then. Below are some small nitpicks, would be cool if you could attend to those.
ps: thanks for not just writing new code but also documenting it *and* writing unit tests, very good!</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/100874/diff/1/?file=11465#file11465line57" style="color: black; font-weight: bold; text-decoration: underline;">interfaces/ibuddydocumentfinder.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="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; ">same IBuddyDocumentFinder for several mimetypes.</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">57</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm">Idem, in your destructor, you'll call</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;">idem? what does that mean?</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/100874/diff/1/?file=11465#file11465line73" style="color: black; font-weight: bold; text-decoration: underline;">interfaces/ibuddydocumentfinder.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="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; ">same IBuddyDocumentFinder for several mimetypes.</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">73</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="cm">/** @return true, if the two documents are buddies.</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;">could you unify the indentation in the doxygen comments, make them all look like the one for e.g. addFinder
thanks</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/100874/diff/1/?file=11467#file11467line107" style="color: black; font-weight: bold; text-decoration: underline;">interfaces/idocumentcontroller.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; ">public Q_SLOTS:</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">107</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> A buddy document can be specified. Typically, the document view will be opened</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;">add a newline before, doxygen is always
brief single line
long description
also please not that the buddy document can be supplied optionally and - if it's 0 - the registered finders will be queried automatically</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/100874/diff/1/?file=11467#file11467line122" style="color: black; font-weight: bold; text-decoration: underline;">interfaces/idocumentcontroller.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; ">public Q_SLOTS:</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="cm"> A buddy document can be specified. Typically, the document view will be opened</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;">same as above</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/100874/diff/1/?file=11469#file11469line388" style="color: black; font-weight: bold; text-decoration: underline;">shell/documentcontroller.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; ">struct DocumentControllerPrivate {</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">388</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span><span class="p">(</span><span class="n">Core</span><span class="o">::</span><span class="n">self</span><span class="p">()</span><span class="o">-></span><span class="n">uiControllerInternal</span><span class="p">()</span><span class="o">-></span><span class="n">arrangeBuddies</span><span class="p">())</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;">what if this setting is false and a buddy was passed to openDocument directly?
I'd assume the config only decides whether to look for buddies automatically. if some plugin passes a buddy manually it should be always honored. or what do you think?</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/100874/diff/1/?file=11471#file11471line167" style="color: black; font-weight: bold; text-decoration: underline;">shell/settings/uiconfig.ui</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; "></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">167</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <string>Arrange buddy documents side by side</string></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;">in a user visible string, I'd prefer "related documents" instead of buddy documents.
the tooltip should also be more descriptive:
"When enabled, plugins can group related files side by side. For example a header file will be opened next to the implementation file."</pre>
</div>
<br />
<p>- Milian</p>
<br />
<p>On March 16th, 2011, 5:58 p.m., Martin Heide 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 KDevelop.</div>
<div>By Martin Heide.</div>
<p style="color: grey;"><i>Updated March 16, 2011, 5:58 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;">!! To apply patch, use (instead of git apply)
patch -p1 < kdevplatform-buddy-1.patch
(bug in reviewboard, sorry) !!
(This review request is based on https://git.reviewboard.kde.org/r/100795/ , to separate cleanup and buddy document features)
Works together with the review request https://git.reviewboard.kde.org/r/100796/ in kdevelop. Apply them both before compiling.
Add document buddy architecture :
Allow kdevelop language plugins to manage related documents (like foo.h and foo.cpp for cpp plugin)
Language plugins may implement IBuddyDocumentFinder to define the rules (how to know if two documents are buddies).
Language plugins that implement IBuddyDocumentFinder must register themselves.
An option was added to the UIConfig to open documents side by side with its buddies.
Buddy documents work like this: Assume that the option is enabled in the config, and there exist the following tabs:
| foo.h | bar.cpp |
If now the user opens foo.cpp, then this tab will be placed to the right of foo.h.
Another feature: Assume that both buddy documents and the option "Open new tab after current" are enabled. There are the following tabs:
| *foo.h* | foo.cpp | bar.h | (foo.h is the active tab)
Normally, if the user opens somefile.cpp now, the tab would be placed between foo.h and foo.cpp, which would separate the buddies. So we changed the bahavior: the new tab will be placed after foo.cpp instead.</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;">Created shell-buddytest and sublime-documentcontrollertest. Both pass.
Fixed an error in sublime-areaoperationtest (which was not related to the buddy document feature, but to the option "Open new tab after current").</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>interfaces/CMakeLists.txt <span style="color: grey">(170cf63)</span></li>
<li>interfaces/ibuddydocumentfinder.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>interfaces/ibuddydocumentfinder.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>interfaces/idocumentcontroller.h <span style="color: grey">(d036afa)</span></li>
<li>shell/documentcontroller.h <span style="color: grey">(8dc3706)</span></li>
<li>shell/documentcontroller.cpp <span style="color: grey">(bc0a3dd)</span></li>
<li>shell/settings/uiconfig.kcfg <span style="color: grey">(60dccb2)</span></li>
<li>shell/settings/uiconfig.ui <span style="color: grey">(61f4b08)</span></li>
<li>shell/tests/CMakeLists.txt <span style="color: grey">(b093565)</span></li>
<li>shell/tests/documentcontrollertest.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>shell/tests/documentcontrollertest.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>shell/tests/shellbuddytest.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>shell/tests/shellbuddytest.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>sublime/area.h <span style="color: grey">(df0e2bb)</span></li>
<li>sublime/area.cpp <span style="color: grey">(efa8025)</span></li>
<li>sublime/controller.h <span style="color: grey">(e8b4794)</span></li>
<li>sublime/controller.cpp <span style="color: grey">(a8dd38a)</span></li>
<li>sublime/tests/areaoperationtest.h <span style="color: grey">(0e0f08e)</span></li>
<li>sublime/tests/areaoperationtest.cpp <span style="color: grey">(28dbeff)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/100874/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Screenshots </h1>
<div>
<a href="http://git.reviewboard.kde.org/r/100874/s/103/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2011/03/16/uisettings_400x100.png" style="border: 1px black solid;" alt="New UiSettings" /></a>
</div>
</td>
</tr>
</table>
</div>
</body>
</html>