From a73f592d7d08ddf588b5a021661021ca06f2119e Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 23 Sep 2019 10:54:16 -0700 Subject: [PATCH] Allow DocumentEngine to elect into diff construction Summary: Ref T13425. Allow DocumentEngines to claim they can produce diffs. If both sides of a change can be diffed by the same document engine and it can produce diffs, have it diff them. This has no impact on runtime behavior because no upstream engine elects into diff generation yet. Test Plan: Loaded some revisions, nothing broke. Maniphest Tasks: T13425 Differential Revision: https://secure.phabricator.com/D20830 --- .../DifferentialChangesetViewController.php | 1 + .../parser/DifferentialChangesetParser.php | 68 +++++++++++++++++++ .../document/PhabricatorDocumentEngine.php | 12 ++++ .../files/document/PhabricatorDocumentRef.php | 24 +++++++ 4 files changed, 105 insertions(+) diff --git a/src/applications/differential/controller/DifferentialChangesetViewController.php b/src/applications/differential/controller/DifferentialChangesetViewController.php index fe6163feb9..f2558ffa1d 100644 --- a/src/applications/differential/controller/DifferentialChangesetViewController.php +++ b/src/applications/differential/controller/DifferentialChangesetViewController.php @@ -166,6 +166,7 @@ final class DifferentialChangesetViewController extends DifferentialController { DifferentialChangesetParser::parseRangeSpecification($spec); $parser = id(new DifferentialChangesetParser()) + ->setViewer($viewer) ->setCoverage($coverage) ->setChangeset($changeset) ->setRenderingReference($rendering_reference) diff --git a/src/applications/differential/parser/DifferentialChangesetParser.php b/src/applications/differential/parser/DifferentialChangesetParser.php index 9843ff3d5d..058fc9c766 100644 --- a/src/applications/differential/parser/DifferentialChangesetParser.php +++ b/src/applications/differential/parser/DifferentialChangesetParser.php @@ -58,6 +58,7 @@ final class DifferentialChangesetParser extends Phobject { private $linesOfContext = 8; private $highlightEngine; + private $viewer; public function setRange($start, $end) { $this->rangeStart = $start; @@ -149,6 +150,15 @@ final class DifferentialChangesetParser extends Phobject { return $this->offsetMode; } + public function setViewer(PhabricatorUser $viewer) { + $this->viewer = $viewer; + return $this; + } + + public function getViewer() { + return $this->viewer; + } + public static function getDefaultRendererForViewer(PhabricatorUser $viewer) { $is_unified = $viewer->compareUserSetting( PhabricatorUnifiedDiffsSetting::SETTINGKEY, @@ -1003,6 +1013,11 @@ final class DifferentialChangesetParser extends Phobject { ->setOldComments($old_comments) ->setNewComments($new_comments); + $engine_view = $this->newDocumentEngineChangesetView(); + if ($engine_view !== null) { + return $engine_view; + } + switch ($this->changeset->getFileType()) { case DifferentialChangeType::FILE_IMAGE: $old = null; @@ -1675,4 +1690,57 @@ final class DifferentialChangesetParser extends Phobject { return $prefix.$line; } + private function newDocumentEngineChangesetView() { + $changeset = $this->changeset; + $viewer = $this->getViewer(); + + // TODO: This should probably be made non-optional in the future. + if (!$viewer) { + return null; + } + + $old_data = $this->old; + $old_data = ipull($old_data, 'text'); + $old_data = implode('', $old_data); + + $new_data = $this->new; + $new_data = ipull($new_data, 'text'); + $new_data = implode('', $new_data); + + $old_ref = id(new PhabricatorDocumentRef()) + ->setName($changeset->getOldFile()) + ->setData($old_data); + + $new_ref = id(new PhabricatorDocumentRef()) + ->setName($changeset->getFilename()) + ->setData($new_data); + + $old_engines = PhabricatorDocumentEngine::getEnginesForRef( + $viewer, + $old_ref); + + $new_engines = PhabricatorDocumentEngine::getEnginesForRef( + $viewer, + $new_ref); + + $shared_engines = array_intersect_key($old_engines, $new_engines); + + $document_engine = null; + foreach ($shared_engines as $shared_engine) { + if ($shared_engine->canDiffDocuments($old_ref, $new_ref)) { + $document_engine = $shared_engine; + break; + } + } + + + if ($document_engine) { + return $document_engine->newDiffView( + $old_ref, + $new_ref); + } + + return null; + } + } diff --git a/src/applications/files/document/PhabricatorDocumentEngine.php b/src/applications/files/document/PhabricatorDocumentEngine.php index c869f5f0d7..7e2a0d3b34 100644 --- a/src/applications/files/document/PhabricatorDocumentEngine.php +++ b/src/applications/files/document/PhabricatorDocumentEngine.php @@ -31,6 +31,18 @@ abstract class PhabricatorDocumentEngine return $this->canRenderDocumentType($ref); } + public function canDiffDocuments( + PhabricatorDocumentRef $uref, + PhabricatorDocumentRef $vref) { + return false; + } + + public function newDiffView( + PhabricatorDocumentRef $uref, + PhabricatorDocumentRef $vref) { + throw new PhutilMethodNotImplementedException(); + } + public function canConfigureEncoding(PhabricatorDocumentRef $ref) { return false; } diff --git a/src/applications/files/document/PhabricatorDocumentRef.php b/src/applications/files/document/PhabricatorDocumentRef.php index 036656c00e..12d9f4f2fd 100644 --- a/src/applications/files/document/PhabricatorDocumentRef.php +++ b/src/applications/files/document/PhabricatorDocumentRef.php @@ -11,6 +11,7 @@ final class PhabricatorDocumentRef private $symbolMetadata = array(); private $blameURI; private $coverage = array(); + private $data; public function setFile(PhabricatorFile $file) { $this->file = $file; @@ -65,6 +66,10 @@ final class PhabricatorDocumentRef return $this->byteLength; } + if ($this->data !== null) { + return strlen($this->data); + } + if ($this->file) { return (int)$this->file->getByteSize(); } @@ -72,7 +77,26 @@ final class PhabricatorDocumentRef return null; } + public function setData($data) { + $this->data = $data; + return $this; + } + public function loadData($begin = null, $end = null) { + if ($this->data !== null) { + $data = $this->data; + + if ($begin !== null && $end !== null) { + $data = substr($data, $begin, $end - $begin); + } else if ($begin !== null) { + $data = substr($data, $begin); + } else if ($end !== null) { + $data = substr($data, 0, $end); + } + + return $data; + } + if ($this->file) { $iterator = $this->file->getFileDataIterator($begin, $end);