<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/122905/">https://git.reviewboard.kde.org/r/122905/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 13th, 2015, 12:54 a.m. CET, <b>Aleix Pol Gonzalez</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="https://git.reviewboard.kde.org/r/122905/diff/2/?file=354710#file354710line208" style="color: black; font-weight: bold; text-decoration: underline;">models/memcheckitemsimpl.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">void MemcheckFrame::incomingData(QString name, QString value)</pre></td>
</tr>
</tbody>
<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">208</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QUrl</span> <span class="n">base</span> <span class="o">=</span> <span class="n">QUrl</span><span class="o">::</span><span class="n">fromLocalFile</span><span class="p">(</span><span class="n">dir</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">That's still not correct. Instead do the following:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">QUrl base = QUrl::fromLocalFile(QDir::cleanPath(dir+'/'));
QUrl url = base.resolved(QUrl(file));</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Also you could only do the resolution if QUrl(file).isRelative()</p></pre>
</blockquote>
<p>On March 13th, 2015, 1:45 a.m. CET, <b>Laszlo Kis-Adam</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 is always relative as the code provides just the filename in the file variable, so it's pointless to check if it's relative.
Also the code works, valgrind provides the data in the format of directory and filename. The directory is always without trailing slash, and the filename is always just a filename.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Here's an example of what memcheck provides, that is then parsed:
<dir>/home/dfighter/projects/kdevtest/my/super/deep/example/directory</dir>
<file>tests.cpp</file></p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As for the other valgrind tools, if they don't provide the right directory ( massif doesn't those items always have the directory set to the base directory ) the resolution will NOT work either way. So nothing lost, nothing gained.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">So what is incorrect exactly?
Are you just being pedantic, or is there an actual use case where this is incorrect? If there is, ofc I will change it :)
Thanks!</p></pre>
</blockquote>
<p>On March 13th, 2015, 3:36 a.m. CET, <b>Aleix Pol Gonzalez</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;">Well, the difference between being thorough and pedantic is can be thin. If you don't want to fix it, then don't.</p></pre>
</blockquote>
<p>On March 14th, 2015, 2:07 a.m. CET, <b>Laszlo Kis-Adam</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;">The question isn't whether I want to fix it or not, the question is whether there really is an issue. I don't think there is. I think this code works just fine.
So let's bottomline this: will you merge it like this, or not? If you are only willing to merge it if I change it the way you suggested, then ofc I will change it.</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Well, if I asked you to change it, was because I would have preferred to have optimal code that doesn't switch from and to QUrl for no reason. You could have saved your last message and done so, instead of continuing this.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'll just commit this because this got into very useless bike-shedding for an otherwise good patch.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Let's leave this behind and keep an open mind.</p></pre>
<br />
<p>- Aleix</p>
<br />
<p>On March 13th, 2015, 12:02 a.m. CET, Laszlo Kis-Adam 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 KDevelop.</div>
<div>By Laszlo Kis-Adam.</div>
<p style="color: grey;"><i>Updated March 13, 2015, 12:02 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdev-valgrind
</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;">Updated kdev-valgrind to be compatible with KF5</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">(31aaf52)</span></li>
<li>cmake/FindKDevPlatform.cmake <span style="color: grey">(1a771c5)</span></li>
<li>config.h <span style="color: grey">(a7ec923)</span></li>
<li>config.cpp <span style="color: grey">(9924dca)</span></li>
<li>config/cachegrindconfigpage.h <span style="color: grey">(070fc06)</span></li>
<li>config/cachegrindconfigpage.cpp <span style="color: grey">(a487cc5)</span></li>
<li>config/callgrindconfigpage.h <span style="color: grey">(eefbb18)</span></li>
<li>config/callgrindconfigpage.cpp <span style="color: grey">(2c97477)</span></li>
<li>config/genericconfigpage.h <span style="color: grey">(e6c9108)</span></li>
<li>config/genericconfigpage.cpp <span style="color: grey">(d0147f9)</span></li>
<li>config/helgrindconfigpage.h <span style="color: grey">(497e328)</span></li>
<li>config/helgrindconfigpage.cpp <span style="color: grey">(3fd97bd)</span></li>
<li>config/massifconfigpage.h <span style="color: grey">(64f0f11)</span></li>
<li>config/massifconfigpage.cpp <span style="color: grey">(49660af)</span></li>
<li>config/memcheckconfigpage.h <span style="color: grey">(f1d5925)</span></li>
<li>config/memcheckconfigpage.cpp <span style="color: grey">(3a661db)</span></li>
<li>config/ui/cachegrindconfig.ui <span style="color: grey">(170731e)</span></li>
<li>config/ui/callgrindconfig.ui <span style="color: grey">(fbb003d)</span></li>
<li>config/ui/genericconfig.ui <span style="color: grey">(a09f301)</span></li>
<li>config/ui/helgrindconfig.ui <span style="color: grey">(6cead3c)</span></li>
<li>config/ui/massifconfig.ui <span style="color: grey">(6d4a8de)</span></li>
<li>config/ui/memcheckconfig.ui <span style="color: grey">(11a163e)</span></li>
<li>config/valgrindcachegrindconfigpage.h <span style="color: grey">(73f65de)</span></li>
<li>config/valgrindcachegrindconfigpage.cpp <span style="color: grey">(22b349b)</span></li>
<li>config/valgrindcallgrindconfigpage.h <span style="color: grey">(fbdc90d)</span></li>
<li>config/valgrindcallgrindconfigpage.cpp <span style="color: grey">(5820af3)</span></li>
<li>config/valgrindgenericconfigpage.h <span style="color: grey">(cdaf0a8)</span></li>
<li>config/valgrindgenericconfigpage.cpp <span style="color: grey">(e05ef52)</span></li>
<li>config/valgrindhelgrindconfigpage.h <span style="color: grey">(c7487c2)</span></li>
<li>config/valgrindhelgrindconfigpage.cpp <span style="color: grey">(b6e877d)</span></li>
<li>config/valgrindmassifconfigpage.h <span style="color: grey">(62e3e79)</span></li>
<li>config/valgrindmassifconfigpage.cpp <span style="color: grey">(9165f6e)</span></li>
<li>config/valgrindmemcheckconfigpage.h <span style="color: grey">(431040a)</span></li>
<li>config/valgrindmemcheckconfigpage.cpp <span style="color: grey">(7c4f92f)</span></li>
<li>debug.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>debug.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>job.h <span style="color: grey">(ec4b1b0)</span></li>
<li>job.cpp <span style="color: grey">(cf832c4)</span></li>
<li>jobs/cachegrindjob.cpp <span style="color: grey">(0028a62)</span></li>
<li>jobs/callgrindjob.cpp <span style="color: grey">(627e4f7)</span></li>
<li>jobs/massifjob.cpp <span style="color: grey">(9fe2fde)</span></li>
<li>jobs/memcheckjob.cpp <span style="color: grey">(6912760)</span></li>
<li>kdevvalgrind.desktop <span style="color: grey">(fb76bb6)</span></li>
<li>kdevvalgrind.desktop.cmake <span style="color: grey">(PRE-CREATION)</span></li>
<li>launcher.h <span style="color: grey">(539e34f)</span></li>
<li>launcher.cpp <span style="color: grey">(d9b52af)</span></li>
<li>marks.cpp <span style="color: grey">(ff2c890)</span></li>
<li>models/cachegrinditem.h <span style="color: grey">(311d228)</span></li>
<li>models/cachegrinditem.cpp <span style="color: grey">(1b50b63)</span></li>
<li>models/cachegrindmodel.cpp <span style="color: grey">(70cad22)</span></li>
<li>models/callgrinditem.h <span style="color: grey">(11c4a33)</span></li>
<li>models/callgrinditem.cpp <span style="color: grey">(845104b)</span></li>
<li>models/callgrindmodel.cpp <span style="color: grey">(5dc971c)</span></li>
<li>models/massifitem.h <span style="color: grey">(6460fba)</span></li>
<li>models/massifitem.cpp <span style="color: grey">(33cd92d)</span></li>
<li>models/massifmodel.cpp <span style="color: grey">(4d5f66a)</span></li>
<li>models/memcheckitemsimpl.h <span style="color: grey">(111e4a6)</span></li>
<li>models/memcheckitemsimpl.cpp <span style="color: grey">(fdee8d2)</span></li>
<li>models/memcheckmodel.cpp <span style="color: grey">(2b96fbe)</span></li>
<li>parsers/cachegrindparser.h <span style="color: grey">(312f09a)</span></li>
<li>parsers/cachegrindparser.cpp <span style="color: grey">(f82fd8a)</span></li>
<li>parsers/callgrindparser.h <span style="color: grey">(e7f522d)</span></li>
<li>parsers/callgrindparser.cpp <span style="color: grey">(5ffe4c9)</span></li>
<li>parsers/massifparser.h <span style="color: grey">(1a97f3b)</span></li>
<li>parsers/memcheckparser.h <span style="color: grey">(2e3d5ae)</span></li>
<li>parsers/memcheckparser.cpp <span style="color: grey">(cf438ea)</span></li>
<li>plugin.h <span style="color: grey">(e0410e4)</span></li>
<li>plugin.cpp <span style="color: grey">(3cd2448)</span></li>
<li>statjob.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>statjob.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>views/cachegrindview.cpp <span style="color: grey">(591f217)</span></li>
<li>views/callgrindview.cpp <span style="color: grey">(6aa551d)</span></li>
<li>views/massifview.cpp <span style="color: grey">(7bb275f)</span></li>
<li>views/memcheckview.cpp <span style="color: grey">(95961fb)</span></li>
<li>widget.cpp <span style="color: grey">(18eacf7)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/122905/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>