diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3139,6 +3139,7 @@ 'PhabricatorDivinerApplication' => 'applications/diviner/application/PhabricatorDivinerApplication.php', 'PhabricatorDocumentEngine' => 'applications/files/document/PhabricatorDocumentEngine.php', 'PhabricatorDocumentEngineBlock' => 'applications/files/diff/PhabricatorDocumentEngineBlock.php', + 'PhabricatorDocumentEngineBlockDiff' => 'applications/files/diff/PhabricatorDocumentEngineBlockDiff.php', 'PhabricatorDocumentEngineBlocks' => 'applications/files/diff/PhabricatorDocumentEngineBlocks.php', 'PhabricatorDocumentRef' => 'applications/files/document/PhabricatorDocumentRef.php', 'PhabricatorDocumentRenderingEngine' => 'applications/files/document/render/PhabricatorDocumentRenderingEngine.php', @@ -9478,6 +9479,7 @@ 'PhabricatorDivinerApplication' => 'PhabricatorApplication', 'PhabricatorDocumentEngine' => 'Phobject', 'PhabricatorDocumentEngineBlock' => 'Phobject', + 'PhabricatorDocumentEngineBlockDiff' => 'Phobject', 'PhabricatorDocumentEngineBlocks' => 'Phobject', 'PhabricatorDocumentRef' => 'Phobject', 'PhabricatorDocumentRenderingEngine' => 'Phobject', 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 @@ -868,7 +868,15 @@ ->setHighlightingDisabled($this->highlightingDisabled) ->setDepthOnlyLines($this->getDepthOnlyLines()); - $engine_blocks = $this->newDocumentEngineBlocks(); + list($engine, $old_ref, $new_ref) = $this->newDocumentEngine(); + if ($engine) { + $engine_blocks = $engine->newEngineBlocks( + $old_ref, + $new_ref); + } else { + $engine_blocks = null; + } + $has_document_engine = ($engine_blocks !== null); $shield = null; @@ -1043,7 +1051,9 @@ $vs = $id; } - $renderer->setDocumentEngineBlocks($engine_blocks); + $renderer + ->setDocumentEngine($engine) + ->setDocumentEngineBlocks($engine_blocks); return $renderer->renderDocumentEngineBlocks( $engine_blocks, @@ -1657,7 +1667,7 @@ return $prefix.$line; } - private function newDocumentEngineBlocks() { + private function newDocumentEngine() { $changeset = $this->changeset; $viewer = $this->getViewer(); @@ -1728,7 +1738,8 @@ } if ($document_engine) { - return $document_engine->newEngineBlocks( + return array( + $document_engine, $old_ref, $new_ref); } 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 @@ -242,7 +242,7 @@ 'type' => 'old-file', 'htype' => '', 'line' => $block->getBlockKey(), - 'render' => $block->newContentView(), + 'render' => $block->getContent(), ); } diff --git a/src/applications/differential/render/DifferentialChangesetRenderer.php b/src/applications/differential/render/DifferentialChangesetRenderer.php --- a/src/applications/differential/render/DifferentialChangesetRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetRenderer.php @@ -35,6 +35,8 @@ private $highlightingDisabled; private $scopeEngine = false; private $depthOnlyLines; + + private $documentEngine; private $documentEngineBlocks; private $oldFile = false; @@ -240,6 +242,15 @@ return $this->oldChangesetID; } + public function setDocumentEngine(PhabricatorDocumentEngine $engine) { + $this->documentEngine = $engine; + return $this; + } + + public function getDocumentEngine() { + return $this->documentEngine; + } + public function setDocumentEngineBlocks( PhabricatorDocumentEngineBlocks $blocks) { $this->documentEngineBlocks = $blocks; diff --git a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php --- a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php @@ -369,6 +369,15 @@ $old_changeset_key, $new_changeset_key) { + $engine = $this->getDocumentEngine(); + + $old_ref = null; + $new_ref = null; + $refs = $block_list->getDocumentRefs(); + if ($refs) { + list($old_ref, $new_ref) = $refs; + } + $old_comments = $this->getOldComments(); $new_comments = $this->getNewComments(); @@ -392,41 +401,16 @@ if ($old) { $old_key = $old->getBlockKey(); - - $old_classes = $old->getClasses(); - - if ($old->getDifferenceType() === '-') { - $old_classes[] = 'old'; - $old_classes[] = 'old-full'; - } - - $old_classes[] = 'diff-flush'; - - $old_classes = implode(' ', $old_classes); - $is_visible = $old->getIsVisible(); } else { $old_key = null; - $old_classes = null; } if ($new) { $new_key = $new->getBlockKey(); - $new_classes = $new->getClasses(); - - if ($new->getDifferenceType() === '+') { - $new_classes[] = 'new'; - $new_classes[] = 'new-full'; - } - - $new_classes[] = 'diff-flush'; - - $new_classes = implode(' ', $new_classes); - $is_visible = $new->getIsVisible(); } else { $new_key = null; - $new_classes = null; } if (!$is_visible) { @@ -454,24 +438,54 @@ } if ($is_rem && $is_add) { - list($old_content, $new_content) = array( - $old->newContentView(), - $new->newContentView(), - ); + $block_diff = $engine->newBlockDiffViews( + $old_ref, + $old, + $new_ref, + $new); + + $old_content = $block_diff->getOldContent(); + $new_content = $block_diff->getNewContent(); + + $old_classes = $block_diff->getOldClasses(); + $new_classes = $block_diff->getNewClasses(); } else { + $old_classes = array(); + $new_classes = array(); + if ($old) { - $old_content = $old->newContentView(); + $old_content = $engine->newBlockContentView( + $old_ref, + $old); + + if ($is_rem) { + $old_classes[] = 'old'; + $old_classes[] = 'old-full'; + } } else { $old_content = null; } if ($new) { - $new_content = $new->newContentView(); + $new_content = $engine->newBlockContentView( + $new_ref, + $new); + + if ($is_add) { + $new_classes[] = 'new'; + $new_classes[] = 'new-full'; + } } else { $new_content = null; } } + $old_classes[] = 'diff-flush'; + $old_classes = implode(' ', $old_classes); + + $new_classes[] = 'diff-flush'; + $new_classes = implode(' ', $new_classes); + $old_inline_rows = array(); if ($old_key !== null) { $old_inlines = idx($old_comments, $old_key, array()); diff --git a/src/applications/files/diff/PhabricatorDocumentEngineBlock.php b/src/applications/files/diff/PhabricatorDocumentEngineBlock.php --- a/src/applications/files/diff/PhabricatorDocumentEngineBlock.php +++ b/src/applications/files/diff/PhabricatorDocumentEngineBlock.php @@ -5,7 +5,6 @@ private $blockKey; private $content; - private $classes = array(); private $differenceHash; private $differenceType; private $isVisible; @@ -19,10 +18,6 @@ return $this->content; } - public function newContentView() { - return $this->getContent(); - } - public function setBlockKey($block_key) { $this->blockKey = $block_key; return $this; @@ -32,15 +27,6 @@ return $this->blockKey; } - public function addClass($class) { - $this->classes[] = $class; - return $this; - } - - public function getClasses() { - return $this->classes; - } - public function setDifferenceHash($difference_hash) { $this->differenceHash = $difference_hash; return $this; diff --git a/src/applications/files/diff/PhabricatorDocumentEngineBlockDiff.php b/src/applications/files/diff/PhabricatorDocumentEngineBlockDiff.php new file mode 100644 --- /dev/null +++ b/src/applications/files/diff/PhabricatorDocumentEngineBlockDiff.php @@ -0,0 +1,47 @@ +oldContent = $old_content; + return $this; + } + + public function getOldContent() { + return $this->oldContent; + } + + public function setNewContent($new_content) { + $this->newContent = $new_content; + return $this; + } + + public function getNewContent() { + return $this->newContent; + } + + public function addOldClass($class) { + $this->oldClasses[] = $class; + return $this; + } + + public function getOldClasses() { + return $this->oldClasses; + } + + public function addNewClass($class) { + $this->newClasses[] = $class; + return $this; + } + + public function getNewClasses() { + return $this->newClasses; + } + +} 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 @@ -26,6 +26,10 @@ return $this; } + public function getDocumentRefs() { + return ipull($this->lists, 'ref'); + } + public function newTwoUpLayout() { $rows = array(); $lists = $this->lists; 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 @@ -37,6 +37,30 @@ return false; } + public function newBlockDiffViews( + PhabricatorDocumentRef $uref, + PhabricatorDocumentEngineBlock $ublock, + PhabricatorDocumentRef $vref, + PhabricatorDocumentEngineBlock $vblock) { + + $u_content = $this->newBlockContentView($uref, $ublock); + $v_content = $this->newBlockContentView($vref, $vblock); + + return id(new PhabricatorDocumentEngineBlockDiff()) + ->setOldContent($u_content) + ->addOldClass('old') + ->addOldClass('old-full') + ->setNewContent($v_content) + ->addNewClass('new') + ->addNewClass('new-full'); + } + + public function newBlockContentView( + PhabricatorDocumentRef $ref, + PhabricatorDocumentEngineBlock $block) { + return $block->getContent(); + } + public function newEngineBlocks( PhabricatorDocumentRef $uref, PhabricatorDocumentRef $vref) { 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 @@ -39,6 +39,23 @@ ->addBlockList($vref, $v_blocks); } + public function newBlockDiffViews( + PhabricatorDocumentRef $uref, + PhabricatorDocumentEngineBlock $ublock, + PhabricatorDocumentRef $vref, + PhabricatorDocumentEngineBlock $vblock) { + + $u_content = $this->newBlockContentView($uref, $ublock); + $v_content = $this->newBlockContentView($vref, $vblock); + + return id(new PhabricatorDocumentEngineBlockDiff()) + ->setOldContent($u_content) + ->addOldClass('diff-image-cell') + ->setNewContent($v_content) + ->addNewClass('diff-image-cell'); + } + + private function newDiffBlocks(PhabricatorDocumentRef $ref) { $blocks = array(); @@ -59,7 +76,6 @@ $blocks[] = id(new PhabricatorDocumentEngineBlock()) ->setBlockKey('1') - ->addClass('diff-image-cell') ->setDifferenceHash($hash) ->setContent($image_view); 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 @@ -60,31 +60,135 @@ return $blocks; } - private function newDiffBlocks(PhabricatorDocumentRef $ref) { + public function newBlockDiffViews( + PhabricatorDocumentRef $uref, + PhabricatorDocumentEngineBlock $ublock, + PhabricatorDocumentRef $vref, + PhabricatorDocumentEngineBlock $vblock) { + + $ucell = $ublock->getContent(); + $vcell = $vblock->getContent(); + + $utype = idx($ucell, 'cell_type'); + $vtype = idx($vcell, 'cell_type'); + + if ($utype === $vtype) { + switch ($utype) { + case 'markdown': + $usource = idx($ucell, 'source'); + $usource = implode('', $usource); + + $vsource = idx($vcell, 'source'); + $vsource = implode('', $vsource); + + $diff = id(new PhutilProseDifferenceEngine()) + ->getDiff($usource, $vsource); + + $u_content = $this->newProseDiffCell($diff, array('=', '-')); + $v_content = $this->newProseDiffCell($diff, array('=', '+')); + + $u_content = $this->newJupyterCell(null, $u_content, null); + $v_content = $this->newJupyterCell(null, $v_content, null); + + $u_content = $this->newCellContainer($u_content); + $v_content = $this->newCellContainer($v_content); + + return id(new PhabricatorDocumentEngineBlockDiff()) + ->setOldContent($u_content) + ->addOldClass('old') + ->setNewContent($v_content) + ->addNewClass('new'); + } + } + + return parent::newBlockDiffViews($uref, $ublock, $vref, $vblock); + } + + public function newBlockContentView( + PhabricatorDocumentRef $ref, + PhabricatorDocumentEngineBlock $block) { + $viewer = $this->getViewer(); - $content = $ref->loadData(); + $cell = $block->getContent(); - $cells = $this->newCells($content, true); + $cell_content = $this->renderJupyterCell($viewer, $cell); - $idx = 1; - $blocks = array(); - foreach ($cells as $cell) { - $cell_content = $this->renderJupyterCell($viewer, $cell); + return $this->newCellContainer($cell_content); + } - $notebook_table = phutil_tag( - 'table', - array( - 'class' => 'jupyter-notebook', - ), - $cell_content); + private function newCellContainer($cell_content) { + $notebook_table = phutil_tag( + 'table', + array( + 'class' => 'jupyter-notebook', + ), + $cell_content); + + $container = phutil_tag( + 'div', + array( + 'class' => 'document-engine-jupyter document-engine-diff', + ), + $notebook_table); - $container = phutil_tag( + return $container; + } + + private function newProseDiffCell(PhutilProseDiff $diff, array $mask) { + $mask = array_fuse($mask); + + $result = array(); + foreach ($diff->getParts() as $part) { + $type = $part['type']; + $text = $part['text']; + + if (!isset($mask[$type])) { + continue; + } + + switch ($type) { + case '-': + $result[] = phutil_tag( + 'span', + array( + 'class' => 'bright', + ), + $text); + break; + case '+': + $result[] = phutil_tag( + 'span', + array( + 'class' => 'bright', + ), + $text); + break; + case '=': + $result[] = $text; + break; + } + } + + return array( + null, + phutil_tag( 'div', array( - 'class' => 'document-engine-jupyter document-engine-diff', + 'class' => 'jupyter-cell-markdown', ), - $notebook_table); + $result), + ); + } + private function newDiffBlocks(PhabricatorDocumentRef $ref) { + $viewer = $this->getViewer(); + $content = $ref->loadData(); + + $cells = $this->newCells($content, true); + + $idx = 1; + $blocks = array(); + foreach ($cells as $cell) { // When the cell is a source code line, we can hash just the raw // input rather than all the cell metadata. @@ -92,6 +196,9 @@ case 'code/line': $hash_input = $cell['raw']; break; + case 'markdown': + $hash_input = implode('', $cell['source']); + break; default: $hash_input = serialize($cell); break; @@ -104,7 +211,7 @@ $blocks[] = id(new PhabricatorDocumentEngineBlock()) ->setBlockKey($idx) ->setDifferenceHash($hash) - ->setContent($container); + ->setContent($cell); $idx++; } @@ -204,6 +311,26 @@ foreach ($cells as $cell) { $cell_type = idx($cell, 'cell_type'); + if ($cell_type === 'markdown') { + $source = $cell['source']; + $source = implode('', $source); + + // Attempt to split contiguous blocks of markdown into smaller + // pieces. + + $chunks = preg_split( + '/\n\n+/', + $source); + + foreach ($chunks as $chunk) { + $result = $cell; + $result['source'] = array($chunk); + $results[] = $result; + } + + continue; + } + if ($cell_type !== 'code') { $results[] = $cell; continue; @@ -261,13 +388,6 @@ list($label, $content) = $this->renderJupyterCellContent($viewer, $cell); - $label_cell = phutil_tag( - 'td', - array( - 'class' => 'jupyter-label', - ), - $label); - $classes = null; switch (idx($cell, 'cell_type')) { case 'code/line': @@ -275,6 +395,20 @@ break; } + return $this->newJupyterCell( + $label, + $content, + $classes); + } + + private function newJupyterCell($label, $content, $classes) { + $label_cell = phutil_tag( + 'td', + array( + 'class' => 'jupyter-label', + ), + $label); + $content_cell = phutil_tag( 'td', array(