Edgewall Software
Modify

Opened 12 years ago

Last modified 3 years ago

#6943 new defect

diff renderer has problems processing file names

Reported by: theultramage@… Owned by:
Priority: normal Milestone: next-stable-1.2.x
Component: rendering Version:
Severity: minor Keywords: diff bitesized
Cc: theultramage@… Branch:
Release Notes:
API Changes:

Description (last modified by theultramage@…)

I discovered a glitch in how trac renders unified diffs. Specifically, depending on the old and new filename, the results will vary a lot.

Following are several examples:


same name

  • x.txt

     
    11this is example text
    22and one more line
    3 no more lines
     3no more lines

different name, no dot in name (in /trunk, this displays as "(a) xxx vs. (b) yyy")

  • (a) xxx vs. (b) yyy

    a b  
    11this is example text
    22and one more line
    3 no more lines
     3no more lines

different name, dot in name

  • .txt

    xxx yyy  
    11this is example text
    22and one more line
    3 no more lines
     3no more lines

for reference, this is an image of how the third case looked like at the time of writing: screenshot of how a diff of two differently named files with a dot renders like

as you can see, the 'xxx' and 'yyy' replaced the 'old'/'new' columns, and the title is completely wrong.

Attachments (1)

trac-diff-glitch.png (4.2 KB ) - added by theultramage@… 12 years ago.
screenshot of how a diff of two differently named files with a dot renders like

Download all attachments as: .zip

Change History (16)

by theultramage@…, 12 years ago

Attachment: trac-diff-glitch.png added

screenshot of how a diff of two differently named files with a dot renders like

comment:1 by Tim Hatch, 12 years ago

I'm a little confused about why '.' is considered a pathsep in source:trunk/trac/mimeview/patch.py line 130. Removing the . seems to make them a little intuitive. What's the use case for having . there (comparing a.txt and a.txt.old?)

comment:2 by theultramage@…, 12 years ago

Oh, and the 'source code' for the diffs above, if anyone needs to reproduce:

{{{
#!diff
--- xxx.txt       Wed Oct 17 08:44:43 2007
+++ yyy.txt       Wed Oct 17 08:44:58 2007
@@ -1,3 +1,3 @@
 this is example text
 and one more line
-no more lines
+no more lines
}}}

comment:3 by Christian Boos, 11 years ago

Milestone: 0.11.1
Severity: normalminor

comment:4 by Christian Boos, 11 years ago

Keywords: diff added

comment:5 by Christian Boos, 11 years ago

Milestone: 0.11-retriage0.12

comment:6 by theultramage@…, 10 years ago

It is possible that the intent of the . separator was to handle names in the form file.c.r9999 vs. file.c.r12345. In that case it would display as file.c in the diff header, and 9999 / 12345 in the column headers.

However, when the names are not in this form, the renderer fails badly. And I'm still wondering why in example 2 the file names get substituted by 'a' and 'b' instead of just using the file names in the column headers. Who came up with this thing?

in reply to:  6 comment:7 by Christian Boos, 10 years ago

Replying to theultramage@…:

It is possible that the intent of the . separator was to handle names in the form file.c.r9999 vs. file.c.r12345. In that case it would display as file.c in the diff header, and 9999 / 12345 in the column headers.

That was the case, IIRC.

However, when the names are not in this form, the renderer fails badly.

Right, I should check for a second dot.

comment:8 by Christian Boos, 10 years ago

Milestone: 0.12next-minor-0.12.x

comment:9 by Christian Boos, 10 years ago

Version: devel

comment:10 by Ryan J Ollos, 5 years ago

Milestone: next-minor-0.12.xnext-stable-1.0.x

comment:11 by theultramage@…, 5 years ago

Description: modified (diff)

Link to attachment image instead of remote url.

comment:12 by Christian Boos, 5 years ago

Keywords: bitesized added

comment:13 by Ryan J Ollos, 4 years ago

Owner: Christian Boos removed

comment:14 by Jun Omae, 4 years ago

#12341 was closed as duplicate.

comment:15 by Ryan J Ollos, 3 years ago

Milestone: next-stable-1.0.xnext-stable-1.2.x

Moved ticket assigned to next-stable-1.0.x since maintenance of 1.0.x is coming to a close. Please move the ticket back if it's critical to fix on 1.0.x.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The ticket will remain with no owner.
The ticket will be disowned. Next status will be 'new'.
as The resolution will be set. Next status will be 'closed'.
The owner will be changed from (none) to anonymous. Next status will be 'assigned'.

Add Comment


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