diff --git a/src/applications/differential/parser/DifferentialChangesetParser.php b/src/applications/differential/parser/DifferentialChangesetParser.php --- a/src/applications/differential/parser/DifferentialChangesetParser.php +++ b/src/applications/differential/parser/DifferentialChangesetParser.php @@ -1686,45 +1686,77 @@ 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); + $old_ref->setData($old_data); + } } - $new_ref = id(new PhabricatorDocumentRef()) - ->setName($changeset->getFilename()); - if ($new_file) { - $new_ref->setFile($new_file); + if ($no_new) { + $new_ref = null; } else { - $new_data = $this->new; - $new_data = ipull($new_data, 'text'); - $new_data = implode('', $new_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); + $new_ref->setData($new_data); + } } - $old_engines = PhabricatorDocumentEngine::getEnginesForRef( - $viewer, - $old_ref); - $new_engines = PhabricatorDocumentEngine::getEnginesForRef( - $viewer, - $new_ref); + $old_engines = null; + if ($old_ref) { + $old_engines = PhabricatorDocumentEngine::getEnginesForRef( + $viewer, + $old_ref); + } - $shared_engines = array_intersect_key($new_engines, $old_engines); - $default_engine = head_key($new_engines); + $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]); + 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(); diff --git a/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php b/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php --- a/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php @@ -371,17 +371,27 @@ $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'; diff --git a/src/applications/files/diff/PhabricatorDocumentEngineBlocks.php b/src/applications/files/diff/PhabricatorDocumentEngineBlocks.php --- a/src/applications/files/diff/PhabricatorDocumentEngineBlocks.php +++ b/src/applications/files/diff/PhabricatorDocumentEngineBlocks.php @@ -15,7 +15,10 @@ 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( diff --git a/src/applications/files/document/PhabricatorDocumentEngine.php b/src/applications/files/document/PhabricatorDocumentEngine.php --- a/src/applications/files/document/PhabricatorDocumentEngine.php +++ b/src/applications/files/document/PhabricatorDocumentEngine.php @@ -32,8 +32,8 @@ } public function canDiffDocuments( - PhabricatorDocumentRef $uref, - PhabricatorDocumentRef $vref) { + PhabricatorDocumentRef $uref = null, + PhabricatorDocumentRef $vref = null) { return false; } diff --git a/src/applications/files/document/PhabricatorImageDocumentEngine.php b/src/applications/files/document/PhabricatorImageDocumentEngine.php --- a/src/applications/files/document/PhabricatorImageDocumentEngine.php +++ b/src/applications/files/document/PhabricatorImageDocumentEngine.php @@ -18,21 +18,38 @@ } 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) diff --git a/src/applications/files/document/PhabricatorJupyterDocumentEngine.php b/src/applications/files/document/PhabricatorJupyterDocumentEngine.php --- a/src/applications/files/document/PhabricatorJupyterDocumentEngine.php +++ b/src/applications/files/document/PhabricatorJupyterDocumentEngine.php @@ -36,20 +36,29 @@ } 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);