mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-29 02:02:41 +01:00
Use a hash-and-diff strategy to produce a diff layout for block-based documents
Summary: Depends on D20835. Ref T13425. Ref T13414. When a document has a list of content blocks, we may not be able to diff it directly, but we can hash each block and then diff the hashes (internally "diff" also does approximately the same thing). We could do this ourselves with slightly fewer layers of indirection, but: diff already exists; we already use it; we already have a bunch of abstractions on top of it; and it's likely much faster on large inputs than the best we can do in PHP. Test Plan: {F6888169} Maniphest Tasks: T13425, T13414 Differential Revision: https://secure.phabricator.com/D20836
This commit is contained in:
parent
932d829af3
commit
281598d65c
7 changed files with 115 additions and 24 deletions
|
@ -11,7 +11,7 @@ return array(
|
||||||
'conpherence.pkg.js' => '020aebcf',
|
'conpherence.pkg.js' => '020aebcf',
|
||||||
'core.pkg.css' => '6a8c9533',
|
'core.pkg.css' => '6a8c9533',
|
||||||
'core.pkg.js' => '6e5c894f',
|
'core.pkg.js' => '6e5c894f',
|
||||||
'differential.pkg.css' => 'ce54994e',
|
'differential.pkg.css' => 'abd2c0d8',
|
||||||
'differential.pkg.js' => '68fa36fc',
|
'differential.pkg.js' => '68fa36fc',
|
||||||
'diffusion.pkg.css' => '42c75c37',
|
'diffusion.pkg.css' => '42c75c37',
|
||||||
'diffusion.pkg.js' => 'a98c0bf7',
|
'diffusion.pkg.js' => 'a98c0bf7',
|
||||||
|
@ -61,7 +61,7 @@ return array(
|
||||||
'rsrc/css/application/dashboard/dashboard.css' => '5a205b9d',
|
'rsrc/css/application/dashboard/dashboard.css' => '5a205b9d',
|
||||||
'rsrc/css/application/diff/inline-comment-summary.css' => '81eb368d',
|
'rsrc/css/application/diff/inline-comment-summary.css' => '81eb368d',
|
||||||
'rsrc/css/application/differential/add-comment.css' => '7e5900d9',
|
'rsrc/css/application/differential/add-comment.css' => '7e5900d9',
|
||||||
'rsrc/css/application/differential/changeset-view.css' => 'db306b82',
|
'rsrc/css/application/differential/changeset-view.css' => '5dda5e53',
|
||||||
'rsrc/css/application/differential/core.css' => '7300a73e',
|
'rsrc/css/application/differential/core.css' => '7300a73e',
|
||||||
'rsrc/css/application/differential/phui-inline-comment.css' => '48acce5b',
|
'rsrc/css/application/differential/phui-inline-comment.css' => '48acce5b',
|
||||||
'rsrc/css/application/differential/revision-comment.css' => '7dbc8d1d',
|
'rsrc/css/application/differential/revision-comment.css' => '7dbc8d1d',
|
||||||
|
@ -554,7 +554,7 @@ return array(
|
||||||
'conpherence-thread-manager' => 'aec8e38c',
|
'conpherence-thread-manager' => 'aec8e38c',
|
||||||
'conpherence-transaction-css' => '3a3f5e7e',
|
'conpherence-transaction-css' => '3a3f5e7e',
|
||||||
'd3' => '9d068042',
|
'd3' => '9d068042',
|
||||||
'differential-changeset-view-css' => 'db306b82',
|
'differential-changeset-view-css' => '5dda5e53',
|
||||||
'differential-core-view-css' => '7300a73e',
|
'differential-core-view-css' => '7300a73e',
|
||||||
'differential-revision-add-comment-css' => '7e5900d9',
|
'differential-revision-add-comment-css' => '7e5900d9',
|
||||||
'differential-revision-comment-css' => '7dbc8d1d',
|
'differential-revision-comment-css' => '7dbc8d1d',
|
||||||
|
@ -1457,6 +1457,9 @@ return array(
|
||||||
'javelin-dom',
|
'javelin-dom',
|
||||||
'phuix-dropdown-menu',
|
'phuix-dropdown-menu',
|
||||||
),
|
),
|
||||||
|
'5dda5e53' => array(
|
||||||
|
'phui-inline-comment-view-css',
|
||||||
|
),
|
||||||
'5faf27b9' => array(
|
'5faf27b9' => array(
|
||||||
'phuix-form-control-view',
|
'phuix-form-control-view',
|
||||||
),
|
),
|
||||||
|
@ -2069,9 +2072,6 @@ return array(
|
||||||
'javelin-uri',
|
'javelin-uri',
|
||||||
'phabricator-notification',
|
'phabricator-notification',
|
||||||
),
|
),
|
||||||
'db306b82' => array(
|
|
||||||
'phui-inline-comment-view-css',
|
|
||||||
),
|
|
||||||
'dfa1d313' => array(
|
'dfa1d313' => array(
|
||||||
'javelin-behavior',
|
'javelin-behavior',
|
||||||
'javelin-dom',
|
'javelin-dom',
|
||||||
|
|
|
@ -379,7 +379,15 @@ final class DifferentialChangesetTwoUpRenderer
|
||||||
if ($old) {
|
if ($old) {
|
||||||
$old_content = $old->newContentView();
|
$old_content = $old->newContentView();
|
||||||
$old_key = $old->getBlockKey();
|
$old_key = $old->getBlockKey();
|
||||||
$old_classes = implode(' ', $old->getClasses());
|
|
||||||
|
$old_classes = $old->getClasses();
|
||||||
|
|
||||||
|
if ($old->getDifferenceType() === '-') {
|
||||||
|
$old_classes[] = 'old';
|
||||||
|
$old_classes[] = 'old-full';
|
||||||
|
}
|
||||||
|
|
||||||
|
$old_classes = implode(' ', $old_classes);
|
||||||
} else {
|
} else {
|
||||||
$old_content = null;
|
$old_content = null;
|
||||||
$old_key = null;
|
$old_key = null;
|
||||||
|
@ -389,7 +397,14 @@ final class DifferentialChangesetTwoUpRenderer
|
||||||
if ($new) {
|
if ($new) {
|
||||||
$new_content = $new->newContentView();
|
$new_content = $new->newContentView();
|
||||||
$new_key = $new->getBlockKey();
|
$new_key = $new->getBlockKey();
|
||||||
$new_classes = implode(' ', $new->getClasses());
|
$new_classes = $new->getClasses();
|
||||||
|
|
||||||
|
if ($new->getDifferenceType() === '+') {
|
||||||
|
$new_classes[] = 'new';
|
||||||
|
$new_classes[] = 'new-full';
|
||||||
|
}
|
||||||
|
|
||||||
|
$new_classes = implode(' ', $new_classes);
|
||||||
} else {
|
} else {
|
||||||
$new_content = null;
|
$new_content = null;
|
||||||
$new_key = null;
|
$new_key = null;
|
||||||
|
|
|
@ -6,6 +6,8 @@ final class PhabricatorDocumentEngineBlock
|
||||||
private $blockKey;
|
private $blockKey;
|
||||||
private $content;
|
private $content;
|
||||||
private $classes = array();
|
private $classes = array();
|
||||||
|
private $differenceHash;
|
||||||
|
private $differenceType;
|
||||||
|
|
||||||
public function setContent($content) {
|
public function setContent($content) {
|
||||||
$this->content = $content;
|
$this->content = $content;
|
||||||
|
@ -38,4 +40,22 @@ final class PhabricatorDocumentEngineBlock
|
||||||
return $this->classes;
|
return $this->classes;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function setDifferenceHash($difference_hash) {
|
||||||
|
$this->differenceHash = $difference_hash;
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getDifferenceHash() {
|
||||||
|
return $this->differenceHash;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function setDifferenceType($difference_type) {
|
||||||
|
$this->differenceType = $difference_type;
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getDifferenceType() {
|
||||||
|
return $this->differenceType;
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -20,28 +20,54 @@ final class PhabricatorDocumentEngineBlocks
|
||||||
$rows = array();
|
$rows = array();
|
||||||
$lists = $this->lists;
|
$lists = $this->lists;
|
||||||
|
|
||||||
$idx = 0;
|
$specs = array();
|
||||||
while (true) {
|
foreach ($this->lists as $list) {
|
||||||
$found_any = false;
|
$specs[] = $this->newDiffSpec($list['blocks']);
|
||||||
|
}
|
||||||
|
|
||||||
$row = array();
|
$old_map = $specs[0]['map'];
|
||||||
foreach ($lists as $list) {
|
$new_map = $specs[1]['map'];
|
||||||
$blocks = $list['blocks'];
|
|
||||||
$cell = idx($blocks, $idx);
|
|
||||||
|
|
||||||
if ($cell !== null) {
|
$old_list = $specs[0]['list'];
|
||||||
$found_any = true;
|
$new_list = $specs[1]['list'];
|
||||||
}
|
|
||||||
|
|
||||||
$row[] = $cell;
|
$changeset = id(new PhabricatorDifferenceEngine())
|
||||||
|
->generateChangesetFromFileContent($old_list, $new_list);
|
||||||
|
|
||||||
|
$hunk_parser = id(new DifferentialHunkParser())
|
||||||
|
->parseHunksForLineData($changeset->getHunks())
|
||||||
|
->reparseHunksForSpecialAttributes();
|
||||||
|
|
||||||
|
$old_lines = $hunk_parser->getOldLines();
|
||||||
|
$new_lines = $hunk_parser->getNewLines();
|
||||||
|
|
||||||
|
$rows = array();
|
||||||
|
|
||||||
|
$count = count($old_lines);
|
||||||
|
for ($ii = 0; $ii < $count; $ii++) {
|
||||||
|
$old_line = idx($old_lines, $ii);
|
||||||
|
$new_line = idx($new_lines, $ii);
|
||||||
|
|
||||||
|
if ($old_line) {
|
||||||
|
$old_hash = rtrim($old_line['text'], "\n");
|
||||||
|
$old_block = array_shift($old_map[$old_hash]);
|
||||||
|
$old_block->setDifferenceType($old_line['type']);
|
||||||
|
} else {
|
||||||
|
$old_block = null;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!$found_any) {
|
if ($new_line) {
|
||||||
break;
|
$new_hash = rtrim($new_line['text'], "\n");
|
||||||
|
$new_block = array_shift($new_map[$new_hash]);
|
||||||
|
$new_block->setDifferenceType($new_line['type']);
|
||||||
|
} else {
|
||||||
|
$new_block = null;
|
||||||
}
|
}
|
||||||
|
|
||||||
$rows[] = $row;
|
$rows[] = array(
|
||||||
$idx++;
|
$old_block,
|
||||||
|
$new_block,
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
return $rows;
|
return $rows;
|
||||||
|
@ -80,4 +106,25 @@ final class PhabricatorDocumentEngineBlocks
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
private function newDiffSpec(array $blocks) {
|
||||||
|
$map = array();
|
||||||
|
$list = array();
|
||||||
|
|
||||||
|
foreach ($blocks as $block) {
|
||||||
|
$hash = $block->getDifferenceHash();
|
||||||
|
|
||||||
|
if (!isset($map[$hash])) {
|
||||||
|
$map[$hash] = array();
|
||||||
|
}
|
||||||
|
$map[$hash][] = $block;
|
||||||
|
|
||||||
|
$list[] = $hash;
|
||||||
|
}
|
||||||
|
|
||||||
|
return array(
|
||||||
|
'map' => $map,
|
||||||
|
'list' => implode("\n", $list)."\n",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -55,9 +55,12 @@ final class PhabricatorImageDocumentEngine
|
||||||
'src' => $file->getBestURI(),
|
'src' => $file->getBestURI(),
|
||||||
)));
|
)));
|
||||||
|
|
||||||
|
$hash = $file->getContentHash();
|
||||||
|
|
||||||
$blocks[] = id(new PhabricatorDocumentEngineBlock())
|
$blocks[] = id(new PhabricatorDocumentEngineBlock())
|
||||||
->setBlockKey('1')
|
->setBlockKey('1')
|
||||||
->addClass('diff-image-cell')
|
->addClass('diff-image-cell')
|
||||||
|
->setDifferenceHash($hash)
|
||||||
->setContent($image_view);
|
->setContent($image_view);
|
||||||
|
|
||||||
return $blocks;
|
return $blocks;
|
||||||
|
|
|
@ -82,8 +82,13 @@ final class PhabricatorJupyterDocumentEngine
|
||||||
),
|
),
|
||||||
$notebook_table);
|
$notebook_table);
|
||||||
|
|
||||||
|
$hash = PhabricatorHash::digestWithNamedKey(
|
||||||
|
serialize($cell),
|
||||||
|
'document-engine.content-digest');
|
||||||
|
|
||||||
$blocks[] = id(new PhabricatorDocumentEngineBlock())
|
$blocks[] = id(new PhabricatorDocumentEngineBlock())
|
||||||
->setBlockKey($idx)
|
->setBlockKey($idx)
|
||||||
|
->setDifferenceHash($hash)
|
||||||
->setContent($container);
|
->setContent($container);
|
||||||
|
|
||||||
$idx++;
|
$idx++;
|
||||||
|
|
|
@ -282,7 +282,8 @@ td.cov-I {
|
||||||
font-weight: bold;
|
font-weight: bold;
|
||||||
}
|
}
|
||||||
|
|
||||||
.differential-diff .diff-image-cell {
|
.differential-diff td.diff-image-cell {
|
||||||
|
background-color: transparent;
|
||||||
background-image: url(/rsrc/image/checker_light.png);
|
background-image: url(/rsrc/image/checker_light.png);
|
||||||
padding: 8px;
|
padding: 8px;
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue