<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/120342/">https://git.reviewboard.kde.org/r/120342/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 25th, 2014, 2:03 p.m. UTC, <b>Boudewijn Rempt</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It looks fine to me, though I'm not the right maintainer :-)</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Thanks Boud. Listed you in cc: because of your experience with that stuff and because I followed Krita's KisFactory2 code. So was looking forward to your comment :)<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
If noone else comments until October 1st, I will go one and ship it :)</p></pre>
<br />
<p>- Friedrich W. H.</p>
<br />
<p>On September 24th, 2014, 9:24 p.m. UTC, Friedrich W. H. Kossebau wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Calligra, Cyrille Berger Skott, Yue Liu, Boudewijn Rempt, and Thorsten Zachmann.</div>
<div>By Friedrich W. H. Kossebau.</div>
<p style="color: grey;"><i>Updated Sept. 24, 2014, 9:24 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
calligra
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Currently blacklisting is done for the two Stage- and KoPA-specific tools calligrastagetoolanimation and kopabackgroundtool in all apps (not done for Braindump because there is no default braindumprc with an "ToolPluginsDisabled" entry).<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
Actually these two tools will even crash other apps because they expect the passed objects, like the canvas, to be of certain classes.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
Als does the current blacklisting have a problem: if the entry "ToolPluginsDisabled" is written to the personal copy of the rc file, installing a new. Which happened to me, because I have a very old flowrc file in personal config dir with only "ToolPluginsDisabled=kopabackgroundtool", so the newer entry in the newer installed flowrc was no longer read, resulting in not blacklisting calligrastagetoolanimation for me in recent calligra versions. And trying that tool made me think Flow is buggy ;)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">So given that those tools are not compatible with all apps, I propose to instead give them own service types:<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
"CalligraPageApp/Tool" and "CalligraStage/Tool".<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
So only programs which explicitely demand services of these types get them, and the rest no longer has to protect against them.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This also helps with potential 3rd-party KoPA-specific tool plugins, as they would not be catched by the existing blacklisting.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Only disadvantage: programs which want them have to explicitely demand them. But that seems okay to me.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Followed the example of Krita's KisFactory2 code how to do refcount-guarded loading of the plugins. Not sure that is perfect. But at least consistent :)</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">calligrastagetoolanimation and kopabackgroundtool now longer show up were they should not show up.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>karbon/data/karbonrc <span style="color: grey">(33ac9b3)</span></li>
<li>krita/animator/kritaanimationrc <span style="color: grey">(60036d9)</span></li>
<li>krita/data/kritarc <span style="color: grey">(9ffcae4)</span></li>
<li>krita/gemini/kritageminirc <span style="color: grey">(354c741)</span></li>
<li>krita/sketch/kritasketchrc <span style="color: grey">(f3efba6)</span></li>
<li>libs/kopageapp/tools/CMakeLists.txt <span style="color: grey">(3dea584)</span></li>
<li>libs/kopageapp/tools/backgroundTool/kopabackgroundtool.desktop <span style="color: grey">(1b8683a)</span></li>
<li>libs/kopageapp/tools/kopa_tool.desktop <span style="color: grey">(PRE-CREATION)</span></li>
<li>sheets/sheetsrc <span style="color: grey">(9b622fb)</span></li>
<li>stage/data/CMakeLists.txt <span style="color: grey">(50e6efa)</span></li>
<li>stage/data/kpr_tool.desktop <span style="color: grey">(PRE-CREATION)</span></li>
<li>stage/part/KPrFactory.cpp <span style="color: grey">(848b15c)</span></li>
<li>stage/part/tools/animationtool/calligrastagetoolanimation.desktop <span style="color: grey">(ac2737e)</span></li>
<li>words/part/author/authorrc <span style="color: grey">(2537c68)</span></li>
<li>words/part/wordsrc <span style="color: grey">(4cd2801)</span></li>
<li>flow/part/FlowFactory.cpp <span style="color: grey">(7f14fb0)</span></li>
<li>flow/part/flowrc <span style="color: grey">(1d1c12f)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/120342/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>