On Wed, May 7, 2014 at 1:36 PM, RjOllos <[email protected]> wrote: > I suspect it is probably handled differently because it is part of a block > that includes non-whitespace changes. We could dig into the code to try and > determine if this was intentional or an oversight, but I'm not bothered too > much by the behavior for this particular case. It could be more troubling > for the case that a large block of code is indented and wrapped in a > conditional statement. There you really want to ignore whitespace so that > you can see that the only change is the addition of a single line. I'd be > interested to see if this same behavior is reproduced for that case.
The same behavior in http://trac.edgewall.org/changeset/10457. "Ignore space changes" doesn't seem it works in the page. The changeset almost is indentation changes. The following page in github works fine. https://github.com/edgewall/trac/commit/80d83370?w=1 (ignore whitespaces) Also, I tried to fix the issue. See 2 attached diff files. -- Jun Omae <[email protected]> (大前 潤) -- You received this message because you are subscribed to the Google Groups "Trac Users" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/trac-users. For more options, visit https://groups.google.com/d/optout.
commit 038b95dcaea8d5f92d552e1632723a7958b602af Author: jomae <[email protected]> Date: Fri May 9 16:55:59 2014 +0900 1.0.2dev: improved diff with "ignore whitespace changes" diff --git a/trac/versioncontrol/diff.py b/trac/versioncontrol/diff.py index 8475b05..b6637f3 100644 --- a/trac/versioncontrol/diff.py +++ b/trac/versioncontrol/diff.py @@ -94,50 +94,61 @@ def filter_ignorable_lines(hunks, fromlines, tolines, context, See `get_filtered_hunks` for the parameter descriptions. """ - def is_ignorable(tag, fromlines, tolines): - if tag == 'delete' and ignore_blank_lines: - if ''.join(fromlines) == '': - return True - elif tag == 'insert' and ignore_blank_lines: - if ''.join(tolines) == '': - return True - elif tag == 'replace' and (ignore_case or ignore_space_changes): - if len(fromlines) != len(tolines): - return False - def f(str): - if ignore_case: - str = str.lower() - if ignore_space_changes: - str = ' '.join(str.split()) - return str - for i in range(len(fromlines)): - if f(fromlines[i]) != f(tolines[i]): - return False - return True + + if ignore_case or ignore_space_changes: + def normalize(value): + if ignore_case: + value = value.lower() + if ignore_space_changes: + value = ' '.join(value.split()) + return value + else: + normalize = None + + def filter_ignores(opcodes): + ignored = False + filtered = [] + for tag, i1, i2, j1, j2 in opcodes: + if normalize and tag == 'replace': + norm_fromlines = map(normalize, fromlines[i1:i2]) + norm_tolines = map(normalize, tolines[j1:j2]) + matcher = difflib.SequenceMatcher(None, norm_fromlines, + norm_tolines) + for tag, norm_i1, norm_i2, norm_j1, norm_j2 in \ + matcher.get_opcodes(): + filtered.append((tag, i1 + norm_i1, i1 + norm_i2, + j1 + norm_j1, j1 + norm_j2)) + if tag != 'replace': + ignored = True + continue + if ignore_blank_lines and tag == 'delete': + if all(fromlines[n] == '' for n in xrange(i1, i2)): + ignored = True + tag = 'equal' + if ignore_blank_lines and tag == 'insert': + if all(tolines[n] == '' for n in xrange(j1, j2)): + ignored = True + tag = 'equal' + filtered.append((tag, i1, i2, j1, j2)) + return ignored, filtered hunks = list(hunks) opcodes = [] ignored_lines = False prev = None for hunk in hunks: - for tag, i1, i2, j1, j2 in hunk: - if tag == 'equal': - if prev: - prev = (tag, prev[1], i2, prev[3], j2) - else: - prev = (tag, i1, i2, j1, j2) + if ignore_blank_lines or ignore_case or ignore_space_changes: + ignored, hunk = filter_ignores(hunk) + if ignored: + ignored_lines = True + for curr in hunk: + if prev is None: + prev = curr + elif prev[0] == curr[0]: + prev = (prev[0], prev[1], curr[2], prev[3], curr[4]) else: - if is_ignorable(tag, fromlines[i1:i2], tolines[j1:j2]): - ignored_lines = True - if prev: - prev = 'equal', prev[1], i2, prev[3], j2 - else: - prev = 'equal', i1, i2, j1, j2 - continue - if prev: - opcodes.append(prev) - opcodes.append((tag, i1, i2, j1, j2)) - prev = None + opcodes.append(prev) + prev = curr if prev: opcodes.append(prev) diff --git a/trac/versioncontrol/tests/diff.py b/trac/versioncontrol/tests/diff.py index 68e6e47..b168f93 100644 --- a/trac/versioncontrol/tests/diff.py +++ b/trac/versioncontrol/tests/diff.py @@ -85,6 +85,40 @@ class DiffTestCase(unittest.TestCase): self.assertEqual(('equal', 0, 2, 0, 2), opcodes.next()) self.assertRaises(StopIteration, opcodes.next) + def test_space_changes_2(self): + left = """\ +try: + try: + func() + commit() + except: + rollback() +finally: + cleanup() +""" + left = left.splitlines() + right = """\ +try: + func() + commit() +except: + rollback() +finally: + cleanup() +""" + right = right.splitlines() + opcodes = get_opcodes(left, right, ignore_space_changes=0) + self.assertEqual(('equal', 0, 1, 0, 1), opcodes.next()) + self.assertEqual(('replace', 1, 6, 1, 5), opcodes.next()) + self.assertEqual(('equal', 6, 8, 5, 7), opcodes.next()) + self.assertRaises(StopIteration, opcodes.next) + + opcodes = get_opcodes(left, right, ignore_space_changes=1) + self.assertEqual(('equal', 0, 1, 0, 1), opcodes.next()) + self.assertEqual(('delete', 1, 2, 1, 1), opcodes.next()) + self.assertEqual(('equal', 2, 8, 1, 7), opcodes.next()) + self.assertRaises(StopIteration, opcodes.next) + def test_case_changes(self): opcodes = get_opcodes(['A', 'B b'], ['A', 'B B'], ignore_case=0) self.assertEqual(('equal', 0, 1, 0, 1), opcodes.next())
diff --git a/trac/versioncontrol/diff.py b/trac/versioncontrol/diff.py index 8034461..27fd525 100644 --- a/trac/versioncontrol/diff.py +++ b/trac/versioncontrol/diff.py @@ -14,12 +14,13 @@ # # Author: Christopher Lenz <[email protected]> +import re +from difflib import SequenceMatcher + +from trac.util.compat import all from trac.util.html import escape, Markup from trac.util.text import expandtabs -from difflib import SequenceMatcher -import re - __all__ = ['get_diff_options', 'hdf_diff', 'diff_blocks', 'unified_diff'] @@ -50,47 +51,49 @@ def _get_opcodes(fromlines, tolines, ignore_blank_lines=False, 'equal' block. """ - def is_ignorable(tag, fromlines, tolines): - if tag == 'delete' and ignore_blank_lines: - if ''.join(fromlines) == '': - return True - elif tag == 'insert' and ignore_blank_lines: - if ''.join(tolines) == '': - return True - elif tag == 'replace' and (ignore_case or ignore_space_changes): - if len(fromlines) != len(tolines): - return False - def f(str): - if ignore_case: - str = str.lower() - if ignore_space_changes: - str = ' '.join(str.split()) - return str - for i in range(len(fromlines)): - if f(fromlines[i]) != f(tolines[i]): - return False - return True + if ignore_case or ignore_space_changes: + def normalize(value): + if ignore_case: + value = value.lower() + if ignore_space_changes: + value = ' '.join(value.split()) + return value + else: + normalize = None - matcher = SequenceMatcher(None, fromlines, tolines) - previous = None - for tag, i1, i2, j1, j2 in matcher.get_opcodes(): - if tag == 'equal': - if previous: - previous = (tag, previous[1], i2, previous[3], j2) - else: - previous = (tag, i1, i2, j1, j2) - else: - if is_ignorable(tag, fromlines[i1:i2], tolines[j1:j2]): - if previous: - previous = 'equal', previous[1], i2, previous[3], j2 - else: - previous = 'equal', i1, i2, j1, j2 + def filter_ignores(opcodes): + for tag, i1, i2, j1, j2 in opcodes: + if normalize and tag == 'replace': + matcher = SequenceMatcher(None, + map(normalize, fromlines[i1:i2]), + map(normalize, tolines[j1:j2])) + for tag, norm_i1, norm_i2, norm_j1, norm_j2 in \ + matcher.get_opcodes(): + yield tag, i1 + norm_i1, i1 + norm_i2, \ + j1 + norm_j1, j1 + norm_j2 continue - if previous: - yield previous + if ignore_blank_lines and tag == 'delete': + if all(fromlines[n] == '' for n in xrange(i1, i2)): + tag = 'equal' + if ignore_blank_lines and tag == 'insert': + if all(tolines[n] == '' for n in xrange(j1, j2)): + tag = 'equal' yield tag, i1, i2, j1, j2 - previous = None + matcher = SequenceMatcher(None, fromlines, tolines) + opcodes = matcher.get_opcodes() + if ignore_blank_lines or ignore_case or ignore_space_changes: + opcodes = filter_ignores(opcodes) + previous = None + for current in opcodes: + if previous is None: + previous = current + elif previous[0] == current[0]: + previous = (previous[0], previous[1], current[2], + previous[3], current[4]) + else: + yield previous + previous = current if previous: yield previous diff --git a/trac/versioncontrol/tests/diff.py b/trac/versioncontrol/tests/diff.py index cc0f02f..2977da6 100644 --- a/trac/versioncontrol/tests/diff.py +++ b/trac/versioncontrol/tests/diff.py @@ -56,6 +56,40 @@ class DiffTestCase(unittest.TestCase): ignore_space_changes=1) self.assertEqual(('equal', 0, 2, 0, 2), opcodes.next()) + def test_space_changes_2(self): + left = """\ +try: + try: + func() + commit() + except: + rollback() +finally: + cleanup() +""" + left = left.splitlines() + right = """\ +try: + func() + commit() +except: + rollback() +finally: + cleanup() +""" + right = right.splitlines() + opcodes = diff._get_opcodes(left, right, ignore_space_changes=0) + self.assertEqual(('equal', 0, 1, 0, 1), opcodes.next()) + self.assertEqual(('replace', 1, 6, 1, 5), opcodes.next()) + self.assertEqual(('equal', 6, 8, 5, 7), opcodes.next()) + self.assertRaises(StopIteration, opcodes.next) + + opcodes = diff._get_opcodes(left, right, ignore_space_changes=1) + self.assertEqual(('equal', 0, 1, 0, 1), opcodes.next()) + self.assertEqual(('delete', 1, 2, 1, 1), opcodes.next()) + self.assertEqual(('equal', 2, 8, 1, 7), opcodes.next()) + self.assertRaises(StopIteration, opcodes.next) + def test_case_changes(self): opcodes = diff._get_opcodes(['A', 'B b'], ['A', 'B B'], ignore_case=0)
