[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