Fix and improve move detection.

Review Request #4921 — Created Nov. 8, 2013 and submitted


Review Board


Fix and improve move detection.

Move detection had some issues that were a result of a stale variable
being accessed. When setting metadata, a previous variable was used,
instead of the instance we were currently operating on. This led to a
bunch of move information getting tacked onto the wrong line and erasing
things. That's been fixed, so move information can be better trusted.

Move detection was also picking up lots of little lines we really didn't
care about. It now has a threshold. It must be at least 2 lines of code,
or 1 line at least 20 characters in length. We may decide to tweak these
later on.

There was also an off-by-one with move ranges. If a range ended with a
line that wasn't considered interesting enough (< 4 characters), but the
range otherwise passed validation, that uninteresting line would be
included. This isn't always bad (might catch an ending brace, for
example), but it's not intended, and shouldn't be provided unless we
design for it.

I also made the move flags more consistent between the left-hand-side
and right-hand-side of the diff viewer. Both the "moved from" and "moved
to" flags are now anchored on the left of their side, so it's easier to
see what lines they span. Basically, I'm applying the new changes
recently made for the right-hand side to the left.

Unit tests pass.

Tested with the diff on /r/4919, which was full of junk and broken
move information before. It looks reasonable now.

Description From Last Updated

Care to add a negative case to this?

  1. Can you include a screenshot of the UI changes?

  2. reviewboard/diffviewer/ (Diff revision 1)
    Show all issues

    Care to add a negative case to this?

  1. Ship It!
Review request changed

Status: Closed (submitted)