Partial Fix: display the plugins that failed to load
AbandonedPublic

Authored by tcanabrava on Jan 12 2017, 6:17 PM.

Details

Summary

This is a partial fix, it shows the plugins that where
active and failed to load during a KDevelop initialization,
but it still doesn't do anything if you try to activate
the plugin when KDevelop is running via the preferences.

Signed-off-by: Tomaz Canabrava <tcanabrava@kde.org>

Test Plan

manual test

Diff Detail

Repository
R33 KDevPlatform
Branch
plugin_work_2
Lint
No Linters Available
Unit
No Unit Test Coverage
tcanabrava updated this revision to Diff 10114.Jan 12 2017, 6:17 PM
tcanabrava retitled this revision from to Partial Fix: display the plugins that failed to load.
tcanabrava updated this object.
tcanabrava edited the test plan for this revision. (Show Details)
tcanabrava added reviewers: kfunk, apol.
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptJan 12 2017, 6:17 PM
brauch added a subscriber: brauch.Jan 17 2017, 8:23 PM

Better than not displaying any info, I guess ;)

shell/plugincontroller.cpp
571

qCDebug

shell/plugincontroller.h
141

QVector and put the QPair into a struct instead

shell/settings/pluginpreferences.cpp
62

auto

63

qCDebug and proper wording please ;)

65

remove this

70

should probably have a colon : in between or so, but then you need an i18n call as well

70

also don't add a newline on the last item, the spacing is weird

brauch requested changes to this revision.Jan 17 2017, 8:23 PM
brauch added a reviewer: brauch.
This revision now requires changes to proceed.Jan 17 2017, 8:23 PM
brauch added inline comments.Jan 17 2017, 8:24 PM
shell/settings/pluginpreferences.cpp
70

And actually, you can return the assembled string already, then you don't need the QPair stuff at all.

mwolff requested changes to this revision.Jan 18 2017, 9:08 AM
mwolff added a reviewer: mwolff.
mwolff added a subscriber: mwolff.

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

shell/plugincontroller.cpp
149

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

571

imo: remove this debug output

586

move { to its own line

shell/settings/pluginpreferences.cpp
61

this looks wrong, it should be

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

62

const auto, even

63

remove altogether

68

needs to use i18np based on number of errors

tcanabrava marked 15 inline comments as done.Jan 18 2017, 11:04 AM
In D4108#78283, @mwolff wrote:

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

I didnt found *where* the plugins are loaded after accepting the preferences, there's a method that will reload all the *global* plugins (git is not one of those, tougth), The preferences dialog only saves the preferences - if one plugin should be loaded or not, and the call to Core::self()->pluginControllerInternal()->updateLoadedPlugins(); don't actually update the loaded plugins.

Besides that, I don't know *where* to put the messages that one plugin failed to load - IMO it should be inside of the KPluginSelector checkbox (click on the checkbox, plugin goes red saying that something is not right with it) but it don't have that option.

Any tougths?

In D4108#78283, @mwolff wrote:

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

I didnt found *where* the plugins are loaded after accepting the preferences, there's a method that will reload all the *global* plugins (git is not one of those, tougth), The preferences dialog only saves the preferences - if one plugin should be loaded or not, and the call to Core::self()->pluginControllerInternal()->updateLoadedPlugins(); don't actually update the loaded plugins.

selector->load() applies this, no? Do you mean the list of plugins in the controller goes out-of-sync? If so, then we must fix this first (in a separate commit).

Besides that, I don't know *where* to put the messages that one plugin failed to load - IMO it should be inside of the KPluginSelector checkbox (click on the checkbox, plugin goes red saying that something is not right with it) but it don't have that option.

The way you added the message is fine for now. I simply want to have the message update itself after a plugin was (un)loaded and an error occurred.

In D4108#78384, @mwolff wrote:
In D4108#78283, @mwolff wrote:

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

I didnt found *where* the plugins are loaded after accepting the preferences, there's a method that will reload all the *global* plugins (git is not one of those, tougth), The preferences dialog only saves the preferences - if one plugin should be loaded or not, and the call to Core::self()->pluginControllerInternal()->updateLoadedPlugins(); don't actually update the loaded plugins.

selector->load() applies this, no? Do you mean the list of plugins in the controller goes out-of-sync? If so, then we must fix this first (in a separate commit).

Selector saves in the configuration what plugins should be loaded, but they don't reload after an initial failure, try this:
Open KDvelop without git installed, you will have the "could not load git" error on the terminal, but the plugin will still be listed on the KPluginSelector (and the plugin object will be nullptr), then check the git plugin on the KPluginSelector and click ok. No message appears (on the terminal or on KDevelop).

Besides that, I don't know *where* to put the messages that one plugin failed to load - IMO it should be inside of the KPluginSelector checkbox (click on the checkbox, plugin goes red saying that something is not right with it) but it don't have that option.

The way you added the message is fine for now. I simply want to have the message update itself after a plugin was (un)loaded and an error occurred.

That's my point, the error only appear when we try to load the plugin in the start of kdevelop.

tcanabrava abandoned this revision.Feb 2 2018, 9:05 AM