From 3d5f03607b8708d195014aa406b65e08ec47fadf Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 30 Mar 2011 17:36:16 -0700 Subject: [PATCH] Rough cut of Diffusion change view. --- src/__phutil_library_map__.php | 5 + ...AphrontDefaultApplicationConfiguration.php | 5 + .../DifferentialChangesetListView.php | 10 +- .../controller/base/DiffusionController.php | 24 +-- .../change/DiffusionChangeController.php | 61 ++------ .../diffusion/controller/change/__init__.php | 3 + .../diff/DiffusionDiffController.php | 75 ++++++++++ .../diffusion/controller/diff/__init__.php | 19 +++ .../data/pathchange/DiffusionPathChange.php | 13 +- .../query/diff/base/DiffusionDiffQuery.php | 60 ++++++++ .../diffusion/query/diff/base/__init__.php | 14 ++ .../query/diff/svn/DiffusionSvnDiffQuery.php | 137 ++++++++++++++++++ .../diffusion/query/diff/svn/__init__.php | 23 +++ .../base/DiffusionPathChangeQuery.php | 1 + .../diffusion/view/base/DiffusionView.php | 4 +- .../DiffusionCommitChangeTableView.php | 3 +- 16 files changed, 393 insertions(+), 64 deletions(-) create mode 100644 src/applications/diffusion/controller/diff/DiffusionDiffController.php create mode 100644 src/applications/diffusion/controller/diff/__init__.php create mode 100644 src/applications/diffusion/query/diff/base/DiffusionDiffQuery.php create mode 100644 src/applications/diffusion/query/diff/base/__init__.php create mode 100644 src/applications/diffusion/query/diff/svn/DiffusionSvnDiffQuery.php create mode 100644 src/applications/diffusion/query/diff/svn/__init__.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index a9ba1ba41d..a23254aa3c 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -157,6 +157,8 @@ phutil_register_library_map(array( 'DiffusionCommitChangeTableView' => 'applications/diffusion/view/commitchangetable', 'DiffusionCommitController' => 'applications/diffusion/controller/commit', 'DiffusionController' => 'applications/diffusion/controller/base', + 'DiffusionDiffController' => 'applications/diffusion/controller/diff', + 'DiffusionDiffQuery' => 'applications/diffusion/query/diff/base', 'DiffusionFileContent' => 'applications/diffusion/data/filecontent', 'DiffusionFileContentQuery' => 'applications/diffusion/query/filecontent/base', 'DiffusionGitBranchQuery' => 'applications/diffusion/query/branch/git', @@ -175,6 +177,7 @@ phutil_register_library_map(array( 'DiffusionRepositoryPath' => 'applications/diffusion/data/repositorypath', 'DiffusionRequest' => 'applications/diffusion/request/base', 'DiffusionSvnBrowseQuery' => 'applications/diffusion/query/browse/svn', + 'DiffusionSvnDiffQuery' => 'applications/diffusion/query/diff/svn', 'DiffusionSvnFileContentQuery' => 'applications/diffusion/query/filecontent/svn', 'DiffusionSvnHistoryQuery' => 'applications/diffusion/query/history/svn', 'DiffusionSvnRequest' => 'applications/diffusion/request/svn', @@ -525,6 +528,7 @@ phutil_register_library_map(array( 'DiffusionCommitChangeTableView' => 'DiffusionView', 'DiffusionCommitController' => 'DiffusionController', 'DiffusionController' => 'PhabricatorController', + 'DiffusionDiffController' => 'DiffusionController', 'DiffusionGitBranchQuery' => 'DiffusionBranchQuery', 'DiffusionGitBrowseQuery' => 'DiffusionBrowseQuery', 'DiffusionGitFileContentQuery' => 'DiffusionFileContentQuery', @@ -535,6 +539,7 @@ phutil_register_library_map(array( 'DiffusionHomeController' => 'DiffusionController', 'DiffusionRepositoryController' => 'DiffusionController', 'DiffusionSvnBrowseQuery' => 'DiffusionBrowseQuery', + 'DiffusionSvnDiffQuery' => 'DiffusionDiffQuery', 'DiffusionSvnFileContentQuery' => 'DiffusionFileContentQuery', 'DiffusionSvnHistoryQuery' => 'DiffusionHistoryQuery', 'DiffusionSvnRequest' => 'DiffusionRequest', diff --git a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php index 6628df4087..268564389a 100644 --- a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php +++ b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php @@ -205,6 +205,11 @@ class AphrontDefaultApplicationConfiguration '(?:[$](?P\d+))?'. '$' => 'DiffusionBrowseController', + 'diff/'. + '(?P.*?)'. + '(?:[;](?P[a-z0-9]+))?'. + '$' + => 'DiffusionDiffController', ), ), diff --git a/src/applications/differential/view/changesetlistview/DifferentialChangesetListView.php b/src/applications/differential/view/changesetlistview/DifferentialChangesetListView.php index fe526ae440..66fa6dd472 100644 --- a/src/applications/differential/view/changesetlistview/DifferentialChangesetListView.php +++ b/src/applications/differential/view/changesetlistview/DifferentialChangesetListView.php @@ -21,6 +21,7 @@ class DifferentialChangesetListView extends AphrontView { private $changesets = array(); private $editable; private $revision; + private $renderURI = '/differential/changeset/'; private $vsMap = array(); public function setChangesets($changesets) { @@ -43,6 +44,11 @@ class DifferentialChangesetListView extends AphrontView { return $this; } + public function setRenderURI($render_uri) { + $this->renderURI = $render_uri; + return $this; + } + public function render() { require_celerity_resource('differential-changeset-view-css'); @@ -105,11 +111,11 @@ class DifferentialChangesetListView extends AphrontView { Javelin::initBehavior('differential-populate', array( 'registry' => $mapping, 'whitespace' => $whitespace, - 'uri' => '/differential/changeset/', + 'uri' => $this->renderURI, )); Javelin::initBehavior('differential-show-more', array( - 'uri' => '/differential/changeset/', + 'uri' => $this->renderURI, )); if ($this->editable) { diff --git a/src/applications/diffusion/controller/base/DiffusionController.php b/src/applications/diffusion/controller/base/DiffusionController.php index 4b227b10a4..6736571ba6 100644 --- a/src/applications/diffusion/controller/base/DiffusionController.php +++ b/src/applications/diffusion/controller/base/DiffusionController.php @@ -144,6 +144,19 @@ abstract class DiffusionController extends PhabricatorController { $view = $spec['view']; + $path = null; + if (isset($spec['path'])) { + $path = $drequest->getPath(); + } + + if ($raw_commit) { + $commit_link = DiffusionView::linkCommit( + $repository, + $raw_commit); + } else { + $commit_link = ''; + } + switch ($view) { case 'history': $view_name = 'History'; @@ -152,16 +165,12 @@ abstract class DiffusionController extends PhabricatorController { $view_name = 'Browse'; break; case 'change': - $crumb_list[] = 'TODO CHANGE'; + $view_name = 'Change'; + $crumb_list[] = phutil_escape_html($path).' ('.$commit_link.')'; $crumbs->setCrumbs($crumb_list); return $crumbs; } - $path = null; - if (isset($spec['path'])) { - $path = $drequest->getPath(); - } - $view_root_uri = "/diffusion/{$callsign}/{$view}/{$branch_uri}"; $jump_href = $view_root_uri; @@ -209,9 +218,6 @@ abstract class DiffusionController extends PhabricatorController { $last_crumb = array_pop($crumb_list); if ($raw_commit) { - $commit_link = DiffusionView::linkCommit( - $repository, - $raw_commit); $jump_link = phutil_render_tag( 'a', array( diff --git a/src/applications/diffusion/controller/change/DiffusionChangeController.php b/src/applications/diffusion/controller/change/DiffusionChangeController.php index 33ec3bd07e..e6cb4318ab 100644 --- a/src/applications/diffusion/controller/change/DiffusionChangeController.php +++ b/src/applications/diffusion/controller/change/DiffusionChangeController.php @@ -21,9 +21,6 @@ class DiffusionChangeController extends DiffusionController { public function processRequest() { $drequest = $this->diffusionRequest; -// $browse_query = DiffusionBrowseQuery::newFromDiffusionRequest($drequest); -// $results = $browse_query->loadPaths(); - $content = array(); $content[] = $this->buildCrumbs( @@ -33,55 +30,23 @@ class DiffusionChangeController extends DiffusionController { 'view' => 'change', )); -/* - if (!$results) { - switch ($browse_query->getReasonForEmptyResultSet()) { - case DiffusionBrowseQuery::REASON_IS_NONEXISTENT: - $title = 'Path Does Not Exist'; - // TODO: Under git, this error message should be more specific. It - // may exist on some other branch. - $body = "This path does not exist anywhere."; - $severity = AphrontErrorView::SEVERITY_ERROR; - break; - case DiffusionBrowseQuery::REASON_IS_DELETED: - // TODO: Format all these commits into nice VCS-agnostic links. - $commit = $drequest->getCommit(); - $deleted = $browse_query->getDeletedAtCommit(); - $existed = $browse_query->getExistedAtCommit(); + $diff_query = DiffusionDiffQuery::newFromDiffusionRequest($drequest); + $changeset = $diff_query->loadChangeset(); - $title = 'Path Was Deleted'; - $body = "This path does not exist at {$commit}. It was deleted in ". - "{$deleted} and last existed at {$existed}."; - $severity = AphrontErrorView::SEVERITY_WARNING; - break; - case DiffusionBrowseQuery::REASON_IS_FILE: - $controller = new DiffusionBrowseFileController($this->getRequest()); - $controller->setDiffusionRequest($drequest); - return $this->delegateToController($controller); - break; - default: - throw new Exception("Unknown failure reason!"); - } + $changeset_view = new DifferentialChangesetListView(); + $changeset_view->setChangesets(array($changeset)); + $changeset_view->setRenderURI( + '/diffusion/'.$drequest->getRepository()->getCallsign().'/diff/'); - $error_view = new AphrontErrorView(); - $error_view->setSeverity($severity); - $error_view->setTitle($title); - $error_view->appendChild('

'.$body.'

'); + // TODO: This is pretty awkward, unify the CSS between Diffusion and + // Differential better. + require_celerity_resource('differential-core-view-css'); + $content[] = + '
'. + $changeset_view->render(). + '
'; - $content[] = $error_view; - - } else { - $browse_table = new DiffusionBrowseTableView(); - $browse_table->setDiffusionRequest($drequest); - $browse_table->setPaths($results); - - $browse_panel = new AphrontPanelView(); - $browse_panel->appendChild($browse_table); - - $content[] = $browse_panel; - } -*/ $nav = $this->buildSideNav('change', true); $nav->appendChild($content); diff --git a/src/applications/diffusion/controller/change/__init__.php b/src/applications/diffusion/controller/change/__init__.php index 5ee7801e41..8c5bc94613 100644 --- a/src/applications/diffusion/controller/change/__init__.php +++ b/src/applications/diffusion/controller/change/__init__.php @@ -6,7 +6,10 @@ +phutil_require_module('phabricator', 'applications/differential/view/changesetlistview'); phutil_require_module('phabricator', 'applications/diffusion/controller/base'); +phutil_require_module('phabricator', 'applications/diffusion/query/diff/base'); +phutil_require_module('phabricator', 'infrastructure/celerity/api'); phutil_require_source('DiffusionChangeController.php'); diff --git a/src/applications/diffusion/controller/diff/DiffusionDiffController.php b/src/applications/diffusion/controller/diff/DiffusionDiffController.php new file mode 100644 index 0000000000..eea585324b --- /dev/null +++ b/src/applications/diffusion/controller/diff/DiffusionDiffController.php @@ -0,0 +1,75 @@ +getRequest(); + if ($request->getStr('id')) { + $parts = explode(';', $request->getStr('id')); + $data['path'] = idx($parts, 0); + $data['commit'] = idx($parts, 1); + } + + $this->diffusionRequest = DiffusionRequest::newFromAphrontRequestDictionary( + $data); + } + + public function processRequest() { + $drequest = $this->getDiffusionRequest(); + $request = $this->getRequest(); + + $diff_query = DiffusionDiffQuery::newFromDiffusionRequest($drequest); + $changeset = $diff_query->loadChangeset(); + + if (!$changeset) { + return new Aphront404Response(); + } + + $parser = new DifferentialChangesetParser(); + $parser->setChangeset($changeset); + $parser->setWhitespaceMode( + DifferentialChangesetParser::WHITESPACE_SHOW_ALL); + + $range_s = null; + $range_e = null; + $mask = array(); + + // TODO: This duplicates a block in DifferentialChangesetViewController. + $range = $request->getStr('range'); + if ($range) { + $match = null; + if (preg_match('@^(\d+)-(\d+)(?:/(\d+)-(\d+))?$@', $range, $match)) { + $range_s = (int)$match[1]; + $range_e = (int)$match[2]; + if (count($match) > 3) { + $start = (int)$match[3]; + $len = (int)$match[4]; + for ($ii = $start; $ii < $start + $len; $ii++) { + $mask[$ii] = true; + } + } + } + } + + $output = $parser->render($range_s, $range_e, $mask); + + return id(new AphrontAjaxResponse()) + ->setContent($output); + } +} diff --git a/src/applications/diffusion/controller/diff/__init__.php b/src/applications/diffusion/controller/diff/__init__.php new file mode 100644 index 0000000000..0ac3138b7c --- /dev/null +++ b/src/applications/diffusion/controller/diff/__init__.php @@ -0,0 +1,19 @@ +path = $path; @@ -63,6 +65,15 @@ final class DiffusionPathChange { return $this->targetPath; } + public function setAwayPaths(array $away_paths) { + $this->awayPaths = $away_paths; + return $this; + } + + public function getAwayPaths() { + return $this->awayPaths; + } + final public function setCommitIdentifier($commit) { $this->commitIdentifier = $commit; return $this; @@ -115,6 +126,4 @@ final class DiffusionPathChange { } - - } diff --git a/src/applications/diffusion/query/diff/base/DiffusionDiffQuery.php b/src/applications/diffusion/query/diff/base/DiffusionDiffQuery.php new file mode 100644 index 0000000000..f9c858f98b --- /dev/null +++ b/src/applications/diffusion/query/diff/base/DiffusionDiffQuery.php @@ -0,0 +1,60 @@ + + } + + final public static function newFromDiffusionRequest( + DiffusionRequest $request) { + + $repository = $request->getRepository(); + + switch ($repository->getVersionControlSystem()) { + case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: + $class = 'DiffusionGitDiffQuery'; + break; + case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: + $class = 'DiffusionSvnDiffQuery'; + break; + default: + throw new Exception("Unsupported VCS!"); + } + + PhutilSymbolLoader::loadClass($class); + $query = new $class(); + + $query->request = $request; + + return $query; + } + + final protected function getRequest() { + return $this->request; + } + + final public function loadChangeset() { + return $this->executeQuery(); + } + + abstract protected function executeQuery(); +} diff --git a/src/applications/diffusion/query/diff/base/__init__.php b/src/applications/diffusion/query/diff/base/__init__.php new file mode 100644 index 0000000000..500696c5a5 --- /dev/null +++ b/src/applications/diffusion/query/diff/base/__init__.php @@ -0,0 +1,14 @@ +getRequest(); + + $path_change_query = DiffusionPathChangeQuery::newFromDiffusionRequest( + $drequest); + $path_changes = $path_change_query->loadChanges(); + + $path = null; + foreach ($path_changes as $change) { + if ($change->getPath() == $drequest->getPath()) { + $path = $change; + } + } + + if (!$path) { + return null; + } + + $change_type = $path->getChangeType(); + switch ($change_type) { + case DifferentialChangeType::TYPE_MULTICOPY: + case DifferentialChangeType::TYPE_DELETE: + if ($path->getTargetPath()) { + $old = array( + $path->getTargetPath(), + $path->getTargetCommitIdentifier()); + } else { + $old = array($path->getPath(), $path->getCommitIdentifier() - 1); + } + $old_name = $path->getPath(); + $new_name = ''; + $new = null; + break; + case DifferentialChangeType::TYPE_ADD: + $old = null; + $new = array($path->getPath(), $path->getCommitIdentifier()); + $old_name = ''; + $new_name = $path->getPath(); + break; + case DifferentialChangeType::TYPE_MOVE_HERE: + case DifferentialChangeType::TYPE_COPY_HERE: + $old = array( + $path->getTargetPath(), + $path->getTargetCommitIdentifier()); + $new = array($path->getPath(), $path->getCommitIdentifier()); + $old_name = $path->getTargetPath(); + $new_name = $path->getPath(); + break; + default: + $old = array($path->getPath(), $path->getCommitIdentifier() - 1); + $new = array($path->getPath(), $path->getCommitIdentifier()); + $old_name = $path->getPath(); + $new_name = $path->getPath(); + break; + } + + $futures = array( + 'old' => $this->buildContentFuture($old), + 'new' => $this->buildContentFuture($new), + ); + $futures = array_filter($futures); + + foreach (Futures($futures) as $key => $future) { + $futures[$key] = $future->resolvex(); + } + + $old_data = idx($futures, 'old', ''); + $new_data = idx($futures, 'new', ''); + + $old_tmp = new TempFile(); + $new_tmp = new TempFile(); + + Filesystem::writeFile($old_tmp, $old_data); + Filesystem::writeFile($new_tmp, $new_data); + + list($err, $raw_diff) = exec_manual( + 'diff -L %s -L %s -U65535 %s %s', + nonempty($old_name, '/dev/universe').' 9999-99-99', + nonempty($new_name, '/dev/universe').' 9999-99-99', + $old_tmp, + $new_tmp); + + $parser = new ArcanistDiffParser(); + $parser->setDetectBinaryFiles(true); + + $change = $parser->parseDiffusionPathChangesAndRawDiff( + $drequest->getPath(), + $path_changes, + $raw_diff); + + $diff = DifferentialDiff::newFromRawChanges(array($change)); + $changesets = $diff->getChangesets(); + $changeset = reset($changesets); + + $id = $drequest->getPath().';'.$drequest->getCommit(); + $changeset->setID($id); + + return $changeset; + } + + private function buildContentFuture($spec) { + if (!$spec) { + return null; + } + + $drequest = $this->getRequest(); + $repository = $drequest->getRepository(); + + list($ref, $rev) = $spec; + return new ExecFuture( + 'svn --non-interactive cat %s%s@%d', + $repository->getDetail('remote-uri'), + $ref, + $rev); + } + +} diff --git a/src/applications/diffusion/query/diff/svn/__init__.php b/src/applications/diffusion/query/diff/svn/__init__.php new file mode 100644 index 0000000000..17a8b4ac46 --- /dev/null +++ b/src/applications/diffusion/query/diff/svn/__init__.php @@ -0,0 +1,23 @@ +setPath(ltrim($raw_change['pathName'], '/')); $change->setChangeType($raw_change['changeType']); $change->setFileType($raw_change['fileType']); + $change->setCommitIdentifier($commit->getCommitIdentifier()); $changes[] = $change; } diff --git a/src/applications/diffusion/view/base/DiffusionView.php b/src/applications/diffusion/view/base/DiffusionView.php index 13020d6a0b..ff1856345a 100644 --- a/src/applications/diffusion/view/base/DiffusionView.php +++ b/src/applications/diffusion/view/base/DiffusionView.php @@ -29,7 +29,7 @@ abstract class DiffusionView extends AphrontView { return $this->diffusionRequest; } - final public function linkChange($change_type, $file_type) { + final public function linkChange($change_type, $file_type, $path = null) { $text = DifferentialChangeType::getFullNameForChangeType($change_type); if ($change_type == DifferentialChangeType::TYPE_CHILD) { @@ -52,7 +52,7 @@ abstract class DiffusionView extends AphrontView { $callsign = $repository->getCallsign(); $branch = $drequest->getBranchURIComponent($drequest->getBranch()); - $path = $branch.$drequest->getPath(); + $path = $branch.($path ? $path : $drequest->getPath()); return phutil_render_tag( 'a', diff --git a/src/applications/diffusion/view/commitchangetable/DiffusionCommitChangeTableView.php b/src/applications/diffusion/view/commitchangetable/DiffusionCommitChangeTableView.php index bfbf911e3a..72c11b1598 100644 --- a/src/applications/diffusion/view/commitchangetable/DiffusionCommitChangeTableView.php +++ b/src/applications/diffusion/view/commitchangetable/DiffusionCommitChangeTableView.php @@ -49,7 +49,8 @@ final class DiffusionCommitChangeTableView extends DiffusionView { $this->linkBrowse($change->getPath()), $this->linkChange( $change->getChangeType(), - $change->getFileType()), + $change->getFileType(), + $change->getPath()), phutil_render_tag( 'a', array(