<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 />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The changes are too large to review them really thoroughly, but I didn't find any obvious issues. Since you say you've tested it, I say ship it. Possible remaining issues can be worked out in master.</pre>
<br />
<p>- David</p>
<br />
<p>On January 31st, 2011, 11:26 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. 31, 2011, 11:26 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>interfaces/ipatchsource.h <span style="color: grey">(ce7a98229be1527bc9feb4396fad677dedccddbc)</span></li>
<li>interfaces/ipatchsource.cpp <span style="color: grey">(a26a14a7dd958c5833fccbf2ca681d2021181aa4)</span></li>
<li>plugins/patchreview/CMakeLists.txt <span style="color: grey">(f92bcd0f9fdbfcb8ca7ef83daf695d79bbba7b31)</span></li>
<li>plugins/patchreview/libdiff2/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</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/kompare.h <span style="color: grey">(febd5e57b3672e6f283d3b209d4929baf1030a0a)</span></li>
<li>plugins/patchreview/libdiff2/komparemodellist.h <span style="color: grey">(576c5c11237d62f8b5c559f257110b4f26542752)</span></li>
<li>plugins/patchreview/libdiff2/komparemodellist.cpp <span style="color: grey">(86e910f99e9c0b9fecdbd42fe160916199279e12)</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/libdiff2/tests/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/patchreview/libdiff2/tests/interactivedifftest.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/patchreview/libdiff2/tests/interactivedifftest.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/patchreview/libdiff2/tests/levenshteintest.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/patchreview/libdiff2/tests/levenshteintest.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/patchreview/localpatchsource.h <span style="color: grey">(26bafde6ad836c1e2be293c31a1fb266e8626973)</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/patchreview.ui <span style="color: grey">(ee75f2e732ed13a7ac19eb6cad8ff83e682e3c96)</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>