[extensions-web] review: Refactor line-numbers code to be more like the diffview

[Jasper St. Pierre <jstpierre src gnome org>]

commit 6cd7c5ce71b522aa4f49bab3573d83b64455a8e6
Author: Jasper St. Pierre <jstpierre mecheye net>
Date:   Tue Feb 7 13:00:45 2012 -0500

    review: Refactor line-numbers code to be more like the diffview

 sweettooth/review/views.py       |   23 ++++++++++++++------
 sweettooth/static/css/review.css |   41 +++++++++++++++----------------------
 sweettooth/static/js/review.js   |   31 ++++++++++++----------------
 3 files changed, 46 insertions(+), 49 deletions(-)
---
diff --git a/sweettooth/review/views.py b/sweettooth/review/views.py
index b230039..aac80b1 100644
--- a/sweettooth/review/views.py
+++ b/sweettooth/review/views.py
@@ -37,7 +37,20 @@ IMAGE_TYPES = {
 
 BINARY_TYPES = set(['.mo', '.png', ',jpg', '.jpeg', '.gif', '.bmp'])
 
-code_formatter = pygments.formatters.HtmlFormatter(style="borland", cssclass="code")
+# Stolen from ReviewBoard
+# See the top of diffutils.py for details
+class NoWrapperHtmlFormatter(pygments.formatters.HtmlFormatter):
+    """An HTML Formatter for Pygments that don't wrap items in a div."""
+
+    def _wrap_div(self, inner):
+        # Method called by the formatter to wrap the contents of inner.
+        # Inner is a list of tuples containing formatted code. If the first item
+        # in the tuple is zero, then it's a wrapper, so we should ignore it.
+        for tup in inner:
+            if tup[0]:
+                yield tup
+
+code_formatter = NoWrapperHtmlFormatter(style="borland", cssclass="code")
 
 def can_review_extension(user, extension):
     if user == extension.creator:
@@ -82,13 +95,9 @@ def html_for_file(filename, version, raw):
 
     elif extension in BINARY_TYPES:
         download_url = reverse('review-download', kwargs=dict(pk=version.pk))
-        html = "<p>This file is binary. Please <a href=\"%s\">" \
-            "download the zipfile</a> to see it.</p>" % (download_url,)
-
-        return dict(html=html, num_lines=0)
+        return dict(binary=True, url=download_url)
     else:
-        return dict(html=highlight_file(filename, raw, code_formatter),
-                    num_lines=len(raw.strip().splitlines()))
+        return dict(binary=False, lines=split_lines(highlight_file(filename, raw, code_formatter)))
 
 def get_zipfiles(version, old_version_number=None):
     extension = version.extension
diff --git a/sweettooth/static/css/review.css b/sweettooth/static/css/review.css
index e083402..7156c4a 100644
--- a/sweettooth/static/css/review.css
+++ b/sweettooth/static/css/review.css
@@ -53,40 +53,42 @@
     border-style: none solid;
 }
 
-.file pre, .filedisplay .diff, .filedisplay .filetable {
-    margin: 0;
-}
-
 #diff, #files {
     overflow-x: auto;
 }
 
+.file {
+    padding-left: 0.5em;
+}
+
+p.nochanges {
+    font-size: 1.4em;
+    text-align: center;
+    color: #aaa;
+}
+
 .code {
     font-family: monospace;
     background-color: #fff;
     margin: 0;
     width: 100%;
-}
-
-.file {
-    padding-left: 0.5em;
-}
-
-.code, .linenumbers pre {
     line-height: 1.2;
 }
 
-p.nochanges {
-    font-size: 1.4em;
-    text-align: center;
+.linum {
     color: #aaa;
+    text-align: right;
+    background-color: #eee;
+    margin: 0;
+    padding-right: 1em;
+    border-right: 1px solid #ccc;
 }
 
 .line {
     white-space: pre;
 
     /* makes blank lines "expand",
-     * keep in sync with line-height */
+     * keep in sync with line-height in .code */
     height: 1.2em;
 }
 
@@ -133,15 +135,6 @@ p.nochanges {
     font-weight: bold;
 }
 
-.linenumbers pre, .linum {
-    color: #aaa;
-    text-align: right;
-    background-color: #eee;
-    margin: 0;
-    padding-right: 1em;
-    border-right: 1px solid #ccc;
-}
-
 #comments {
     resize: vertical;
     width: 100%;
diff --git a/sweettooth/static/js/review.js b/sweettooth/static/js/review.js
index aaabacc..6a709b2 100644
--- a/sweettooth/static/js/review.js
+++ b/sweettooth/static/js/review.js
@@ -4,28 +4,23 @@ define(['jquery', 'diff'], function($, diff) {
 
     var REVIEW_URL_BASE = '/review/ajax';
 
-    function addLineNumbers(data) {
+    function buildFileView(data) {
         var $fileView, $table, $tr;
 
-        $tr = $('<tr>');
-        $table = $('<table>', {'class': 'filetable'}).append($tr);
-
-        if (data.num_lines) {
-            var count = data.num_lines;
-            var lines = [];
-            lines.push("<td class=\"linenumbers\"><pre>");
-            for (var i = 1; i < (count + 1); i ++) {
-                lines.push("<span rel=\"L" + i + "\">" + i + "</span>\n");
-            }
-            lines.push("</pre></td>");
-
-            $tr.append(lines.join(''));
+        if (data.binary) {
+            return $("<p>").
+                append("This file is binary. Please ").
+                append($("<a>", {'href': data.url}).text("download the zipfile")).
+                append("to see it.");
         }
 
-        $fileView = $('<div>', {'class': 'file'}).
-            appendTo($('<td>', {'width': '100%'}).appendTo($tr));
+        var $table = $('<table>', {'class': 'code'});
 
-        $fileView.html(data.html);
+        $.each(data.lines, function(i) {
+            $table.append($('<tr>', {'class': 'line'}).
+                          append($('<td>', {'class': 'linum'}).text(i + 1)).
+                          append($('<td>', {'class': 'contents'}).html(this)));
+        });
 
         return $table;
     }
@@ -47,7 +42,7 @@ define(['jquery', 'diff'], function($, diff) {
             if (isDiff) {
                 $html = diff.buildDiffTable(data.chunks, data.oldlines, data.newlines);
             } else {
-                $html = addLineNumbers(data);
+                $html = buildFileView(data);
             }
             deferred.resolve($html);
         });