<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/106988/">http://git.reviewboard.kde.org/r/106988/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 23rd, 2012, 6:45 a.m., <b>René Küttner</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;">Patch works fine.
However, while working on the eclipses plugin, the need to add custom menu items and sub menus to the main menu (e.g. below the 'View' menu) came up. The plugin requires this features in order to allow users to select a certain eclipse from the menu. It could be implemented using a flexible createPluginAction( QtMainWindow::Menu parentMenu ) method in QtMainWindow. This method could create an action (replacing createRenderPluginAction()) below a given QtMainWindow::Menu (enum) and return it. Additionally, this would allow the cross-hairs plugin to create its menu action from within the plugin. Therefore removing cross-hairs plugin code from the core. It would also fix the problems this patch targets at. A second method createPluginMenu( QtMainWindow::Menu parentMenu ) could provide similar functionality for sub menus (e.g. a sub menu containing a list of eclipses).
Since the above changes could be implemented easily, it may be a good idea to bundle or replace them with this patch. Are there any objections? If you think, the additions should go into a separate patch, ship it!</pre>
</blockquote>
<p>On October 24th, 2012, 5:22 p.m., <b>Bernhard Beschow</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;">It seems to me that the existing RenderPlugin::actionGroups() is supposed to do exactly what you want (adding eclipse actions to the menu). I wonder if this is implemented in the core, though.
As for the crosshairs plugin, I plan to implement it as a float item. The reason is that it can be seen as a 2D item, just like the float items. And once the crosshairs plugin is a float item, it can be swiched on and off via the menu w/o the core knowing about it. :)
Do you see further potential issues when removing RenderPlugin::action()? Otherwise I'd commit this patch.</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;">Ship it. :)</pre>
<br />
<p>- René</p>
<br />
<p>On October 22nd, 2012, 4:24 p.m., Bernhard Beschow 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 Marble.</div>
<div>By Bernhard Beschow.</div>
<p style="color: grey;"><i>Updated Oct. 22, 2012, 4:24 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;">API change: remove RenderPlugin::action()
With this patch, the actions are created outside of the plugins which is more flexible.
In the KDE version, for instance, creation of KActions (rather than QActions) avoids 'marble(26988)/kdeui (kdelibs): Attempt to use QAction "" with KXMLGUIFactory!' warnings.</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>src/QtMainWindow.h <span style="color: grey">(707855f33b12a4508853dd76c8a18bc2bb06cdf6)</span></li>
<li>src/QtMainWindow.cpp <span style="color: grey">(171190097748cadffd2c7aba268fa0ec5f5a8067)</span></li>
<li>src/lib/MarbleWidgetPopupMenu.cpp <span style="color: grey">(4da64b0c84cbe66b59b51f59f3641c5f7879cb05)</span></li>
<li>src/lib/RenderPlugin.h <span style="color: grey">(63df0432bfc3ae4de851cabe92d839bf93c50662)</span></li>
<li>src/lib/RenderPlugin.cpp <span style="color: grey">(373205de5a4e1cd6016ffaf181bf55a1ecf368ba)</span></li>
<li>src/marble_part.h <span style="color: grey">(098d63793904761f997223eff166a6ccd2568389)</span></li>
<li>src/marble_part.cpp <span style="color: grey">(85269d80f1d164fa12a6b6f13f2e3b8ab0172ca1)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/106988/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>