Edgewall Software
Modify

Opened 6 years ago

Closed 6 years ago

#11687 closed defect (fixed)

Ignore whitespace changes does not always work

Reported by: Ryan J Ollos Owned by: Jun Omae
Priority: normal Milestone: 1.0.5
Component: version control/changeset view Version:
Severity: normal Keywords:
Cc: Branch:
Release Notes:

Fix not working diff with ignore whitespace changes.

API Changes:
Internal Changes:

Description

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).

Attachments (2)

13474.diff (6.8 KB ) - added by Ryan J Ollos 6 years ago.
diff_ignore_ws_1.0-stable.diff (5.6 KB ) - added by Ryan J Ollos 6 years ago.
Patch from gmessage:trac-users:trac-users:arIqj4o1HNs/t8tD-XfeZ0cJ.

Download all attachments as: .zip

Change History (11)

comment:1 by Ryan J Ollos, 6 years ago

Milestone: 0.12.60.12.7

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.

comment:2 by Ryan J Ollos, 6 years ago

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

comment:3 by Ryan J Ollos, 6 years ago

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.

in reply to:  3 comment:4 by Jun Omae, 6 years ago

Component: version control/browserversion control/changeset view

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

     
    8282        return 'wiki'
    8383
    8484    def get_navigation_items(self, req):
    85         if 'WIKI_VIEW' in req.perm('wiki'):
     85        if 'WIKI_VIEW' in req.perm('wiki', 'WikiStart'):
    8686            yield ('mainnav', 'wiki',
    87                    tag.a(_('Wiki'), href=req.href.wiki(), accesskey=1))
     87                   tag.a(_("Wiki"), href=req.href.wiki(), accesskey=1))
    8888        if 'WIKI_VIEW' in req.perm('wiki', 'TracGuide'):
    8989            yield ('metanav', 'help',
    90                    tag.a(_('Help/Guide'), href=req.href.wiki('TracGuide'),
     90                   tag.a(_("Help/Guide"), href=req.href.wiki('TracGuide'),
    9191                         accesskey=6))
    9292
    9393    # 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].

Last edited 6 years ago by Jun Omae (previous) (diff)

comment:5 by Ryan J Ollos, 6 years ago

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.

by Ryan J Ollos, 6 years ago

Attachment: 13474.diff added

comment:6 by Jun Omae, 6 years ago

Milestone: 0.12.71.0.4

comment:7 by Ryan J Ollos, 6 years ago

Milestone: 1.0.41.0.5

comment:8 by Jun Omae, 6 years ago

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:
Last edited 6 years ago by Jun Omae (previous) (diff)

comment:9 by Jun Omae, 6 years ago

Release Notes: modified (diff)
Resolution: fixed
Status: newclosed

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

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Jun Omae.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Jun Omae to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.