Convert commits to use unified table of contents

Summary:
Fixes T2183. We now use the same rendering element in both places.

Intentional changes:

  - Package highlighting is out, coming back to both apps in next diff.
  - removed redundant-feeling "Change" link. The information is now shown with a character ("M", "V", etc.) and the page is a click away under "History". Clicking the path also jumps you to substantially similar content. (We could restore it fairly easily, I just think it's probably the least useful thing in the table right now.)

Test Plan: Viewed a bunch of commits in Diffusion.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T2183

Differential Revision: https://secure.phabricator.com/D13910

src/__phutil_library_map__.php

@@ -367,7 +367,6 @@ phutil_register_library_map(array(
     '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',
@@ -519,7 +518,6 @@ phutil_register_library_map(array(
     'DiffusionCommitAutocloseHeraldField' => 'applications/diffusion/herald/DiffusionCommitAutocloseHeraldField.php',
     'DiffusionCommitBranchesController' => 'applications/diffusion/controller/DiffusionCommitBranchesController.php',
     'DiffusionCommitBranchesHeraldField' => 'applications/diffusion/herald/DiffusionCommitBranchesHeraldField.php',
-    'DiffusionCommitChangeTableView' => 'applications/diffusion/view/DiffusionCommitChangeTableView.php',
     'DiffusionCommitCommitterHeraldField' => 'applications/diffusion/herald/DiffusionCommitCommitterHeraldField.php',
     'DiffusionCommitController' => 'applications/diffusion/controller/DiffusionCommitController.php',
     'DiffusionCommitDiffContentAddedHeraldField' => 'applications/diffusion/herald/DiffusionCommitDiffContentAddedHeraldField.php',
@@ -4006,7 +4004,6 @@ phutil_register_library_map(array(
     'DifferentialDiffQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
     'DifferentialDiffRepositoryHeraldField' => 'DifferentialDiffHeraldField',
     'DifferentialDiffRepositoryProjectsHeraldField' => 'DifferentialDiffHeraldField',
-    'DifferentialDiffTableOfContentsView' => 'AphrontView',
     'DifferentialDiffTestCase' => 'PhutilTestCase',
     'DifferentialDiffTransaction' => 'PhabricatorApplicationTransaction',
     'DifferentialDiffTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
@@ -4178,7 +4175,6 @@ phutil_register_library_map(array(
     'DiffusionCommitAutocloseHeraldField' => 'DiffusionCommitHeraldField',
     'DiffusionCommitBranchesController' => 'DiffusionController',
     'DiffusionCommitBranchesHeraldField' => 'DiffusionCommitHeraldField',
-    'DiffusionCommitChangeTableView' => 'DiffusionView',
     'DiffusionCommitCommitterHeraldField' => 'DiffusionCommitHeraldField',
     'DiffusionCommitController' => 'DiffusionController',
     'DiffusionCommitDiffContentAddedHeraldField' => 'DiffusionCommitHeraldField',

src/applications/differential/controller/DifferentialController.php

@@ -21,4 +21,33 @@ abstract class DifferentialController extends PhabricatorController {
     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;
+  }
+
 }

src/applications/differential/controller/DifferentialDiffViewController.php

@@ -116,10 +116,10 @@ final class DifferentialDiffViewController extends DifferentialController {
     $changesets = $diff->loadChangesets();
     $changesets = msort($changesets, 'getSortKey');
 
-    $table_of_contents = id(new DifferentialDiffTableOfContentsView())
+    $table_of_contents = $this->buildTableOfContents(
-      ->setChangesets($changesets)
+      $changesets,
-      ->setVisibleChangesets($changesets)
+      $changesets,
-      ->setCoverageMap($diff->loadCoverageMap($viewer));
+      $diff->loadCoverageMap($viewer));
 
     $refs = array();
     foreach ($changesets as $changeset) {

src/applications/differential/controller/DifferentialRevisionViewController.php

@@ -1036,34 +1036,4 @@ final class DifferentialRevisionViewController extends DifferentialController {
     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;
-  }
-
-
 }

src/applications/differential/view/DifferentialDiffTableOfContentsView.php

@@ -1,300 +0,0 @@
-<?php
-
-final class DifferentialDiffTableOfContentsView extends AphrontView {
-
-  private $changesets = array();
-  private $visibleChangesets = array();
-  private $references = array();
-  private $repository;
-  private $diff;
-  private $renderURI = '/differential/changeset/';
-  private $coverageMap;
-
-  public function setChangesets($changesets) {
-    $this->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);
-  }
-
-}

src/applications/diffusion/controller/DiffusionCommitController.php

@@ -75,7 +75,6 @@ final class DiffusionCommitController extends DiffusionController {
 
     $commit_data = $commit->getCommitData();
     $is_foreign = $commit_data->getCommitDetail('foreign-svn-stub');
-    $changesets = null;
     if ($is_foreign) {
       $subpath = $commit_data->getCommitDetail('svn-subpath');
 
@@ -181,24 +180,6 @@ final class DiffusionCommitController extends DiffusionController {
       $user,
       $this->auditAuthorityPHIDs);
 
-    $owners_paths = array();
-    if ($highlighted_audits) {
-      $packages = id(new PhabricatorOwnersPackage())->loadAllWhere(
-        'phid IN (%Ls)',
-        mpull($highlighted_audits, 'getAuditorPHID'));
-      if ($packages) {
-        $owners_paths = id(new PhabricatorOwnersPath())->loadAllWhere(
-          'repositoryPHID = %s AND packageID IN (%Ld)',
-          $repository->getPHID(),
-          mpull($packages, 'getID'));
-      }
-    }
-
-    $change_table = new DiffusionCommitChangeTableView();
-    $change_table->setDiffusionRequest($drequest);
-    $change_table->setPathChanges($changes);
-    $change_table->setOwnersPaths($owners_paths);
-
     $count = count($changes);
 
     $bad_commit = null;
@@ -210,6 +191,7 @@ final class DiffusionCommitController extends DiffusionController {
         'r'.$callsign.$commit->getCommitIdentifier());
     }
 
+    $show_changesets = false;
     if ($bad_commit) {
       $content[] = $this->renderStatusMessage(
         pht('Bad Commit'),
@@ -235,6 +217,8 @@ final class DiffusionCommitController extends DiffusionController {
           'Changes are not shown.',
           $hard_limit));
     } else {
+      $show_changesets = true;
+
       // The user has clicked "Show All Changes", and we should show all the
       // changes inline even if there are more than the soft limit.
       $show_all_details = $request->getBool('show_all');
@@ -264,15 +248,20 @@ final class DiffusionCommitController extends DiffusionController {
         $header->addActionLink($button);
       }
 
+      $changesets = DiffusionPathChange::convertToDifferentialChangesets(
+        $user,
+        $changes);
+
+      // TODO: This table and panel shouldn't really be separate, but we need
+      // to clean up the "Load All Files" interaction first.
+      $change_table = $this->buildTableOfContents(
+        $changesets);
+
       $change_panel->setTable($change_table);
       $change_panel->setHeader($header);
 
       $content[] = $change_panel;
 
-      $changesets = DiffusionPathChange::convertToDifferentialChangesets(
-        $user,
-        $changes);
-
       $vcs = $repository->getVersionControlSystem();
       switch ($vcs) {
         case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
@@ -353,12 +342,6 @@ final class DiffusionCommitController extends DiffusionController {
       $change_list->setInlineCommentControllerURI(
         '/diffusion/inline/edit/'.phutil_escape_uri($commit->getPHID()).'/');
 
-      $change_references = array();
-      foreach ($changesets as $key => $changeset) {
-        $change_references[$changeset->getID()] = $references[$key];
-      }
-      $change_table->setRenderingReferences($change_references);
-
       $content[] = $change_list->render();
     }
 
@@ -375,7 +358,7 @@ final class DiffusionCommitController extends DiffusionController {
     $show_filetree = $prefs->getPreference($pref_filetree);
     $collapsed = $prefs->getPreference($pref_collapse);
 
-    if ($changesets && $show_filetree) {
+    if ($show_changesets && $show_filetree) {
       $nav = id(new DifferentialChangesetFileTreeSideNavBuilder())
         ->setTitle($short_name)
         ->setBaseURI(new PhutilURI('/'.$commit_id))
@@ -1082,4 +1065,40 @@ final class DiffusionCommitController extends DiffusionController {
     return $parser->processCorpus($corpus);
   }
 
+  private function buildTableOfContents(array $changesets) {
+    $viewer = $this->getViewer();
+
+    $toc_view = id(new PHUIDiffTableOfContentsListView())
+      ->setUser($viewer);
+
+    // TODO: This is hacky, we just want access to the linkX() methods on
+    // DiffusionView.
+    $diffusion_view = id(new DiffusionEmptyResultView())
+      ->setDiffusionRequest($this->getDiffusionRequest());
+
+    // TODO: Restore package stuff here.
+
+    foreach ($changesets as $changeset_id => $changeset) {
+      $path = $changeset->getFilename();
+      $anchor = substr(md5($path), 0, 8);
+
+      $history_link = $diffusion_view->linkHistory($path);
+      $browse_link = $diffusion_view->linkBrowse($path);
+
+      $item = id(new PHUIDiffTableOfContentsItemView())
+        ->setChangeset($changeset)
+        ->setAnchor($anchor)
+        ->setContext(
+          array(
+            $history_link,
+            ' ',
+            $browse_link,
+          ));
+
+      $toc_view->addItem($item);
+    }
+
+    return $toc_view;
+  }
+
 }

src/applications/diffusion/view/DiffusionCommitChangeTableView.php

@@ -1,103 +0,0 @@
-<?php
-
-final class DiffusionCommitChangeTableView extends DiffusionView {
-
-  private $pathChanges;
-  private $ownersPaths = array();
-  private $renderingReferences;
-
-  public function setPathChanges(array $path_changes) {
-    assert_instances_of($path_changes, 'DiffusionPathChange');
-    $this->pathChanges = $path_changes;
-    return $this;
-  }
-
-  public function setOwnersPaths(array $owners_paths) {
-    assert_instances_of($owners_paths, 'PhabricatorOwnersPath');
-    $this->ownersPaths = $owners_paths;
-    return $this;
-  }
-
-  public function setRenderingReferences(array $value) {
-    $this->renderingReferences = $value;
-    return $this;
-  }
-
-  public function render() {
-    $rows = array();
-    $rowc = array();
-
-    // TODO: Experiment with path stack rendering.
-
-    // TODO: Copy Away and Move Away are rendered junkily still.
-
-    foreach ($this->pathChanges as $id => $change) {
-      $path = $change->getPath();
-      $hash = substr(md5($path), 0, 8);
-      if ($change->getFileType() == DifferentialChangeType::FILE_DIRECTORY) {
-        $path .= '/';
-      }
-
-      if (isset($this->renderingReferences[$id])) {
-        $path_column = javelin_tag(
-          'a',
-          array(
-            'href' => '#'.$hash,
-            'meta' => array(
-              'id' => 'diff-'.$hash,
-              'ref' => $this->renderingReferences[$id],
-            ),
-            'sigil' => 'differential-load',
-          ),
-          $path);
-      } else {
-        $path_column = $path;
-      }
-
-      $rows[] = array(
-        $this->linkHistory($change->getPath()),
-        $this->linkBrowse($change->getPath()),
-        $this->linkChange(
-          $change->getChangeType(),
-          $change->getFileType(),
-          $change->getPath()),
-        $path_column,
-      );
-
-      $row_class = null;
-      foreach ($this->ownersPaths as $owners_path) {
-        $excluded = $owners_path->getExcluded();
-        $owners_path = $owners_path->getPath();
-        if (strncmp('/'.$path, $owners_path, strlen($owners_path)) == 0) {
-          if ($excluded) {
-            $row_class = null;
-            break;
-          }
-          $row_class = 'highlighted';
-        }
-      }
-      $rowc[] = $row_class;
-    }
-
-    $view = new AphrontTableView($rows);
-    $view->setHeaders(
-      array(
-        pht('History'),
-        pht('Browse'),
-        pht('Change'),
-        pht('Path'),
-      ));
-    $view->setColumnClasses(
-      array(
-        '',
-        '',
-        '',
-        'wide',
-      ));
-    $view->setRowClasses($rowc);
-    $view->setNoDataString(pht('This change has not been fully parsed yet.'));
-
-    return $view->render();
-  }
-
-}

src/infrastructure/diff/view/PHUIDiffTableOfContentsItemView.php

@@ -3,10 +3,12 @@
 final class PHUIDiffTableOfContentsItemView extends AphrontView {
 
   private $changeset;
-  private $isVisible;
+  private $isVisible = true;
   private $anchor;
   private $coverage;
   private $coverageID;
+  private $context;
+  private $package;
 
   public function setChangeset(DifferentialChangeset $changeset) {
     $this->changeset = $changeset;
@@ -53,11 +55,31 @@ final class PHUIDiffTableOfContentsItemView extends AphrontView {
     return $this->coverageID;
   }
 
+  public function setContext($context) {
+    $this->context = $context;
+    return $this;
+  }
+
+  public function getContext() {
+    return $this->context;
+  }
+
+  public function setPackage(PhabricatorOwnersPackage $package) {
+    $this->package = $package;
+    return $this;
+  }
+
+  public function getPackage() {
+    return $this->package;
+  }
+
   public function render() {
     $changeset = $this->getChangeset();
 
     $cells = array();
 
+    $cells[] = $this->getContext();
+
     $cells[] = $this->renderPathChangeCharacter();
     $cells[] = $this->renderPropertyChangeCharacter();
     $cells[] = $this->renderPropertyChangeDescription();
@@ -75,6 +97,8 @@ final class PHUIDiffTableOfContentsItemView extends AphrontView {
     $cells[] = $this->renderCoverage();
     $cells[] = $this->renderModifiedCoverage();
 
+    $cells[] = $this->renderPackage();
+
     return $cells;
   }
 
@@ -89,7 +113,7 @@ final class PHUIDiffTableOfContentsItemView extends AphrontView {
     return javelin_tag(
       'span',
       array(
-        'sigil' => 'has-tip',
+        'sigil' => 'has-tooltip',
         'meta' => array(
           'tip' => $title,
           'align' => 'E',
@@ -112,10 +136,11 @@ final class PHUIDiffTableOfContentsItemView extends AphrontView {
     return javelin_tag(
       'span',
       array(
-        'sigil' => 'has-tip',
+        'sigil' => 'has-tooltip',
         'meta' => array(
           'tip' => pht('Properties Modified'),
           'align' => 'E',
+          'size' => 200,
         ),
       ),
       'M');
@@ -259,6 +284,16 @@ final class PHUIDiffTableOfContentsItemView extends AphrontView {
       $meta);
   }
 
+  private function renderPackage() {
+    $package = $this->getPackage();
+
+    if (!$package) {
+      return null;
+    }
+
+    return $this->getUser()->renderHandle($package->getPHID());
+  }
+
   private function renderRename($self, $other, $arrow) {
     $old = explode('/', $self);
     $new = explode('/', $other);

src/infrastructure/diff/view/PHUIDiffTableOfContentsListView.php

@@ -14,13 +14,35 @@ final class PHUIDiffTableOfContentsListView extends AphrontView {
     $this->requireResource('differential-table-of-contents-css');
     $this->requireResource('phui-text-css');
 
+    Javelin::initBehavior('phabricator-tooltips');
+
     $items = $this->items;
 
     $rows = array();
     foreach ($items as $item) {
+      $item->setUser($this->getUser());
       $rows[] = $item->render();
     }
 
+    // Check if any item has content in these columns. If no item does, we'll
+    // just hide them.
+    $any_coverage = false;
+    $any_context = false;
+    $any_package = false;
+    foreach ($items as $item) {
+      if ($item->getContext() !== null) {
+        $any_context = true;
+      }
+
+      if (strlen($item->getCoverage())) {
+        $any_coverage = true;
+      }
+
+      if ($item->getPackage() !== null) {
+        $any_package = true;
+      }
+    }
+
     $reveal_link = javelin_tag(
       'a',
       array(
@@ -40,21 +62,36 @@ final class PHUIDiffTableOfContentsListView extends AphrontView {
     $table = id(new AphrontTableView($rows))
       ->setHeaders(
         array(
-          '',
+          null,
-          '',
+          null,
-          '',
+          null,
+          null,
           pht('Path'),
           pht('Coverage (All)'),
           pht('Coverage (Touched)'),
+          null,
         ))
       ->setColumnClasses(
         array(
+          'center',
           'differential-toc-char center',
           'differential-toc-prop center',
           'differential-toc-ftype center',
           'differential-toc-file wide',
           'differential-toc-cov',
           'differential-toc-cov',
+          'center',
+        ))
+      ->setColumnVisibility(
+        array(
+          $any_context,
+          true,
+          true,
+          true,
+          true,
+          $any_coverage,
+          $any_coverage,
+          $any_package,
         ))
       ->setDeviceVisibility(
         array(
@@ -62,8 +99,10 @@ final class PHUIDiffTableOfContentsListView extends AphrontView {
           true,
           true,
           true,
+          true,
           false,
           false,
+          true,
         ));
 
     $anchor = id(new PhabricatorAnchorView())