<table><tr><td style="">mwolff requested changes to this revision.<br />mwolff added a reviewer: mwolff.<br />mwolff added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D4108" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p>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</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4108#inline-16568" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">plugincontroller.cpp:149</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; ">    <span class="n">QVector</span><span style="color: #aa2211"><</span><span class="n">KPluginMetaData</span><span style="color: #aa2211">></span> <span class="n">plugins</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;">    <span class="n">QList</span><span style="color: #aa2211"><</span><span class="n">QPair</span><span style="color: #aa2211"><</span><span class="n">QString</span><span class="p">,</span><span class="n">QString</span><span style="color: #aa2211">>></span> <span class="n">failedToLoad</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">rename to: pluginLoadErrors, make it a QVector of QStrings (see Sven's comment below)</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4108#inline-16538" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">brauch</span> wrote in <span style="color: #4b4d51; font-weight: bold;">plugincontroller.cpp:571</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">qCDebug</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">imo: remove this debug output</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4108#inline-16567" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">plugincontroller.cpp:586</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span class="n">QList</span><span style="color: #aa2211"><</span><span class="n">QPair</span><span style="color: #aa2211"><</span><span class="n">QString</span><span class="p">,</span><span class="n">QString</span><span style="color: #aa2211">>></span> <span class="n">PluginController</span><span style="color: #aa2211">::</span><span class="n">failedPlugins</span><span class="p">()</span> <span style="color: #aa4000">const</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;">    <span style="color: #aa4000">return</span> <span class="n">d</span><span style="color: #aa2211">-></span><span class="n">failedToLoad</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">move { to its own line</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4108#inline-16572" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">pluginpreferences.cpp:61</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;">    <span style="color: #74777d">/* If the plugin was marked to be loaded, but it's failing, this applies */</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;">    <span class="n">PluginController</span><span style="color: #aa2211">*</span> <span class="n">controller</span> <span style="color: #aa2211">=</span> <span class="n">qobject_cast</span><span style="color: #aa2211"><</span><span class="n">PluginController</span><span style="color: #aa2211">*></span><span class="p">(</span><span class="n">Core</span><span style="color: #aa2211">::</span><span class="n">self</span><span class="p">()</span><span style="color: #aa2211">-></span><span class="n">pluginControllerInternal</span><span class="p">());</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;">    <span class="n">QList</span><span style="color: #aa2211"><</span><span class="n">QPair</span><span style="color: #aa2211"><</span><span class="n">QString</span><span class="p">,</span><span class="n">QString</span><span style="color: #aa2211">>></span> <span class="n">failedPlugins</span> <span style="color: #aa2211">=</span> <span class="n">controller</span><span style="color: #aa2211">-></span><span class="n">failedPlugins</span><span class="p">();</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">this looks wrong, it should be</p>

<p style="padding: 0; margin: 8px;">auto controller =  Core::self()->pluginControllerInternal();</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4108#inline-16539" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">brauch</span> wrote in <span style="color: #4b4d51; font-weight: bold;">pluginpreferences.cpp:62</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">auto</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">const auto, even</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4108#inline-16542" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">brauch</span> wrote in <span style="color: #4b4d51; font-weight: bold;">pluginpreferences.cpp:63</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">qCDebug and proper wording please ;)</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">remove altogether</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4108#inline-16571" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">pluginpreferences.cpp:68</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;">        <span class="n">messageWidget</span><span style="color: #aa2211">-></span><span class="n">setCloseButtonVisible</span><span class="p">(</span><span style="color: #304a96">false</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;">        <span class="n">QString</span> <span class="n">text</span> <span style="color: #aa2211">=</span> <span class="n">i18n</span><span class="p">(</span><span style="color: #766510">"Some plugins failed do load:</span><span style="color: #bb6622">\n</span><span style="color: #766510">"</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;">        <span class="n">foreach</span><span class="p">(</span><span style="color: #aa4000">const</span> <span style="color: #aa4000">auto</span><span style="color: #aa2211">&</span> <span class="n">plugin</span><span class="p">,</span> <span class="n">failedPlugins</span><span class="p">)</span> <span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">needs to use i18np based on number of errors</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R33 KDevPlatform</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D4108" rel="noreferrer">https://phabricator.kde.org/D4108</a></div></div><br /><div><strong>EMAIL PREFERENCES</strong><div><a href="https://phabricator.kde.org/settings/panel/emailpreferences/" rel="noreferrer">https://phabricator.kde.org/settings/panel/emailpreferences/</a></div></div><br /><div><strong>To: </strong>tcanabrava, kfunk, apol, brauch, mwolff<br /><strong>Cc: </strong>mwolff, brauch, kdevelop-devel<br /></div>