<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/121586/">https://git.reviewboard.kde.org/r/121586/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 18th, 2014, 2:09 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;">This breaks the integration with libkomparediff2, doesn't it?</p></pre>
</blockquote>
<p>On December 18th, 2014, 7:47 a.m. CET, <b>René J.V. Bertin</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;">Apart from the hunk count issue I mentioned above, I haven't seen anything that's off - at least not when compared with kdevelop kf5. <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Maybe</em> that changes are no longer highlighted in the review plugin, now that you mention it (no time to go check right now).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What exactly is libkomparediff2 used for, if not for creating the patchfile itself?</p></pre>
</blockquote>
<p>On December 18th, 2014, 1:03 p.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;">No, as you can see in the patch you are submitting, the patch is generated by the vcs plugin.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">komparediff2 is used to analyze the generated patch and provide the highlighting.</p></pre>
</blockquote>
<p>On December 18th, 2014, 2:30 p.m. CET, <b>René J.V. Bertin</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;">And the hunk count as well?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Is it also komparediff2 that extracts the file paths from the patch? I've tried to follow execution flow, but with the inheritance going on it's very hard to figure out where it goes if not to the base classes...</p></pre>
</blockquote>
<p>On December 18th, 2014, 2:44 p.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;">Yes, hunk count as well. KDevelop itself doesn't do any of the patch analysis.</p></pre>
</blockquote>
<p>On December 18th, 2014, 2:46 p.m. CET, <b>René J.V. Bertin</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;">Ok, then that explains why there isn't a hunk count nor highlighting in KDevelop KF5 either ...</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This may be naive, but</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%"> m_kompareInfo<span style="color: #666666">-></span>localSource <span style="color: #666666">=</span> m_patch<span style="color: #666666">-></span>baseDir().toLocalFile();
m_kompareInfo<span style="color: #666666">-></span>depth <span style="color: #666666">=</span> m_patch<span style="color: #666666">-></span>depth();
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Shouldn't these be updated to something like</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%"> m_kompareInfo<span style="color: #666666">-></span>localSource <span style="color: #666666">=</span> m_patch<span style="color: #666666">-></span>baseDir().upURL().toLocalFile();
m_kompareInfo<span style="color: #666666">-></span>depth <span style="color: #666666">=</span> m_patch<span style="color: #666666">-></span>depth() <span style="color: #666666">+</span> <span style="color: #666666">1</span>;
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">?</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;">Answer: indeed that restores the hunk count and highlighting.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The question now becomes: is this the right place to make that modification, or if not, of what class is m_patch an instance when doing a git patch review? I <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">think</em> I deduced it was a VCSDiffPatchSource, but how to be sure?</p></pre>
<br />
<p>- René J.V.</p>
<br />
<p>On December 17th, 2014, 11:25 p.m. CET, René J.V. Bertin 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 Software on Mac OS X and KDevelop.</div>
<div>By René J.V. Bertin.</div>
<p style="color: grey;"><i>Updated Dec. 17, 2014, 11:25 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdevplatform
</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;">A change was made on git.reviewboard.kde.org on December 13th a bit before 22h Europe/Paris time, ending support for p0 patch files. This broke the upload functionality in the current/stable version KDevelop's patchreview plugin.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The patch under review is a workaround (<code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">*</code>) that restores this functionality by invoking the git helper process which generates the patchfiles with the correct arguments.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">*</code>) The real bug is in the reviewboard software, and should be addressed at that level. I work with a software distribution system that requires p0 patches, for which I created patches with KDevelop, something which will now require an additional step.</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;">Under OS X 10.9; this RR has been created from a patched copy of KDevelop 4.7 .</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>plugins/git/gitplugin.cpp <span style="color: grey">(f38dc71)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/121586/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>