Ignore whitespace changes does not always work

[Ryan J Ollos]

It was discussed in ​gdiscussion:trac-users:arIqj4o1HNs that ignore whitespace changes does not always ignore all lines with whitespace-only changes. This can result in very large diffs for simple changes that result in a lot of indentation changes (e.g. wrapping a large block of code in a conditional). For example, see [12739/trunk/trac/ticket/web_ui.py] and [10457] (compare with ​github).

[Ryan J Ollos]

Not sure if this is more appropriate for 0.12.7 or 1.0.3, so I'm setting the milestone to 0.12.7 initially.

[Ryan J Ollos]

[12941] is another changeset with whitespace changes that are not properly ignored. The proposed patch fixes the issue. That patch looks good.

[Ryan J Ollos]

Another minor issue can be observed in attachment:ticket:11067:t11067-r11682-1.patch​. 'Help/Guide' → "Help/Guide" change is highlighted, but 'Wiki' → "Wiki" is not. That's a very small issue, but I notice behavior like this from time to time, and I'm trying to capture instances in which the highlighting is not as expected.

[Jun Omae]

Replying to rjollos:

Another minor issue can be observed in attachment:ticket:11067:t11067-r11682-1.patch​. 'Help/Guide' → "Help/Guide" change is highlighted, but 'Wiki' → "Wiki" is not. That's a very small issue, but I notice behavior like this from time to time, and I'm trying to capture instances in which the highlighting is not as expected.

I think that behavior is not a issue. The diff renderer highlights if only the added and removed blocks have the same number of the lines. For example, replacing + at line 88 with a space, as a result 'Wiki' and "Wiki" would be highlighted. See also tags/trac-1.0.1/trac/mimeview/patch.py@:276-277#L264​.

trac-trunk/trac/wiki/web_ui.py

@@ -82 +82 @@
         return 'wiki'
 
     def get_navigation_items(self, req):
-        if 'WIKI_VIEW' in req.perm('wiki'):
+        if 'WIKI_VIEW' in req.perm('wiki', 'WikiStart'):
             yield ('mainnav', 'wiki',
-                   tag.a(_('Wiki'), href=req.href.wiki(), accesskey=1))
+                   tag.a(_("Wiki"), href=req.href.wiki(), accesskey=1))
         if 'WIKI_VIEW' in req.perm('wiki', 'TracGuide'):
             yield ('metanav', 'help',
-                   tag.a(_('Help/Guide'), href=req.href.wiki('TracGuide'),
+                   tag.a(_("Help/Guide"), href=req.href.wiki('TracGuide'),
                          accesskey=6))
 
     # IPermissionRequestor methods

Also, github commit viewer highlights if probably only both added and removed blocks are 1 line, e.g. ​https://github.com/jun66j5/trac/commit/f824e52 and [12878].

[Ryan J Ollos]

Some code cleanup in [13474:13475]. The diff makes it look as though a large block of code was moved, but that's not the case. It might serve as a good example for improving the diff viewer. Actual diff is 13474.diff​.

[Ryan J Ollos]

Patch from ​gmessage:trac-users:trac-users:arIqj4o1HNs/t8tD-XfeZ0cJ.

[Jun Omae]

Revised patch in jomae.git@t11687.

I found another issue with ignore whitespace changes. Removing leading and trailing spaces in the line could not be detected.

Trac 1.0.4:

>>> print '\n'.join(unified_diff(['    except:'], ['except:'], ignore_space_changes=True))
@@ -1,1 +1,1 @@
     except:
>>> print '\n'.join(unified_diff(['    except:'], [' except:'], ignore_space_changes=True))
@@ -1,1 +1,1 @@
     except:

jomae.git@t11687:

>>> print '\n'.join(unified_diff(['    except:'], ['except:'], ignore_space_changes=True))
@@ -1,1 +1,1 @@
-    except:
+except:
>>> print '\n'.join(unified_diff(['    except:'], [' except:'], ignore_space_changes=True))
@@ -1,1 +1,1 @@
     except:

[Jun Omae]

Committed in [13789] and merged to trunk in [13790].