• 
      

    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:
    Completed
    Change Summary:
    Pushed to release-4.0.x (cc6a91f)