<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/112527/">http://git.reviewboard.kde.org/r/112527/</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 8th, 2013, 8:45 a.m. UTC, <b>David Faure</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/112527/diff/4/?file=188118#file188118line85" style="color: black; font-weight: bold; text-decoration: underline;">staging/kemoticons/src/core/kemoticonsprovider.h</a>
<span style="font-weight: normal;">
(Diff revision 4)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">class KEMOTICONS_EXPORT KEmoticonsProvider : public QObject</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">83</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * @param emo path to the emoticon image</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">82</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * @param emo path to the emoticon image</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">absolute path, or relative path?</pre>
</blockquote>
<p>On September 8th, 2013, 5:07 p.m. UTC, <b>David Gil Oliva</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;">All of them are absolute. Should I specify it in the docs? Or should I modify the code so that it also accepts relative paths and converts them in absolute?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I guess I got confused by the broken line of code below. Path in KDE usually means absolute, yes.
Add "full" before "path", or leave it as is.
Definitely don't add support for relative paths if this wasn't there before, it means the base dir is unclear :)</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 8th, 2013, 8:45 a.m. UTC, <b>David Faure</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/112527/diff/4/?file=188119#file188119line146" style="color: black; font-weight: bold; text-decoration: underline;">staging/kemoticons/src/core/kemoticonsprovider.cpp</a>
<span style="font-weight: normal;">
(Diff revision 4)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">110</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QString</span> <span class="nf">newPath</span><span class="p">(</span><span class="n">d</span><span class="o">-></span><span class="n">m_themePath</span> <span class="o">+</span> <span class="n">QLatin1Char</span><span class="p">(</span><span class="sc">'/'</span><span class="p">)</span> <span class="o">+</span> <span class="n">file</span><span class="p">.</span><span class="n">fileName</span><span class="p">());</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">file.fileName() is the same as emo. Yes, the name of the methods in QFile are a bit confusing.
If this code was meant to extract the actual filename only (/tmp/foo.png -> foo.png) then you have to use QFileInfo::fileName().
Hence my questions about absolute or relative paths, I don't actually know what "emo" contains in this method.</pre>
</blockquote>
<p>On September 8th, 2013, 5:07 p.m. UTC, <b>David Gil Oliva</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;">As I said before, emo is an absolute path. </pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">OK then this line of code is broken :)
file.fileName() is the same as emo, i.e. an absolute path.
Concatenating that to themePath + '/' makes no sense, you'd end up with /usr/path/to/theme/usr/full/path/to/emoticon.</pre>
<br />
<p>- David</p>
<br />
<p>On September 8th, 2013, 12:58 a.m. UTC, David Gil Oliva wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Frameworks.</div>
<div>By David Gil Oliva.</div>
<p style="color: grey;"><i>Updated Sept. 8, 2013, 12:58 a.m.</i></p>
<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;">Clean up KEmoticons framework (prior to splitting)
--Clean up includes
--Turn KEmoticonsProvider abstract and reorganize some code
--Revise documentation
--Use QScopedPointer where useful
--Port away from KIO in KEmoticonsProvider
--Put const where fit
TODO:
--Port away from KServiceTypeTrader. Sebas is working on a way to get
the plugin list without KServiceTypeTrader.</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;">It compiles and test passes</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>staging/kemoticons/src/core/kemoticons.h <span style="color: grey">(57912b550c5e941f751d93c084b37764635e11c7)</span></li>
<li>staging/kemoticons/autotests/kemoticontest.cpp <span style="color: grey">(0ca1671d26970998c13022daa839e1aae4907220)</span></li>
<li>staging/kemoticons/src/core/kemoticons.cpp <span style="color: grey">(317089ed94179aac2b7e448414df930dcfd0c0dd)</span></li>
<li>staging/kemoticons/src/core/kemoticonsprovider.h <span style="color: grey">(3f43ca4c8a1fe74ce01a1aac1abdeb84b9da08d2)</span></li>
<li>staging/kemoticons/src/core/kemoticonsprovider.cpp <span style="color: grey">(1fcad79bd49919058bb21b262b57b7a9bb6ac5c9)</span></li>
<li>staging/kemoticons/src/core/kemoticonstheme.h <span style="color: grey">(d0e1a899e2f8a89b34e6337848c1aeab7f60ef88)</span></li>
<li>staging/kemoticons/src/core/kemoticonstheme.cpp <span style="color: grey">(cd385fde6095b2da930de9ec03a501ae3d1ab0f7)</span></li>
<li>staging/kemoticons/src/providers/adium/adium_emoticons.cpp <span style="color: grey">(f465b64e230639f16ca073ed7ab08a79f04afd4a)</span></li>
<li>staging/kemoticons/src/providers/kde/kde_emoticons.cpp <span style="color: grey">(7d777b5aa0a869a0009814bb43f84f802444d5d6)</span></li>
<li>staging/kemoticons/src/providers/pidgin/pidgin_emoticons.cpp <span style="color: grey">(65b2c5e979eeccbc98986432962e3ab05f39ca57)</span></li>
<li>staging/kemoticons/src/providers/xmpp/xmpp_emoticons.cpp <span style="color: grey">(2535b71263f468dacc830e7cf92fead5e61528e8)</span></li>
<li>staging/kemoticons/tests/main.cpp <span style="color: grey">(f46265e97243aa776f901120d832368154c5c2ed)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/112527/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>