From 9728c65e935813d706d4f0f71dad79ed042f2827 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 6 Jan 2016 06:15:25 -0800 Subject: [PATCH] Drive blame generation through `diffusion.blame` Summary: Ref T2450. Ref T9319. This is still a bit messy, but not quite so bad as it was: instead of using a single call to get both blame information and file content, use `diffusion.blame` for blame information. This will make optimizations to both blame and file content easier. Test Plan: Viewed a bunch of blame (color on/off, blame on/off). Reviewers: chad Reviewed By: chad Maniphest Tasks: T2450, T9319 Differential Revision: https://secure.phabricator.com/D14958 --- src/__phutil_library_map__.php | 2 - .../diffusion/DiffusionLintSaveRunner.php | 6 +- ...fusionFileContentQueryConduitAPIMethod.php | 16 +- .../controller/DiffusionBrowseController.php | 206 +++++++++++------- .../DiffusionGitFileContentQueryTestCase.php | 32 --- .../filecontent/DiffusionFileContentQuery.php | 116 +--------- .../DiffusionGitFileContentQuery.php | 39 +--- .../DiffusionMercurialFileContentQuery.php | 58 +---- .../DiffusionSvnFileContentQuery.php | 15 +- .../storage/PhabricatorRepositoryCommit.php | 26 +++ 10 files changed, 183 insertions(+), 333 deletions(-) delete mode 100644 src/applications/diffusion/query/__tests__/DiffusionGitFileContentQueryTestCase.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 465108ac7c..e3175a5440 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -607,7 +607,6 @@ phutil_register_library_map(array( 'DiffusionGitBranch' => 'applications/diffusion/data/DiffusionGitBranch.php', 'DiffusionGitBranchTestCase' => 'applications/diffusion/data/__tests__/DiffusionGitBranchTestCase.php', 'DiffusionGitFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php', - 'DiffusionGitFileContentQueryTestCase' => 'applications/diffusion/query/__tests__/DiffusionGitFileContentQueryTestCase.php', 'DiffusionGitRawDiffQuery' => 'applications/diffusion/query/rawdiff/DiffusionGitRawDiffQuery.php', 'DiffusionGitReceivePackSSHWorkflow' => 'applications/diffusion/ssh/DiffusionGitReceivePackSSHWorkflow.php', 'DiffusionGitRequest' => 'applications/diffusion/request/DiffusionGitRequest.php', @@ -4577,7 +4576,6 @@ phutil_register_library_map(array( 'DiffusionGitBranch' => 'Phobject', 'DiffusionGitBranchTestCase' => 'PhabricatorTestCase', 'DiffusionGitFileContentQuery' => 'DiffusionFileContentQuery', - 'DiffusionGitFileContentQueryTestCase' => 'PhabricatorTestCase', 'DiffusionGitRawDiffQuery' => 'DiffusionRawDiffQuery', 'DiffusionGitReceivePackSSHWorkflow' => 'DiffusionGitSSHWorkflow', 'DiffusionGitRequest' => 'DiffusionRequest', diff --git a/src/applications/diffusion/DiffusionLintSaveRunner.php b/src/applications/diffusion/DiffusionLintSaveRunner.php index b726089fe2..e32392be40 100644 --- a/src/applications/diffusion/DiffusionLintSaveRunner.php +++ b/src/applications/diffusion/DiffusionLintSaveRunner.php @@ -257,8 +257,10 @@ final class DiffusionLintSaveRunner extends Phobject { 'path' => $path, 'commit' => $this->lintCommit, )); - $query = DiffusionFileContentQuery::newFromDiffusionRequest($drequest) - ->setNeedsBlame(true); + + // TODO: Restore blame information / generally fix this workflow. + + $query = DiffusionFileContentQuery::newFromDiffusionRequest($drequest); $queries[$path] = $query; $futures[$path] = $query->getFileContentFuture(); } diff --git a/src/applications/diffusion/conduit/DiffusionFileContentQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionFileContentQueryConduitAPIMethod.php index 61987b4d77..73ed4161fe 100644 --- a/src/applications/diffusion/conduit/DiffusionFileContentQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionFileContentQueryConduitAPIMethod.php @@ -19,7 +19,6 @@ final class DiffusionFileContentQueryConduitAPIMethod return array( 'path' => 'required string', 'commit' => 'required string', - 'needsBlame' => 'optional bool', 'timeout' => 'optional int', 'byteLimit' => 'optional int', ); @@ -27,12 +26,9 @@ final class DiffusionFileContentQueryConduitAPIMethod protected function getResult(ConduitAPIRequest $request) { $drequest = $this->getDiffusionRequest(); - $needs_blame = $request->getValue('needsBlame'); - $file_query = DiffusionFileContentQuery::newFromDiffusionRequest( - $drequest); - $file_query - ->setViewer($request->getUser()) - ->setNeedsBlame($needs_blame); + + $file_query = DiffusionFileContentQuery::newFromDiffusionRequest($drequest) + ->setViewer($request->getUser()); $timeout = $request->getValue('timeout'); if ($timeout) { @@ -46,11 +42,7 @@ final class DiffusionFileContentQueryConduitAPIMethod $file_content = $file_query->loadFileContent(); - if ($needs_blame) { - list($text_list, $rev_list, $blame_dict) = $file_query->getBlameData(); - } else { - $text_list = $rev_list = $blame_dict = array(); - } + $text_list = $rev_list = $blame_dict = array(); $file_content ->setBlameDict($blame_dict) diff --git a/src/applications/diffusion/controller/DiffusionBrowseController.php b/src/applications/diffusion/controller/DiffusionBrowseController.php index d512ad035f..033050402b 100644 --- a/src/applications/diffusion/controller/DiffusionBrowseController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseController.php @@ -130,7 +130,6 @@ final class DiffusionBrowseController extends DiffusionController { $params = array( 'commit' => $drequest->getCommit(), 'path' => $drequest->getPath(), - 'needsBlame' => $needs_blame, ); $byte_limit = null; @@ -572,7 +571,24 @@ final class DiffusionBrowseController extends DiffusionController { $path, $data) { + $viewer = $this->getViewer(); + + $blame_handles = array(); + if ($needs_blame) { + $blame = $this->loadBlame($path, $drequest->getCommit()); + if ($blame) { + $author_phids = mpull($blame, 'getAuthorPHID'); + $blame_handles = $viewer->loadHandles($author_phids); + } + } else { + $blame = array(); + } + + $file_corpus = $file_content->getCorpus(); + if (!$show_color) { + $lines = phutil_split_lines($file_corpus); + $style = 'border: none; width: 100%; height: 80em; font-family: monospace'; if (!$show_blame) { @@ -581,18 +597,24 @@ final class DiffusionBrowseController extends DiffusionController { array( 'style' => $style, ), - $file_content->getCorpus()); + $file_corpus); } else { - $text_list = $file_content->getTextList(); - $rev_list = $file_content->getRevList(); - $blame_dict = $file_content->getBlameDict(); - $rows = array(); - foreach ($text_list as $k => $line) { - $rev = $rev_list[$k]; - $author = $blame_dict[$rev]['author']; - $rows[] = - sprintf('%-10s %-20s %s', substr($rev, 0, 7), $author, $line); + foreach ($lines as $line_number => $line) { + $commit = idx($blame, $line_number); + if ($commit) { + $author = $commit->renderAuthorShortName($blame_handles); + $commit_name = $commit->getShortName(); + } else { + $author = null; + $commit_name = null; + } + + $rows[] = sprintf( + '%-10s %-20s %s', + $commit_name, + $author, + $line); } $corpus = phutil_tag( @@ -600,22 +622,21 @@ final class DiffusionBrowseController extends DiffusionController { array( 'style' => $style, ), - implode("\n", $rows)); + implode('', $rows)); } } else { require_celerity_resource('syntax-highlighting-css'); - $text_list = $file_content->getTextList(); - $rev_list = $file_content->getRevList(); - $blame_dict = $file_content->getBlameDict(); - $text_list = implode("\n", $text_list); - $text_list = PhabricatorSyntaxHighlighter::highlightWithFilename( + $highlighted = PhabricatorSyntaxHighlighter::highlightWithFilename( $path, - $text_list); - $text_list = explode("\n", $text_list); + $file_corpus); + $lines = phutil_split_lines($highlighted); - $rows = $this->buildDisplayRows($text_list, $rev_list, $blame_dict, - $needs_blame, $drequest, $show_blame, $show_color); + $rows = $this->buildDisplayRows( + $lines, + $blame, + $show_blame, + $show_color); $corpus_table = javelin_tag( 'table', @@ -824,25 +845,26 @@ final class DiffusionBrowseController extends DiffusionController { private function buildDisplayRows( - array $text_list, - array $rev_list, - array $blame_dict, - $needs_blame, - DiffusionRequest $drequest, - $show_blame, - $show_color) { + array $lines, + array $blame, + $show_color, + $show_blame) { + + $drequest = $this->getDiffusionRequest(); $handles = array(); - if ($blame_dict) { - $epoch_list = ipull(ifilter($blame_dict, 'epoch'), 'epoch'); + if ($blame) { + $epoch_list = mpull($blame, 'getEpoch', 'getID'); + $epoch_list = array_filter($epoch_list); + $epoch_list = array_unique($epoch_list); + $epoch_list = array_values($epoch_list); + $epoch_min = min($epoch_list); $epoch_max = max($epoch_list); $epoch_range = ($epoch_max - $epoch_min) + 1; - - $author_phids = ipull(ifilter($blame_dict, 'authorPHID'), 'authorPHID'); - $handles = $this->loadViewerHandles($author_phids); } + $line_arr = array(); $line_str = $drequest->getLine(); $ranges = explode(',', $line_str); @@ -864,9 +886,9 @@ final class DiffusionBrowseController extends DiffusionController { $display = array(); $line_number = 1; - $last_rev = null; + $last_commit = null; $color = null; - foreach ($text_list as $k => $line) { + foreach ($lines as $line_index => $line) { $display_line = array( 'epoch' => null, 'commit' => null, @@ -882,17 +904,26 @@ final class DiffusionBrowseController extends DiffusionController { // with same color; otherwise generate blame info. The newer a change // is, the more saturated the color. - $rev = idx($rev_list, $k, $last_rev); + $commit = idx($blame, $line_index, $last_commit); - if ($last_rev == $rev) { + if ($commit && $last_commit && + ($last_commit->getID() == $commit->getID())) { $display_line['color'] = $color; } else { - $blame = $blame_dict[$rev]; - - if (!isset($blame['epoch'])) { - $color = '#ffd'; // Render as warning. + if ($commit) { + $epoch = $commit->getEpoch(); } else { - $color_ratio = ($blame['epoch'] - $epoch_min) / $epoch_range; + $epoch = null; + } + + if (!$epoch) { + if (!$blame) { + $color = '#f6f6f6'; + } else { + $color = '#ffd'; // Render as warning. + } + } else { + $color_ratio = ($epoch - $epoch_min) / $epoch_range; $color_value = 0xE6 * (1.0 - $color_ratio); $color = sprintf( '#%02x%02x%02x', @@ -901,19 +932,16 @@ final class DiffusionBrowseController extends DiffusionController { $color_value); } - $display_line['epoch'] = idx($blame, 'epoch'); + $display_line['epoch'] = $epoch; $display_line['color'] = $color; - $display_line['commit'] = $rev; - $author_phid = idx($blame, 'authorPHID'); - if ($author_phid && $handles[$author_phid]) { - $author_link = $handles[$author_phid]->renderLink(); + if ($commit) { + $display_line['commit'] = $commit; } else { - $author_link = $blame['author']; + $display_line['commit'] = null; } - $display_line['author'] = $author_link; - $last_rev = $rev; + $last_commit = $commit; } } @@ -936,15 +964,7 @@ final class DiffusionBrowseController extends DiffusionController { $request = $this->getRequest(); $viewer = $request->getUser(); - $commits = array_filter(ipull($display, 'commit')); - if ($commits) { - $commits = id(new DiffusionCommitQuery()) - ->setViewer($viewer) - ->withRepository($drequest->getRepository()) - ->withIdentifiers($commits) - ->execute(); - $commits = mpull($commits, null, 'getCommitIdentifier'); - } + $commits = mpull($blame, null, 'getCommitIdentifier'); $revision_ids = id(new DifferentialRevision()) ->loadIDsByCommitPHIDs(mpull($commits, 'getPHID')); @@ -957,9 +977,9 @@ final class DiffusionBrowseController extends DiffusionController { } $phids = array(); - foreach ($commits as $commit) { - if ($commit->getAuthorPHID()) { - $phids[] = $commit->getAuthorPHID(); + foreach ($commits as $blame_commit) { + if ($blame_commit->getAuthorPHID()) { + $phids[] = $blame_commit->getAuthorPHID(); } } foreach ($revisions as $revision) { @@ -1002,7 +1022,6 @@ final class DiffusionBrowseController extends DiffusionController { $engine); foreach ($display as $line) { - $line_href = $drequest->generateURI( array( 'action' => 'browse', @@ -1023,11 +1042,11 @@ final class DiffusionBrowseController extends DiffusionController { if (idx($line, 'commit')) { $commit = $line['commit']; - if (idx($commits, $commit)) { + if ($commit) { $tooltip = $this->renderCommitTooltip( - $commits[$commit], + $commit, $handles, - $line['author']); + $commit->renderAuthorLink($handles)); } else { $tooltip = null; } @@ -1041,7 +1060,7 @@ final class DiffusionBrowseController extends DiffusionController { 'href' => $drequest->generateURI( array( 'action' => 'commit', - 'commit' => $line['commit'], + 'commit' => $commit->getCommitIdentifier(), )), 'sigil' => 'has-tooltip', 'meta' => array( @@ -1050,14 +1069,11 @@ final class DiffusionBrowseController extends DiffusionController { 'size' => 600, ), ), - id(new PhutilUTF8StringTruncator()) - ->setMaximumGlyphs(9) - ->setTerminator('') - ->truncateString($line['commit'])); + $commit->getShortName()); $revision_id = null; - if (idx($commits, $commit)) { - $revision_id = idx($revision_ids, $commits[$commit]->getPHID()); + if ($commit) { + $revision_id = idx($revision_ids, $commit->getPHID()); } if ($revision_id) { @@ -1207,7 +1223,7 @@ final class DiffusionBrowseController extends DiffusionController { private function renderInlines( array $inlines, - $needs_blame, + $show_blame, $has_coverage, $engine) { @@ -1222,7 +1238,7 @@ final class DiffusionBrowseController extends DiffusionController { ->setInlineComment($inline) ->render(); - $row = array_fill(0, ($needs_blame ? 3 : 1), phutil_tag('th')); + $row = array_fill(0, ($show_blame ? 3 : 1), phutil_tag('th')); $row[] = phutil_tag('td', array(), $inline_view); @@ -1722,4 +1738,44 @@ final class DiffusionBrowseController extends DiffusionController { return $view; } + private function loadBlame($path, $commit) { + $blame = $this->callConduitWithDiffusionRequest( + 'diffusion.blame', + array( + 'commit' => $commit, + 'paths' => array($path), + )); + + $identifiers = idx($blame, $path, array()); + + if ($identifiers) { + $viewer = $this->getViewer(); + $drequest = $this->getDiffusionRequest(); + $repository = $drequest->getRepository(); + + $commits = id(new DiffusionCommitQuery()) + ->setViewer($viewer) + ->withRepository($repository) + ->withIdentifiers($identifiers) + // TODO: We only fetch this to improve author display behavior, but + // shouldn't really need to? + ->needCommitData(true) + ->execute(); + $commits = mpull($commits, null, 'getCommitIdentifier'); + } else { + $commits = array(); + } + + foreach ($identifiers as $key => $identifier) { + $commit = idx($commits, $identifier); + if ($commit) { + $identifiers[$key] = $commit; + } else { + $identifiers[$key] = null; + } + } + + return $identifiers; + } + } diff --git a/src/applications/diffusion/query/__tests__/DiffusionGitFileContentQueryTestCase.php b/src/applications/diffusion/query/__tests__/DiffusionGitFileContentQueryTestCase.php deleted file mode 100644 index 1e69ecd5fa..0000000000 --- a/src/applications/diffusion/query/__tests__/DiffusionGitFileContentQueryTestCase.php +++ /dev/null @@ -1,32 +0,0 @@ -call()'); - $this->assertEqual($result[0], '8220d5d54f6d5d5552a636576cbe9c35f15b65b2'); - $this->assertEqual($result[1], 'Andrew Gallagher'); - $this->assertEqual($result[2], ' $somevar = $this->call()'); - - // User name like 'Jimmy (He) Zhang' - $result = DiffusionGitFileContentQuery::match( - '8220d5d54f6d5d5552a636576cbe9c35f15b65b2 '. - '( Jimmy (He) Zhang 2013-10-11 5) '. - 'code(); "(string literal 9999-99-99 2)"; more_code();'); - $this->assertEqual($result[1], 'Jimmy (He) Zhang'); - $this->assertEqual($result[2], - ' code(); "(string literal 9999-99-99 2)"; more_code();'); - - // User name like 'Scott Shapiro (Ads Product Marketing)' - $result = DiffusionGitFileContentQuery::match( - '8220d5d54f6d5d5552a636576cbe9c35f15b65b2 '. - '( Scott Shapiro (Ads Product Marketing) 2013-10-11 5) '. - 'code(); "(string literal 9999-99-99 2)"; more_code();'); - $this->assertEqual($result[1], 'Scott Shapiro (Ads Product Marketing)'); - $this->assertEqual($result[2], - ' code(); "(string literal 9999-99-99 2)"; more_code();'); - } -} diff --git a/src/applications/diffusion/query/filecontent/DiffusionFileContentQuery.php b/src/applications/diffusion/query/filecontent/DiffusionFileContentQuery.php index 9664d98f07..6a9cf2e021 100644 --- a/src/applications/diffusion/query/filecontent/DiffusionFileContentQuery.php +++ b/src/applications/diffusion/query/filecontent/DiffusionFileContentQuery.php @@ -8,7 +8,6 @@ */ abstract class DiffusionFileContentQuery extends DiffusionQuery { - private $needsBlame; private $fileContent; private $viewer; private $timeout; @@ -32,6 +31,15 @@ abstract class DiffusionFileContentQuery extends DiffusionQuery { return $this->byteLimit; } + public function setViewer(PhabricatorUser $user) { + $this->viewer = $user; + return $this; + } + + public function getViewer() { + return $this->viewer; + } + final public static function newFromDiffusionRequest( DiffusionRequest $request) { return parent::newQueryObject(__CLASS__, $request); @@ -90,110 +98,4 @@ abstract class DiffusionFileContentQuery extends DiffusionQuery { return $this->fileContent->getCorpus(); } - /** - * Pretty hairy function. If getNeedsBlame is false, this returns - * - * ($text_list, array(), array()) - * - * Where $text_list is the raw file content with trailing new lines stripped. - * - * If getNeedsBlame is true, this returns - * - * ($text_list, $line_rev_dict, $blame_dict) - * - * Where $text_list is just the lines of code -- the raw file content will - * contain lots of blame data, $line_rev_dict is a dictionary of line number - * => revision id, and $blame_dict is another complicated data structure. - * In detail, $blame_dict contains [revision id][author] keys, as well - * as [commit id][authorPhid] and [commit id][epoch] keys. - * - * @return ($text_list, $line_rev_dict, $blame_dict) - */ - final public function getBlameData() { - $raw_data = preg_replace('/\n$/', '', $this->getRawData()); - - $text_list = array(); - $line_rev_dict = array(); - $blame_dict = array(); - - if (!$this->getNeedsBlame()) { - $text_list = explode("\n", $raw_data); - } else if ($raw_data != '') { - $lines = array(); - foreach (explode("\n", $raw_data) as $k => $line) { - $lines[$k] = $this->tokenizeLine($line); - - list($rev_id, $author, $text) = $lines[$k]; - $text_list[$k] = $text; - $line_rev_dict[$k] = $rev_id; - } - - $line_rev_dict = $this->processRevList($line_rev_dict); - - foreach ($lines as $k => $line) { - list($rev_id, $author, $text) = $line; - $rev_id = $line_rev_dict[$k]; - - if (!isset($blame_dict[$rev_id])) { - $blame_dict[$rev_id]['author'] = $author; - } - } - - $repository = $this->getRequest()->getRepository(); - - $commits = id(new DiffusionCommitQuery()) - ->setViewer($this->getViewer()) - ->withDefaultRepository($repository) - ->withIdentifiers(array_unique($line_rev_dict)) - ->execute(); - - foreach ($commits as $commit) { - $blame_dict[$commit->getCommitIdentifier()]['epoch'] = - $commit->getEpoch(); - } - - if ($commits) { - $commits_data = id(new PhabricatorRepositoryCommitData())->loadAllWhere( - 'commitID IN (%Ls)', - mpull($commits, 'getID')); - - foreach ($commits_data as $data) { - $author_phid = $data->getCommitDetail('authorPHID'); - if (!$author_phid) { - continue; - } - $commit = $commits[$data->getCommitID()]; - $commit_identifier = $commit->getCommitIdentifier(); - $blame_dict[$commit_identifier]['authorPHID'] = $author_phid; - } - } - - } - - return array($text_list, $line_rev_dict, $blame_dict); - } - - abstract protected function tokenizeLine($line); - - public function setNeedsBlame($needs_blame) { - $this->needsBlame = $needs_blame; - return $this; - } - - public function getNeedsBlame() { - return $this->needsBlame; - } - - public function setViewer(PhabricatorUser $user) { - $this->viewer = $user; - return $this; - } - - public function getViewer() { - return $this->viewer; - } - - protected function processRevList(array $rev_list) { - return $rev_list; - } } diff --git a/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php b/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php index 67c6369940..448d2886dc 100644 --- a/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php +++ b/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php @@ -9,17 +9,10 @@ final class DiffusionGitFileContentQuery extends DiffusionFileContentQuery { $path = $drequest->getPath(); $commit = $drequest->getCommit(); - if ($this->getNeedsBlame()) { - return $repository->getLocalCommandFuture( - '--no-pager blame -c -l --date=short %s -- %s', - $commit, - $path); - } else { - return $repository->getLocalCommandFuture( - 'cat-file blob %s:%s', - $commit, - $path); - } + return $repository->getLocalCommandFuture( + 'cat-file blob %s:%s', + $commit, + $path); } protected function executeQueryFromFuture(Future $future) { @@ -31,28 +24,4 @@ final class DiffusionGitFileContentQuery extends DiffusionFileContentQuery { return $file_content; } - protected function tokenizeLine($line) { - return self::match($line); - } - - public static function match($line) { - $m = array(); - // sample lines: - // - // d1b4fcdd2a7c8c0f8cbdd01ca839d992135424dc - // ( hzhao 2009-05-01 202)function print(); - // - // 8220d5d54f6d5d5552a636576cbe9c35f15b65b2 - // (Andrew Gallagher 2010-12-03 324) - // // Add the lines for trailing context - preg_match( - '/^\s*?(\S+?)\s*\(\s*(.*?)\s+\d{4}-\d{2}-\d{2}\s+\d+\)(.*)?$/', - $line, - $m); - $rev_id = $m[1]; - $author = $m[2]; - $text = idx($m, 3); - return array($rev_id, $author, $text); - } - } diff --git a/src/applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php b/src/applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php index c97b53e75a..ec49414666 100644 --- a/src/applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php +++ b/src/applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php @@ -10,19 +10,10 @@ final class DiffusionMercurialFileContentQuery $path = $drequest->getPath(); $commit = $drequest->getCommit(); - 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. - return $repository->getLocalCommandFuture( - 'annotate --user --number --rev %s -- %s', - $commit, - $path); - } else { - return $repository->getLocalCommandFuture( - 'cat --rev %s -- %s', - $commit, - $path); - } + return $repository->getLocalCommandFuture( + 'cat --rev %s -- %s', + $commit, + $path); } protected function executeQueryFromFuture(Future $future) { @@ -34,45 +25,4 @@ final class DiffusionMercurialFileContentQuery return $file_content; } - protected function tokenizeLine($line) { - $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/DiffusionSvnFileContentQuery.php b/src/applications/diffusion/query/filecontent/DiffusionSvnFileContentQuery.php index be7487969b..4976d7a27b 100644 --- a/src/applications/diffusion/query/filecontent/DiffusionSvnFileContentQuery.php +++ b/src/applications/diffusion/query/filecontent/DiffusionSvnFileContentQuery.php @@ -10,8 +10,7 @@ final class DiffusionSvnFileContentQuery extends DiffusionFileContentQuery { $commit = $drequest->getCommit(); return $repository->getRemoteCommandFuture( - '%C %s', - $this->getNeedsBlame() ? 'blame --force' : 'cat', + 'cat %s', $repository->getSubversionPathURI($path, $commit)); } @@ -41,16 +40,4 @@ final class DiffusionSvnFileContentQuery extends DiffusionFileContentQuery { return $file_content; } - protected function tokenizeLine($line) { - // sample line: - // 347498 yliu function print(); - $m = array(); - preg_match('/^\s*(\d+)\s+(\S+)(?: (.*))?$/', $line, $m); - $rev_id = $m[1]; - $author = $m[2]; - $text = idx($m, 3); - - return array($rev_id, $author, $text); - } - } diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommit.php b/src/applications/repository/storage/PhabricatorRepositoryCommit.php index 987c42492a..d7a2e111e2 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryCommit.php +++ b/src/applications/repository/storage/PhabricatorRepositoryCommit.php @@ -266,6 +266,32 @@ final class PhabricatorRepositoryCommit return $repository->formatCommitName($identifier); } + public function getShortName() { + $identifier = $this->getCommitIdentifier(); + return substr($identifier, 0, 9); + } + + public function renderAuthorLink($handles) { + $author_phid = $this->getAuthorPHID(); + if ($author_phid && isset($handles[$author_phid])) { + return $handles[$author_phid]->renderLink(); + } + + return $this->renderAuthorShortName($handles); + } + + public function renderAuthorShortName($handles) { + $author_phid = $this->getAuthorPHID(); + if ($author_phid && isset($handles[$author_phid])) { + return $handles[$author_phid]->getName(); + } + + $data = $this->getCommitData(); + $name = $data->getAuthorName(); + + $parsed = new PhutilEmailAddress($name); + return nonempty($parsed->getDisplayName(), $parsed->getAddress()); + } /* -( PhabricatorPolicyInterface )----------------------------------------- */