Redesign, improve, and fix support for getting changed chunk ranges in diffs.
Review Request #8834 — Created March 22, 2017 and submitted
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).