<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/121095/">https://git.reviewboard.kde.org/r/121095/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 11th, 2014, 3:39 p.m. CET, <b>Albert Astals Cid</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;">I sincerely think this is a <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">VERY BAD</em> idea. I don't want my app behaving differently depending if a third party misterious compononent that is not documented anywhere is installed or not. If you have a dependency, well put it in a tier with direct access to that dependency, or put a huge warning in the docu saying this will lose the functionality if something else is not there.</p></pre>
</blockquote>
<p>On November 11th, 2014, 3:48 p.m. CET, <b>Daniel Vrátil</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;">This requirement is documented in ktexttohtml.h in /r/121094.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%"><span style="color: #408080; font-style: italic">/**</span>
<span style="color: #408080; font-style: italic"> * Replace text emoticons smileys by emoticons images.</span>
<span style="color: #408080; font-style: italic"> *</span>
<span style="color: #408080; font-style: italic"> * @note</span>
<span style="color: #408080; font-style: italic"> * This option works only when FrameworkIntegration plugin is installed,</span>
<span style="color: #408080; font-style: italic"> * and requires QGuiApplication. This will not work with QCoreApplication.</span>
<span style="color: #408080; font-style: italic"> * If the FrameworkIntegration plugin is not available, or this is called</span>
<span style="color: #408080; font-style: italic"> * from a QCoreApplication, this option will not do anything.</span>
<span style="color: #408080; font-style: italic"> */</span>
ReplaceSmileys <span style="color: #666666">=</span> <span style="color: #666666">1</span> <span style="color: #666666"><<</span> <span style="color: #666666">2</span>,
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The behaviour change in case the conditions above are not fulfiled are not drastical - the flag will simply be ignored. I don't see much difference over MessageBox not remembering "Don't show this again" between sessions, because it would be using only memory storage when FrameworkIntegrationPlugin is not installed (which btw is not documented at all)</p></pre>
</blockquote>
<p>On November 11th, 2014, 8:07 p.m. CET, <b>Albert Astals Cid</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;">Why is the plugin in FrameworkIntegration and not in KEmoticons?</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;">Theoretically it could be in KEmoticons as some sort of plugin, but the original idea was to simply follow the same path as KMessageBox to create a runtime frameworks integration (for which "FrameworkIntegrationPlugin" seems to be exactly the right place).</p></pre>
<br />
<p>- Daniel</p>
<br />
<p>On November 11th, 2014, 3:51 p.m. CET, Daniel Vrátil 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 KDE Frameworks, David Faure and Michael Pyne.</div>
<div>By Daniel Vrátil.</div>
<p style="color: grey;"><i>Updated Nov. 11, 2014, 3:51 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
frameworkintegration
</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;">This patch is related to /r/121094, which moves KTextToHTML conversion utility from KPimUtils to KCoreAddons. Since KCoreAddons can't depend on KEmoticons needed for smileys conversion, I added the actual KEmoticons code here, to create a run-time dependency, similar to the KWidgetsAddons-KConfig dependency for KMessageBox.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This patch refactors the FrameworkIntegrationPlugin a bit - I split the KMessageBox-specific code into a separate file, and added a new file with the KTextToHTMLEmoticonsInterface implementation, as we can't just keep stacking more and more classes into a single file :-)</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;">Tested with KTextToHTML code from /r/121094 in a QGuiApplication and it seems to work.</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>CMakeLists.txt <span style="color: grey">(3721bfa)</span></li>
<li>src/integrationplugin/CMakeLists.txt <span style="color: grey">(3395368)</span></li>
<li>src/integrationplugin/frameworkintegrationplugin.h <span style="color: grey">(6dc6825)</span></li>
<li>src/integrationplugin/frameworkintegrationplugin.cpp <span style="color: grey">(a45ba9d)</span></li>
<li>src/integrationplugin/kmessagebox.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/integrationplugin/kmessagebox.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/integrationplugin/ktexttohtml.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/integrationplugin/ktexttohtml.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/121095/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>