diff --git a/resources/celerity/map.php b/resources/celerity/map.php --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -12,7 +12,7 @@ 'core.pkg.css' => '589cd2fe', 'core.pkg.js' => '49814bac', 'dark-console.pkg.js' => '187792c2', - 'differential.pkg.css' => '8e1a7922', + 'differential.pkg.css' => '73cf6fb1', 'differential.pkg.js' => 'c8f88d74', 'diffusion.pkg.css' => '42c75c37', 'diffusion.pkg.js' => 'a98c0bf7', @@ -69,7 +69,7 @@ 'rsrc/css/application/differential/revision-comment.css' => '7dbc8d1d', 'rsrc/css/application/differential/revision-history.css' => '8aa3eac5', 'rsrc/css/application/differential/revision-list.css' => '93d2df7d', - 'rsrc/css/application/differential/table-of-contents.css' => '0e3364c7', + 'rsrc/css/application/differential/table-of-contents.css' => 'e7f8c50e', 'rsrc/css/application/diffusion/diffusion-icons.css' => '23b31a1b', 'rsrc/css/application/diffusion/diffusion-readme.css' => 'b68a76e4', 'rsrc/css/application/diffusion/diffusion-repository.css' => 'b89e8c6c', @@ -566,7 +566,7 @@ 'differential-revision-comment-css' => '7dbc8d1d', 'differential-revision-history-css' => '8aa3eac5', 'differential-revision-list-css' => '93d2df7d', - 'differential-table-of-contents-css' => '0e3364c7', + 'differential-table-of-contents-css' => 'e7f8c50e', 'diffusion-css' => 'b54c77b0', 'diffusion-icons-css' => '23b31a1b', 'diffusion-readme-css' => 'b68a76e4', diff --git a/src/applications/differential/constants/DifferentialChangeType.php b/src/applications/differential/constants/DifferentialChangeType.php --- a/src/applications/differential/constants/DifferentialChangeType.php +++ b/src/applications/differential/constants/DifferentialChangeType.php @@ -22,50 +22,6 @@ const FILE_NORMAL = 7; const FILE_SUBMODULE = 8; - public static function getSummaryCharacterForChangeType($type) { - static $types = array( - self::TYPE_ADD => 'A', - self::TYPE_CHANGE => 'M', - self::TYPE_DELETE => 'D', - self::TYPE_MOVE_AWAY => 'V', - self::TYPE_COPY_AWAY => 'P', - self::TYPE_MOVE_HERE => 'V', - self::TYPE_COPY_HERE => 'P', - self::TYPE_MULTICOPY => 'P', - self::TYPE_MESSAGE => 'Q', - self::TYPE_CHILD => '@', - ); - return idx($types, coalesce($type, '?'), '~'); - } - - public static function getSummaryColorForChangeType($type) { - static $types = array( - self::TYPE_ADD => 'green', - self::TYPE_CHANGE => 'black', - self::TYPE_DELETE => 'red', - self::TYPE_MOVE_AWAY => 'orange', - self::TYPE_COPY_AWAY => 'black', - self::TYPE_MOVE_HERE => 'green', - self::TYPE_COPY_HERE => 'green', - self::TYPE_MULTICOPY => 'orange', - self::TYPE_MESSAGE => 'black', - self::TYPE_CHILD => 'black', - ); - return idx($types, coalesce($type, '?'), 'black'); - } - - public static function getShortNameForFileType($type) { - static $names = array( - self::FILE_TEXT => null, - self::FILE_DIRECTORY => 'dir', - self::FILE_IMAGE => 'img', - self::FILE_BINARY => 'bin', - self::FILE_SYMLINK => 'sym', - self::FILE_SUBMODULE => 'sub', - ); - return idx($names, coalesce($type, '?'), '???'); - } - public static function getIconForFileType($type) { static $icons = array( self::FILE_TEXT => 'fa-file-text-o', diff --git a/src/applications/differential/storage/DifferentialChangeset.php b/src/applications/differential/storage/DifferentialChangeset.php --- a/src/applications/differential/storage/DifferentialChangeset.php +++ b/src/applications/differential/storage/DifferentialChangeset.php @@ -315,6 +315,30 @@ return $this->assertAttached($this->diff); } + public function getOldStatePathVector() { + $path = $this->getOldFile(); + if (!strlen($path)) { + $path = $this->getFilename(); + } + + $path = trim($path, '/'); + $path = explode('/', $path); + + return $path; + } + + public function getNewStatePathVector() { + if (!$this->hasNewState()) { + return null; + } + + $path = $this->getFilename(); + $path = trim($path, '/'); + $path = explode('/', $path); + + return $path; + } + public function newFileTreeIcon() { $icon = $this->getPathIconIcon(); $color = $this->getPathIconColor(); @@ -335,16 +359,8 @@ } public function getIsLowImportanceChangeset() { - $change_type = $this->getChangeType(); - - $change_map = array( - DifferentialChangeType::TYPE_DELETE => true, - DifferentialChangeType::TYPE_MOVE_AWAY => true, - DifferentialChangeType::TYPE_MULTICOPY => true, - ); - - if (isset($change_map[$change_type])) { - return $change_map[$change_type]; + if (!$this->hasNewState()) { + return true; } if ($this->isGeneratedChangeset()) { 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 @@ -81,9 +81,7 @@ $cells[] = $this->getContext(); - $cells[] = $this->renderPathChangeCharacter(); - $cells[] = $this->renderPropertyChangeCharacter(); - $cells[] = $this->renderPropertyChangeDescription(); + $cells[] = $changeset->newFileTreeIcon(); $link = $this->renderChangesetLink(); $lines = $this->renderChangesetLines(); @@ -103,82 +101,12 @@ return $cells; } - private function renderPathChangeCharacter() { - $changeset = $this->getChangeset(); - $type = $changeset->getChangeType(); - - $color = DifferentialChangeType::getSummaryColorForChangeType($type); - $char = DifferentialChangeType::getSummaryCharacterForChangeType($type); - $title = DifferentialChangeType::getFullNameForChangeType($type); - - return javelin_tag( - 'span', - array( - 'sigil' => 'has-tooltip', - 'meta' => array( - 'tip' => $title, - 'align' => 'E', - ), - 'class' => 'phui-text-'.$color, - ), - $char); - } - - private function renderPropertyChangeCharacter() { - $changeset = $this->getChangeset(); - - $old = $changeset->getOldProperties(); - $new = $changeset->getNewProperties(); - - if ($old === $new) { - return null; - } - - return javelin_tag( - 'span', - array( - 'sigil' => 'has-tooltip', - 'meta' => array( - 'tip' => pht('Properties Modified'), - 'align' => 'E', - 'size' => 200, - ), - ), - 'M'); - } - - private function renderPropertyChangeDescription() { - $changeset = $this->getChangeset(); - - $file_type = $changeset->getFileType(); - - $desc = DifferentialChangeType::getShortNameForFileType($file_type); - if ($desc === null) { - return null; - } - - return pht('(%s)', $desc); - } - - private function renderChangesetLink() { + public function newLink() { $anchor = $this->getAnchor(); $changeset = $this->getChangeset(); $name = $changeset->getDisplayFilename(); - - $change_type = $changeset->getChangeType(); - if (DifferentialChangeType::isOldLocationChangeType($change_type)) { - $away = $changeset->getAwayPaths(); - if (count($away) == 1) { - if ($change_type == DifferentialChangeType::TYPE_MOVE_AWAY) { - $right_arrow = "\xE2\x86\x92"; - $name = $this->renderRename($name, head($away), $right_arrow); - } - } - } else if ($change_type == DifferentialChangeType::TYPE_MOVE_HERE) { - $left_arrow = "\xE2\x86\x90"; - $name = $this->renderRename($name, $changeset->getOldFile(), $left_arrow); - } + $name = basename($name); return javelin_tag( 'a', @@ -192,18 +120,22 @@ $name); } - private function renderChangesetLines() { + public function renderChangesetLines() { $changeset = $this->getChangeset(); + if ($changeset->getIsLowImportanceChangeset()) { + return null; + } + $line_count = $changeset->getAffectedLineCount(); if (!$line_count) { return null; } - return ' '.pht('(%d line(s))', $line_count); + return pht('%d line(s)', $line_count); } - private function renderCoverage() { + public function renderCoverage() { $not_applicable = '-'; $coverage = $this->getCoverage(); @@ -221,7 +153,7 @@ return sprintf('%d%%', 100 * ($covered / ($covered + $not_covered))); } - private function renderModifiedCoverage() { + public function renderModifiedCoverage() { $not_applicable = '-'; $coverage = $this->getCoverage(); @@ -285,50 +217,18 @@ $meta); } - private function renderPackages() { + public function renderPackages() { $packages = $this->getPackages(); + if (!$packages) { return null; } - $viewer = $this->getUser(); + $viewer = $this->getViewer(); $package_phids = mpull($packages, 'getPHID'); return $viewer->renderHandleList($package_phids) ->setGlyphLimit(48); } - private function renderRename($self, $other, $arrow) { - $old = explode('/', $self); - $new = explode('/', $other); - - $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); - } - } 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 @@ -9,6 +9,8 @@ private $background; private $bare; + private $components = array(); + public function addItem(PHUIDiffTableOfContentsItemView $item) { $this->items[] = $item; return $this; @@ -61,57 +63,179 @@ } $items = $this->items; + $viewer = $this->getViewer(); - $rows = array(); - $rowc = array(); + $item_map = array(); + + $vector_tree = new ArcanistDiffVectorTree(); foreach ($items as $item) { - $item->setUser($this->getUser()); - $rows[] = $item->render(); + $item->setViewer($viewer); - $have_authority = false; + $changeset = $item->getChangeset(); - $packages = $item->getPackages(); - if ($packages) { - if (array_intersect_key($packages, $authority)) { - $have_authority = true; - } + $old_vector = $changeset->getOldStatePathVector(); + $new_vector = $changeset->getNewStatePathVector(); + + $tree_vector = $this->newTreeVector($old_vector, $new_vector); + + $item_map[implode("\n", $tree_vector)] = $item; + + $vector_tree->addVector($tree_vector); + } + $node_list = $vector_tree->newDisplayList(); + + $node_map = array(); + foreach ($node_list as $node) { + $path_vector = $node->getVector(); + $path_vector = implode("\n", $path_vector); + $node_map[$path_vector] = $node; + } + + // Mark all nodes which contain at least one path which exists in the new + // state. Nodes we don't mark contain only deleted or moved files, so they + // can be rendered with a less-prominent style. + + foreach ($node_map as $node_key => $node) { + $item = idx($item_map, $node_key); + + if (!$item) { + continue; } - if ($have_authority) { - $rowc[] = 'highlighted'; - } else { - $rowc[] = null; + $changeset = $item->getChangeset(); + if (!$changeset->getIsLowImportanceChangeset()) { + $node->setAncestralAttribute('important', true); } } - // Check if any item has content in these columns. If no item does, we'll - // just hide them. + $any_packages = false; $any_coverage = false; $any_context = false; - $any_packages = false; - foreach ($items as $item) { - if ($item->getContext() !== null) { - $any_context = true; + + $rows = array(); + $rowc = array(); + foreach ($node_map as $node_key => $node) { + $display_vector = $node->getDisplayVector(); + $item = idx($item_map, $node_key); + + if ($item) { + $changeset = $item->getChangeset(); + $icon = $changeset->newFileTreeIcon(); + } else { + $changeset = null; + $icon = id(new PHUIIconView()) + ->setIcon('fa-folder-open-o grey'); } - if (strlen($item->getCoverage())) { - $any_coverage = true; + if ($node->getChildren()) { + $old_dir = true; + $new_dir = true; + } else { + // TODO: When properties are set on a directory in SVN directly, this + // might be incorrect. + $old_dir = false; + $new_dir = false; } - if ($item->getPackages() !== null) { + $display_view = $this->newComponentView( + $icon, + $display_vector, + $old_dir, + $new_dir, + $item); + + $depth = $node->getDisplayDepth(); + + $style = sprintf('padding-left: %dpx;', $depth * 16); + + if ($item) { + $packages = $item->renderPackages(); + } else { + $packages = null; + } + + if ($packages) { $any_packages = true; } + + if ($item) { + if ($item->getCoverage()) { + $any_coverage = true; + } + $coverage = $item->renderCoverage(); + $modified_coverage = $item->renderModifiedCoverage(); + } else { + $coverage = null; + $modified_coverage = null; + } + + if ($item) { + $context = $item->getContext(); + if ($context) { + $any_context = true; + } + } else { + $context = null; + } + + if ($item) { + $lines = $item->renderChangesetLines(); + } else { + $lines = null; + } + + $rows[] = array( + $context, + phutil_tag( + 'div', + array( + 'style' => $style, + ), + $display_view), + $lines, + $coverage, + $modified_coverage, + $packages, + ); + + $classes = array(); + + $have_authority = false; + + if ($item) { + $packages = $item->getPackages(); + if ($packages) { + if (array_intersect_key($packages, $authority)) { + $have_authority = true; + } + } + } + + if ($have_authority) { + $classes[] = 'highlighted'; + } + + if (!$node->getAttribute('important')) { + $classes[] = 'diff-toc-low-importance-row'; + } + + if ($changeset) { + $classes[] = 'diff-toc-changeset-row'; + } else { + $classes[] = 'diff-toc-no-changeset-row'; + } + + $rowc[] = implode(' ', $classes); } $table = id(new AphrontTableView($rows)) ->setRowClasses($rowc) + ->setClassName('aphront-table-view-compact') ->setHeaders( array( - null, - null, - null, null, pht('Path'), + pht('Size'), pht('Coverage (All)'), pht('Coverage (Touched)'), pht('Packages'), @@ -119,10 +243,8 @@ ->setColumnClasses( array( null, - 'differential-toc-char center', - 'differential-toc-prop center', - 'differential-toc-ftype center', - 'differential-toc-file wide', + 'diff-toc-path wide', + 'right', 'differential-toc-cov', 'differential-toc-cov', null, @@ -132,8 +254,6 @@ $any_context, true, true, - true, - true, $any_coverage, $any_coverage, $any_packages, @@ -142,9 +262,7 @@ array( true, true, - true, - true, - true, + false, false, false, true, @@ -174,7 +292,240 @@ if ($this->infoView) { $box->setInfoView($this->infoView); } + return $box; } + private function newTreeVector($old, $new) { + if ($old === null && $new === null) { + throw new Exception(pht('Changeset has no path vectors!')); + } + + $vector = null; + if ($old === null) { + $vector = $new; + } else if ($new === null) { + $vector = $old; + } else if ($old === $new) { + $vector = $new; + } + + if ($vector) { + foreach ($vector as $k => $v) { + $vector[$k] = $this->newScalarComponent($v); + } + return $vector; + } + + $matrix = id(new PhutilEditDistanceMatrix()) + ->setSequences($old, $new) + ->setComputeString(true); + $edits = $matrix->getEditString(); + + // If the edit sequence contains deletions followed by edits, move + // the deletions to the end to left-align the new path. + $edits = preg_replace('/(d+)(x+)/', '\2\1', $edits); + + $vector = array(); + $length = strlen($edits); + + $old_cursor = 0; + $new_cursor = 0; + + for ($ii = 0; $ii < strlen($edits); $ii++) { + $c = $edits[$ii]; + switch ($c) { + case 'i': + $vector[] = $this->newPairComponent(null, $new[$new_cursor]); + $new_cursor++; + break; + case 'd': + $vector[] = $this->newPairComponent($old[$old_cursor], null); + $old_cursor++; + break; + case 's': + case 'x': + case 't': + $vector[] = $this->newPairComponent( + $old[$old_cursor], + $new[$new_cursor]); + $old_cursor++; + $new_cursor++; + break; + default: + throw new Exception(pht('Unknown edit string "%s"!', $c)); + } + } + + return $vector; + } + + private function newScalarComponent($v) { + $key = sprintf('path(%s)', $v); + + if (!isset($this->components[$key])) { + $this->components[$key] = $v; + } + + return $key; + } + + private function newPairComponent($u, $v) { + if ($u === $v) { + return $this->newScalarComponent($u); + } + + $key = sprintf('pair(%s > %s)', $u, $v); + + if (!isset($this->components[$key])) { + $this->components[$key] = array($u, $v); + } + + return $key; + } + + private function newComponentView( + $icon, + array $keys, + $old_dir, + $new_dir, + $item) { + + $is_simple = true; + + $items = array(); + foreach ($keys as $key) { + $component = $this->components[$key]; + + if (is_array($component)) { + $is_simple = false; + } else { + $component = array( + $component, + $component, + ); + } + + $items[] = $component; + } + + $move_icon = id(new PHUIIconView()) + ->setIcon('fa-angle-double-right pink'); + + $old_row = array( + phutil_tag('td', array(), $move_icon), + ); + $new_row = array( + phutil_tag('td', array(), $icon), + ); + + $last_old_key = null; + $last_new_key = null; + + foreach ($items as $key => $component) { + if (!is_array($component)) { + $last_old_key = $key; + $last_new_key = $key; + } else { + if ($component[0] !== null) { + $last_old_key = $key; + } + if ($component[1] !== null) { + $last_new_key = $key; + } + } + } + + foreach ($items as $key => $component) { + if (!is_array($component)) { + $old = $component; + $new = $component; + } else { + $old = $component[0]; + $new = $component[1]; + } + + $old_classes = array(); + $new_classes = array(); + + if ($old === $new) { + // Do nothing. + } else if ($old === null) { + $new_classes[] = 'diff-path-component-new'; + } else if ($new === null) { + $old_classes[] = 'diff-path-component-old'; + } else { + $old_classes[] = 'diff-path-component-old'; + $new_classes[] = 'diff-path-component-new'; + } + + if ($old !== null) { + if (($key === $last_old_key) && !$old_dir) { + // Do nothing. + } else { + $old = $old.'/'; + } + } + + if ($new !== null) { + if (($key === $last_new_key) && $item) { + $new = $item->newLink(); + } else if (($key === $last_new_key) && !$new_dir) { + // Do nothing. + } else { + $new = $new.'/'; + } + } + + $old_row[] = phutil_tag( + 'td', + array(), + phutil_tag( + 'div', + array( + 'class' => implode(' ', $old_classes), + ), + $old)); + $new_row[] = phutil_tag( + 'td', + array(), + phutil_tag( + 'div', + array( + 'class' => implode(' ', $new_classes), + ), + $new)); + } + + $old_row = phutil_tag( + 'tr', + array( + 'class' => 'diff-path-old', + ), + $old_row); + + $new_row = phutil_tag( + 'tr', + array( + 'class' => 'diff-path-new', + ), + $new_row); + + $rows = array(); + $rows[] = $new_row; + if (!$is_simple) { + $rows[] = $old_row; + } + + $body = phutil_tag('tbody', array(), $rows); + + $table = phutil_tag( + 'table', + array( + ), + $body); + + return $table; + } + } diff --git a/webroot/rsrc/css/application/differential/table-of-contents.css b/webroot/rsrc/css/application/differential/table-of-contents.css --- a/webroot/rsrc/css/application/differential/table-of-contents.css +++ b/webroot/rsrc/css/application/differential/table-of-contents.css @@ -88,3 +88,44 @@ margin: 4px 0; border: 1px solid {$thinblueborder}; } + +.diff-toc-path .phui-icon-view { + margin-right: 2px; + text-align: center; + width: 20px; + display: inline-block; +} + +table.aphront-table-view-compact td { + padding: 3px 8px; +} + +td.diff-toc-path td { + padding: 0; + color: {$greytext}; +} + +.diff-toc-path div.diff-path-component-new { + padding: 0 4px; + background: {$new-bright}; + margin: 0 2px; +} + +.diff-toc-path div.diff-path-component-old { + padding: 0 4px; + background: {$old-bright}; + margin: 0 2px; +} + +.diff-toc-path a { + color: {$darkbluetext}; +} + +td.diff-toc-path .diff-path-old td { + color: {$lightgreytext}; +} + +.diff-toc-low-importance-row, +.alt-diff-toc-low-importance-row { + opacity: 0.5; +}