diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -367,7 +367,6 @@ 'DifferentialDiffQuery' => 'applications/differential/query/DifferentialDiffQuery.php', 'DifferentialDiffRepositoryHeraldField' => 'applications/differential/herald/DifferentialDiffRepositoryHeraldField.php', 'DifferentialDiffRepositoryProjectsHeraldField' => 'applications/differential/herald/DifferentialDiffRepositoryProjectsHeraldField.php', - 'DifferentialDiffTableOfContentsView' => 'applications/differential/view/DifferentialDiffTableOfContentsView.php', 'DifferentialDiffTestCase' => 'applications/differential/storage/__tests__/DifferentialDiffTestCase.php', 'DifferentialDiffTransaction' => 'applications/differential/storage/DifferentialDiffTransaction.php', 'DifferentialDiffTransactionQuery' => 'applications/differential/query/DifferentialDiffTransactionQuery.php', @@ -3998,7 +3997,6 @@ 'DifferentialDiffQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'DifferentialDiffRepositoryHeraldField' => 'DifferentialDiffHeraldField', 'DifferentialDiffRepositoryProjectsHeraldField' => 'DifferentialDiffHeraldField', - 'DifferentialDiffTableOfContentsView' => 'AphrontView', 'DifferentialDiffTestCase' => 'PhutilTestCase', 'DifferentialDiffTransaction' => 'PhabricatorApplicationTransaction', 'DifferentialDiffTransactionQuery' => 'PhabricatorApplicationTransactionQuery', diff --git a/src/applications/differential/controller/DifferentialController.php b/src/applications/differential/controller/DifferentialController.php --- a/src/applications/differential/controller/DifferentialController.php +++ b/src/applications/differential/controller/DifferentialController.php @@ -21,4 +21,33 @@ return $this->buildSideNavView(true)->getMenu(); } + protected function buildTableOfContents( + array $changesets, + array $visible_changesets, + array $coverage) { + $viewer = $this->getViewer(); + + $toc_view = id(new PHUIDiffTableOfContentsListView()) + ->setUser($viewer); + + foreach ($changesets as $changeset_id => $changeset) { + $is_visible = isset($visible_changesets[$changeset_id]); + $anchor = $changeset->getAnchorName(); + + $filename = $changeset->getFilename(); + $coverage_id = 'differential-mcoverage-'.md5($filename); + + $item = id(new PHUIDiffTableOfContentsItemView()) + ->setChangeset($changeset) + ->setIsVisible($is_visible) + ->setAnchor($anchor) + ->setCoverage(idx($coverage, $filename)) + ->setCoverageID($coverage_id); + + $toc_view->addItem($item); + } + + return $toc_view; + } + } diff --git a/src/applications/differential/controller/DifferentialDiffViewController.php b/src/applications/differential/controller/DifferentialDiffViewController.php --- a/src/applications/differential/controller/DifferentialDiffViewController.php +++ b/src/applications/differential/controller/DifferentialDiffViewController.php @@ -116,10 +116,10 @@ $changesets = $diff->loadChangesets(); $changesets = msort($changesets, 'getSortKey'); - $table_of_contents = id(new DifferentialDiffTableOfContentsView()) - ->setChangesets($changesets) - ->setVisibleChangesets($changesets) - ->setCoverageMap($diff->loadCoverageMap($viewer)); + $table_of_contents = $this->buildTableOfContents( + $changesets, + $changesets, + $diff->loadCoverageMap($viewer)); $refs = array(); foreach ($changesets as $changeset) { diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -1036,34 +1036,4 @@ return $view; } - private function buildTableOfContents( - array $changesets, - array $visible_changesets, - array $coverage) { - $viewer = $this->getViewer(); - - $toc_view = id(new PHUIDiffTableOfContentsListView()) - ->setUser($viewer); - - foreach ($changesets as $changeset_id => $changeset) { - $is_visible = isset($visible_changesets[$changeset_id]); - $anchor = $changeset->getAnchorName(); - - $filename = $changeset->getFilename(); - $coverage_id = 'differential-mcoverage-'.md5($filename); - - $item = id(new PHUIDiffTableOfContentsItemView()) - ->setChangeset($changeset) - ->setIsVisible($is_visible) - ->setAnchor($anchor) - ->setCoverage(idx($coverage, $filename)) - ->setCoverageID($coverage_id); - - $toc_view->addItem($item); - } - - return $toc_view; - } - - } diff --git a/src/applications/differential/view/DifferentialDiffTableOfContentsView.php b/src/applications/differential/view/DifferentialDiffTableOfContentsView.php deleted file mode 100644 --- a/src/applications/differential/view/DifferentialDiffTableOfContentsView.php +++ /dev/null @@ -1,300 +0,0 @@ -changesets = $changesets; - return $this; - } - - public function setVisibleChangesets($visible_changesets) { - $this->visibleChangesets = $visible_changesets; - return $this; - } - - public function setRenderingReferences(array $references) { - $this->references = $references; - return $this; - } - - public function setRepository(PhabricatorRepository $repository) { - $this->repository = $repository; - return $this; - } - - public function setDiff(DifferentialDiff $diff) { - $this->diff = $diff; - return $this; - } - - public function setCoverageMap(array $coverage_map) { - $this->coverageMap = $coverage_map; - return $this; - } - - public function render() { - - $this->requireResource('differential-core-view-css'); - $this->requireResource('differential-table-of-contents-css'); - $this->requireResource('phui-text-css'); - - $rows = array(); - - $changesets = $this->changesets; - $paths = array(); - foreach ($changesets as $id => $changeset) { - $type = $changeset->getChangeType(); - $ftype = $changeset->getFileType(); - $ref = idx($this->references, $id); - $display_file = $changeset->getDisplayFilename(); - - $meta = null; - if (DifferentialChangeType::isOldLocationChangeType($type)) { - $away = $changeset->getAwayPaths(); - if (count($away) > 1) { - $meta = array(); - if ($type == DifferentialChangeType::TYPE_MULTICOPY) { - $meta[] = pht('Deleted after being copied to multiple locations:'); - } else { - $meta[] = pht('Copied to multiple locations:'); - } - foreach ($away as $path) { - $meta[] = $path; - } - $meta = phutil_implode_html(phutil_tag('br'), $meta); - } else { - if ($type == DifferentialChangeType::TYPE_MOVE_AWAY) { - $display_file = $this->renderRename( - $display_file, - reset($away), - "\xE2\x86\x92"); - } else { - $meta = pht('Copied to %s', reset($away)); - } - } - } else if ($type == DifferentialChangeType::TYPE_MOVE_HERE) { - $old_file = $changeset->getOldFile(); - $display_file = $this->renderRename( - $display_file, - $old_file, - "\xE2\x86\x90"); - } else if ($type == DifferentialChangeType::TYPE_COPY_HERE) { - $meta = pht('Copied from %s', $changeset->getOldFile()); - } - - $link = $this->renderChangesetLink($changeset, $ref, $display_file); - - $line_count = $changeset->getAffectedLineCount(); - if ($line_count == 0) { - $lines = ''; - } else { - $lines = ' '.pht('(%d line(s))', $line_count); - } - - $char = DifferentialChangeType::getSummaryCharacterForChangeType($type); - $chartitle = DifferentialChangeType::getFullNameForChangeType($type); - $desc = DifferentialChangeType::getShortNameForFileType($ftype); - $color = DifferentialChangeType::getSummaryColorForChangeType($type); - if ($desc) { - $desc = '('.$desc.')'; - } - $pchar = - ($changeset->getOldProperties() === $changeset->getNewProperties()) - ? '' - : phutil_tag( - 'span', - array('title' => pht('Properties Changed')), - 'M'); - - $fname = $changeset->getFilename(); - $cov = $this->renderCoverage($this->coverageMap, $fname); - if ($cov === null) { - $mcov = $cov = phutil_tag('em', array(), '-'); - } else { - $mcov = phutil_tag( - 'div', - array( - 'id' => 'differential-mcoverage-'.md5($fname), - 'class' => 'differential-mcoverage-loading', - ), - (isset($this->visibleChangesets[$id]) ? - pht('Loading...') : pht('?'))); - } - - if ($meta) { - $meta = phutil_tag( - 'div', - array( - 'class' => 'differential-toc-meta', - ), - $meta); - } - - if ($this->diff && $this->repository) { - $paths[] = - $changeset->getAbsoluteRepositoryPath($this->repository, $this->diff); - } - - $char = phutil_tag('span', array('class' => 'phui-text-'.$color), $char); - - $rows[] = array( - $char, - $pchar, - $desc, - array($link, $lines, $meta), - $cov, - $mcov, - ); - } - - $editor_link = null; - if ($paths && $this->user) { - $editor_link = $this->user->loadEditorLink( - $paths, - 1, // line number - $this->repository->getCallsign()); - if ($editor_link) { - $editor_link = - phutil_tag( - 'a', - array( - 'href' => $editor_link, - 'class' => 'button differential-toc-edit-all', - ), - pht('Open All in Editor')); - } - } - - $reveal_link = javelin_tag( - 'a', - array( - 'sigil' => 'differential-reveal-all', - 'mustcapture' => true, - 'class' => 'button differential-toc-reveal-all', - ), - pht('Show All Context')); - - $buttons = phutil_tag( - 'div', - array( - 'class' => 'differential-toc-buttons grouped', - ), - array( - $editor_link, - $reveal_link, - )); - - $table = id(new AphrontTableView($rows)); - $table->setHeaders( - array( - '', - '', - '', - pht('Path'), - pht('Coverage (All)'), - pht('Coverage (Touched)'), - )); - $table->setColumnClasses( - array( - 'differential-toc-char center', - 'differential-toc-prop center', - 'differential-toc-ftype center', - 'differential-toc-file wide', - 'differential-toc-cov', - 'differential-toc-cov', - )); - $table->setDeviceVisibility( - array( - true, - true, - true, - true, - false, - false, - )); - $anchor = id(new PhabricatorAnchorView()) - ->setAnchorName('toc') - ->setNavigationMarker(true); - - return id(new PHUIObjectBoxView()) - ->setHeaderText(pht('Table of Contents')) - ->setTable($table) - ->appendChild($anchor) - ->appendChild($buttons); - } - - private function renderRename($display_file, $other_file, $arrow) { - $old = explode('/', $display_file); - $new = explode('/', $other_file); - - $start = count($old); - foreach ($old as $index => $part) { - if (!isset($new[$index]) || $part != $new[$index]) { - $start = $index; - break; - } - } - - $end = count($old); - foreach (array_reverse($old) as $from_end => $part) { - $index = count($new) - $from_end - 1; - if (!isset($new[$index]) || $part != $new[$index]) { - $end = $from_end; - break; - } - } - - $rename = - '{'. - implode('/', array_slice($old, $start, count($old) - $end - $start)). - ' '.$arrow.' '. - implode('/', array_slice($new, $start, count($new) - $end - $start)). - '}'; - - array_splice($new, $start, count($new) - $end - $start, $rename); - return implode('/', $new); - } - - private function renderCoverage(array $coverage, $file) { - $info = idx($coverage, $file); - if (!$info) { - return null; - } - - $not_covered = substr_count($info, 'U'); - $covered = substr_count($info, 'C'); - - if (!$not_covered && !$covered) { - return null; - } - - return sprintf('%d%%', 100 * ($covered / ($covered + $not_covered))); - } - - - private function renderChangesetLink( - DifferentialChangeset $changeset, - $ref, - $display_file) { - - return javelin_tag( - 'a', - array( - 'href' => '#'.$changeset->getAnchorName(), - 'sigil' => 'differential-load', - 'meta' => array( - 'id' => 'diff-'.$changeset->getAnchorName(), - ), - ), - $display_file); - } - -} diff --git a/src/infrastructure/diff/view/PHUIDiffTableOfContentsItemView.php b/src/infrastructure/diff/view/PHUIDiffTableOfContentsItemView.php --- a/src/infrastructure/diff/view/PHUIDiffTableOfContentsItemView.php +++ b/src/infrastructure/diff/view/PHUIDiffTableOfContentsItemView.php @@ -3,7 +3,7 @@ final class PHUIDiffTableOfContentsItemView extends AphrontView { private $changeset; - private $isVisible; + private $isVisible = true; private $anchor; private $coverage; private $coverageID; @@ -89,7 +89,7 @@ return javelin_tag( 'span', array( - 'sigil' => 'has-tip', + 'sigil' => 'has-tooltip', 'meta' => array( 'tip' => $title, 'align' => 'E', @@ -112,10 +112,11 @@ return javelin_tag( 'span', array( - 'sigil' => 'has-tip', + 'sigil' => 'has-tooltip', 'meta' => array( 'tip' => pht('Properties Modified'), 'align' => 'E', + 'size' => 200, ), ), 'M'); diff --git a/src/infrastructure/diff/view/PHUIDiffTableOfContentsListView.php b/src/infrastructure/diff/view/PHUIDiffTableOfContentsListView.php --- a/src/infrastructure/diff/view/PHUIDiffTableOfContentsListView.php +++ b/src/infrastructure/diff/view/PHUIDiffTableOfContentsListView.php @@ -14,6 +14,8 @@ $this->requireResource('differential-table-of-contents-css'); $this->requireResource('phui-text-css'); + Javelin::initBehavior('phabricator-tooltips'); + $items = $this->items; $rows = array(); @@ -21,6 +23,14 @@ $rows[] = $item->render(); } + // If no item has any coverage information, just hide the coverage columns. + $any_coverage = false; + foreach ($items as $item) { + if (strlen($item->getCoverage())) { + $any_coverage = true; + } + } + $reveal_link = javelin_tag( 'a', array( @@ -56,6 +66,15 @@ 'differential-toc-cov', 'differential-toc-cov', )) + ->setColumnVisibility( + array( + true, + true, + true, + true, + $any_coverage, + $any_coverage, + )) ->setDeviceVisibility( array( true,