diff --git a/reviewboard/diffviewer/processors.py b/reviewboard/diffviewer/processors.py index a0f9133754626e941b334035fe53e70a44089351..82699ffb76bd97ac09e41a6b420dd65fc94ccd3d 100644 --- a/reviewboard/diffviewer/processors.py +++ b/reviewboard/diffviewer/processors.py @@ -9,13 +9,6 @@ CHUNK_RANGE_RE = re.compile( re.M) -# Number of lines of context we assume we have in uploaded diffs. This number -# may vary between diffs, but only if people go out of their way to change it -# when generating the diff. In practice, we can trust this value in 99.9% of -# the cases. -CHUNK_RANGE_ASSUMED_CONTEXT_LEN = 3 - - def filter_interdiff_opcodes(opcodes, filediff_data, interfilediff_data): """Filters the opcodes for an interdiff to remove unnecessary lines. @@ -28,20 +21,40 @@ def filter_interdiff_opcodes(opcodes, filediff_data, interfilediff_data): ranges of lines dictated in the uploaded diff files. """ def _find_range_info(diff): + lines = diff.splitlines() + process_changes = False ranges = [] - for m in CHUNK_RANGE_RE.finditer(diff): - new_start = int(m.group('new_start')) - new_len = int(m.group('new_len')) or 1 - - if new_len >= 0: - new_end = new_start + new_len - - # We reduce by 1 because the chunk ranges in diffs start at 1. - ranges.append(( - new_start - 1 + CHUNK_RANGE_ASSUMED_CONTEXT_LEN, - new_end - 1 + CHUNK_RANGE_ASSUMED_CONTEXT_LEN - )) + # Look through the chunks of the diff, trying to find the amount + # of context shown at the beginning of each chunk. Though this + # will usually be 3 lines, it may be fewer or more, depending + # on file length and diff generation settings. + for line in lines: + if process_changes: + if line.startswith(('-', '+')): + # We've found the first change in the chunk. We now + # know how many lines of context we have. + # + # We reduce the indexes by 1 because the chunk ranges + # in diffs start at 1, and we want a 0-based index. + start = chunk_start - 1 + lines_of_context + ranges.append((start, start + chunk_len)) + process_changes = False + continue + else: + lines_of_context += 1 + + # This was not a change within a chunk, or we weren't processing, + # so check to see if this is a chunk header instead. + m = CHUNK_RANGE_RE.match(line) + + if m: + # It is a chunk header. Reset the state for the next range, + # and pull the line number and length from the header. + chunk_start = int(m.group('new_start')) + chunk_len = int(m.group('new_len') or '1') + process_changes = True + lines_of_context = 0 return ranges diff --git a/reviewboard/diffviewer/tests.py b/reviewboard/diffviewer/tests.py index 1771784f6322e3b0b44e4363eb58f6529ef2a3a9..e397306bf66644f6ed1cfadb324011d9755bb837 100644 --- a/reviewboard/diffviewer/tests.py +++ b/reviewboard/diffviewer/tests.py @@ -908,12 +908,18 @@ class ProcessorsTests(TestCase): ('insert', 40, 40, 40, 45), ] - # NOTE: Only the "@@" lines in the diff matter below for this - # processor, so the rest can be left out. - orig_diff = '@@ -22,7 +22,7 @@\n' + # NOTE: Only the "@@" lines and the lines leading up to the first + # change in a chunk matter to the processor, so the rest can + # be left out. + orig_diff = ( + '@@ -22,7 +22,7 @@\n' + ' #\n #\n #\n-#\n' + ) new_diff = ( '@@ -2,11 +2,6 @@\n' + ' #\n #\n #\n-#\n' '@@ -22,7 +22,7 @@\n' + ' #\n #\n #\n-#\n' ) new_opcodes = list(filter_interdiff_opcodes(opcodes, orig_diff, @@ -929,6 +935,56 @@ class ProcessorsTests(TestCase): ('equal', 40, 40, 40, 45), ]) + def test_filter_interdiff_opcodes_1_line(self): + """Testing filter_interdiff_opcodes with a 1 line file""" + opcodes = [ + ('replace', 0, 1, 0, 1), + ] + + # NOTE: Only the "@@" lines and the lines leading up to the first + # change in a chunk matter to the processor, so the rest can + # be left out. + orig_diff = ( + '@@ -0,0 +1 @@\n' + '+#\n' + ) + new_diff = ( + '@@ -0,0 +1 @@\n' + '+##\n' + ) + + new_opcodes = list(filter_interdiff_opcodes(opcodes, orig_diff, + new_diff)) + + self.assertEqual(new_opcodes, [ + ('replace', 0, 1, 0, 1), + ]) + + def test_filter_interdiff_opcodes_early_change(self): + """Testing filter_interdiff_opcodes with a change early in the file""" + opcodes = [ + ('replace', 2, 3, 2, 3), + ] + + # NOTE: Only the "@@" lines and the lines leading up to the first + # change in a chunk matter to the processor, so the rest can + # be left out. + orig_diff = ( + '@@ -1,5 +1,5 @@\n' + ' #\n#\n+#\n' + ) + new_diff = ( + '@@ -1,5 +1,5 @@\n' + ' #\n#\n+#\n' + ) + + new_opcodes = list(filter_interdiff_opcodes(opcodes, orig_diff, + new_diff)) + + self.assertEqual(new_opcodes, [ + ('replace', 2, 3, 2, 3), + ]) + def test_filter_interdiff_opcodes_with_inserts_right(self): """Testing filter_interdiff_opcodes with inserts on the right""" # These opcodes were taken from the r1-r2 interdiff at @@ -945,10 +1001,17 @@ class ProcessorsTests(TestCase): ('equal', 190, 232, 197, 239), ] - # NOTE: Only the "@@" lines in the diff matter below for this - # processor, so the rest can be left out. - orig_diff = '@@ -0,0 +1,232 @@\n' - new_diff = '@@ -0,0 +1,239 @@\n' + # NOTE: Only the "@@" lines and the lines leading up to the first + # change in a chunk matter to the processor, so the rest can + # be left out. + orig_diff = ( + '@@ -0,0 +1,232 @@\n' + ' #\n #\n #\n+#\n' + ) + new_diff = ( + '@@ -0,0 +1,239 @@\n' + ' #\n #\n #\n+#\n' + ) new_opcodes = list(filter_interdiff_opcodes(opcodes, orig_diff, new_diff)) @@ -976,24 +1039,37 @@ class ProcessorsTests(TestCase): ('equal', 632, 882, 633, 883), ] - # NOTE: Only the "@@" lines in the diff matter below for this - # processor, so the rest can be left out. - orig_diff = ( + # NOTE: Only the "@@" lines and the lines leading up to the first + # change in a chunk matter to the processor, so the rest can + # be left out. + orig_diff = '\n'.join([ '@@ -413,6 +413,8 @@\n' + ' #\n #\n #\n+#\n' '@@ -422,9 +424,13 @@\n' + ' #\n #\n #\n+#\n' '@@ -433,6 +439,8 @@\n' + ' #\n #\n #\n+#\n' '@@ -442,6 +450,9 @@\n' + ' #\n #\n #\n+#\n' '@@ -595,6 +605,205 @@\n' + ' #\n #\n #\n+#\n' '@@ -636,6 +845,36 @@\n' - ) - new_diff = ( + ' #\n #\n #\n+#\n' + ]) + new_diff = '\n'.join([ '@@ -413,6 +413,8 @@\n' + ' #\n #\n #\n+#\n' '@@ -422,9 +424,13 @@\n' + ' #\n #\n #\n+#\n' '@@ -433,6 +439,8 @@\n' + ' #\n #\n #\n+#\n' '@@ -442,6 +450,8 @@\n' + ' #\n #\n #\n+#\n' '@@ -595,6 +605,206 @@\n' + ' #\n #\n #\n+#\n' '@@ -636,6 +846,36 @@\n' - ) + ' #\n #\n #\n+#\n' + ]) new_opcodes = list(filter_interdiff_opcodes(opcodes, orig_diff, new_diff))