Redesign, improve, and fix support for getting changed chunk ranges in diffs.

Review Request #8834 — Created March 22, 2017 and submitted

Information

Review Board
release-2.0.x
4459b7e...

Reviewers

The code that attempts to filter out unwanted changes for interdiffs
relies on a parser for diff data that seeks out ranges of modifications,
factoring in headers, deleted and inserted lines, and the number of
lines of context in a diff. The existing code parsed the diff in such a
way as to meet the needs of the interdiff filtering code at the time it
was written, and was a private function within it. That made the code
harder to change and test, and it turned out to be flawed in how it
performed some calculations.

This change replaces it with a more generic, testable version. Unlike
the previous version, which tried to generate ranges corresponding to
the modified version of the file (but taking into consideration ranges
in the original file), the new function is built to generate a wider set
of information about each range independently. It returns a list of
dictionaries containing information about the original and modified
ranges, including data from the headers and ranges for the changed
sections within each chunk.

This is a diff parser, but is not designed to be as complex as the main
diff parser we use for splitting up diffs and gathering useful data.
Down the road, it may make sense to merge these, but that would be
overkill for now.

This new version does correct a major flaw in the original, which is
that the lines of context after a change would be set to 0 when
preceding another section. This resulted in larger ranges, which caused
problems in some of our tests (which we inadvertently worked around --
recently caught in a recent change), and may have caused interdiff
algorithm issues.

The existing interdiff algorithm's been updated to use this new method
to generate the ranges it uses. This helped to highlight some problems
in the way we generate and use ranges, which will help with the
interdiff fixes that are in progress.

Unit tests pass.

Tested a handful of interdiffs. Didn't see any changes in interdiff
output from before this change (though further tests will be run before
this change ends up going in).

reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/diffviewer/tests/test_diffutils.py
        reviewboard/diffviewer/processors.py
        reviewboard/diffviewer/tests/test_processors.py
        reviewboard/diffviewer/diffutils.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/diffviewer/tests/test_diffutils.py
        reviewboard/diffviewer/processors.py
        reviewboard/diffviewer/tests/test_processors.py
        reviewboard/diffviewer/diffutils.py
    
    
  2. 
      
david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-4.0.x (cc6a91f)
Loading...