diff --git a/reviewboard/diffviewer/opcode_generator.py b/reviewboard/diffviewer/opcode_generator.py index 0cc8b2418e7889832c7794ba3d321b5f2f577f93..5e7549b7d19e7da382be8941b035851203f7d388 100644 --- a/reviewboard/diffviewer/opcode_generator.py +++ b/reviewboard/diffviewer/opcode_generator.py @@ -7,8 +7,7 @@ from django.utils import six from django.utils.six.moves import range from reviewboard.diffviewer.processors import (filter_interdiff_opcodes, - merge_adjacent_chunks, - post_process_interdiff_chunks) + post_process_filtered_equals) class DiffOpcodeGenerator(object): @@ -58,10 +57,6 @@ class DiffOpcodeGenerator(object): opcodes = filter_interdiff_opcodes(opcodes, self.filediff.diff, self.interfilediff.diff) - # From the filtered content, we may have ended up with consecutive - # "equal" chunks, so merge them. - opcodes = merge_adjacent_chunks(opcodes) - for opcode in opcodes: yield opcode @@ -114,7 +109,7 @@ class DiffOpcodeGenerator(object): # "filtered-equal" chunks. This allowed us to skip any additional # processing, particularly the indentation highlighting. It's # now time to turn those back into "equal" chunks. - opcodes = post_process_interdiff_chunks(opcodes) + opcodes = post_process_filtered_equals(opcodes) for opcode in opcodes: yield opcode diff --git a/reviewboard/diffviewer/processors.py b/reviewboard/diffviewer/processors.py index 354c7425cefc4b710745bc9fa14f466217927637..9edf2c7e9cd0613bf56db8e265adb02e5e083b94 100644 --- a/reviewboard/diffviewer/processors.py +++ b/reviewboard/diffviewer/processors.py @@ -62,6 +62,11 @@ def filter_interdiff_opcodes(opcodes, filediff_data, interfilediff_data): return ranges + def _is_range_valid(line_range, tag, i1, i2): + return (line_range is not None and + i1 >= line_range[0] and + (tag == 'delete' or i1 != i2)) + orig_ranges = _find_range_info(filediff_data) new_ranges = _find_range_info(interfilediff_data) @@ -109,14 +114,39 @@ def filter_interdiff_opcodes(opcodes, filediff_data, interfilediff_data): # See if the chunk we're looking at is in the range of the chunk in # one of the uploaded diffs. If so, allow it through. - valid_chunk = ( - (orig_range is not None and - (tag == 'delete' or i1 != i2) and - (i1 >= orig_range[0] and i2 <= orig_range[1])) or - (new_range is not None and - (tag == 'delete' or j1 != j2) and - (j1 >= new_range[0] and j2 <= new_range[1])) - ) + orig_starts_valid = _is_range_valid(orig_range, tag, i1, i2) + new_starts_valid = _is_range_valid(new_range, tag, j1, j2) + valid_chunk = orig_starts_valid or new_starts_valid + + if valid_chunk: + # This chunk is valid. It may only be a portion of the real + # chunk, though. We'll need to split it up into a known valid + # segment first, and yield that. + if orig_starts_valid: + valid_i2 = min(i2, orig_range[1] + 1) + else: + valid_i2 = i2 + + if new_starts_valid: + valid_j2 = min(j2, new_range[1] + 1) + else: + valid_j2 = j2 + + yield tag, i1, valid_i2, j1, valid_j2 + + if valid_i2 == i2 and valid_j2 == j2: + continue + + # There were more parts of this range remaining. We know they're + # all invalid, so let's update i1 and j1 to point to the start + # of those invalid ranges, and mark them. + if orig_range is not None and i2 > orig_range[1]: + i1 = orig_range[1] + 1 + + if new_range is not None and j2 > new_range[1]: + j1 = new_range[1] + 1 + + valid_chunk = False if not valid_chunk: # Turn this into an "filtered-equal" chunk. The left-hand and @@ -126,40 +156,43 @@ def filter_interdiff_opcodes(opcodes, filediff_data, interfilediff_data): # # These will get turned back into "equal" chunks in the # post-processing step. - tag = 'filtered-equal' + yield 'filtered-equal', i1, i2, j1, j2 - yield tag, i1, i2, j1, j2 +def post_process_filtered_equals(opcodes): + """Post-processes filtered-equal and equal chunks from interdiffs. -def merge_adjacent_chunks(opcodes): - """Merges adjacent chunks of the same tag. + Any filtered-out "filtered-equal" chunks will get turned back into "equal" + chunks and merged into any prior equal chunks. Likewise, simple "equal" + chunks will also get merged. - This will take any chunks that have the same tag (such as two "equal" - chunks) and merge them together. + "equal" chunks that have any indentation information will remain + their own chunks, with nothing merged in. """ cur_chunk = None - for tag, i1, i2, j1, j2 in opcodes: - if cur_chunk and cur_chunk[0] == tag: - cur_chunk = (tag, cur_chunk[1], i2, cur_chunk[3], j2) + for tag, i1, i2, j1, j2, meta in opcodes: + if ((tag == 'equal' and not meta.get('indentation_changes')) or + tag == 'filtered-equal'): + # We either have a plain equal chunk without any indentation + # changes, or a filtered-equal chunk. In these cases, we can + # safely merge the chunks together and transform them into + # an "equal" chunk. + if cur_chunk: + i1 = cur_chunk[1] + j1 = cur_chunk[3] + meta = cur_chunk[5] + + cur_chunk = ('equal', i1, i2, j1, j2, meta) else: + # This is some sort of changed chunk (insert, delete, replace, + # or equal with indentation changes). Yield the previous chunk + # we were working with, if any, and then yield the current chunk. if cur_chunk: yield cur_chunk + cur_chunk = None - cur_chunk = (tag, i1, i2, j1, j2) + yield tag, i1, i2, j1, j2, meta if cur_chunk: yield cur_chunk - - -def post_process_interdiff_chunks(opcodes): - """Post-processes interdiff chunks. - - Any filtered-out "filtered-equal" chunks will get turned back into "equal" - chunks. - """ - for tag, i1, i2, j1, j2, meta in opcodes: - if tag == 'filtered-equal': - tag = 'equal' - - yield tag, i1, i2, j1, j2, meta diff --git a/reviewboard/diffviewer/tests.py b/reviewboard/diffviewer/tests.py index d91475ab8a8f27c65a5299f5669f847767a4ef56..3f6557849e3700ecdb95ac4eb9ff1498e0fd8ca2 100644 --- a/reviewboard/diffviewer/tests.py +++ b/reviewboard/diffviewer/tests.py @@ -21,7 +21,7 @@ from reviewboard.diffviewer.myersdiff import MyersDiffer from reviewboard.diffviewer.opcode_generator import get_diff_opcode_generator from reviewboard.diffviewer.renderers import DiffRenderer from reviewboard.diffviewer.processors import (filter_interdiff_opcodes, - merge_adjacent_chunks) + post_process_filtered_equals) from reviewboard.diffviewer.templatetags.difftags import highlightregion from reviewboard.scmtools.models import Repository, Tool from reviewboard.testing import TestCase @@ -1007,9 +1007,11 @@ class ProcessorsTests(TestCase): ('filtered-equal', 0, 0, 0, 1), ('filtered-equal', 0, 5, 1, 5), ('delete', 5, 10, 5, 5), - ('filtered-equal', 10, 25, 5, 20), + ('equal', 10, 25, 5, 11), + ('filtered-equal', 10, 25, 11, 20), ('replace', 25, 26, 20, 26), - ('filtered-equal', 26, 40, 26, 40), + ('equal', 26, 32, 26, 32), + ('filtered-equal', 32, 40, 32, 40), ('filtered-equal', 40, 40, 40, 45), ]) @@ -1156,30 +1158,98 @@ class ProcessorsTests(TestCase): ('filtered-equal', 0, 631, 0, 631), ('replace', 631, 632, 631, 632), ('insert', 632, 632, 632, 633), - ('filtered-equal', 632, 882, 633, 883), + ('equal', 632, 813, 633, 814), + ('filtered-equal', 813, 882, 814, 883), ]) - def test_merge_adjacent_chunks(self): - """Testing merge_adjacent_chunks""" + def test_post_process_filtered_equals(self): + """Testing post_process_filtered_equals""" opcodes = [ - ('equal', 0, 0, 0, 1), - ('equal', 0, 5, 1, 5), - ('delete', 5, 10, 5, 5), - ('equal', 10, 25, 5, 20), - ('replace', 25, 26, 20, 26), - ('equal', 26, 40, 26, 40), - ('equal', 40, 40, 40, 45), + ('equal', 0, 10, 0, 10, {}), + ('insert', 10, 20, 0, 10, {}), + ('equal', 20, 30, 10, 20, {}), + ('equal', 30, 40, 20, 30, {}), + ('filtered-equal', 40, 50, 30, 40, {}), ] - new_opcodes = list(merge_adjacent_chunks(opcodes)) + new_opcodes = list(post_process_filtered_equals(opcodes)) - self.assertEqual(new_opcodes, [ - ('equal', 0, 5, 0, 5), - ('delete', 5, 10, 5, 5), - ('equal', 10, 25, 5, 20), - ('replace', 25, 26, 20, 26), - ('equal', 26, 40, 26, 45), - ]) + self.assertEqual( + new_opcodes, + [ + ('equal', 0, 10, 0, 10, {}), + ('insert', 10, 20, 0, 10, {}), + ('equal', 20, 50, 10, 40, {}), + ]) + + def test_post_process_filtered_equals_with_indentation(self): + """Testing post_process_filtered_equals with indentation changes""" + opcodes = [ + ('equal', 0, 10, 0, 10, {}), + ('insert', 10, 20, 0, 10, {}), + ('equal', 20, 30, 10, 20, { + 'indentation_changes': { + '21-11': (True, 4), + } + }), + ('equal', 30, 40, 20, 30, {}), + ('filtered-equal', 30, 50, 20, 40, {}), + ] + + new_opcodes = list(post_process_filtered_equals(opcodes)) + + self.assertEqual( + new_opcodes, + [ + ('equal', 0, 10, 0, 10, {}), + ('insert', 10, 20, 0, 10, {}), + ('equal', 20, 30, 10, 20, { + 'indentation_changes': { + '21-11': (True, 4), + } + }), + ('equal', 30, 50, 20, 40, {}), + ]) + + def test_post_process_filtered_equals_with_adjacent_indentation(self): + """Testing post_process_filtered_equals with + adjacent indentation changes + """ + opcodes = [ + ('equal', 0, 10, 0, 10, {}), + ('insert', 10, 20, 0, 10, {}), + ('equal', 20, 30, 10, 20, { + 'indentation_changes': { + '21-11': (True, 4), + } + }), + ('equal', 30, 40, 20, 30, { + 'indentation_changes': { + '31-21': (False, 8), + } + }), + ('filtered-equal', 40, 50, 30, 40, {}), + ] + + new_opcodes = list(post_process_filtered_equals(opcodes)) + + self.assertEqual( + new_opcodes, + [ + ('equal', 0, 10, 0, 10, {}), + ('insert', 10, 20, 0, 10, {}), + ('equal', 20, 30, 10, 20, { + 'indentation_changes': { + '21-11': (True, 4), + } + }), + ('equal', 30, 40, 20, 30, { + 'indentation_changes': { + '31-21': (False, 8), + } + }), + ('equal', 40, 50, 30, 40, {}), + ]) class DiffChunkGeneratorTests(TestCase):