diff --git a/reviewboard/diffviewer/chunk_generator.py b/reviewboard/diffviewer/chunk_generator.py index 8dd2df137c843d3f74e77519f30bf210f6598dfd..e0a313ae009d255b08e8db312bf1085a2cc00050 100644 --- a/reviewboard/diffviewer/chunk_generator.py +++ b/reviewboard/diffviewer/chunk_generator.py @@ -2,7 +2,6 @@ from __future__ import unicode_literals import fnmatch import re -from difflib import SequenceMatcher from django.utils.html import escape from django.utils.safestring import mark_safe @@ -17,7 +16,8 @@ from pygments.lexers import get_lexer_for_filename from pygments.formatters import HtmlFormatter from reviewboard.diffviewer.differ import get_differ -from reviewboard.diffviewer.diffutils import (get_original_file, +from reviewboard.diffviewer.diffutils import (get_line_changed_regions, + get_original_file, get_patched_file, convert_to_unicode) from reviewboard.diffviewer.opcode_generator import (DiffOpcodeGenerator, @@ -328,8 +328,8 @@ class DiffChunkGenerator(object): len(old_line) <= self.STYLED_MAX_LINE_LEN and len(new_line) <= self.STYLED_MAX_LINE_LEN and old_line != new_line): - old_region, new_region = self._get_line_changed_regions(old_line, - new_line) + old_region, new_region = \ + get_line_changed_regions(old_line, new_line) else: old_region = new_region = [] @@ -586,51 +586,6 @@ class DiffChunkGenerator(object): return highlight(data, lexer, NoWrapperHtmlFormatter()).splitlines() - def _get_line_changed_regions(self, oldline, newline): - """Returns regions of changes between two similar lines.""" - if oldline is None or newline is None: - return (None, None) - - # Use the SequenceMatcher directly. It seems to give us better results - # for this. We should investigate steps to move to the new differ. - differ = SequenceMatcher(None, oldline, newline) - - # This thresholds our results -- we don't want to show inter-line diffs - # if most of the line has changed, unless those lines are very short. - - # FIXME: just a plain, linear threshold is pretty crummy here. Short - # changes in a short line get lost. I haven't yet thought of a fancy - # nonlinear test. - if differ.ratio() < 0.6: - return (None, None) - - oldchanges = [] - newchanges = [] - back = (0, 0) - - for tag, i1, i2, j1, j2 in differ.get_opcodes(): - if tag == 'equal': - if (i2 - i1 < 3) or (j2 - j1 < 3): - back = (j2 - j1, i2 - i1) - continue - - oldstart, oldend = i1 - back[0], i2 - newstart, newend = j1 - back[1], j2 - - if oldchanges != [] and oldstart <= oldchanges[-1][1] < oldend: - oldchanges[-1] = (oldchanges[-1][0], oldend) - elif not oldline[oldstart:oldend].isspace(): - oldchanges.append((oldstart, oldend)) - - if newchanges != [] and newstart <= newchanges[-1][1] < newend: - newchanges[-1] = (newchanges[-1][0], newend) - elif not newline[newstart:newend].isspace(): - newchanges.append((newstart, newend)) - - back = (0, 0) - - return oldchanges, newchanges - def compute_chunk_last_header(lines, numlines, meta, last_header=None): """Computes information for the displayed function/class headers. diff --git a/reviewboard/diffviewer/diffutils.py b/reviewboard/diffviewer/diffutils.py index 0b55ab0f8dd7e58f70bae6038f11db52bd590f8f..64a725a1fa0c6921dd41121453ea130879303aea 100644 --- a/reviewboard/diffviewer/diffutils.py +++ b/reviewboard/diffviewer/diffutils.py @@ -4,6 +4,7 @@ import os import re import subprocess import tempfile +from difflib import SequenceMatcher from django.utils.translation import ugettext as _ from djblets.log import log_timed @@ -551,3 +552,50 @@ def get_enable_highlighting(user): return (siteconfig.get('diffviewer_syntax_highlighting') and user_syntax_highlighting and get_can_enable_syntax_highlighting()) + + +def get_line_changed_regions(oldline, newline): + """Returns regions of changes between two similar lines.""" + if oldline is None or newline is None: + return None, None + + # Use the SequenceMatcher directly. It seems to give us better results + # for this. We should investigate steps to move to the new differ. + differ = SequenceMatcher(None, oldline, newline) + + # This thresholds our results -- we don't want to show inter-line diffs + # if most of the line has changed, unless those lines are very short. + + # FIXME: just a plain, linear threshold is pretty crummy here. Short + # changes in a short line get lost. I haven't yet thought of a fancy + # nonlinear test. + if differ.ratio() < 0.6: + return None, None + + oldchanges = [] + newchanges = [] + back = (0, 0) + + for tag, i1, i2, j1, j2 in differ.get_opcodes(): + if tag == 'equal': + if (i2 - i1 < 3) or (j2 - j1 < 3): + back = (j2 - j1, i2 - i1) + + continue + + oldstart, oldend = i1 - back[0], i2 + newstart, newend = j1 - back[1], j2 + + if oldchanges and oldstart <= oldchanges[-1][1] < oldend: + oldchanges[-1] = (oldchanges[-1][0], oldend) + elif not oldline[oldstart:oldend].isspace(): + oldchanges.append((oldstart, oldend)) + + if newchanges and newstart <= newchanges[-1][1] < newend: + newchanges[-1] = (newchanges[-1][0], newend) + elif not newline[newstart:newend].isspace(): + newchanges.append((newstart, newend)) + + back = (0, 0) + + return oldchanges, newchanges diff --git a/reviewboard/diffviewer/tests.py b/reviewboard/diffviewer/tests.py index 66d5b68334740e54c0e793c69b79a91c95e7054f..fb9cd63424c87d83f7f454196bb9f1f1dacfc0cb 100644 --- a/reviewboard/diffviewer/tests.py +++ b/reviewboard/diffviewer/tests.py @@ -1110,36 +1110,6 @@ class DiffChunkGeneratorTests(TestCase): filediff = FileDiff(source_file='foo', diffset=DiffSet()) self.generator = DiffChunkGenerator(None, filediff) - def test_get_line_changed_regions(self): - """Testing DiffChunkGenerator._get_line_changed_regions""" - def deep_equal(A, B): - typea, typeb = type(A), type(B) - self.assertEqual(typea, typeb) - if typea is tuple or typea is list: - for a, b in zip_longest(A, B): - deep_equal(a, b) - else: - self.assertEqual(A, B) - - deep_equal(self.generator._get_line_changed_regions(None, None), - (None, None)) - - old = 'submitter = models.ForeignKey(Person, verbose_name="Submitter")' - new = 'submitter = models.ForeignKey(User, verbose_name="Submitter")' - regions = self.generator._get_line_changed_regions(old, new) - deep_equal(regions, ([(30, 36)], [(30, 34)])) - - old = '-from reviews.models import ReviewRequest, Person, Group' - new = '+from .reviews.models import ReviewRequest, Group' - regions = self.generator._get_line_changed_regions(old, new) - deep_equal(regions, ([(0, 1), (6, 6), (43, 51)], - [(0, 1), (6, 7), (44, 44)])) - - old = 'abcdefghijklm' - new = 'nopqrstuvwxyz' - regions = self.generator._get_line_changed_regions(old, new) - deep_equal(regions, (None, None)) - def test_indent_spaces(self): """Testing DiffChunkGenerator._serialize_indentation with spaces""" self.assertEqual(self.generator._serialize_indentation(' '), @@ -1540,3 +1510,37 @@ class DiffRendererTests(SpyAgency, TestCase): chunk = diff_file['chunks'][0] self.assertEqual(chunk['change'], 'replace') + + +class DiffUtilsTests(TestCase): + """Unit tests for diffutils.""" + def test_get_line_changed_regions(self): + """Testing DiffChunkGenerator._get_line_changed_regions""" + def deep_equal(A, B): + typea, typeb = type(A), type(B) + self.assertEqual(typea, typeb) + + if typea is tuple or typea is list: + for a, b in zip_longest(A, B): + deep_equal(a, b) + else: + self.assertEqual(A, B) + + deep_equal(diffutils.get_line_changed_regions(None, None), + (None, None)) + + old = 'submitter = models.ForeignKey(Person, verbose_name="Submitter")' + new = 'submitter = models.ForeignKey(User, verbose_name="Submitter")' + regions = diffutils.get_line_changed_regions(old, new) + deep_equal(regions, ([(30, 36)], [(30, 34)])) + + old = '-from reviews.models import ReviewRequest, Person, Group' + new = '+from .reviews.models import ReviewRequest, Group' + regions = diffutils.get_line_changed_regions(old, new) + deep_equal(regions, ([(0, 1), (6, 6), (43, 51)], + [(0, 1), (6, 7), (44, 44)])) + + old = 'abcdefghijklm' + new = 'nopqrstuvwxyz' + regions = diffutils.get_line_changed_regions(old, new) + deep_equal(regions, (None, None))