[Marble-devel] Review Request: Avoid some QAction-related warnings in the KDE version
René Küttner
kde at bitquirl.net
Fri Oct 26 10:33:02 UTC 2012
> On Oct. 23, 2012, 6:45 a.m., René Küttner wrote:
> > 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!
>
> Bernhard Beschow wrote:
> 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.
Ship it. :)
- René
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106988/#review20716
-----------------------------------------------------------
On Oct. 22, 2012, 4:24 p.m., Bernhard Beschow wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106988/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2012, 4:24 p.m.)
>
>
> Review request for Marble.
>
>
> Description
> -------
>
> 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.
>
>
> Diffs
> -----
>
> src/QtMainWindow.h 707855f33b12a4508853dd76c8a18bc2bb06cdf6
> src/QtMainWindow.cpp 171190097748cadffd2c7aba268fa0ec5f5a8067
> src/lib/MarbleWidgetPopupMenu.cpp 4da64b0c84cbe66b59b51f59f3641c5f7879cb05
> src/lib/RenderPlugin.h 63df0432bfc3ae4de851cabe92d839bf93c50662
> src/lib/RenderPlugin.cpp 373205de5a4e1cd6016ffaf181bf55a1ecf368ba
> src/marble_part.h 098d63793904761f997223eff166a6ccd2568389
> src/marble_part.cpp 85269d80f1d164fa12a6b6f13f2e3b8ab0172ca1
>
> Diff: http://git.reviewboard.kde.org/r/106988/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bernhard Beschow
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20121026/9936f386/attachment.html>
More information about the Marble-devel
mailing list