PatchRenderer does not render chunk separator

[Shunichi Goto <gotoh@…>]

Trac does not render chunk separator row on ticket or wiki page because of typo in template string in patch.py.

patch.py

@@ -40 +40 @@
    <colgroup><col class="lineno" /><col class="lineno" /><col class="content" /></colgroup>
    <thead><tr>
     <th title="<?cs var:file.oldrev ?>"><?cs var:file.oldrev ?></th>
-    <th title="<?cs var:file.newrev ?>"><?cs var:item.newrev ?></th>
+    <th title="<?cs var:file.newrev ?>"><?cs var:file.newrev ?></th>
     <th>&nbsp;</th>
    </tr></thead><?cs
    each:change = file.diff ?><?cs
     call:diff_display(change, diff.style) ?><?cs
-    if:name(change) < len(item.diff) - 1 ?>
+    if:name(change) < len(file.diff) - 1 ?>
      <tbody class="skipped">
       <tr><th>&hellip;</th><th>&hellip;</th><td>&nbsp;</td></tr>
      </tbody><?cs

[Christian Boos]

Also, the PatchRenderer shouldn't be used at all when used in a wiki text for which the source is not available (i.e. in the ticket description or in ticket comments), as it's impossible to get the actual diff source in that case.

But this would require an API change, IHTMLPreviewRenderer.get_quality_ratio would have to know about the context of use (add an optional context argument?).

Note that I see additional benefits for such a change, related to the idea of using Mimeview renderers in order to show svn:properties in the browser (see #1601 and #1947): different renderers could be used depending on where the property is shown (either the list of properties for the current path or in additional custom column #1601).

[Matthew Good]

cboos: I don't see why the PatchRenderer should not be used in the Wiki. Embeding the content in the Wiki is no different than rendering a ".diff" file that's an attachment or in the repository. In any case where the PatchRenderer is used it's not actually connected back to the actual source file, so if you don't want to use it you should simply disable that mimeviewer.

I also don't think that mime types are an appropriate way to handle the SVN properties, but I'll follow up on that in #1947 where you mention it.

[Christopher Lenz]

cboos, the statement that the PatchRenderer makes the original patch inaccessible (e.g. for copy&paste) is certainly valid, but doesn't have anything to do with this ticket.

I've been thinking about ways to solve this particular problem, and the only thing that seems realistic would be some javascript that would toggle between the "pretty" diff and a plain diff view. Simply forbidding the PatchRenderer to handle formatting in the context of wiki text doesn't sound attractive; I think the prettified view helps a lot getting a quick idea of what the patch actually changes.

[Matthew Good]

Ah, cmlenz's comments made me realize what cboos meant by the "source" in his comments. I was thinking "source" was in reference to the files that were being diffed, not the plain text diff itself. I can understand that, although cmlenz is right that it's a separate issue.

[Christian Boos]

Ok, I'll apply the patch, and create a new ticket to discuss about the above problem.

[Christopher Lenz]

I'll take this (sorry cboos).

[Christopher Lenz]

Fixed in [2264].