[Differential] [Requested Changes To] D4108: Partial Fix: display the plugins that failed to load

Milian Wolff noreply at phabricator.kde.org
Wed Jan 18 09:08:53 UTC 2017

mwolff requested changes to this revision.
mwolff added a reviewer: mwolff.
mwolff added a comment.

  where do you update the error message when the user tries to enable a plugin from the setting dialog but it fails to load? you only construct the error display in the ctor, so this won't update properly, one will have to restart the dialog to show effect - please fix this


> plugincontroller.cpp:149
>      QVector<KPluginMetaData> plugins;
> +    QList<QPair<QString,QString>> failedToLoad;

rename to: pluginLoadErrors, make it a QVector of QStrings (see Sven's comment below)

> brauch wrote in plugincontroller.cpp:571
> qCDebug

imo: remove this debug output

> plugincontroller.cpp:586
> +QList<QPair<QString,QString>> PluginController::failedPlugins() const {
> +    return d->failedToLoad;

move { to its own line

> pluginpreferences.cpp:61
> +    /* If the plugin was marked to be loaded, but it's failing, this applies */
> +    PluginController* controller = qobject_cast<PluginController*>(Core::self()->pluginControllerInternal());
> +    QList<QPair<QString,QString>> failedPlugins = controller->failedPlugins();

this looks wrong, it should be

auto controller =  Core::self()->pluginControllerInternal();

> brauch wrote in pluginpreferences.cpp:62
> auto

const auto, even

> brauch wrote in pluginpreferences.cpp:63
> qCDebug and proper wording please ;)

remove altogether

> pluginpreferences.cpp:68
> +        messageWidget->setCloseButtonVisible(false);
> +        QString text = i18n("Some plugins failed do load:\n");
> +        foreach(const auto& plugin, failedPlugins) {

needs to use i18np based on number of errors

  R33 KDevPlatform



To: tcanabrava, kfunk, apol, brauch, mwolff
Cc: mwolff, brauch, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170118/3d9f6411/attachment-0001.html>

More information about the KDevelop-devel mailing list