diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 5ad020ec06..47e68b31c2 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -556,6 +556,7 @@ phutil_register_library_map(array( 'DifferentialRevisionViewController' => 'applications/differential/controller/DifferentialRevisionViewController.php', 'DifferentialSchemaSpec' => 'applications/differential/storage/DifferentialSchemaSpec.php', 'DifferentialSetDiffPropertyConduitAPIMethod' => 'applications/differential/conduit/DifferentialSetDiffPropertyConduitAPIMethod.php', + 'DifferentialStackGraph' => 'applications/differential/edge/DifferentialStackGraph.php', 'DifferentialStoredCustomField' => 'applications/differential/customfield/DifferentialStoredCustomField.php', 'DifferentialSubscribersField' => 'applications/differential/customfield/DifferentialSubscribersField.php', 'DifferentialSummaryField' => 'applications/differential/customfield/DifferentialSummaryField.php', @@ -1599,6 +1600,7 @@ phutil_register_library_map(array( 'PHUICurtainExtension' => 'view/extension/PHUICurtainExtension.php', 'PHUICurtainPanelView' => 'view/layout/PHUICurtainPanelView.php', 'PHUICurtainView' => 'view/layout/PHUICurtainView.php', + 'PHUIDiffGraphView' => 'infrastructure/diff/view/PHUIDiffGraphView.php', 'PHUIDiffInlineCommentDetailView' => 'infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php', 'PHUIDiffInlineCommentEditView' => 'infrastructure/diff/view/PHUIDiffInlineCommentEditView.php', 'PHUIDiffInlineCommentRowScaffold' => 'infrastructure/diff/view/PHUIDiffInlineCommentRowScaffold.php', @@ -4928,6 +4930,7 @@ phutil_register_library_map(array( 'DifferentialRevisionViewController' => 'DifferentialController', 'DifferentialSchemaSpec' => 'PhabricatorConfigSchemaSpec', 'DifferentialSetDiffPropertyConduitAPIMethod' => 'DifferentialConduitAPIMethod', + 'DifferentialStackGraph' => 'AbstractDirectedGraph', 'DifferentialStoredCustomField' => 'DifferentialCustomField', 'DifferentialSubscribersField' => 'DifferentialCoreCustomField', 'DifferentialSummaryField' => 'DifferentialCoreCustomField', @@ -6132,6 +6135,7 @@ phutil_register_library_map(array( 'PHUICurtainExtension' => 'Phobject', 'PHUICurtainPanelView' => 'AphrontTagView', 'PHUICurtainView' => 'AphrontTagView', + 'PHUIDiffGraphView' => 'Phobject', 'PHUIDiffInlineCommentDetailView' => 'PHUIDiffInlineCommentView', 'PHUIDiffInlineCommentEditView' => 'PHUIDiffInlineCommentView', 'PHUIDiffInlineCommentRowScaffold' => 'AphrontView', diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index c66da78035..83c6d10288 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -341,6 +341,21 @@ final class DifferentialRevisionViewController extends DifferentialController { ->setKey('commits') ->appendChild($local_table)); + $stack_graph = id(new DifferentialStackGraph()) + ->setSeedRevision($revision) + ->loadGraph(); + if (!$stack_graph->isEmpty()) { + $stack_view = $this->renderStackView($revision, $stack_graph); + list($stack_name, $stack_color, $stack_table) = $stack_view; + + $tab_group->addTab( + id(new PHUITabView()) + ->setName($stack_name) + ->setKey('stack') + ->setColor($stack_color) + ->appendChild($stack_table)); + } + if ($other_view) { $tab_group->addTab( id(new PHUITabView()) @@ -1198,4 +1213,149 @@ final class DifferentialRevisionViewController extends DifferentialController { } + private function renderStackView( + DifferentialRevision $current, + DifferentialStackGraph $graph) { + + $ancestry = $graph->getParentEdges(); + $viewer = $this->getViewer(); + + $revisions = id(new DifferentialRevisionQuery()) + ->setViewer($viewer) + ->withPHIDs(array_keys($ancestry)) + ->execute(); + $revisions = mpull($revisions, null, 'getPHID'); + + $order = id(new PhutilDirectedScalarGraph()) + ->addNodes($ancestry) + ->getTopographicallySortedNodes(); + + $ancestry = array_select_keys($ancestry, $order); + + $traces = id(new PHUIDiffGraphView()) + ->renderGraph($ancestry); + + // Load author handles, and also revision handles for any revisions which + // we failed to load (they might be policy restricted). + $handle_phids = mpull($revisions, 'getAuthorPHID'); + foreach ($order as $phid) { + if (empty($revisions[$phid])) { + $handle_phids[] = $phid; + } + } + $handles = $viewer->loadHandles($handle_phids); + + $rows = array(); + $rowc = array(); + + $ii = 0; + $seen = false; + foreach ($ancestry as $phid => $ignored) { + $revision = idx($revisions, $phid); + if ($revision) { + $status_icon = $revision->getStatusIcon(); + $status_name = $revision->getStatusDisplayName(); + + $status = array( + id(new PHUIIconView())->setIcon($status_icon), + ' ', + $status_name, + ); + + $author = $viewer->renderHandle($revision->getAuthorPHID()); + $title = phutil_tag( + 'a', + array( + 'href' => $revision->getURI(), + ), + array( + $revision->getMonogram(), + ' ', + $revision->getTitle(), + )); + } else { + $status = null; + $author = null; + $title = $viewer->renderHandle($phid); + } + + $rows[] = array( + $traces[$ii++], + $status, + $author, + $title, + ); + + if ($phid == $current->getPHID()) { + $rowc[] = 'highlighted'; + } else { + $rowc[] = null; + } + } + + $stack_table = id(new AphrontTableView($rows)) + ->setHeaders( + array( + null, + pht('Status'), + pht('Author'), + pht('Revision'), + )) + ->setRowClasses($rowc) + ->setColumnClasses( + array( + 'threads', + null, + null, + 'wide', + )); + + // Count how many revisions this one depends on that are not yet closed. + $seen = array(); + $look = array($current->getPHID()); + while ($look) { + $phid = array_pop($look); + + $parents = idx($ancestry, $phid, array()); + foreach ($parents as $parent) { + if (isset($seen[$parent])) { + continue; + } + + $seen[$parent] = $parent; + $look[] = $parent; + } + } + + $blocking_count = 0; + foreach ($seen as $parent) { + if ($parent == $current->getPHID()) { + continue; + } + + $revision = idx($revisions, $parent); + if (!$revision) { + continue; + } + + if ($revision->isClosed()) { + continue; + } + + $blocking_count++; + } + + if (!$blocking_count) { + $stack_name = pht('Stack'); + $stack_color = null; + } else { + $stack_name = pht( + 'Stack (%s Open)', + new PhutilNumber($blocking_count)); + $stack_color = PHUIListItemView::STATUS_FAIL; + } + + return array($stack_name, $stack_color, $stack_table); + } + } diff --git a/src/applications/differential/customfield/DifferentialChildRevisionsField.php b/src/applications/differential/customfield/DifferentialChildRevisionsField.php index 0965fc2d7c..1d9406a448 100644 --- a/src/applications/differential/customfield/DifferentialChildRevisionsField.php +++ b/src/applications/differential/customfield/DifferentialChildRevisionsField.php @@ -19,22 +19,4 @@ final class DifferentialChildRevisionsField return pht('Lists revisions this one is depended on by.'); } - public function shouldAppearInPropertyView() { - return true; - } - - public function renderPropertyViewLabel() { - return $this->getFieldName(); - } - - public function getRequiredHandlePHIDsForPropertyView() { - return PhabricatorEdgeQuery::loadDestinationPHIDs( - $this->getObject()->getPHID(), - DifferentialRevisionDependedOnByRevisionEdgeType::EDGECONST); - } - - public function renderPropertyViewValue(array $handles) { - return $this->renderHandleList($handles); - } - } diff --git a/src/applications/differential/customfield/DifferentialParentRevisionsField.php b/src/applications/differential/customfield/DifferentialParentRevisionsField.php index 3654fdc780..30ecea4bf2 100644 --- a/src/applications/differential/customfield/DifferentialParentRevisionsField.php +++ b/src/applications/differential/customfield/DifferentialParentRevisionsField.php @@ -23,24 +23,6 @@ final class DifferentialParentRevisionsField return pht('Lists revisions this one depends on.'); } - public function shouldAppearInPropertyView() { - return true; - } - - public function renderPropertyViewLabel() { - return $this->getFieldName(); - } - - public function getRequiredHandlePHIDsForPropertyView() { - return PhabricatorEdgeQuery::loadDestinationPHIDs( - $this->getObject()->getPHID(), - DifferentialRevisionDependsOnRevisionEdgeType::EDGECONST); - } - - public function renderPropertyViewValue(array $handles) { - return $this->renderHandleList($handles); - } - public function getProTips() { return array( pht( diff --git a/src/applications/differential/edge/DifferentialStackGraph.php b/src/applications/differential/edge/DifferentialStackGraph.php new file mode 100644 index 0000000000..cc7d735d79 --- /dev/null +++ b/src/applications/differential/edge/DifferentialStackGraph.php @@ -0,0 +1,58 @@ +addNodes( + array( + '' => array($revision->getPHID()), + )); + } + + public function isEmpty() { + return (count($this->getNodes()) <= 2); + } + + public function getParentEdges() { + return $this->parentEdges; + } + + protected function loadEdges(array $nodes) { + $query = id(new PhabricatorEdgeQuery()) + ->withSourcePHIDs($nodes) + ->withEdgeTypes( + array( + DifferentialRevisionDependsOnRevisionEdgeType::EDGECONST, + DifferentialRevisionDependedOnByRevisionEdgeType::EDGECONST, + )); + + $query->execute(); + + $map = array(); + foreach ($nodes as $node) { + $parents = $query->getDestinationPHIDs( + array($node), + array( + DifferentialRevisionDependsOnRevisionEdgeType::EDGECONST, + )); + + $children = $query->getDestinationPHIDs( + array($node), + array( + DifferentialRevisionDependedOnByRevisionEdgeType::EDGECONST, + )); + + $this->parentEdges[$node] = $parents; + $this->childEdges[$node] = $children; + + $map[$node] = array_values(array_fuse($parents) + array_fuse($children)); + } + + return $map; + } + +} diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php index e0c8effada..97d5a5e581 100644 --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -136,6 +136,10 @@ final class DifferentialRevision extends DifferentialDAO return "D{$id}"; } + public function getURI() { + return '/'.$this->getMonogram(); + } + public function setTitle($title) { $this->title = $title; if (!$this->getID()) { @@ -426,6 +430,31 @@ final class DifferentialRevision extends DifferentialDAO return DifferentialRevisionStatus::isClosedStatus($this->getStatus()); } + public function getStatusIcon() { + $map = array( + ArcanistDifferentialRevisionStatus::NEEDS_REVIEW + => 'fa-code grey', + ArcanistDifferentialRevisionStatus::NEEDS_REVISION + => 'fa-refresh red', + ArcanistDifferentialRevisionStatus::CHANGES_PLANNED + => 'fa-headphones red', + ArcanistDifferentialRevisionStatus::ACCEPTED + => 'fa-check green', + ArcanistDifferentialRevisionStatus::CLOSED + => 'fa-check-square-o black', + ArcanistDifferentialRevisionStatus::ABANDONED + => 'fa-plane black', + ); + + return idx($map, $this->getStatus()); + } + + public function getStatusDisplayName() { + $status = $this->getStatus(); + return ArcanistDifferentialRevisionStatus::getNameForRevisionStatus( + $status); + } + public function getFlag(PhabricatorUser $viewer) { return $this->assertAttachedKey($this->flags, $viewer->getPHID()); } diff --git a/src/applications/differential/view/DifferentialRevisionListView.php b/src/applications/differential/view/DifferentialRevisionListView.php index 6451bd9d2a..74e3f7633f 100644 --- a/src/applications/differential/view/DifferentialRevisionListView.php +++ b/src/applications/differential/view/DifferentialRevisionListView.php @@ -104,10 +104,6 @@ final class DifferentialRevisionListView extends AphrontView { $modified = $revision->getDateModified(); - $status = $revision->getStatus(); - $status_name = - ArcanistDifferentialRevisionStatus::getNameForRevisionStatus($status); - if (isset($icons['flag'])) { $item->addHeadIcon($icons['flag']); } @@ -155,29 +151,14 @@ final class DifferentialRevisionListView extends AphrontView { $item->addAttribute(pht('Reviewers: %s', $reviewers)); $item->setEpoch($revision->getDateModified()); - switch ($status) { - case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW: - $item->setStatusIcon('fa-code grey', pht('Needs Review')); - break; - case ArcanistDifferentialRevisionStatus::NEEDS_REVISION: - $item->setStatusIcon('fa-refresh red', pht('Needs Revision')); - break; - case ArcanistDifferentialRevisionStatus::CHANGES_PLANNED: - $item->setStatusIcon('fa-headphones red', pht('Changes Planned')); - break; - case ArcanistDifferentialRevisionStatus::ACCEPTED: - $item->setStatusIcon('fa-check green', pht('Accepted')); - break; - case ArcanistDifferentialRevisionStatus::CLOSED: - $item->setDisabled(true); - $item->setStatusIcon('fa-check-square-o black', pht('Closed')); - break; - case ArcanistDifferentialRevisionStatus::ABANDONED: - $item->setDisabled(true); - $item->setStatusIcon('fa-plane black', pht('Abandoned')); - break; + if ($revision->isClosed()) { + $item->setDisabled(true); } + $item->setStatusIcon( + $revision->getStatusIcon(), + $revision->getStatusDisplayName()); + $list->addItem($item); } diff --git a/src/applications/diffusion/view/DiffusionHistoryTableView.php b/src/applications/diffusion/view/DiffusionHistoryTableView.php index a9fb34e683..80e8aacf40 100644 --- a/src/applications/diffusion/view/DiffusionHistoryTableView.php +++ b/src/applications/diffusion/view/DiffusionHistoryTableView.php @@ -82,7 +82,10 @@ final class DiffusionHistoryTableView extends DiffusionView { $graph = null; if ($this->parents) { - $graph = $this->renderGraph(); + $graph = id(new PHUIDiffGraphView()) + ->setIsHead($this->isHead) + ->setIsTail($this->isTail) + ->renderGraph($this->parents); } $show_builds = PhabricatorApplication::isClassInstalledForViewer( @@ -219,166 +222,4 @@ final class DiffusionHistoryTableView extends DiffusionView { return $view->render(); } - /** - * Draw a merge/branch graph from the parent revision data. We're basically - * building up a bunch of strings like this: - * - * ^ - * |^ - * o| - * |o - * o - * - * ...which form an ASCII representation of the graph we eventually want to - * draw. - * - * NOTE: The actual implementation is black magic. - */ - private function renderGraph() { - // This keeps our accumulated information about each line of the - // merge/branch graph. - $graph = array(); - - // This holds the next commit we're looking for in each column of the - // graph. - $threads = array(); - - // This is the largest number of columns any row has, i.e. the width of - // the graph. - $count = 0; - - foreach ($this->history as $key => $history) { - $joins = array(); - $splits = array(); - - $parent_list = $this->parents[$history->getCommitIdentifier()]; - - // Look for some thread which has this commit as the next commit. If - // we find one, this commit goes on that thread. Otherwise, this commit - // goes on a new thread. - - $line = ''; - $found = false; - $pos = count($threads); - for ($n = 0; $n < $count; $n++) { - if (empty($threads[$n])) { - $line .= ' '; - continue; - } - - if ($threads[$n] == $history->getCommitIdentifier()) { - if ($found) { - $line .= ' '; - $joins[] = $n; - unset($threads[$n]); - } else { - $line .= 'o'; - $found = true; - $pos = $n; - } - } else { - - // We render a "|" for any threads which have a commit that we haven't - // seen yet, this is later drawn as a vertical line. - $line .= '|'; - } - } - - // If we didn't find the thread this commit goes on, start a new thread. - // We use "o" to mark the commit for the rendering engine, or "^" to - // indicate that there's nothing after it so the line from the commit - // upward should not be drawn. - - if (!$found) { - if ($this->isHead) { - $line .= '^'; - } else { - $line .= 'o'; - foreach ($graph as $k => $meta) { - // Go back across all the lines we've already drawn and add a - // "|" to the end, since this is connected to some future commit - // we don't know about. - for ($jj = strlen($meta['line']); $jj <= $count; $jj++) { - $graph[$k]['line'] .= '|'; - } - } - } - } - - // Update the next commit on this thread to the commit's first parent. - // This might have the effect of making a new thread. - $threads[$pos] = head($parent_list); - - // If we made a new thread, increase the thread count. - $count = max($pos + 1, $count); - - // Now, deal with splits (merges). I picked this terms opposite to the - // underlying repository term to confuse you. - foreach (array_slice($parent_list, 1) as $parent) { - $found = false; - - // Try to find the other parent(s) in our existing threads. If we find - // them, split to that thread. - - foreach ($threads as $idx => $thread_commit) { - if ($thread_commit == $parent) { - $found = true; - $splits[] = $idx; - } - } - - // If we didn't find the parent, we don't know about it yet. Find the - // first free thread and add it as the "next" commit in that thread. - // This might create a new thread. - - if (!$found) { - for ($n = 0; $n < $count; $n++) { - if (empty($threads[$n])) { - break; - } - } - $threads[$n] = $parent; - $splits[] = $n; - $count = max($n + 1, $count); - } - } - - $graph[] = array( - 'line' => $line, - 'split' => $splits, - 'join' => $joins, - ); - } - - // If this is the last page in history, replace the "o" with an "x" so we - // do not draw a connecting line downward, and replace "^" with an "X" for - // repositories with exactly one commit. - if ($this->isTail && $graph) { - $last = array_pop($graph); - $last['line'] = str_replace('o', 'x', $last['line']); - $last['line'] = str_replace('^', 'X', $last['line']); - $graph[] = $last; - } - - // Render into tags for the behavior. - - foreach ($graph as $k => $meta) { - $graph[$k] = javelin_tag( - 'div', - array( - 'sigil' => 'commit-graph', - 'meta' => $meta, - ), - ''); - } - - Javelin::initBehavior( - 'diffusion-commit-graph', - array( - 'count' => $count, - )); - - return $graph; - } - } diff --git a/src/infrastructure/diff/view/PHUIDiffGraphView.php b/src/infrastructure/diff/view/PHUIDiffGraphView.php new file mode 100644 index 0000000000..02893cc5d4 --- /dev/null +++ b/src/infrastructure/diff/view/PHUIDiffGraphView.php @@ -0,0 +1,171 @@ +isHead = $is_head; + return $this; + } + + public function getIsHead() { + return $this->isHead; + } + + public function setIsTail($is_tail) { + $this->isTail = $is_tail; + return $this; + } + + public function getIsTail() { + return $this->isTail; + } + + public function renderGraph(array $parents) { + // This keeps our accumulated information about each line of the + // merge/branch graph. + $graph = array(); + + // This holds the next commit we're looking for in each column of the + // graph. + $threads = array(); + + // This is the largest number of columns any row has, i.e. the width of + // the graph. + $count = 0; + + foreach ($parents as $cursor => $parent_list) { + $joins = array(); + $splits = array(); + + // Look for some thread which has this commit as the next commit. If + // we find one, this commit goes on that thread. Otherwise, this commit + // goes on a new thread. + + $line = ''; + $found = false; + $pos = count($threads); + for ($n = 0; $n < $count; $n++) { + if (empty($threads[$n])) { + $line .= ' '; + continue; + } + + if ($threads[$n] == $cursor) { + if ($found) { + $line .= ' '; + $joins[] = $n; + unset($threads[$n]); + } else { + $line .= 'o'; + $found = true; + $pos = $n; + } + } else { + + // We render a "|" for any threads which have a commit that we haven't + // seen yet, this is later drawn as a vertical line. + $line .= '|'; + } + } + + // If we didn't find the thread this commit goes on, start a new thread. + // We use "o" to mark the commit for the rendering engine, or "^" to + // indicate that there's nothing after it so the line from the commit + // upward should not be drawn. + + if (!$found) { + if ($this->getIsHead()) { + $line .= '^'; + } else { + $line .= 'o'; + foreach ($graph as $k => $meta) { + // Go back across all the lines we've already drawn and add a + // "|" to the end, since this is connected to some future commit + // we don't know about. + for ($jj = strlen($meta['line']); $jj <= $count; $jj++) { + $graph[$k]['line'] .= '|'; + } + } + } + } + + // Update the next commit on this thread to the commit's first parent. + // This might have the effect of making a new thread. + $threads[$pos] = head($parent_list); + + // If we made a new thread, increase the thread count. + $count = max($pos + 1, $count); + + // Now, deal with splits (merges). I picked this terms opposite to the + // underlying repository term to confuse you. + foreach (array_slice($parent_list, 1) as $parent) { + $found = false; + + // Try to find the other parent(s) in our existing threads. If we find + // them, split to that thread. + + foreach ($threads as $idx => $thread_commit) { + if ($thread_commit == $parent) { + $found = true; + $splits[] = $idx; + } + } + + // If we didn't find the parent, we don't know about it yet. Find the + // first free thread and add it as the "next" commit in that thread. + // This might create a new thread. + + if (!$found) { + for ($n = 0; $n < $count; $n++) { + if (empty($threads[$n])) { + break; + } + } + $threads[$n] = $parent; + $splits[] = $n; + $count = max($n + 1, $count); + } + } + + $graph[] = array( + 'line' => $line, + 'split' => $splits, + 'join' => $joins, + ); + } + + // If this is the last page in history, replace the "o" with an "x" so we + // do not draw a connecting line downward, and replace "^" with an "X" for + // repositories with exactly one commit. + if ($this->getIsTail() && $graph) { + $last = array_pop($graph); + $last['line'] = str_replace('o', 'x', $last['line']); + $last['line'] = str_replace('^', 'X', $last['line']); + $graph[] = $last; + } + + // Render into tags for the behavior. + + foreach ($graph as $k => $meta) { + $graph[$k] = javelin_tag( + 'div', + array( + 'sigil' => 'commit-graph', + 'meta' => $meta, + ), + ''); + } + + Javelin::initBehavior( + 'diffusion-commit-graph', + array( + 'count' => $count, + )); + + return $graph; + } + +} diff --git a/src/view/phui/PHUITabView.php b/src/view/phui/PHUITabView.php index 670fc0759a..bcc80bf774 100644 --- a/src/view/phui/PHUITabView.php +++ b/src/view/phui/PHUITabView.php @@ -6,6 +6,7 @@ final class PHUITabView extends AphrontTagView { private $key; private $keyLocked; private $contentID; + private $color; public function setKey($key) { if ($this->keyLocked) { @@ -58,8 +59,17 @@ final class PHUITabView extends AphrontTagView { return $this->contentID; } + public function setColor($color) { + $this->color = $color; + return $this; + } + + public function getColor() { + return $this->color; + } + public function newMenuItem() { - return id(new PHUIListItemView()) + $item = id(new PHUIListItemView()) ->setName($this->getName()) ->setKey($this->getKey()) ->setType(PHUIListItemView::TYPE_LINK) @@ -69,6 +79,13 @@ final class PHUITabView extends AphrontTagView { array( 'tabKey' => $this->getKey(), )); + + $color = $this->getColor(); + if ($color !== null) { + $item->setStatusColor($color); + } + + return $item; } }