First patch attachment previewable; 2nd one not

[dave@…]

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.

[Remy Blank]

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

[Christian Boos]

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

[Remy Blank]

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?

[Christian Boos]

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)

[Christian Boos]

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.

[Christian Boos]

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.

[Christian Boos]

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.