[Differential] [Requested Changes To] D4541: Fix loading/unloading of GDB plugin
Milian Wolff
noreply at phabricator.kde.org
Fri Feb 10 08:49:16 UTC 2017
mwolff requested changes to this revision.
mwolff added a reviewer: mwolff.
mwolff added a comment.
This revision now requires changes to proceed.
good in principle, but some style nitpicks
INLINE COMMENTS
> debuggerplugin.cpp:58
>
> - QList<IPlugin*> plugins = KDevelop::ICore::self()->pluginController()->allPluginsForExtension("org.kdevelop.IExecutePlugin");
> - foreach(IPlugin* plugin, plugins) {
> - IExecutePlugin* iface = plugin->extension<IExecutePlugin>();
> - Q_ASSERT(iface);
> - KDevelop::LaunchConfigurationType* type = core()->runController()->launchConfigurationTypeForId( iface->nativeAppConfigTypeId() );
> - Q_ASSERT(type);
> - type->addLauncher( new GdbLauncher( this, iface ) );
> + foreach(auto plugin, core()->pluginController()->allPluginsForExtension("org.kdevelop.IExecutePlugin")) {
> + setupExecutePlugin(plugin, true);
use range-based-for in new code please, and wrap strings in QStringLiteral
> debuggerplugin.cpp:64
> + this, [this](KDevelop::IPlugin* plugin) {
> + setupExecutePlugin(plugin, true);
> + });
indent, such that this line is four spaces deeper than the this in the line above
> debuggerplugin.cpp:65
> + setupExecutePlugin(plugin, true);
> + });
> +
indent, such that this line aligns with the this
> debuggerplugin.cpp:69
> + this, [this](KDevelop::IPlugin* plugin) {
> + setupExecutePlugin(plugin, false);
> + });
dito indent
> debuggerplugin.cpp:75
> +{
> + foreach(auto plugin, core()->pluginController()->allPluginsForExtension("org.kdevelop.IExecutePlugin")) {
> + setupExecutePlugin(plugin, false);
dito above
> debuggerplugin.h:87
> DebuggerToolFactory<MemoryViewerWidget, CppDebuggerPlugin>* memoryviewerfactory;
> + QMap<KDevelop::IPlugin*, GdbLauncher*> m_launchers;
> };
QHash
QMap is sorted, but pointers are inherently random in their value (i.e. the address), so you definitely don't want to sort them.
REPOSITORY
R32 KDevelop
REVISION DETAIL
https://phabricator.kde.org/D4541
EMAIL PREFERENCES
https://phabricator.kde.org/settings/panel/emailpreferences/
To: antonanikin, #kdevelop, mwolff
Cc: mwolff, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170210/347c2c7b/attachment-0001.html>
More information about the KDevelop-devel
mailing list