Support rich diff rendering with DocumentEngine for added/removed files

Summary: Ref T13425. When a file (like a Jupyter notebook) is added or removed, we can still render a useful line-by-line diff.

Test Plan:
  - Viewed add/modify/remove of Jupyter, source code, and images in 2up/1up mode, everything looked okay.

Maniphest Tasks: T13425

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

src/applications/differential/parser/DifferentialChangesetParser.php

@@ -1686,47 +1686,79 @@ final class DifferentialChangesetParser extends Phobject {
         break;
     }
 
-    $old_ref = id(new PhabricatorDocumentRef())
-      ->setName($changeset->getOldFile());
-    if ($old_file) {
-      $old_ref->setFile($old_file);
+    $type_delete = DifferentialChangeType::TYPE_DELETE;
+    $type_add = DifferentialChangeType::TYPE_ADD;
+    $change_type = $changeset->getChangeType();
+
+    $no_old = ($change_type == $type_add);
+    $no_new = ($change_type == $type_delete);
+
+    if ($no_old) {
+      $old_ref = null;
     } else {
-      $old_data = $this->old;
-      $old_data = ipull($old_data, 'text');
-      $old_data = implode('', $old_data);
+      $old_ref = id(new PhabricatorDocumentRef())
+        ->setName($changeset->getOldFile());
+      if ($old_file) {
+        $old_ref->setFile($old_file);
+      } else {
+        $old_data = $this->old;
+        $old_data = ipull($old_data, 'text');
+        $old_data = implode('', $old_data);
 
-      $old_ref->setData($old_data);
-    }
-
-    $new_ref = id(new PhabricatorDocumentRef())
-      ->setName($changeset->getFilename());
-    if ($new_file) {
-      $new_ref->setFile($new_file);
-    } else {
-      $new_data = $this->new;
-      $new_data = ipull($new_data, 'text');
-      $new_data = implode('', $new_data);
-
-      $new_ref->setData($new_data);
-    }
-
-    $old_engines = PhabricatorDocumentEngine::getEnginesForRef(
-      $viewer,
-      $old_ref);
-
-    $new_engines = PhabricatorDocumentEngine::getEnginesForRef(
-      $viewer,
-      $new_ref);
-
-    $shared_engines = array_intersect_key($new_engines, $old_engines);
-    $default_engine = head_key($new_engines);
-
-    foreach ($shared_engines as $key => $shared_engine) {
-      if (!$shared_engine->canDiffDocuments($old_ref, $new_ref)) {
-        unset($shared_engines[$key]);
+        $old_ref->setData($old_data);
       }
     }
 
+    if ($no_new) {
+      $new_ref = null;
+    } else {
+      $new_ref = id(new PhabricatorDocumentRef())
+        ->setName($changeset->getFilename());
+      if ($new_file) {
+        $new_ref->setFile($new_file);
+      } else {
+        $new_data = $this->new;
+        $new_data = ipull($new_data, 'text');
+        $new_data = implode('', $new_data);
+
+        $new_ref->setData($new_data);
+      }
+    }
+
+
+    $old_engines = null;
+    if ($old_ref) {
+      $old_engines = PhabricatorDocumentEngine::getEnginesForRef(
+        $viewer,
+        $old_ref);
+    }
+
+    $new_engines = null;
+    if ($new_ref) {
+      $new_engines = PhabricatorDocumentEngine::getEnginesForRef(
+        $viewer,
+        $new_ref);
+    }
+
+    if ($new_engines !== null && $old_engines !== null) {
+      $shared_engines = array_intersect_key($new_engines, $old_engines);
+      $default_engine = head_key($new_engines);
+
+      foreach ($shared_engines as $key => $shared_engine) {
+        if (!$shared_engine->canDiffDocuments($old_ref, $new_ref)) {
+          unset($shared_engines[$key]);
+        }
+      }
+    } else if ($new_engines !== null) {
+      $shared_engines = $new_engines;
+      $default_engine = head_key($shared_engines);
+    } else if ($old_engines !== null) {
+      $shared_engines = $old_engines;
+      $default_engine = head_key($shared_engines);
+    } else {
+      return null;
+    }
+
     $engine_key = $this->getDocumentEngineKey();
     if (strlen($engine_key)) {
       if (isset($shared_engines[$engine_key])) {

src/applications/differential/render/DifferentialChangesetOneUpRenderer.php

@@ -371,17 +371,27 @@ final class DifferentialChangesetOneUpRenderer
           $cell_classes = $block_diff->getNewClasses();
         }
       } else if ($row_type === 'old') {
+        if (!$old_ref) {
+          continue;
+        }
+
         $cell_content = $engine->newBlockContentView(
           $old_ref,
           $old);
+
         $cell_classes[] = 'old';
         $cell_classes[] = 'old-full';
 
         $new_key = null;
       } else if ($row_type === 'new') {
+        if (!$new_ref) {
+          continue;
+        }
+
         $cell_content = $engine->newBlockContentView(
           $new_ref,
           $new);
+
         $cell_classes[] = 'new';
         $cell_classes[] = 'new-full';
 

src/applications/files/diff/PhabricatorDocumentEngineBlocks.php

@@ -15,7 +15,10 @@ final class PhabricatorDocumentEngineBlocks
     return $this->messages;
   }
 
-  public function addBlockList(PhabricatorDocumentRef $ref, array $blocks) {
+  public function addBlockList(
+    PhabricatorDocumentRef $ref = null,
+    array $blocks = array()) {
+
     assert_instances_of($blocks, 'PhabricatorDocumentEngineBlock');
 
     $this->lists[] = array(

src/applications/files/document/PhabricatorDocumentEngine.php

@@ -32,8 +32,8 @@ abstract class PhabricatorDocumentEngine
   }
 
   public function canDiffDocuments(
-    PhabricatorDocumentRef $uref,
-    PhabricatorDocumentRef $vref) {
+    PhabricatorDocumentRef $uref = null,
+    PhabricatorDocumentRef $vref = null) {
     return false;
   }
 

src/applications/files/document/PhabricatorImageDocumentEngine.php

@@ -18,21 +18,38 @@ final class PhabricatorImageDocumentEngine
   }
 
   public function canDiffDocuments(
-    PhabricatorDocumentRef $uref,
-    PhabricatorDocumentRef $vref) {
+    PhabricatorDocumentRef $uref = null,
+    PhabricatorDocumentRef $vref = null) {
 
-    // For now, we can only render a rich image diff if both documents have
+    // For now, we can only render a rich image diff if the documents have
     // their data stored in Files already.
 
-    return ($uref->getFile() && $vref->getFile());
+    if ($uref && !$uref->getFile()) {
+      return false;
+    }
+
+    if ($vref && !$vref->getFile()) {
+      return false;
+    }
+
+    return true;
   }
 
   public function newEngineBlocks(
-    PhabricatorDocumentRef $uref,
-    PhabricatorDocumentRef $vref) {
+    PhabricatorDocumentRef $uref = null,
+    PhabricatorDocumentRef $vref = null) {
 
-    $u_blocks = $this->newDiffBlocks($uref);
-    $v_blocks = $this->newDiffBlocks($vref);
+    if ($uref) {
+      $u_blocks = $this->newDiffBlocks($uref);
+    } else {
+      $u_blocks = array();
+    }
+
+    if ($vref) {
+      $v_blocks = $this->newDiffBlocks($vref);
+    } else {
+      $v_blocks = array();
+    }
 
     return id(new PhabricatorDocumentEngineBlocks())
       ->addBlockList($uref, $u_blocks)

src/applications/files/document/PhabricatorJupyterDocumentEngine.php

@@ -36,20 +36,29 @@ final class PhabricatorJupyterDocumentEngine
   }
 
   public function canDiffDocuments(
-    PhabricatorDocumentRef $uref,
-    PhabricatorDocumentRef $vref) {
+    PhabricatorDocumentRef $uref = null,
+    PhabricatorDocumentRef $vref = null) {
     return true;
   }
 
   public function newEngineBlocks(
-    PhabricatorDocumentRef $uref,
-    PhabricatorDocumentRef $vref) {
+    PhabricatorDocumentRef $uref = null,
+    PhabricatorDocumentRef $vref = null) {
 
     $blocks = new PhabricatorDocumentEngineBlocks();
 
     try {
-      $u_blocks = $this->newDiffBlocks($uref);
-      $v_blocks = $this->newDiffBlocks($vref);
+      if ($uref) {
+        $u_blocks = $this->newDiffBlocks($uref);
+      } else {
+        $u_blocks = array();
+      }
+
+      if ($vref) {
+        $v_blocks = $this->newDiffBlocks($vref);
+      } else {
+        $v_blocks = array();
+      }
 
       $blocks->addBlockList($uref, $u_blocks);
       $blocks->addBlockList($vref, $v_blocks);