Add very basic 1-up HTML renderer support

Summary:
  - Add a query parameter to select the diff renderer.
  - When we populate diffs, use the 1-up renderer if the device isn't a desktop. Don't switch renderers once the page has loaded since it will be a tremendous mess. We can add an "always use 1-up" preference later.
  - Tweak JX.Device so we avoid a race condition, where `JX.Device.getDevice()` may not be valid when other behaviors run.
  - Implement enough of the 1-up HTML renderer to provide a vague hint that it actually works.
  - Fix a couple of bugs with primitive construction.

Test Plan:
{F29168}
{F29169}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2009

Differential Revision: https://secure.phabricator.com/D4423

src/applications/differential/controller/DifferentialChangesetViewController.php

@@ -157,6 +157,11 @@ final class DifferentialChangesetViewController extends DifferentialController {
     $parser->setLeftSideCommentMapping($left_source, $left_new);
     $parser->setWhitespaceMode($request->getStr('whitespace'));
 
+    if ($request->getStr('renderer') == '1up') {
+      $parser->setRenderer(new DifferentialChangesetOneUpRenderer());
+    }
+
+
     if ($left && $right) {
       $parser->setOriginals($left, $right);
     }

src/applications/differential/render/DifferentialChangesetOneUpRenderer.php

@@ -7,15 +7,61 @@ final class DifferentialChangesetOneUpRenderer
     return true;
   }
 
