mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-26 00:32:42 +01:00
Perform basic block interdiffs when diffing abstract blocks, and interdiff markdown in Jupyter notebooks
Summary: Depends on D20844. Ref T13425. When we line up two blocks and they can be interdiffed (generally: they both have the same type of content), let the Engine interdiff them. Then, make the Jupyter engine interdiff markdown. Test Plan: {F6898583} Maniphest Tasks: T13425 Differential Revision: https://secure.phabricator.com/D20845
This commit is contained in:
parent
5afdc620db
commit
d9515e82a3
11 changed files with 324 additions and 75 deletions
|
@ -3139,6 +3139,7 @@ phutil_register_library_map(array(
|
|||
'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 @@ phutil_register_library_map(array(
|
|||
'PhabricatorDivinerApplication' => 'PhabricatorApplication',
|
||||
'PhabricatorDocumentEngine' => 'Phobject',
|
||||
'PhabricatorDocumentEngineBlock' => 'Phobject',
|
||||
'PhabricatorDocumentEngineBlockDiff' => 'Phobject',
|
||||
'PhabricatorDocumentEngineBlocks' => 'Phobject',
|
||||
'PhabricatorDocumentRef' => 'Phobject',
|
||||
'PhabricatorDocumentRenderingEngine' => 'Phobject',
|
||||
|
|
|
@ -868,7 +868,15 @@ final class DifferentialChangesetParser extends Phobject {
|
|||
->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 @@ final class DifferentialChangesetParser extends Phobject {
|
|||
$vs = $id;
|
||||
}
|
||||
|
||||
$renderer->setDocumentEngineBlocks($engine_blocks);
|
||||
$renderer
|
||||
->setDocumentEngine($engine)
|
||||
->setDocumentEngineBlocks($engine_blocks);
|
||||
|
||||
return $renderer->renderDocumentEngineBlocks(
|
||||
$engine_blocks,
|
||||
|
@ -1657,7 +1667,7 @@ final class DifferentialChangesetParser extends Phobject {
|
|||
return $prefix.$line;
|
||||
}
|
||||
|
||||
private function newDocumentEngineBlocks() {
|
||||
private function newDocumentEngine() {
|
||||
$changeset = $this->changeset;
|
||||
$viewer = $this->getViewer();
|
||||
|
||||
|
@ -1728,7 +1738,8 @@ final class DifferentialChangesetParser extends Phobject {
|
|||
}
|
||||
|
||||
if ($document_engine) {
|
||||
return $document_engine->newEngineBlocks(
|
||||
return array(
|
||||
$document_engine,
|
||||
$old_ref,
|
||||
$new_ref);
|
||||
}
|
||||
|
|
|
@ -242,7 +242,7 @@ final class DifferentialChangesetOneUpRenderer
|
|||
'type' => 'old-file',
|
||||
'htype' => '',
|
||||
'line' => $block->getBlockKey(),
|
||||
'render' => $block->newContentView(),
|
||||
'render' => $block->getContent(),
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
@ -35,6 +35,8 @@ abstract class DifferentialChangesetRenderer extends Phobject {
|
|||
private $highlightingDisabled;
|
||||
private $scopeEngine = false;
|
||||
private $depthOnlyLines;
|
||||
|
||||
private $documentEngine;
|
||||
private $documentEngineBlocks;
|
||||
|
||||
private $oldFile = false;
|
||||
|
@ -240,6 +242,15 @@ abstract class DifferentialChangesetRenderer extends Phobject {
|
|||
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;
|
||||
|
|
|
@ -369,6 +369,15 @@ final class DifferentialChangesetTwoUpRenderer
|
|||
$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 @@ final class DifferentialChangesetTwoUpRenderer
|
|||
|
||||
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 @@ final class DifferentialChangesetTwoUpRenderer
|
|||
}
|
||||
|
||||
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());
|
||||
|
|
|
@ -5,7 +5,6 @@ final class PhabricatorDocumentEngineBlock
|
|||
|
||||
private $blockKey;
|
||||
private $content;
|
||||
private $classes = array();
|
||||
private $differenceHash;
|
||||
private $differenceType;
|
||||
private $isVisible;
|
||||
|
@ -19,10 +18,6 @@ final class PhabricatorDocumentEngineBlock
|
|||
return $this->content;
|
||||
}
|
||||
|
||||
public function newContentView() {
|
||||
return $this->getContent();
|
||||
}
|
||||
|
||||
public function setBlockKey($block_key) {
|
||||
$this->blockKey = $block_key;
|
||||
return $this;
|
||||
|
@ -32,15 +27,6 @@ final class PhabricatorDocumentEngineBlock
|
|||
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;
|
||||
|
|
|
@ -0,0 +1,47 @@
|
|||
<?php
|
||||
|
||||
final class PhabricatorDocumentEngineBlockDiff
|
||||
extends Phobject {
|
||||
|
||||
private $oldContent;
|
||||
private $newContent;
|
||||
private $oldClasses = array();
|
||||
private $newClasses = array();
|
||||
|
||||
public function setOldContent($old_content) {
|
||||
$this->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;
|
||||
}
|
||||
|
||||
}
|
|
@ -26,6 +26,10 @@ final class PhabricatorDocumentEngineBlocks
|
|||
return $this;
|
||||
}
|
||||
|
||||
public function getDocumentRefs() {
|
||||
return ipull($this->lists, 'ref');
|
||||
}
|
||||
|
||||
public function newTwoUpLayout() {
|
||||
$rows = array();
|
||||
$lists = $this->lists;
|
||||
|
|
|
@ -37,6 +37,30 @@ abstract class PhabricatorDocumentEngine
|
|||
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) {
|
||||
|
|
|
@ -39,6 +39,23 @@ final class PhabricatorImageDocumentEngine
|
|||
->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 @@ final class PhabricatorImageDocumentEngine
|
|||
|
||||
$blocks[] = id(new PhabricatorDocumentEngineBlock())
|
||||
->setBlockKey('1')
|
||||
->addClass('diff-image-cell')
|
||||
->setDifferenceHash($hash)
|
||||
->setContent($image_view);
|
||||
|
||||
|
|
|
@ -60,6 +60,126 @@ final class PhabricatorJupyterDocumentEngine
|
|||
return $blocks;
|
||||
}
|
||||
|
||||
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();
|
||||
$cell = $block->getContent();
|
||||
|
||||
$cell_content = $this->renderJupyterCell($viewer, $cell);
|
||||
|
||||
return $this->newCellContainer($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);
|
||||
|
||||
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' => 'jupyter-cell-markdown',
|
||||
),
|
||||
$result),
|
||||
);
|
||||
}
|
||||
|
||||
private function newDiffBlocks(PhabricatorDocumentRef $ref) {
|
||||
$viewer = $this->getViewer();
|
||||
$content = $ref->loadData();
|
||||
|
@ -69,22 +189,6 @@ final class PhabricatorJupyterDocumentEngine
|
|||
$idx = 1;
|
||||
$blocks = array();
|
||||
foreach ($cells as $cell) {
|
||||
$cell_content = $this->renderJupyterCell($viewer, $cell);
|
||||
|
||||
$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);
|
||||
|
||||
// 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 @@ final class PhabricatorJupyterDocumentEngine
|
|||
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 @@ final class PhabricatorJupyterDocumentEngine
|
|||
$blocks[] = id(new PhabricatorDocumentEngineBlock())
|
||||
->setBlockKey($idx)
|
||||
->setDifferenceHash($hash)
|
||||
->setContent($container);
|
||||
->setContent($cell);
|
||||
|
||||
$idx++;
|
||||
}
|
||||
|
@ -204,6 +311,26 @@ final class PhabricatorJupyterDocumentEngine
|
|||
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 @@ final class PhabricatorJupyterDocumentEngine
|
|||
|
||||
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 @@ final class PhabricatorJupyterDocumentEngine
|
|||
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(
|
||||
|
|
Loading…
Reference in a new issue