[extensions-web] Move diffview entirely client-side

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

commit 40097ca2126c828b32b8001d7c10cbe3f0ca179c
Author: Jasper St. Pierre <jstpierre mecheye net>
Date:   Mon Jan 30 18:10:06 2012 -0500

    Move diffview entirely client-side
    
    Instead of calculating the diffview HTML and sending it over
    to the client, construct the DOM elements on the client with
    some jQuery. This will give us better line number support,
    as well as make things generally line up in the diff.
    
    For now, disable highlighting. I have a plan for adding back
    highlighting, but it will be done entirely client-side. The
    main problem with the current approach is that any syntax
    differences will show up in the diff viewer, resulting in
    things that look like this:
    
        tx">foo bar
    
    as part of an inline change region. It's because something
    changed syntactic categories, so we see that in the diff
    output.

 sweettooth/review/diffview.py    |  109 --------------------------
 sweettooth/review/views.py       |   30 +++-----
 sweettooth/static/css/review.css |   15 ++--
 sweettooth/static/js/diff.js     |  158 ++++++++++++++++++++++++++++++++++++++
 sweettooth/static/js/review.js   |   19 +----
 5 files changed, 181 insertions(+), 150 deletions(-)
---
diff --git a/sweettooth/review/views.py b/sweettooth/review/views.py
index 1bda386..e976b3d 100644
--- a/sweettooth/review/views.py
+++ b/sweettooth/review/views.py
@@ -19,7 +19,7 @@ from django.utils.html import escape
 from django.utils import simplejson as json
 from django.views.decorators.http import require_POST
 
-from review.diffview import get_chunks_html, split_lines, NoWrapperHtmlFormatter
+from review.diffutils import get_chunks, split_lines
 from review.models import CodeReview, ChangeStatusLog, get_all_reviewers
 from extensions import models
 
