Filter unwanted upstream lines from interdiffs.

Review Request #4141 — Created May 12, 2013 and submitted

Information

Review Board
release-1.7.x

Reviewers

Filter unwanted upstream lines from interdiffs.

Interdiffs had a long-standing problem where, after merging, any
upstream changes from a subsequent diff would get rolled up into the
display of the diff. This was annoying particularly for very active
trees, and made it hard to see exactly what was changing.

We now filter these from the interdiffs. We start off by parsing both
diff files and find the line ranges (the "@@" lines). We can then use
that information to completely filter out changes that aren't in the
ranges.

To get as close as possible to what the user actually uploaded, we
strip away some additional lines from those ranges. By default, all diff
programs we've seen generate 3 lines of context to make the diff more
readable, meaning that the ranges are extended by 6 lines on either
side. We just reduce the ranges by 6, ensuring that only the modified
lines are considered for display.

Any lines that aren't to be displayed as inserts, deletes or replaces
get turned into "equal" lines. The user may notice that they're not in
fact equal (especially when it's deleted/inserted lines), but that
should be fine as the point is to get them to focus on the changes from
the diff itself, not the upstream changes.

It's still possible that some additional upstream lines will make it in,
but only when they're right inbetween some modified lines within the
same hunk of the diff.
Tested with some changes locally, and some changes pulled down from
diffs on reviews.reviewboard.org.

Unit tests pass.
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        reviewboard/diffviewer/processors.py
        reviewboard/diffviewer/tests.py
        reviewboard/diffviewer/diffutils.py
      Ignored Files:
    
    
  2. 
      
chipx86
david
  1. Is this really something we want to put on release-1.7.x? I wouldn't want a big regression...
    1. Wasn't sure. I can move it to master.
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (8e5a4b7)
Loading...