diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 788dfc1dc7..17e3a8fd73 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -295,7 +295,7 @@ celerity_register_resource_map(array( ), 'diffusion-source-css' => array( - 'uri' => '/res/5f5ac1d6/rsrc/css/application/diffusion/diffusion-source.css', + 'uri' => '/res/9f79ed12/rsrc/css/application/diffusion/diffusion-source.css', 'type' => 'css', 'requires' => array( diff --git a/src/applications/audit/query/commit/PhabricatorAuditCommitQuery.php b/src/applications/audit/query/commit/PhabricatorAuditCommitQuery.php index 35fce40425..f49b6803e6 100644 --- a/src/applications/audit/query/commit/PhabricatorAuditCommitQuery.php +++ b/src/applications/audit/query/commit/PhabricatorAuditCommitQuery.php @@ -24,6 +24,7 @@ final class PhabricatorAuditCommitQuery { private $commitPHIDs; private $authorPHIDs; private $packagePHIDs; + private $identifiers = array(); private $needCommitData; @@ -51,6 +52,11 @@ final class PhabricatorAuditCommitQuery { return $this; } + public function withIdentifiers($repository_id, array $identifiers) { + $this->identifiers[] = array($repository_id, $identifiers); + return $this; + } + public function needCommitData($need) { $this->needCommitData = $need; return $this; @@ -152,6 +158,23 @@ final class PhabricatorAuditCommitQuery { $this->packagePHIDs); } + if ($this->identifiers) { + $clauses = array(); + foreach ($this->identifiers as $spec) { + list($repository_id, $identifiers) = $spec; + if ($identifiers) { + $clauses[] = qsprintf( + $conn_r, + 'c.repositoryID = %d AND c.commitIdentifier IN (%Ls)', + $repository_id, + $identifiers); + } + } + if ($clauses) { + $where[] = '('.implode(') OR (', $clauses).')'; + } + } + $status = $this->status; switch ($status) { case self::STATUS_OPEN: diff --git a/src/applications/diffusion/controller/file/DiffusionBrowseFileController.php b/src/applications/diffusion/controller/file/DiffusionBrowseFileController.php index f52a252cbe..b254ca7e9f 100644 --- a/src/applications/diffusion/controller/file/DiffusionBrowseFileController.php +++ b/src/applications/diffusion/controller/file/DiffusionBrowseFileController.php @@ -23,11 +23,17 @@ final class DiffusionBrowseFileController extends DiffusionController { public function processRequest() { $request = $this->getRequest(); - $drequest = $this->getDiffusionRequest(); + + $before = $request->getStr('before'); + if ($before) { + return $this->buildBeforeResponse($before); + } + $path = $drequest->getPath(); $selected = $request->getStr('view'); $needs_blame = ($selected == 'blame' || $selected == 'plainblame'); + $file_query = DiffusionFileContentQuery::newFromDiffusionRequest( $this->diffusionRequest); $file_query->setNeedsBlame($needs_blame); @@ -99,8 +105,6 @@ final class DiffusionBrowseFileController extends DiffusionController { } } - - // TODO: blame of blame. switch ($selected) { case 'plain': $style = @@ -166,7 +170,7 @@ final class DiffusionBrowseFileController extends DiffusionController { $corpus = phutil_render_tag( 'div', array( - 'style' => 'padding: 0pt 2em;', + 'style' => 'padding: 0 2em;', ), $corpus_table); @@ -245,20 +249,13 @@ final class DiffusionBrowseFileController extends DiffusionController { DiffusionFileContentQuery $file_query, $selected) { - $last_rev = null; - $color = '#eeeeee'; - $rows = array(); - $n = 1; - $view = $this->getRequest()->getStr('view'); - if ($blame_dict) { - $epoch_list = ipull($blame_dict, 'epoch'); - $epoch_max = max($epoch_list); - $epoch_min = min($epoch_list); - $epoch_range = $epoch_max - $epoch_min + 1; + $epoch_list = ipull($blame_dict, 'epoch'); + $epoch_min = min($epoch_list); + $epoch_max = max($epoch_list); + $epoch_range = ($epoch_max - $epoch_min) + 1; } - $targ = ''; $min_line = 0; $line = $drequest->getLine(); if (strpos($line, '-') !== false) { @@ -270,109 +267,219 @@ final class DiffusionBrowseFileController extends DiffusionController { $max_line = $line; } + $display = array(); + + $line_number = 1; + $last_rev = null; + $color = null; foreach ($text_list as $k => $line) { + $display_line = array( + 'color' => null, + 'epoch' => null, + 'commit' => null, + 'author' => null, + 'target' => null, + 'highlighted' => null, + 'line' => $line_number, + 'data' => $line, + ); + if ($needs_blame) { // If the line's rev is same as the line above, show empty content // with same color; otherwise generate blame info. The newer a change - // is, the darker the color. - $rev = $rev_list[$k]; + // is, the more saturated the color. + + // TODO: SVN doesn't always give us blame for the last line, if empty? + // Bug with our stuff or with SVN? + $rev = idx($rev_list, $k, $last_rev); + if ($last_rev == $rev) { - $blame_info = - ($file_query->getSupportsBlameOnBlame() ? - '' : ''). - ''. - ''; + $display_line['color'] = $color; } else { + $blame = $blame_dict[$rev]; - $revision_time = null; - if ($blame_dict) { - $color_number = (int)(0xEE - - 0xEE * ($blame_dict[$rev]['epoch'] - $epoch_min) / $epoch_range); - $color = sprintf('#%02xee%02x', $color_number, $color_number); - $revision_time = phabricator_datetime( - $blame_dict[$rev]['epoch'], - $this->getRequest()->getUser()); - } + $color_ratio = ($blame['epoch'] - $epoch_min) / $epoch_range; - $revision_link = self::renderRevision( - $drequest, - substr($rev, 0, 7)); + $color_value = 0xF6 * (1.0 - $color_ratio); + $color = sprintf( + '#%02x%02x%02x', + $color_value, + 0xF6, + $color_value); - if (!$file_query->getSupportsBlameOnBlame()) { - $prev_link = ''; - } else { - $prev_rev = $file_query->getPrevRev($rev); - $path = $drequest->getPath(); - $prev_link = self::renderBrowse( - $drequest, - $path, - "\xC2\xAB", - $prev_rev, - $n, - $selected, - 'Blame previous revision'); - $prev_link = phutil_render_tag( - 'th', - array( - 'class' => 'diffusion-wide-link', - 'style' => 'background: '.$color.'; width: 2em;', - ), - $prev_link); - } + $display_line['epoch'] = $blame['epoch']; + $display_line['color'] = $color; + $display_line['commit'] = $rev; if (isset($blame_dict[$rev]['handle'])) { $author_link = $blame_dict[$rev]['handle']->renderLink(); } else { - $author_link = phutil_escape_html($blame_dict[$rev]['author']); + $author_link = phutil_render_tag( + 'span', + array( + ), + phutil_escape_html($blame_dict[$rev]['author'])); } - $blame_info = - $prev_link . - ''.$revision_link.''. - ''.$author_link.''; + $display_line['author'] = $author_link; + $last_rev = $rev; } - } else { - $blame_info = null; } - // Highlight the line of interest if needed. - if ($min_line > 0 && ($n >= $min_line && $n <= $max_line)) { - $tr = ''; - if ($targ == '') { - $targ = ''; - Javelin::initBehavior('diffusion-jump-to', - array('target' => 'scroll_target')); + if ($min_line) { + if ($line_number == $min_line) { + $display_line['target'] = true; + } + if ($line_number >= $min_line && $line_number <= $max_line) { + $display_line['highlighted'] = true; } - } else { - $tr = ''; - $targ = null; } - $href = $drequest->generateURI( + $display[] = $display_line; + ++$line_number; + } + + $commits = id(new PhabricatorAuditCommitQuery()) + ->withIdentifiers( + $drequest->getRepository()->getID(), + array_filter(ipull($display, 'commit'))) + ->needCommitData(true) + ->execute(); + $commits = mpull($commits, null, 'getCommitIdentifier'); + + $request = $this->getRequest(); + $user = $request->getUser(); + + $rows = array(); + foreach ($display as $line) { + + $line_href = $drequest->generateURI( array( - 'action' => 'browse', - 'stable' => true, + 'action' => 'browse', + 'line' => $line['line'], + 'stable' => true, )); - $href = (string)$href; - $query_params = null; - if ($view) { - $query_params = '?view='.$view; + $line_href->setQueryParams($request->getRequestURI()->getQueryParams()); + + $blame = array(); + if ($line['color']) { + $color = $line['color']; + + $before_link = null; + $commit_link = null; + if (idx($line, 'commit')) { + $commit = $line['commit']; + + $summary = 'Unknown'; + if (idx($commits, $commit)) { + $summary = $commits[$commit]->getCommitData()->getSummary(); + } + + $tooltip = phabricator_date( + $line['epoch'], + $user)." \xC2\xB7 ".$summary; + + Javelin::initBehavior('phabricator-tooltips', array()); + require_celerity_resource('aphront-tooltip-css'); + + $commit_link = javelin_render_tag( + 'a', + array( + 'href' => $drequest->generateURI( + array( + 'action' => 'commit', + 'commit' => $line['commit'], + )), + 'sigil' => 'has-tooltip', + 'meta' => array( + 'tip' => $tooltip, + 'align' => 'E', + 'size' => 600, + ), + ), + phutil_escape_html(phutil_utf8_shorten($line['commit'], 9, ''))); + + $before_link = javelin_render_tag( + 'a', + array( + 'href' => $line_href->alter('before', $commit), + 'sigil' => 'has-tooltip', + 'meta' => array( + 'tip' => 'Skip Past This Commit', + 'align' => 'E', + 'size' => 300, + ), + ), + "\xC2\xAB"); + } + + $blame[] = phutil_render_tag( + 'th', + array( + 'class' => 'diffusion-blame-link', + 'style' => 'background: '.$color, + ), + $before_link); + + $blame[] = phutil_render_tag( + 'th', + array( + 'class' => 'diffusion-rev-link', + 'style' => 'background: '.$color, + ), + $commit_link); + + $blame[] = phutil_render_tag( + 'th', + array( + 'class' => 'diffusion-author-link', + 'style' => 'background: '.$color, + ), + idx($line, 'author')); } - $link = phutil_render_tag( + $line_link = phutil_render_tag( 'a', array( - 'href' => $href.'$'.$n.$query_params, + 'href' => $line_href, ), - $n); + phutil_escape_html($line['line'])); - $rows[] = $tr.$blame_info. - ''.$link.''. - ''.$targ.$line.''; - ++$n; + $blame[] = phutil_render_tag( + 'th', + array( + 'class' => 'diffusion-line-link', + 'style' => isset($color) ? 'background: '.$color : null, + ), + $line_link); + + $blame = implode('', $blame); + + if ($line['target']) { + Javelin::initBehavior( + 'diffusion-jump-to', + array( + 'target' => 'scroll_target', + )); + $anchor_text = ''; + } else { + $anchor_text = null; + } + + $line_text = phutil_render_tag( + 'td', + array( + ), + $anchor_text.$line['data']); + + $rows[] = phutil_render_tag( + 'tr', + array( + 'style' => ($line['highlighted'] ? 'background: #ffff00;' : null), + ), + $blame. + $line_text); } return $rows; @@ -493,5 +600,34 @@ final class DiffusionBrowseFileController extends DiffusionController { return $panel; } + private function buildBeforeResponse($before) { + $request = $this->getRequest(); + $drequest = $this->getDiffusionRequest(); + + $before_req = DiffusionRequest::newFromDictionary( + array( + 'repository' => $drequest->getRepository(), + 'commit' => $before, + )); + + $query = DiffusionCommitParentsQuery::newFromDiffusionRequest($before_req); + $parents = $query->loadParents(); + $parent = head($parents); + + // NOTE: If they get back to the very first commit, we just keep them there. + // We could maybe show a message or something. + + $before_uri = $drequest->generateURI( + array( + 'action' => 'browse', + 'commit' => $parent ? $parent->getCommitIdentifier() : $before, + 'line' => $drequest->getLine(), + )); + + $before_uri->setQueryParams($request->getRequestURI()->getQueryParams()); + $before_uri = $before_uri->alter('before', null); + + return id(new AphrontRedirectResponse())->setURI($before_uri); + } } diff --git a/src/applications/diffusion/controller/file/__init__.php b/src/applications/diffusion/controller/file/__init__.php index 6ed4027961..39a096ffec 100644 --- a/src/applications/diffusion/controller/file/__init__.php +++ b/src/applications/diffusion/controller/file/__init__.php @@ -10,12 +10,16 @@ phutil_require_module('arcanist', 'difference'); phutil_require_module('phabricator', 'aphront/response/redirect'); phutil_require_module('phabricator', 'aphront/writeguard'); +phutil_require_module('phabricator', 'applications/audit/query/commit'); phutil_require_module('phabricator', 'applications/diffusion/controller/base'); phutil_require_module('phabricator', 'applications/diffusion/query/filecontent/base'); +phutil_require_module('phabricator', 'applications/diffusion/query/parents/base'); +phutil_require_module('phabricator', 'applications/diffusion/request/base'); phutil_require_module('phabricator', 'applications/files/storage/file'); phutil_require_module('phabricator', 'applications/markup/syntax'); phutil_require_module('phabricator', 'infrastructure/celerity/api'); phutil_require_module('phabricator', 'infrastructure/javelin/api'); +phutil_require_module('phabricator', 'infrastructure/javelin/markup'); phutil_require_module('phabricator', 'infrastructure/util/hash'); phutil_require_module('phabricator', 'view/form/control/select'); phutil_require_module('phabricator', 'view/layout/panel'); diff --git a/src/applications/diffusion/query/filecontent/base/DiffusionFileContentQuery.php b/src/applications/diffusion/query/filecontent/base/DiffusionFileContentQuery.php index 6b291c348f..cf38d0bb06 100644 --- a/src/applications/diffusion/query/filecontent/base/DiffusionFileContentQuery.php +++ b/src/applications/diffusion/query/filecontent/base/DiffusionFileContentQuery.php @@ -26,16 +26,6 @@ abstract class DiffusionFileContentQuery extends DiffusionQuery { return parent::newQueryObject(__CLASS__, $request); } - public function getSupportsBlameOnBlame() { - return false; - } - - public function getPrevRev($rev) { - // TODO: support git once the 'parent' info of a commit is saved - // to the database. - throw new Exception("Unsupported VCS!"); - } - final public function loadFileContent() { $this->fileContent = $this->executeQuery(); } @@ -54,11 +44,20 @@ abstract class DiffusionFileContentQuery extends DiffusionQuery { if (!$this->getNeedsBlame()) { $text_list = explode("\n", rtrim($raw_data)); } else { + $lines = array(); foreach (explode("\n", rtrim($raw_data)) as $k => $line) { - list($rev_id, $author, $text) = $this->tokenizeLine($line); + $lines[$k] = $this->tokenizeLine($line); + list($rev_id, $author, $text) = $lines[$k]; $text_list[$k] = $text; $rev_list[$k] = $rev_id; + } + + $rev_list = $this->processRevList($rev_list); + + foreach ($lines as $k => $line) { + list($rev_id, $author, $text) = $line; + $rev_id = $rev_list[$k]; if (!isset($blame_dict[$rev_id]) && !isset($blame_dict[$rev_id]['author'] )) { @@ -68,9 +67,11 @@ abstract class DiffusionFileContentQuery extends DiffusionQuery { $repository = $this->getRequest()->getRepository(); - $commits = id(new PhabricatorRepositoryCommit())->loadAllWhere( - 'repositoryID = %d AND commitIdentifier IN (%Ls)', $repository->getID(), - array_unique($rev_list)); + $commits = id(new PhabricatorAuditCommitQuery()) + ->withIdentifiers( + $repository->getID(), + array_unique($rev_list)) + ->execute(); foreach ($commits as $commit) { $blame_dict[$commit->getCommitIdentifier()]['epoch'] = @@ -115,4 +116,8 @@ abstract class DiffusionFileContentQuery extends DiffusionQuery { public function getNeedsBlame() { return $this->needsBlame; } + + protected function processRevList(array $rev_list) { + return $rev_list; + } } diff --git a/src/applications/diffusion/query/filecontent/base/__init__.php b/src/applications/diffusion/query/filecontent/base/__init__.php index cba7efae94..db6a0b7560 100644 --- a/src/applications/diffusion/query/filecontent/base/__init__.php +++ b/src/applications/diffusion/query/filecontent/base/__init__.php @@ -6,9 +6,9 @@ +phutil_require_module('phabricator', 'applications/audit/query/commit'); phutil_require_module('phabricator', 'applications/diffusion/query/base'); phutil_require_module('phabricator', 'applications/phid/handle/data'); -phutil_require_module('phabricator', 'applications/repository/storage/commit'); phutil_require_module('phabricator', 'applications/repository/storage/commitdata'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/diffusion/query/filecontent/mercurial/DiffusionMercurialFileContentQuery.php b/src/applications/diffusion/query/filecontent/mercurial/DiffusionMercurialFileContentQuery.php index 383120c36c..070176ec03 100644 --- a/src/applications/diffusion/query/filecontent/mercurial/DiffusionMercurialFileContentQuery.php +++ b/src/applications/diffusion/query/filecontent/mercurial/DiffusionMercurialFileContentQuery.php @@ -1,7 +1,7 @@ getPath(); $commit = $drequest->getCommit(); - list($corpus) = $repository->execxLocalCommand( - 'cat --rev %s -- %s', - $commit, - $path); + if ($this->getNeedsBlame()) { + // NOTE: We're using "--number" instead of "--changeset" because there is + // no way to get "--changeset" to show us the full commit hashes. + list($corpus) = $repository->execxLocalCommand( + 'annotate --user --number --rev %s -- %s', + $commit, + $path); + } else { + list($corpus) = $repository->execxLocalCommand( + 'cat --rev %s -- %s', + $commit, + $path); + } $file_content = new DiffusionFileContent(); $file_content->setCorpus($corpus); @@ -37,11 +46,45 @@ final class DiffusionMercurialFileContentQuery return $file_content; } - protected function tokenizeLine($line) { - // TODO: Support blame. - throw new Exception( - "Diffusion does not currently support blame for Mercurial."); + $matches = null; + + preg_match( + '/^(.*?)\s+([0-9]+): (.*)$/', + $line, + $matches); + + return array($matches[2], $matches[1], $matches[3]); + } + + /** + * Convert local revision IDs into full commit identifier hashes. + */ + protected function processRevList(array $rev_list) { + $drequest = $this->getRequest(); + $repository = $drequest->getRepository(); + + $revs = array_unique($rev_list); + foreach ($revs as $key => $rev) { + $revs[$key] = '--rev '.(int)$rev; + } + + list($stdout) = $repository->execxLocalCommand( + 'log --template=%s %C', + '{rev} {node}\\n', + implode(' ', $revs)); + + $map = array(); + foreach (explode("\n", trim($stdout)) as $line) { + list($rev, $node) = explode(' ', $line); + $map[$rev] = $node; + } + + foreach ($rev_list as $k => $rev) { + $rev_list[$k] = $map[$rev]; + } + + return $rev_list; } } diff --git a/src/applications/diffusion/query/filecontent/svn/DiffusionSvnFileContentQuery.php b/src/applications/diffusion/query/filecontent/svn/DiffusionSvnFileContentQuery.php index 2104823b64..29b9baccb7 100644 --- a/src/applications/diffusion/query/filecontent/svn/DiffusionSvnFileContentQuery.php +++ b/src/applications/diffusion/query/filecontent/svn/DiffusionSvnFileContentQuery.php @@ -1,7 +1,7 @@ getRequest(); diff --git a/src/applications/diffusion/request/base/DiffusionRequest.php b/src/applications/diffusion/request/base/DiffusionRequest.php index 7a820777a5..fe1c74a0a2 100644 --- a/src/applications/diffusion/request/base/DiffusionRequest.php +++ b/src/applications/diffusion/request/base/DiffusionRequest.php @@ -348,6 +348,7 @@ abstract class DiffusionRequest { $req_callsign = false; $req_branch = false; + $req_commit = false; switch ($action) { case 'history': @@ -360,6 +361,10 @@ abstract class DiffusionRequest { $req_callsign = true; $req_branch = true; break; + case 'commit': + $req_callsign = true; + $req_commit = true; + break; } if ($req_callsign && !strlen($callsign)) { @@ -372,6 +377,11 @@ abstract class DiffusionRequest { "Diffusion URI action '{$action}' requires branch!"); } + if ($req_commit && !strlen($commit)) { + throw new Exception( + "Diffusion URI action '{$action}' requires commit!"); + } + switch ($action) { case 'change': case 'history': @@ -392,6 +402,11 @@ abstract class DiffusionRequest { // it came from a URI. $uri = "{$path}{$commit}"; break; + case 'commit': + $commit = ltrim($commit, ';'); + $callsign = rtrim($callsign, '/'); + $uri = "/r{$callsign}{$commit}"; + break; default: throw new Exception("Unknown Diffusion URI action '{$action}'!"); } diff --git a/webroot/rsrc/css/application/diffusion/diffusion-source.css b/webroot/rsrc/css/application/diffusion/diffusion-source.css index c2a32fd97d..572b896620 100644 --- a/webroot/rsrc/css/application/diffusion/diffusion-source.css +++ b/webroot/rsrc/css/application/diffusion/diffusion-source.css @@ -4,7 +4,6 @@ .diffusion-source { margin: 1em 0 2em; - table-layout: fixed; width: 100%; font-family: "Monaco", Consolas, monospace; font-size: 10px; @@ -12,35 +11,46 @@ .diffusion-source th { text-align: right; - padding: 2px 6px; - width: 44px; vertical-align: top; background: #eeeeee; color: #888888; border-style: solid; border-width: 0px 1px; border-color: #eeeeee #999999 #eeeeee #dddddd; - font-weight: bold; font-family: "Verdana"; font-size: 11px; - overflow: hidden; } .diffusion-source td { letter-spacing: 0.0083334px; vertical-align: top; - white-space: pre; + white-space: pre-wrap; padding-bottom: 1px; padding-left: 8px; line-height: 16px; - overflow: hidden; -} - -.diffusion-wide-link a { - /* Give the user a larger click target. */ - display: block; + width: 100%; } .diffusion-browse-type-form { float: right; } + +.diffusion-blame-link, +.diffusion-rev-link, +.diffusion-author-link { + white-space: nowrap; +} + +.diffusion-blame-link a, +.diffusion-rev-link a, +.diffusion-author-link a, +.diffusion-line-link a { + /* Give the user a larger click target. */ + display: block; + padding: 2px 8px; + font-weight: bold; +} + +.diffusion-author-link span { + padding: 2px 8px; +}