<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/100795/">http://git.reviewboard.kde.org/r/100795/</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 5th, 2011, 10:45 a.m., <b>Dmitry Risenberg</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 does not apply cleanly on current master:
patching file sublime/container.h
Hunk #2 FAILED at 63.
1 out of 2 hunks FAILED -- saving rejects to file sublime/container.h.rej
patching file sublime/container.cpp
Hunk #9 succeeded at 519 with fuzz 2 (offset 12 lines).
Please rebase on current master branch and re-send the patch.
Also the whitespace and newline changes make it hard to see what has actually changed. Some new trailing whitespaces are introduced - they should be removed.
Replace 'buddys' with 'buddies' - this is the correct spelling.
I think IBuddyDocumentFinder should be not in sublime, but in interfaces, because C++ support should not depend on the UI library (your other patch).
As a feature request: this should sometimes be triggered automatically, for example on closing KDevelop (whether to trigger or not can be configured in the settings).
</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Ok sorry about that, we'll create a new patch.
Yes it's not optimal to have IBuddyDocumentFinder in sublime. The reason why we put it there:
Currently sublime does not depend on any other library of kdevplatform. We use IBuddyDocumentFinder and BuddyFinderRegistry in the tabbar cleanup, to keep foo.cpp always open when foo.h is still in use (and vice versa). The tabbar cleanup is in sublime as it is closely related to Sublime::Container, so we create a cleanup object in the constructor of Container, this seems to be the only proper way to add the cleanup feature.
So I see several possiblities:
a) Put the buddy functionality in interfaces. So the c++ plugin would not depend on sublime[1], but instead sublime would depend on interfaces.
b) Keep the buddy functionality in sublime.
c) Put the buddies in interfaces, and remove buddy integration in the cleanup, so keeping sublime independent of interfaces. I'd discourage this option, because someone who is editing foo.h will very likely also edit foo.cpp soon.
What do you think?
(Without our patch, the library dependencies are like this at the moment (correct me if I'm wrong):
shell -> sublime
shell -> interfaces
c++plugin -> interfaces)
The idea of triggering the cleanup on closing kdevelop is good, I think. As soon as our patch is accepted in general, we could add this.
[1] In fact, CppLanguageSupport::createActionsForMainWindow already has a parameter of type Sublime::MainWindow*, but it is not used, so perhaps it could be removed. At least, the plugin is not linked against sublime.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 5th, 2011, 10:45 a.m., <b>Dmitry Risenberg</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/100795/diff/1/?file=10539#file10539line137" style="color: black; font-weight: bold; text-decoration: underline;">shell/documentcontroller.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:</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">137</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">virtual</span> <span class="n">KDevelop</span><span class="o">::</span><span class="n">IDocument</span><span class="o">*</span> <span class="n">findBuddyDocument</span><span class="p">(</span><span class="k">const</span> <span class="n">KUrl</span> <span class="o">&</span><span class="n">url</span><span class="p">,</span> <span class="n">Sublime</span><span class="o">::</span><span class="n">IBuddyDocumentFinder</span><span class="o">*</span> <span class="n">finder</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;">Why does this belong to IDocumentController and not IBuddyDocumentFinder?</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 belongs to DocumentController because here we search for a buddy document in all open documents, using ICore::self()->documentController()->openDocuments(). So currently, if we'd put it in IBuddyDocumentFinder (which is currently in sublime), sublime would depend on interfaces (and perhaps on shell, I'm not sure, as the real implementations of ICore and IDocumentController seem to be in the shell, in Core and DocumentController).
(see above, regarding the problem where to put IBuddyDocumentFinder)</pre>
<br />
<p>- Martin</p>
<br />
<p>On March 4th, 2011, 5:31 p.m., Yannick Motta 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 Yannick Motta.</div>
<p style="color: grey;"><i>Updated March 4, 2011, 5:31 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;">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 with BuddyFinderRegistry.
An option was added to open documents side by side with its buddies.
Add smart cleanup feature to allow user to easily close outdated tabs related to each tab-bar.
Outdated tabs are files that are not currently used.
This feature is configurable, user can choose the time after which smart cleanup considers an unused tab outdated.
An document is considered active if it is active at least five seconds.
Last five documents are never outdated.
If an outdated document has an active buddy, it is immune to cleanup.
User must manually start cleanup feature on the context menu of the tab-bar.
A dialog opens and user can see which documents are considered outdated. User can remove documents from the list.
If the user clicks "ok", tabs will be close.
Add an option to hide close buttons on tabs in order to free some space.</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 have added two unit tests :
sublime-cleanuptest
shell-buddytest
both pass without error.
manual test seems also working as expected.</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/idocumentcontroller.h <span style="color: grey">(d036afa)</span></li>
<li>kdevplatform.kdev4 <span style="color: grey">(f5f408c)</span></li>
<li>project/CMakeLists.txt <span style="color: grey">(a5b7f74)</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.kcfgc <span style="color: grey">(3c02f72)</span></li>
<li>shell/settings/uiconfig.ui <span style="color: grey">(61f4b08)</span></li>
<li>shell/settings/uipreferences.cpp <span style="color: grey">(43c63d6)</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/CMakeLists.txt <span style="color: grey">(f9aaaeb)</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/buddyfinderregistry.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>sublime/buddyfinderregistry.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>sublime/cleanup.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>sublime/cleanup.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>sublime/cleanupdialog.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>sublime/cleanupdialog.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>sublime/cleanupdialog.ui <span style="color: grey">(PRE-CREATION)</span></li>
<li>sublime/container.h <span style="color: grey">(6b79df7)</span></li>
<li>sublime/container.cpp <span style="color: grey">(d45604f)</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/ibuddydocumentfinder.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>sublime/mainwindow.cpp <span style="color: grey">(08ca648)</span></li>
<li>sublime/tests/CMakeLists.txt <span style="color: grey">(410412b)</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>
<li>sublime/tests/cleanuptest.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>sublime/tests/cleanuptest.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/100795/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/100795/s/90/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2011/03/04/Ui_settings_400x100.png" style="border: 1px black solid;" alt="New ui settings" /></a>
<a href="http://git.reviewboard.kde.org/r/100795/s/91/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2011/03/04/Cleanup_dialog_400x100.png" style="border: 1px black solid;" alt="Cleanup dialog and hided close buttons" /></a>
<a href="http://git.reviewboard.kde.org/r/100795/s/92/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2011/03/04/tab_bat_context_menu_400x100.png" style="border: 1px black solid;" alt="Tab bar context menu" /></a>
</div>
</td>
</tr>
</table>
</div>
</body>
</html>