-  public function renderChangesetTable($contents) {
-    throw new Exception("Not implemented!");
-  }
-
   public function renderTextChange(
     $range_start,
     $range_len,
     $rows) {
-    throw new Exception("Not implemented!");
+
+    $primitives = $this->buildPrimitives($range_start, $range_len);
+
+    $out = array();
+    foreach ($primitives as $p) {
+      $type = $p['type'];
+      switch ($type) {
+        case 'old':
+        case 'new':
+          $out[] = '<tr>';
+          if ($type == 'old') {
+            if ($p['htype']) {
+              $class = 'left old';
+            } else {
+              $class = 'left';
+            }
+            $out[] = '<th>'.$p['line'].'</th>';
+            $out[] = '<th></th>';
+            $out[] = '<td class="'.$class.'">'.$p['render'].'</td>';
+          } else if ($type == 'new') {
+            if ($p['htype']) {
+              $class = 'right new';
+              $out[] = '<th />';
+            } else {
+              $class = 'right';
+              $out[] = '<th>'.$p['oline'].'</th>';
+            }
+            $out[] = '<th>'.$p['line'].'</th>';
+            $out[] = '<td class="'.$class.'">'.$p['render'].'</td>';
+          }
+          $out[] = '</tr>';
+          break;
+        case 'inline':
+          $out[] = '<tr><th /><th />';
+          $out[] = '<td style="background: #ddd; color: #888;">';
+
+          $out[] = 'INLINE COMMENT<br />';
+          $out[] = phutil_escape_html($p['comment']->getContent());
+
+          $out[] = '</td></tr>';
+          break;
+        default:
+          $out[] = '<tr><th /><th /><td>'.$type.'</td></tr>';
+          break;
+      }
+    }
+
+    if ($out) {
+      return $this->wrapChangeInTable(implode('', $out));
+    }
+    return null;
   }
 
   public function renderFileChange($old_file = null,

src/applications/differential/render/DifferentialChangesetRenderer.php

@@ -356,6 +356,7 @@ abstract class DifferentialChangesetRenderer {
         'htype' => null,
         'cursor' => $ii,
         'line' => null,
+        'oline' => null,
         'render' => null,
       );
 
@@ -364,13 +365,15 @@ abstract class DifferentialChangesetRenderer {
         'htype' => null,
         'cursor' => $ii,
         'line' => null,
+        'oline' => null,
         'render' => null,
         'copy' => null,
         'coverage' => null,
       );
 
       if (isset($old[$ii])) {
-        $ospec['line'] = $old[$ii]['line'];
+        $ospec['line'] = (int)$old[$ii]['line'];
+        $nspec['oline'] = (int)$old[$ii]['line'];
         $ospec['htype'] = $old[$ii]['type'];
         if (isset($old_render[$ii])) {
           $ospec['render'] = $old_render[$ii];
@@ -378,7 +381,8 @@ abstract class DifferentialChangesetRenderer {
       }
 
       if (isset($new[$ii])) {
-        $nspec['line'] = $new[$ii]['line'];
+        $nspec['line'] = (int)$new[$ii]['line'];
+        $ospec['oline'] = (int)$new[$ii]['line'];
         $nspec['htype'] = $new[$ii]['type'];
         if (isset($new_render[$ii])) {
           $nspec['render'] = $new_render[$ii];
@@ -408,7 +412,7 @@ abstract class DifferentialChangesetRenderer {
         foreach ($new_comments[$nspec['line']] as $comment) {
           $primitives[] = array(
             'type' => 'inline',
-            'comment' => 'comment',
+            'comment' => $comment,
             'right' => true,
           );
         }
@@ -445,6 +449,7 @@ abstract class DifferentialChangesetRenderer {
     $new_buf = array();
     foreach ($primitives as $primitive) {
       $type = $primitive['type'];
+
       if ($type == 'old') {
         if (!$primitive['htype']) {
           // This is a line which appears in both the old file and the new
@@ -475,11 +480,12 @@ abstract class DifferentialChangesetRenderer {
         $new_buf = array();
         $out[] = array($primitive);
       } else if ($type == 'inline') {
-        if ($primitive['right']) {
+        $out[] = $old_buf;
-          $new_buf[] = $primitive;
+        $out[] = $new_buf;
-        } else {
+        $old_buf = array();
-          $old_buf[] = $primitive;
+        $new_buf = array();
-        }
+
+        $out[] = array($primitive);
       } else {
         throw new Exception("Unknown primitive type '{$primitive}'!");
       }

webroot/rsrc/js/application/core/behavior-device.js

@@ -10,40 +10,44 @@
 JX.install('Device', {
   statics : {
     _device : null,
+    recalculate: function() {
+      var v = JX.Vector.getViewport();
+      var self = JX.Device;
+
+      var device = 'desktop';
+      if (v.x <= 768) {
+        device = 'tablet';
+      }
+      if (v.x <= 480) {
+        device = 'phone';
+      }
+
+      if (device == self._device) {
+        return;
+      }
+
+      self._device = device;
+
+      var e = document.body;
+      JX.DOM.alterClass(e, 'device-phone', (device == 'phone'));
+      JX.DOM.alterClass(e, 'device-tablet', (device == 'tablet'));
+      JX.DOM.alterClass(e, 'device-desktop', (device == 'desktop'));
+      JX.DOM.alterClass(e, 'device', (device != 'desktop'));
+
+      JX.Stratcom.invoke('phabricator-device-change', null, device);
+    },
+
     getDevice : function() {
-      return JX.Device._device;
+      var self = JX.Device;
+      if (self._device === null) {
+        self.recalculate();
+      }
+      return self._device;
     }
   }
 });
 
 JX.behavior('device', function() {
-
+  JX.Stratcom.listen('resize', null, JX.Device.recalculate);
-  function onresize() {
+  JX.Device.recalculate();
-    var v = JX.Vector.getViewport();
-
-    var device = 'desktop';
-    if (v.x <= 768) {
-      device = 'tablet';
-    }
-    if (v.x <= 480) {
-      device = 'phone';
-    }
-
-    if (device == JX.Device.getDevice()) {
-      return;
-    }
-
-    JX.Device._device = device;
-
-    var e = document.body;
-    JX.DOM.alterClass(e, 'device-phone', (device == 'phone'));
-    JX.DOM.alterClass(e, 'device-tablet', (device == 'tablet'));
-    JX.DOM.alterClass(e, 'device-desktop', (device == 'desktop'));
-    JX.DOM.alterClass(e, 'device', (device != 'desktop'));
-
-    JX.Stratcom.invoke('phabricator-device-change', null, device);
-  }
-
-  JX.Stratcom.listen('resize', null, onresize);
-  onresize();
 });

webroot/rsrc/js/application/differential/behavior-populate.js

@@ -5,6 +5,7 @@
  *           javelin-util
  *           javelin-dom
  *           javelin-stratcom
+ *           javelin-behavior-device
  *           phabricator-tooltip
  */
 
@@ -23,10 +24,16 @@ JX.behavior('differential-populate', function(config) {
     }
   }
 
+  // NOTE: If you load the page at one device resolution and then resize to
+  // a different one we don't re-render the diffs, because it's a complicated
+  // mess and you could lose inline comments, cursor positions, etc.
+  var renderer = (JX.Device.getDevice() == 'desktop') ? '2up' : '1up';
+
   for (var k in config.registry) {
     var data = {
       ref : config.registry[k],
-      whitespace: config.whitespace
+      whitespace: config.whitespace,
+      renderer: renderer
     };
 
     new JX.Workflow(config.uri, data)