Review Request: Add interactivity to diff highlighting

[David Nolden]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/100440/#review1155
-----------------------------------------------------------

Ship it!


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.

- David


On Jan. 31, 2011, 11:26 p.m., Dmitry Risenberg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100440/
> -----------------------------------------------------------
> 
> (Updated Jan. 31, 2011, 11:26 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Summary
> -------
> 
> 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 at git.kde.org:clones/kdevplatform/risenberg/kdevplatform, branch 'review'.
> 
> Upd. This patch depends on latest bugfixes to Kate main branch (up to fd7fcf2d6d368fccdc0a28326516f85ac8e120e2).
> 
> 
> Diffs
> -----
> 
>   interfaces/ipatchsource.h ce7a98229be1527bc9feb4396fad677dedccddbc 
>   interfaces/ipatchsource.cpp a26a14a7dd958c5833fccbf2ca681d2021181aa4 
>   plugins/patchreview/CMakeLists.txt f92bcd0f9fdbfcb8ca7ef83daf695d79bbba7b31 
>   plugins/patchreview/libdiff2/CMakeLists.txt PRE-CREATION 
>   plugins/patchreview/libdiff2/difference.h 00b3c5469168337bbbbe116b051e1d0976b5e46f 
>   plugins/patchreview/libdiff2/difference.cpp 10a55aa3b3609f4f0fb5d67cc004884b79ad173e 
>   plugins/patchreview/libdiff2/differencestringpair.h PRE-CREATION 
>   plugins/patchreview/libdiff2/diffhunk.cpp 97c79673dbe4e1de5176d20395d6092489a9d1b0 
>   plugins/patchreview/libdiff2/diffmodel.h 0d7a20210437070c0f621b85714d1be51d846a00 
>   plugins/patchreview/libdiff2/diffmodel.cpp acaadb5035e8a3d011068fdda79aee4f45a27a3a 
>   plugins/patchreview/libdiff2/kompare.h febd5e57b3672e6f283d3b209d4929baf1030a0a 
>   plugins/patchreview/libdiff2/komparemodellist.h 576c5c11237d62f8b5c559f257110b4f26542752 
>   plugins/patchreview/libdiff2/komparemodellist.cpp 86e910f99e9c0b9fecdbd42fe160916199279e12 
>   plugins/patchreview/libdiff2/levenshteintable.h e57c4723ac6f1c506d168858be32271eec0526c9 
>   plugins/patchreview/libdiff2/levenshteintable.cpp ffc9e3de5a252c36d605d46d6ece4beaf958a8cc 
>   plugins/patchreview/libdiff2/marker.h PRE-CREATION 
>   plugins/patchreview/libdiff2/stringlistpair.h PRE-CREATION 
>   plugins/patchreview/libdiff2/stringlistpair.cpp PRE-CREATION 
>   plugins/patchreview/libdiff2/tests/CMakeLists.txt PRE-CREATION 
>   plugins/patchreview/libdiff2/tests/interactivedifftest.h PRE-CREATION 
>   plugins/patchreview/libdiff2/tests/interactivedifftest.cpp PRE-CREATION 
>   plugins/patchreview/libdiff2/tests/levenshteintest.h PRE-CREATION 
>   plugins/patchreview/libdiff2/tests/levenshteintest.cpp PRE-CREATION 
>   plugins/patchreview/localpatchsource.h 26bafde6ad836c1e2be293c31a1fb266e8626973 
>   plugins/patchreview/patchreview.h c517f34c151a0813522f31d6843939abe1d03055 
>   plugins/patchreview/patchreview.cpp 492522691ebb76b28f970660eea68461dc605ad0 
>   plugins/patchreview/patchreview.ui ee75f2e732ed13a7ac19eb6cad8ff83e682e3c96 
> 
> Diff: http://git.reviewboard.kde.org/r/100440/diff
> 
> 
> Testing
> -------
> 
> Unit tests for the algorithmic part and some manual testing.
> 
> 
> Thanks,
> 
> Dmitry
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20110202/5330e665/attachment.html>