Review Request: Add interactivity to diff highlighting

Wed Jan 26 10:40:43 UTC 2011

> On Jan. 25, 2011, 11:45 p.m., David Nolden wrote:
> > 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.

> 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.

- Dmitry

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

On Jan. 25, 2011, 12:20 p.m., Dmitry Risenberg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100440/
> -----------------------------------------------------------
> 
> (Updated Jan. 25, 2011, 12:20 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
> -----
> 
>   plugins/patchreview/CMakeLists.txt f92bcd0f9fdbfcb8ca7ef83daf695d79bbba7b31 
>   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/komparemodellist.h 576c5c11237d62f8b5c559f257110b4f26542752 
>   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/patchreview.h c517f34c151a0813522f31d6843939abe1d03055 
>   plugins/patchreview/patchreview.cpp 492522691ebb76b28f970660eea68461dc605ad0 
>   plugins/patchreview/tests/CMakeLists.txt PRE-CREATION 
>   plugins/patchreview/tests/levenshteintest.h PRE-CREATION 
>   plugins/patchreview/tests/levenshteintest.cpp PRE-CREATION 
>   plugins/patchreview/tests/patchreviewtest.h PRE-CREATION 
>   plugins/patchreview/tests/patchreviewtest.cpp PRE-CREATION 
> 
> 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/20110126/57710a9a/attachment.html>