<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/107310/">http://git.reviewboard.kde.org/r/107310/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 17th, 2012, 5:19 p.m., <b>Milian Wolff</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 think this is ready to go... except - putting that context menu into its own plugin seems overkill. Personally I think this could be put into the context browser plugin, as this is about "browsing", no?
It would remove the need for a lot of boiler plate code in your plugin, just to get a simple context menu action up and running. What do you (and others) say?</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;">I first also thought about integration this functionality somewhere else. Either another plugin or the document controller itself. But still I think it's better to leave it in its own plugin:
- Like this it's possible to just turn off the context menu extension by disabling the plugin. Integration in an other plugin would mean to introduce a new option to turn it off.
- I think from the point of view of maintainability it's better to leave in its own plugin. One of the things I really like about Kdevelop's code base is its modularity: Every little feature could be a plugin and doesn't have to be stuffed somewhere else. Also from the point of view of "Single responsibility" it's better to just leave there as the context browser plugin for example would be responsible for too many things otherwise.
- Thanks to Kdevelop's well thought plugin system I really think the boiler plate is kept to a minimum. If I just think about other "extensible" systems... :)</pre>
<br />
<p>- André</p>
<br />
<p>On November 16th, 2012, 1:35 p.m., André Stein 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 KDevelop.</div>
<div>By André Stein.</div>
<p style="color: grey;"><i>Updated Nov. 16, 2012, 1:35 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 request adds a new plugin "Switch to buddy" which enables in the c++ language plugin a new context menu action to switch to corresponding header or implementation file. This request is split in two parts because it needs changes in kdevelop too (see Request 107311).
Changes made:
- A new function has been added to IBuddyDocumentFinder to allow language plugins to implement logic to find buddy or related files.
- The new plugin "switch to buddy" implements a context menu extension which is only trigged if at least one buddy can be found. To get possible buddies IBuddyDocumentFinder is used using the editor's current document's mime type.
- See plugin
This implements a feature known from many IDEs. Using the Buddy interface and a separate plugin I think it's rather clean and generic.</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;">Manual testing.</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/ibuddydocumentfinder.h <span style="color: grey">(7807df7)</span></li>
<li>plugins/CMakeLists.txt <span style="color: grey">(8aac3ae)</span></li>
<li>plugins/switchtobuddy/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/switchtobuddy/Messages.sh <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/switchtobuddy/kdevswitchtobuddy.desktop <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/switchtobuddy/switchtobuddyplugin.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/switchtobuddy/switchtobuddyplugin.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>shell/tests/shellbuddytest.cpp <span style="color: grey">(981daa3)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/107310/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/107310/s/830/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2012/11/14/switch_screenshot1_400x100.png" style="border: 1px black solid;" alt="Context menu in action" /></a>
</div>
</td>
</tr>
</table>
</div>
</body>
</html>