@@ -38,7 +38,6 @@ IMAGE_TYPES = {
 BINARY_TYPES = set(['.mo', '.png', ',jpg', '.jpeg', '.gif', '.bmp'])
 
 code_formatter = pygments.formatters.HtmlFormatter(style="borland", cssclass="code")
-diff_formatter = NoWrapperHtmlFormatter(style="borland")
 
 def get_filelist(zipfile, disallow_binary):
     for name in zipfile.namelist():
@@ -120,7 +119,7 @@ def get_zipfiles(version, old_version_number=None):
 
     return old_zipfile, new_zipfile
 
-def get_diff(old_zipfile, new_zipfile, filename, highlight):
+def get_diff(old_zipfile, new_zipfile, filename):
     old, new = old_zipfile.open(filename, 'r'), new_zipfile.open(filename, 'r')
     oldcontent, newcontent = old.read(), new.read()
 
@@ -130,21 +129,16 @@ def get_diff(old_zipfile, new_zipfile, filename, highlight):
     old.close()
     new.close()
 
-    if highlight:
-        oldmarkup = highlight_file(filename, oldcontent, diff_formatter)
-        newmarkup = highlight_file(filename, newcontent, diff_formatter)
-    else:
-        oldmarkup = escape(oldcontent)
-        newmarkup = escape(newcontent)
+    oldmarkup = escape(oldcontent)
+    newmarkup = escape(newcontent)
 
     oldlines = split_lines(oldmarkup)
     newlines = split_lines(newmarkup)
 
-    old_htmls, new_htmls = get_chunks_html(oldlines, newlines)
-    return dict(old=dict(html='\n'.join(old_htmls),
-                         num_lines=len(old_htmls)),
-                new=dict(html='\n'.join(new_htmls),
-                         num_lines=len(new_htmls)))
+    chunks = list(get_chunks(oldlines, newlines))
+    return dict(chunks=chunks,
+                oldlines=oldlines,
+                newlines=newlines)
 
 @ajax_view
 @model_view(models.ExtensionVersion)
@@ -180,15 +174,14 @@ def ajax_get_file_diff_view(request, obj):
     version, extension = obj, obj.extension
 
     filename = request.GET['filename']
-    highlight = request.GET.get('highlight', True)
     old_version_number = request.GET.get('oldver', None)
 
     file_base, file_extension = os.path.splitext(filename)
     if file_extension in IMAGE_TYPES:
-        return
+        return None
 
     if file_extension in BINARY_TYPES:
-        return
+        return None
 
     old_zipfile, new_zipfile = get_zipfiles(version, old_version_number)
 
@@ -196,8 +189,7 @@ def ajax_get_file_diff_view(request, obj):
     old_filelist = set(old_zipfile.namelist())
 
     if filename in old_filelist and filename in new_filelist:
-        return get_diff(old_zipfile, new_zipfile,
-                        filename, highlight)
+        return get_diff(old_zipfile, new_zipfile, filename)
     else:
         return None
 
diff --git a/sweettooth/static/css/review.css b/sweettooth/static/css/review.css
index 251f9ca..380d623 100644
--- a/sweettooth/static/css/review.css
+++ b/sweettooth/static/css/review.css
@@ -65,6 +65,7 @@
     font-family: monospace;
     background-color: #fff;
     margin: 0;
+    width: 100%;
 }
 
 .file {
@@ -81,9 +82,7 @@ p.nochanges {
     color: #aaa;
 }
 
-span.line {
-    width: 100%;
-    display: block;
+.line {
     white-space: pre;
 
     /* makes blank lines "expand",
@@ -92,23 +91,23 @@ span.line {
 }
 
 /* colors stolen from Splinter */
-span.deleted {
+.deleted {
     background-color: #ffccaa;
 }
 
-span.inserted {
+.inserted {
     background-color: #bbffbb;
 }
 
-span.replaced {
+.replaced {
     background-color: #aaccff;
 }
 
-span.changed {
+.changed {
     background-color: #cceeff;
 }
 
-.linenumbers pre {
+.linenumbers pre, .linum {
     color: #aaa;
     text-align: right;
     background-color: #eee;
diff --git a/sweettooth/static/js/diff.js b/sweettooth/static/js/diff.js
new file mode 100644
index 0000000..9598645
--- /dev/null
+++ b/sweettooth/static/js/diff.js
@@ -0,0 +1,158 @@
+"use strict";
+
+define(['jquery'], function($) {
+
+    // Each table row has four columns:
+    // ===================================================================
+    // | Old Line Number | Old Contents | New Line Number | New Contents |
+    //
+    // Each "buildChunk" function below should build full row(s).
+
+    // For some reason it's hard to turn an array of jQuery objects into
+    // one jQuery object.
+    function flatten(list) {
+        var $elems = $();
+        $.each(list, function() {
+            $elems = $elems.add(this);
+        });
+        return $elems;
+    }
+
+    function buildEqualChunk(chunk, oldContents, newContents) {
+        return $.map(chunk.lines, function(line, i) {
+            var oldLinum = line[1];
+            var newLinum = line[2];
+
+            var contents = oldContents[oldLinum - 1];
+
+            var $row = $('<tr>', {'class': 'line equal'}).
+                append($('<td>', {'class': 'old linum'}).text(oldLinum)).
+                append($('<td>', {'class': 'old contents'}).html(contents)).
+                append($('<td>', {'class': 'new linum'}).text(newLinum)).
+                append($('<td>', {'class': 'new contents'}).html(contents));
+
+            if (chunk.collapsable) {
+                if (i == 0)
+                    $row.addClass('collapsable-first');
+                else
+                    $row.addClass('collapsable-rest');
+            }
+
+            return $row;
+        });
+    }
+
+    function buildInsertChunk(chunk, oldContents, newContents) {
+        return $.map(chunk.lines, function(line) {
+            var linum = line[2];
+            var contents = newContents[linum - 1];
+
+            return $('<tr>', {'class': 'line inserted'}).
+                append($('<td>', {'class': 'linum'})).
+                append($('<td>')).
+                append($('<td>', {'class': 'new linum'}).text(linum)).
+                append($('<td>', {'class': 'new contents'}).html(contents));
+        });
+    }
+
+    function buildDeleteChunk(chunk, oldContents, newContents) {
+        return $.map(chunk.lines, function(line) {
+            var linum = line[1];
+            var contents = newContents[linum - 1];
+
+            return $('<tr>', {'class': 'line deleted'}).
+                append($('<td>', {'class': 'old linum'}).text(linum)).
+                append($('<td>', {'class': 'old contents'}).html(contents)).
+                append($('<td>', {'class': 'linum'})).
+                append($('<td>'));
+        });
+    }
+
+    // This is called for changes within lines in a 'replace' chunk,
+    // one half-row at a time.  'contents' here is the line's contents
+    //
+    // If we replace:
+    //     "this is a long, long line."
+    //
+    // with:
+    //     "this is yet another long, long line."
+    //
+    // then we get regions that look like:
+    //     [8, 9]  ,  [8, 13]
+    // Our job is to highlight the replaced regions on the respective
+    // half-row.
+    function buildReplaceRegions(regions, contents) {
+        function span(tag, text) {
+            return $('<span>', {'class': 'inline'}).addClass(tag).html(text);
+        }
+
+        function unchanged(text) { return span('unchanged', text); }
+        function changed(text) { return span('changed', text); }
+
+        // If there's no region, then SequentialMatcher failed to
+        // find something useful (or the ratio was too low). Just
+        // highlight the entire region as changed.
+        if (!regions || regions.length === 0)
+            return changed(contents);
+
+        var regionElems = [];
+        var lastEnd = 0;
+
+        $.each(regions, function() {
+            var start = this[0], end = this[1];
+
+            // The indexes in the 'regions' are the changed regions. We
+            // can expect that the regions in between the indexes are
+            // unchanged regions, so build those.
+
+            regionElems.push(unchanged(contents.slice(lastEnd, start)));
+            regionElems.push(changed(contents.slice(start, end)));
+
+            lastEnd = end;
+        });
+
+        // We may have an unchanged region left over at the end of a row.
+        if (contents.slice(lastEnd))
+            regionElems.push(unchanged(contents.slice(lastEnd)));
+
+        return regionElems;
+    }
+
+    function buildReplaceChunk(chunk, oldContents, newContents) {
+        return $.map(chunk.lines, function(line) {
+            var oldLinum = line[1], newLinum = line[2];
+            var oldRegion = line[3], newRegion = line[4];
+
+            var oldContent = oldContents[oldLinum - 1];
+            var newContent = newContents[newLinum - 1];
+
+            return $('<tr>', {'class': 'line replaced'}).
+                append($('<td>', {'class': 'old linum'}).text(oldLinum)).
+                append($('<td>', {'class': 'old contents'})
+                       .append(flatten(buildReplaceRegions(oldRegion, oldContent)))).
+                append($('<td>', {'class': 'new linum'}).text(newLinum)).
+                append($('<td>', {'class': 'new contents'})
+                       .append(flatten(buildReplaceRegions(newRegion, newContent))));
+        });
+    }
+
+    var operations = {
+        'equal': buildEqualChunk,
+        'insert': buildInsertChunk,
+        'delete': buildDeleteChunk,
+        'replace': buildReplaceChunk
+    };
+
+    function buildDiffTable(chunks, oldContents, newContents) {
+        var $table = $('<table>', {'class': 'code'});
+
+        $.each(chunks, function() {
+            $table.append(flatten(operations[this.change](this, oldContents, newContents)));
+        });
+
+        return $table;
+    };
+
+    return { buildDiffTable: buildDiffTable };
+
+});
diff --git a/sweettooth/static/js/review.js b/sweettooth/static/js/review.js
index 382cc4a..3045e76 100644
--- a/sweettooth/static/js/review.js
+++ b/sweettooth/static/js/review.js
@@ -1,6 +1,6 @@
 "use strict";
 
-define(['jquery'], function($) {
+define(['jquery', 'diff'], function($, diff) {
 
     var REVIEW_URL_BASE = '/review/ajax';
 
@@ -30,8 +30,8 @@ define(['jquery'], function($) {
         return $table;
     }
 
-    function createFileView(filename, pk, diff) {
-        var frag = diff ? '/get-file-diff/' : '/get-file/';
+    function createFileView(filename, pk, isDiff) {
+        var frag = isDiff ? '/get-file-diff/' : '/get-file/';
 
         var req = $.ajax({
             type: 'GET',
@@ -44,21 +44,12 @@ define(['jquery'], function($) {
 
         req.done(function(data) {
             var $html;
-            if (diff) {
+            if (isDiff) {
                 if (data === null) {
                     $html = $('<p>', {'class': 'nochanges'}).
                         text("There have been no changes in this file.");
                 } else {
-                    var $old = addLineNumbers(data['old']);
-                    var $new = addLineNumbers(data['new']);
-
-                    $html = $('<table>', {'class': 'diff'});
-                    var $tr = $('<tr>').appendTo($html);
-
-                    $tr.append($('<td>', {'width': '50%',
-                                          'class': 'code'}).append($old));
-                    $tr.append($('<td>', {'width': '50%',
-                                          'class': 'code'}).append($new));
+                    $html = diff.buildDiffTable(data.chunks, data.oldlines, data.newlines);
                 }
             } else {
                 $html = addLineNumbers(data);