Edgewall Software
Modify

Opened 15 years ago

Closed 15 years ago

#8047 closed defect (fixed)

First patch attachment previewable; 2nd one not

Reported by: dave@… Owned by: Christian Boos
Priority: normal Milestone: 0.12
Component: rendering Version: 0.11
Severity: normal Keywords: patch diff
Cc: dikim@… Branch:
Release Notes:
API Changes:
Internal Changes:

Description

Take a look at the attachments to http://svn.boost.org/trac/boost/ticket/2744

The first one gives a nice HTML preview; the 2nd one does not.

Attachments (0)

Change History (7)

comment:1 by Remy Blank, 15 years ago

Component: generalrendering
Milestone: 0.11.4
Owner: set to Remy Blank
Version: none0.11

Strange indeed. I guess the diff renderer raises an exception, which is caught by the renderer selection code. I'll check that.

comment:2 by Christian Boos, 15 years ago

Well, it's simple, that second diff is ill-formed, the first hunk is:

@@ -139,15 +139,16 @@
       dict d;
       d["__slots__"] = tuple();
       d["values"] = dict();
+      d["names"] = dict();
 
       object module_name = module_prefix();
       if (module_name)

when it should be:

@@ -139,6 +139,7 @@
       dict d;
       d["__slots__"] = tuple();
       d["values"] = dict();
+      d["names"] = dict();
 
       object module_name = module_prefix();
       if (module_name)

So we fail by hitting an unexpected '@' command. It should be possible to recover from this though, but nevertheless visually indicate that the hunk has a problem.

Remy, unless you had already something in preparation I'd like to take this one over as I already have pending work there (support for GIT binary diffs).

comment:3 by Remy Blank, 15 years ago

Owner: changed from Remy Blank to Christian Boos

Sure, go ahead.

One thing I found odd in the renderer selection was that a renderer is discarded on any Exception, and we move to the next one. This tends to hide issues in the renderer. Maybe we could only catch a specific exception, and report all others as internal errors?

comment:4 by Christian Boos, 15 years ago

Keywords: patch diff unicode added

You're right, in this case the PatchRenderer itself did something sensible, raising a TracError "Invalid unified diff content".

I think we can convert such errors into warnings before moving on to the next renderer.

Besides that patch is also interesting because it contains non-ascii characters (UTF-8 encoded), and that somehow fails to be detected properly, as the rendering says:

DEBUG: Rendering preview of file enum_with_duplicated_values_v2.2.patch with mime-type text/x-diff; charset=iso-8859-15

and the Unified diff view shows:

Index: libs/python/src/object/enum.cpp
===================================================================
--- libs/python/src/object/enum.cpp (revisão 51177)
+++ libs/python/src/object/enum.cpp (cópia de trabalho)

(i.e. UTF-8 encoded string decoded with iso-8859-15)

in reply to:  4 comment:5 by Christian Boos, 15 years ago

Keywords: unicode removed

Besides that patch is also interesting because it contains non-ascii characters (UTF-8 encoded), and that somehow fails to be detected properly

Oops, that was due to my [trac] default_charset. See #7724 for eventually having a way to set the encoding for a given attachment.

For the rest of the issue, it's now fixed and I'll push the changes tomorrow on trunk . It should be possible to backport some simpler change for 0.11.4 later on.

comment:6 by Christian Boos, 15 years ago

Fixed in r7881, which needs r7879.

Also, the rendering errors are now converted to warnings in r7883.

Leaving open for an eventual backport to 0.11.4.

comment:7 by Christian Boos, 15 years ago

Milestone: 0.11.40.12
Resolution: fixed
Status: newclosed

Well, I'm getting lazy here: the patch renderer evolved too much between 0.11 and 0.12, let's make things simpler and declare that as fixed in 0.12 only.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Christian Boos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Christian Boos 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.