Make images work in the unified diff view

Summary: Ref T2009. Still a touch glitch-ish but essentially functional now.

Test Plan: Viewed image diffs in 1up and 2up views. Made inline comments on them.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T2009

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

src/applications/differential/render/DifferentialChangesetHTMLRenderer.php

@@ -575,4 +575,17 @@ abstract class DifferentialChangesetHTMLRenderer
     return array($left_prefix, $right_prefix);
   }
 
+  protected function renderImageStage(PhabricatorFile $file) {
+    return phutil_tag(
+      'div',
+      array(
+        'class' => 'differential-image-stage',
+      ),
+      phutil_tag(
+        'img',
+        array(
+          'src' => $file->getBestURI(),
+        )));
+  }
+
 }

src/applications/differential/render/DifferentialChangesetOneUpRenderer.php

@@ -30,7 +30,10 @@ final class DifferentialChangesetOneUpRenderer
     $rows) {
 
     $primitives = $this->buildPrimitives($range_start, $range_len);
+    return $this->renderPrimitives($primitives, $rows);
+  }
 
+  protected function renderPrimitives(array $primitives, $rows) {
     list($left_prefix, $right_prefix) = $this->getLineIDPrefixes();
 
     $no_copy = phutil_tag('td', array('class' => 'copy'));
@@ -44,29 +47,37 @@ final class DifferentialChangesetOneUpRenderer
       switch ($type) {
         case 'old':
         case 'new':
-          $out[] = hsprintf('<tr>');
+        case 'old-file':
-          if ($type == 'old') {
+        case 'new-file':
+          $is_old = ($type == 'old' || $type == 'old-file');
+
+          $cells = array();
+          if ($is_old) {
             if ($p['htype']) {
               $class = 'left old';
             } else {
               $class = 'left';
             }
 
+            if ($type == 'old-file') {
+              $class = "{$class} differential-old-image";
+            }
+
             if ($left_prefix) {
               $left_id = $left_prefix.$p['line'];
             } else {
               $left_id = null;
             }
-            $out[] = phutil_tag('th', array('id' => $left_id), $p['line']);
+            $cells[] = phutil_tag('th', array('id' => $left_id), $p['line']);
 
-            $out[] = phutil_tag('th', array());
+            $cells[] = phutil_tag('th', array());
-            $out[] = $no_copy;
+            $cells[] = $no_copy;
-            $out[] = phutil_tag('td', array('class' => $class), $p['render']);
+            $cells[] = phutil_tag('td', array('class' => $class), $p['render']);
-            $out[] = $no_coverage;
+            $cells[] = $no_coverage;
           } else {
             if ($p['htype']) {
               $class = 'right new';
-              $out[] = phutil_tag('th', array());
+              $cells[] = phutil_tag('th', array());
             } else {
               $class = 'right';
               if ($left_prefix) {
@@ -74,7 +85,11 @@ final class DifferentialChangesetOneUpRenderer
               } else {
                 $left_id = null;
               }
-              $out[] = phutil_tag('th', array('id' => $left_id), $p['oline']);
+              $cells[] = phutil_tag('th', array('id' => $left_id), $p['oline']);
+            }
+
+            if ($type == 'new-file') {
+              $class = "{$class} differential-new-image";
             }
 
             if ($right_prefix) {
@@ -82,14 +97,16 @@ final class DifferentialChangesetOneUpRenderer
             } else {
               $right_id = null;
             }
-            $out[] = phutil_tag('th', array('id' => $right_id), $p['line']);
+            $cells[] = phutil_tag('th', array('id' => $right_id), $p['line']);
 
 
-            $out[] = $no_copy;
+            $cells[] = $no_copy;
-            $out[] = phutil_tag('td', array('class' => $class), $p['render']);
+            $cells[] = phutil_tag('td', array('class' => $class), $p['render']);
-            $out[] = $no_coverage;
+            $cells[] = $no_coverage;
           }
-          $out[] = hsprintf('</tr>');
+
+          $out[] = phutil_tag('tr', array(), $cells);
+
           break;
         case 'inline':
           $inline = $this->buildInlineComment(
@@ -137,6 +154,7 @@ final class DifferentialChangesetOneUpRenderer
     if ($out) {
       return $this->wrapChangeInTable(phutil_implode_html('', $out));
     }
+
     return null;
   }
 
@@ -146,7 +164,55 @@ final class DifferentialChangesetOneUpRenderer
     $id = 0,
     $vs = 0) {
 
-    throw new PhutilMethodNotImplementedException();
+    // TODO: This should eventually merge into the normal primitives pathway,
+    // but fake it for now and just share as much code as possible.
+
+    $primitives = array();
+    if ($old_file) {
+      $primitives[] = array(
+        'type' => 'old-file',
+        'htype' => ($new_file ? 'new-file' : null),
+        'file' => $old_file,
+        'line' => 1,
+        'render' => $this->renderImageStage($old_file),
+      );
+    }
+
+    if ($new_file) {
+      $primitives[] = array(
+        'type' => 'new-file',
+        'htype' => ($old_file ? 'old-file' : null),
+        'file' => $new_file,
+        'line' => 1,
+        'oline' => ($old_file ? 1 : null),
+        'render' => $this->renderImageStage($old_file),
+      );
+    }
+
+    // TODO: We'd like to share primitive code here, but buildPrimitives()
+    // currently chokes on changesets with no textual data.
+    foreach ($this->getOldComments() as $line => $group) {
+      foreach ($group as $comment) {
+        $primitives[] = array(
+          'type' => 'inline',
+          'comment' => $comment,
+          'right' => false,
+        );
+      }
+    }
+
+    foreach ($this->getNewComments() as $line => $group) {
+      foreach ($group as $comment) {
+        $primitives[] = array(
+          'type' => 'inline',
+          'comment' => $comment,
+          'right' => true,
+        );
+      }
+    }
+
+    $output = $this->renderPrimitives($primitives, 1);
+    return $this->renderChangesetTable($output);
   }
 
   public function getRowScaffoldForInline(PHUIDiffInlineCommentView $view) {

src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php

@@ -304,30 +304,12 @@ final class DifferentialChangesetTwoUpRenderer
                                    $vs = 0) {
     $old = null;
     if ($old_file) {
-      $old = phutil_tag(
+      $old = $this->renderImageStage($old_file);
-        'div',
-        array(
-          'class' => 'differential-image-stage',
-        ),
-        phutil_tag(
-          'img',
-          array(
-            'src' => $old_file->getBestURI(),
-          )));
     }
 
     $new = null;
     if ($new_file) {
-      $new = phutil_tag(
+      $new = $this->renderImageStage($new_file);
-        'div',
-        array(
-          'class' => 'differential-image-stage',
-        ),
-        phutil_tag(
-          'img',
-          array(
-            'src' => $new_file->getBestURI(),
-          )));
     }
 
     $html_old = array();