Page MenuHomePhabricator
Files F13986270

D16990.diff
No OneTemporary
Actions

D16990.diff
View Options

diff --git a/src/applications/diffusion/controller/DiffusionBrowseController.php b/src/applications/diffusion/controller/DiffusionBrowseController.php
--- a/src/applications/diffusion/controller/DiffusionBrowseController.php
+++ b/src/applications/diffusion/controller/DiffusionBrowseController.php
@@ -1651,6 +1651,7 @@
protected function buildCurtain(DiffusionRequest $drequest) {
$viewer = $this->getViewer();
+ $repository = $drequest->getRepository();
$curtain = $this->newCurtainView($drequest);
@@ -1666,6 +1667,21 @@
->setIcon('fa-list'));
$behind_head = $drequest->getSymbolicCommit();
+
+ if ($repository->supportsBranchComparison()) {
+ $compare_uri = $drequest->generateURI(
+ array(
+ 'action' => 'compare',
+ ));
+
+ $curtain->addAction(
+ id(new PhabricatorActionView())
+ ->setName(pht('Compare Against...'))
+ ->setIcon('fa-code-fork')
+ ->setWorkflow(true)
+ ->setHref($compare_uri));
+ }
+
$head_uri = $drequest->generateURI(
array(
'commit' => '',
diff --git a/src/applications/diffusion/controller/DiffusionCompareController.php b/src/applications/diffusion/controller/DiffusionCompareController.php
--- a/src/applications/diffusion/controller/DiffusionCompareController.php
+++ b/src/applications/diffusion/controller/DiffusionCompareController.php
@@ -16,104 +16,87 @@
$drequest = $this->getDiffusionRequest();
$repository = $drequest->getRepository();
- $content = array();
-
- $crumbs = $this->buildCrumbs(array('view' => 'compare'));
-
- $empty_title = null;
- $error_message = null;
-
- if ($repository->getVersionControlSystem() !=
- PhabricatorRepositoryType::REPOSITORY_TYPE_GIT) {
- $empty_title = pht('Not supported');
- $error_message = pht(
- 'This feature is not yet supported for %s repositories.',
- $repository->getVersionControlSystem());
- $content[] = id(new PHUIInfoView())
- ->setTitle($empty_title)
- ->setSeverity(PHUIInfoView::SEVERITY_WARNING)
- ->setErrors(array($error_message));
+ if (!$repository->supportsBranchComparison()) {
+ return $this->newDialog()
+ ->setTitle(pht('Not Supported'))
+ ->appendParagraph(
+ pht(
+ 'Branch comparison is not supported for this version control '.
+ 'system.'))
+ ->addCancelButton($this->getApplicationURI(), pht('Okay'));
}
$head_ref = $request->getStr('head');
$against_ref = $request->getStr('against');
- $refs = id(new DiffusionCachedResolveRefsQuery())
- ->setRepository($repository)
- ->withRefs(array($head_ref, $against_ref))
- ->execute();
+ $must_prompt = false;
+ if (!$request->isFormPost()) {
+ if (!strlen($head_ref)) {
+ $head_ref = $drequest->getSymbolicCommit();
+ if (!strlen($head_ref)) {
+ $head_ref = $drequest->getBranch();
+ }
+ }
+ if (!strlen($against_ref)) {
+ $default_branch = $repository->getDefaultBranch();
+ if ($default_branch != $head_ref) {
+ $against_ref = $default_branch;
- if (count($refs) == 2) {
- $content[] = $this->buildCompareContent($drequest);
- } else {
- $content[] = $this->buildCompareForm($request, $refs);
+ // If we filled this in by default, we want to prompt the user to
+ // confirm that this is really what they want.
+ $must_prompt = true;
+ }
+ }
}
-
- return $this->newPage()
- ->setTitle(
+ $refs = $drequest->resolveRefs(
+ array_filter(
array(
- $repository->getName(),
- $repository->getDisplayName(),
- ))
- ->setCrumbs($crumbs)
- ->appendChild($content);
- }
-
- private function buildCompareForm(AphrontRequest $request, array $resolved) {
- $viewer = $this->getViewer();
-
- $head_ref = $request->getStr('head');
- $against_ref = $request->getStr('against');
+ $head_ref,
+ $against_ref,
+ )));
- if (idx($resolved, $head_ref)) {
- $e_head = null;
+ $identical = false;
+ if ($head_ref === $against_ref) {
+ $identical = true;
} else {
- $e_head = 'Not Found';
+ if (count($refs) == 2) {
+ if ($refs[$head_ref] === $refs[$against_ref]) {
+ $identical = true;
+ }
+ }
}
- if (idx($resolved, $against_ref)) {
- $e_against = null;
- } else {
- $e_against = 'Not Found';
+ if ($must_prompt || count($refs) != 2 || $identical) {
+ return $this->buildCompareDialog(
+ $head_ref,
+ $against_ref,
+ $refs,
+ $identical);
}
+ if ($request->isFormPost()) {
+ // Redirect to a stable URI that can be copy/pasted.
+ $compare_uri = $drequest->generateURI(
+ array(
+ 'action' => 'compare',
+ 'head' => $head_ref,
+ 'against' => $against_ref,
+ ));
- $form = id(new AphrontFormView())
- ->setUser($viewer)
- ->setMethod('GET')
- ->appendControl(
- id(new AphrontFormTextControl())
- ->setLabel(pht('Head'))
- ->setName('head')
- ->setError($e_head)
- ->setValue($head_ref))
- ->appendControl(
- id(new AphrontFormTextControl())
- ->setLabel(pht('Against'))
- ->setName('against')
- ->setError($e_against)
- ->setValue($against_ref))
- ->appendControl(
- id(new AphrontFormSubmitControl())
- ->setValue('Compare'));
-
- return $form;
- }
-
- private function buildCompareContent(DiffusionRequest $drequest) {
- $request = $this->getRequest();
- $repository = $drequest->getRepository();
-
- $head_ref = $request->getStr('head');
- $against_ref = $request->getStr('against');
+ return id(new AphrontRedirectResponse())->setURI($compare_uri);
+ }
- $content = array();
+ $crumbs = $this->buildCrumbs(
+ array(
+ 'view' => 'compare',
+ ));
$pager = id(new PHUIPagerView())
->readFromRequest($request);
+ $history = null;
try {
$history_results = $this->callConduitWithDiffusionRequest(
'diffusion.historyquery',
@@ -126,111 +109,189 @@
));
$history = DiffusionPathChange::newFromConduit(
$history_results['pathChanges']);
-
$history = $pager->sliceResults($history);
- $history_exception = null;
+ $history_view = $this->newHistoryView(
+ $history_results,
+ $history,
+ $pager,
+ $head_ref,
+ $against_ref);
+
} catch (Exception $ex) {
- $history_results = null;
- $history = null;
- $history_exception = $ex;
+ if ($repository->isImporting()) {
+ $history_view = $this->renderStatusMessage(
+ pht('Still Importing...'),
+ pht(
+ 'This repository is still importing. History is not yet '.
+ 'available.'));
+ } else {
+ $history_view = $this->renderStatusMessage(
+ pht('Unable to Retrieve History'),
+ $ex->getMessage());
+ }
}
- if ($history_results) {
- $content[] = $this->buildCompareProperties($drequest, $history_results);
- }
- $content[] = $this->buildCompareForm(
- $request,
- array($head_ref => true, $against_ref => true));
+ $header = id(new PHUIHeaderView())
+ ->setHeader(
+ pht(
+ 'Changes on %s but not %s',
+ phutil_tag('em', array(), $head_ref),
+ phutil_tag('em', array(), $against_ref)));
- $content[] = $this->buildHistoryTable(
- $history_results,
- $history,
- $pager,
- $history_exception);
+ $curtain = $this->buildCurtain($head_ref, $against_ref);
- $content[] = $this->renderTablePagerBox($pager);
+ $column_view = id(new PHUITwoColumnView())
+ ->setHeader($header)
+ ->setCurtain($curtain)
+ ->setMainColumn(
+ array(
+ $history_view,
+ ));
- return $content;
+ return $this->newPage()
+ ->setTitle(
+ array(
+ $repository->getName(),
+ $repository->getDisplayName(),
+ ))
+ ->setCrumbs($crumbs)
+ ->appendChild($column_view);
}
- private function buildCompareProperties($drequest, array $history_results) {
- $viewer = $this->getViewer();
+ private function buildCompareDialog(
+ $head_ref,
+ $against_ref,
+ array $resolved,
+ $identical) {
+ $viewer = $this->getViewer();
$request = $this->getRequest();
+ $drequest = $this->getDiffusionRequest();
$repository = $drequest->getRepository();
- $head_ref = $request->getStr('head');
- $against_ref = $request->getStr('against');
+ $e_head = null;
+ $e_against = null;
+ $errors = array();
+ if ($request->isFormPost()) {
+ if (!strlen($head_ref)) {
+ $e_head = pht('Required');
+ $errors[] = pht(
+ 'You must provide two different commits to compare.');
+ } else if (!isset($resolved[$head_ref])) {
+ $e_head = pht('Not Found');
+ $errors[] = pht(
+ 'Commit "%s" is not a valid commit in this repository.',
+ $head_ref);
+ }
+
+ if (!strlen($against_ref)) {
+ $e_against = pht('Required');
+ $errors[] = pht(
+ 'You must provide two different commits to compare.');
+ } else if (!isset($resolved[$against_ref])) {
+ $e_against = pht('Not Found');
+ $errors[] = pht(
+ 'Commit "%s" is not a valid commit in this repository.',
+ $against_ref);
+ }
- $reverse_uri = $repository->getPathURI(
- "compare/?head=${against_ref}&against=${head_ref}");
- $actions = id(new PhabricatorActionListView());
- $actions->setUser($viewer);
+ if ($identical) {
+ $e_head = pht('Identical');
+ $e_against = pht('Identical');
+ $errors[] = pht(
+ 'Both references identify the same commit. You can not compare a '.
+ 'commit against itself.');
+ }
+ }
+ $form = id(new AphrontFormView())
+ ->setViewer($viewer)
+ ->appendControl(
+ id(new AphrontFormTextControl())
+ ->setLabel(pht('Head'))
+ ->setName('head')
+ ->setError($e_head)
+ ->setValue($head_ref))
+ ->appendControl(
+ id(new AphrontFormTextControl())
+ ->setLabel(pht('Against'))
+ ->setName('against')
+ ->setError($e_against)
+ ->setValue($against_ref));
+
+ $cancel_uri = $repository->generateURI(
+ array(
+ 'action' => 'browse',
+ ));
+
+ return $this->newDialog()
+ ->setTitle(pht('Compare Against'))
+ ->setWidth(AphrontDialogView::WIDTH_FORM)
+ ->setErrors($errors)
+ ->appendForm($form)
+ ->addSubmitButton(pht('Compare'))
+ ->addCancelButton($cancel_uri, pht('Cancel'));
+ }
- $actions->addAction(id(new PhabricatorActionView())
+ private function buildCurtain($head_ref, $against_ref) {
+ $viewer = $this->getViewer();
+ $request = $this->getRequest();
+ $drequest = $this->getDiffusionRequest();
+ $repository = $drequest->getRepository();
+
+ $curtain = $this->newCurtainView(null);
+
+ $reverse_uri = $drequest->generateURI(
+ array(
+ 'action' => 'compare',
+ 'head' => $against_ref,
+ 'against' => $head_ref,
+ ));
+
+ $curtain->addAction(
+ id(new PhabricatorActionView())
->setName(pht('Reverse Comparison'))
->setHref($reverse_uri)
- ->setIcon('fa-list'));
+ ->setIcon('fa-refresh'));
- $view = id(new PHUIPropertyListView())
- ->setUser($viewer)
- ->setActionList($actions);
+ $compare_uri = $drequest->generateURI(
+ array(
+ 'action' => 'compare',
+ 'head' => $head_ref,
+ ));
- $readme =
- 'These are the commits that are reachable from **Head** commit and not '.
- 'from the **Against** commit.';
- $readme = new PHUIRemarkupView($viewer, $readme);
- $view->addTextContent($readme);
+ $curtain->addAction(
+ id(new PhabricatorActionView())
+ ->setName(pht('Compare Against...'))
+ ->setIcon('fa-code-fork')
+ ->setWorkflow(true)
+ ->setHref($compare_uri));
- $view->addProperty(pht('Head'), $head_ref);
- $view->addProperty(pht('Against'), $against_ref);
+ // TODO: Provide a "Show Diff" action.
- $header = id(new PHUIHeaderView())
- ->setUser($viewer)
- ->setPolicyObject($drequest->getRepository());
-
- $box = id(new PHUIObjectBoxView())
- ->setUser($viewer)
- ->setHeader($header)
- ->addPropertyList($view);
- return $box;
+ return $curtain;
}
- private function buildHistoryTable(
- $history_results,
- $history,
- $pager,
- $history_exception) {
+ private function newHistoryView(
+ array $results,
+ array $history,
+ PHUIPagerView $pager,
+ $head_ref,
+ $against_ref) {
$request = $this->getRequest();
- $viewer = $request->getUser();
+ $viewer = $this->getViewer();
$drequest = $this->getDiffusionRequest();
$repository = $drequest->getRepository();
- if ($history_exception) {
- if ($repository->isImporting()) {
- return $this->renderStatusMessage(
- pht('Still Importing...'),
- pht(
- 'This repository is still importing. History is not yet '.
- 'available.'));
- } else {
- return $this->renderStatusMessage(
- pht('Unable to Retrieve History'),
- $history_exception->getMessage());
- }
- }
-
if (!$history) {
return $this->renderStatusMessage(
- pht('Up to date'),
- new PHUIRemarkupView(
- $viewer,
- pht(
- '**Against** is strictly ahead of **Head** '.
- '- no commits are missing.')));
+ pht('Up To Date'),
+ pht(
+ 'There are no commits on %s that are not already on %s.',
+ phutil_tag('strong', array(), $head_ref),
+ phutil_tag('strong', array(), $against_ref)));
}
$history_table = id(new DiffusionHistoryTableView())
@@ -238,23 +299,27 @@
->setDiffusionRequest($drequest)
->setHistory($history);
- // TODO: Super sketchy?
$history_table->loadRevisions();
- if ($history_results) {
- $history_table->setParents($history_results['parents']);
+ if ($history) {
+ $history_table->setParents($results['parents']);
$history_table->setIsHead(!$pager->getOffset());
$history_table->setIsTail(!$pager->getHasMorePages());
}
- // TODO also expose diff.
-
- $panel = new PHUIObjectBoxView();
$header = id(new PHUIHeaderView())
- ->setHeader(pht('Missing Commits'));
- $panel->setHeader($header);
- $panel->setTable($history_table);
+ ->setHeader(pht('Commits'));
+
+ $object_box = id(new PHUIObjectBoxView())
+ ->setHeader($header)
+ ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY)
+ ->setTable($history_table);
+
+ $pager_box = $this->renderTablePagerBox($pager);
- return $panel;
+ return array(
+ $object_box,
+ $pager_box,
+ );
}
}
diff --git a/src/applications/diffusion/request/DiffusionRequest.php b/src/applications/diffusion/request/DiffusionRequest.php
--- a/src/applications/diffusion/request/DiffusionRequest.php
+++ b/src/applications/diffusion/request/DiffusionRequest.php
@@ -644,7 +644,7 @@
return $match;
}
- private function resolveRefs(array $refs, array $types) {
+ public function resolveRefs(array $refs, array $types = array()) {
// First, try to resolve refs from fast cache sources.
$cached_query = id(new DiffusionCachedResolveRefsQuery())
->setRepository($this->getRepository())
diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php
--- a/src/applications/repository/storage/PhabricatorRepository.php
+++ b/src/applications/repository/storage/PhabricatorRepository.php
@@ -689,6 +689,7 @@
case 'lint':
case 'pathtree':
case 'refs':
+ case 'compare':
break;
case 'branch':
// NOTE: This does not actually require a branch, and won't have one
@@ -710,6 +711,9 @@
$commit = idx($params, 'commit');
$line = idx($params, 'line');
+ $head = idx($params, 'head');
+ $against = idx($params, 'against');
+
if ($req_commit && !strlen($commit)) {
throw new Exception(
pht(
@@ -734,11 +738,13 @@
$path = phutil_escape_uri($path);
}
+ $raw_branch = $branch;
if (strlen($branch)) {
$branch = phutil_escape_uri_path_component($branch);
$path = "{$branch}/{$path}";
}
+ $raw_commit = $commit;
if (strlen($commit)) {
$commit = str_replace('$', '$$', $commit);
$commit = ';'.phutil_escape_uri($commit);
@@ -748,6 +754,7 @@
$line = '$'.phutil_escape_uri($line);
}
+ $query = array();
switch ($action) {
case 'change':
case 'history':
@@ -760,6 +767,20 @@
case 'refs':
$uri = $this->getPathURI("/{$action}/{$path}{$commit}{$line}");
break;
+ case 'compare':
+ $uri = $this->getPathURI("/{$action}/");
+ if (strlen($head)) {
+ $query['head'] = $head;
+ } else if (strlen($raw_commit)) {
+ $query['commit'] = $raw_commit;
+ } else if (strlen($raw_branch)) {
+ $query['head'] = $raw_branch;
+ }
+
+ if (strlen($against)) {
+ $query['against'] = $against;
+ }
+ break;
case 'branch':
if (strlen($path)) {
$uri = $this->getPathURI("/repository/{$path}");
@@ -791,8 +812,10 @@
);
}
- if (idx($params, 'params')) {
- $uri->setQueryParams($params['params']);
+ $query = idx($params, 'params', array()) + $query;
+
+ if ($query) {
+ $uri->setQueryParams($query);
}
return $uri;
@@ -2066,6 +2089,10 @@
return $result;
}
+ public function supportsBranchComparison() {
+ return $this->isGit();
+ }
+
/* -( Repository URIs )---------------------------------------------------- */

File Metadata

Mime Type
text/plain
Expires
Tue, Oct 22, 1:42 AM (4 w, 3 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6740403
Default Alt Text
D16990.diff (18 KB)

Event Timeline

Phabricator · Built by Phacility