<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/100440/">http://git.reviewboard.kde.org/r/100440/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 25th, 2011, 11:45 p.m., <b>David Nolden</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;">Interesting work, I will review this thoroughly (not now though).
However, the assumption that the edited side is always the "destination" side does not always hold. For example, you might review have a VCS diff which someone else has produced, and which currently is _not_ applied, then you will be editing the "source" side. It's not a desaster if the highlighting is not updated nicely then, because the diff won't be applyable anymore anyway in the affected areas. The highlighting-ranges and sidebar symbols should stay alive though in such a situation, because the user migth manually be applying a specific portion of the diff, and still wants to know what was inserted where.
Basically, we have to make sure that this breaks nothing.
Regarding the basic problem, I think we also need an "Update" button, which can ask the VCS for the _new_ diff and somewhat fluidly updates the views, though this is a different thing for the future.</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;">> For example, you might review have a VCS diff which someone else has produced, and which currently is _not_ applied...
Could you explain how to do this in KDevelop, so that I can also test this case.</pre>
<br />
<p>- Dmitry</p>
<br />
<p>On January 25th, 2011, 12:20 p.m., Dmitry Risenberg wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDevelop.</div>
<div>By Dmitry Risenberg.</div>
<p style="color: grey;"><i>Updated Jan. 25, 2011, 12:20 p.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;">This patch adds interactivity to Review area - whenever you change text in a reviewed document, the highlighting updates itself to match the new difference. If a line is manually reverted to the state it is in the source, highlighting is removed from this line. The markers to the left of the lines are also updated as the set of lines affected by the difference changes.
The patch tries to preserve "reverted" (by clicking on them) differences when changes are made in adjacent lines, though once changes are made inside the "reverted" area, it loses highlighting and is treated as plain text.
Diffs are updated by dynamically computing the Levenshtein distance between the newly inserted/deleted lines and their source counterparts (LevenshteinTable class is turned into a template). This plain algorithm seems suitable because big volumes of text aren't expected to be inserted interactively - latency only observable for 200+ lines insertion from clipboard on my machine.
Probable issues:
1) libdiff2 has a notion of hunks (DiffHunk) beside differences (Difference) - the former come from the hunks if .diff file and are not updated in any way when the diff changes. This is not a problem now, for the DiffHunks aren't used anywhere in KDevelop, but might become one possibly.
2) The patch assumes that changes are always made to the destination side of the patch. It is so for the use cases I saw (differences from version control plugins), though not sure that it's always so - expect someone with better overall knowledge to clarify on that.
The code may be checked from git@git.kde.org:clones/kdevplatform/risenberg/kdevplatform, branch 'review'.
Upd. This patch depends on latest bugfixes to Kate main branch (up to fd7fcf2d6d368fccdc0a28326516f85ac8e120e2).</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;">Unit tests for the algorithmic part and some manual testing.</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/patchreview/CMakeLists.txt <span style="color: grey">(f92bcd0f9fdbfcb8ca7ef83daf695d79bbba7b31)</span></li>
<li>plugins/patchreview/libdiff2/difference.h <span style="color: grey">(00b3c5469168337bbbbe116b051e1d0976b5e46f)</span></li>
<li>plugins/patchreview/libdiff2/difference.cpp <span style="color: grey">(10a55aa3b3609f4f0fb5d67cc004884b79ad173e)</span></li>
<li>plugins/patchreview/libdiff2/differencestringpair.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/patchreview/libdiff2/diffhunk.cpp <span style="color: grey">(97c79673dbe4e1de5176d20395d6092489a9d1b0)</span></li>
<li>plugins/patchreview/libdiff2/diffmodel.h <span style="color: grey">(0d7a20210437070c0f621b85714d1be51d846a00)</span></li>
<li>plugins/patchreview/libdiff2/diffmodel.cpp <span style="color: grey">(acaadb5035e8a3d011068fdda79aee4f45a27a3a)</span></li>
<li>plugins/patchreview/libdiff2/komparemodellist.h <span style="color: grey">(576c5c11237d62f8b5c559f257110b4f26542752)</span></li>
<li>plugins/patchreview/libdiff2/levenshteintable.h <span style="color: grey">(e57c4723ac6f1c506d168858be32271eec0526c9)</span></li>
<li>plugins/patchreview/libdiff2/levenshteintable.cpp <span style="color: grey">(ffc9e3de5a252c36d605d46d6ece4beaf958a8cc)</span></li>
<li>plugins/patchreview/libdiff2/marker.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/patchreview/libdiff2/stringlistpair.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/patchreview/libdiff2/stringlistpair.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/patchreview/patchreview.h <span style="color: grey">(c517f34c151a0813522f31d6843939abe1d03055)</span></li>
<li>plugins/patchreview/patchreview.cpp <span style="color: grey">(492522691ebb76b28f970660eea68461dc605ad0)</span></li>
<li>plugins/patchreview/tests/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/patchreview/tests/levenshteintest.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/patchreview/tests/levenshteintest.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/patchreview/tests/patchreviewtest.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/patchreview/tests/patchreviewtest.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/100440/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>