From 47a184e2a543d4e947bcf19c618a7c3f501285e5 Mon Sep 17 00:00:00 2001 From: vrana Date: Tue, 6 Nov 2012 22:46:23 -0800 Subject: [PATCH] Display saved lint messages in Diffusion browse file Test Plan: Looked at https://secure.phabricator.com/diffusion/ARC/browse/master/src/difference/ArcanistDiffUtils.php. Saw "Show 16 Lint Messages". Clicked on it, saw the messages. Clicked on "Hide Lint Messages". Went to https://secure.phabricator.com/diffusion/ARC/browse/master/src/difference/ArcanistDiffUtils.php;5be54e. Saw "Switch Commit to See Lint". Clicked on it, saw the messages. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Differential Revision: https://secure.phabricator.com/D3920 --- .../DiffusionBrowseFileController.php | 132 +++++++++++++++++- .../diffusion/request/DiffusionRequest.php | 11 ++ .../diffusion/request/DiffusionSvnRequest.php | 4 + .../PhabricatorBaseEnglishTranslation.php | 10 ++ 4 files changed, 150 insertions(+), 7 deletions(-) diff --git a/src/applications/diffusion/controller/DiffusionBrowseFileController.php b/src/applications/diffusion/controller/DiffusionBrowseFileController.php index b88d41bafc..551a210828 100644 --- a/src/applications/diffusion/controller/DiffusionBrowseFileController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseFileController.php @@ -4,6 +4,9 @@ final class DiffusionBrowseFileController extends DiffusionController { private $corpusType = 'text'; + private $lintCommit; + private $lintMessages; + public function processRequest() { $request = $this->getRequest(); @@ -45,6 +48,8 @@ final class DiffusionBrowseFileController extends DiffusionController { return $this->buildRawResponse($path, $data); } + $this->loadLintMessages(); + // Build the content of the file. $corpus = $this->buildCorpus( $selected, @@ -119,6 +124,37 @@ final class DiffusionBrowseFileController extends DiffusionController { )); } + private function loadLintMessages() { + $drequest = $this->getDiffusionRequest(); + $branch = $drequest->loadBranch(); + + if (!$branch || !$branch->getLintCommit()) { + return; + } + + $file_history = DiffusionHistoryQuery::newFromDiffusionRequest( + $drequest)->setLimit(1)->loadHistory(); + + $lint_request = clone $drequest; + $lint_request->setCommit($branch->getLintCommit()); + $lint_history = DiffusionHistoryQuery::newFromDiffusionRequest( + $lint_request)->setLimit(1)->loadHistory(); + + $this->lintCommit = ''; + if (!$file_history || !$lint_history || + reset($file_history)->getCommitIdentifier() != + reset($lint_history)->getCommitIdentifier()) { + $this->lintCommit = $branch->getLintCommit(); + } + + $this->lintMessages = queryfx_all( + id(new PhabricatorRepository())->establishConnection('r'), + 'SELECT * FROM %T WHERE branchID = %d AND path = %s', + PhabricatorRepository::TABLE_LINTMESSAGE, + $branch->getID(), + '/'.$drequest->getPath()); + } + private function buildCorpus($selected, DiffusionFileContentQuery $file_query, $needs_blame, @@ -267,6 +303,7 @@ final class DiffusionBrowseFileController extends DiffusionController { ); $user = $this->getRequest()->getUser(); + $base_uri = $this->getRequest()->getRequestURI(); $blame_on = ($selected == 'blame' || $selected == 'plainblame'); if ($blame_on) { @@ -277,7 +314,7 @@ final class DiffusionBrowseFileController extends DiffusionController { $blame_button = $this->createViewAction( $blame_text, - $toggle_blame[$selected], + $base_uri->alter('view', $toggle_blame[$selected]), $user); @@ -289,13 +326,48 @@ final class DiffusionBrowseFileController extends DiffusionController { } $highlight_button = $this->createViewAction( $highlight_text, - $toggle_highlight[$selected], + $base_uri->alter('view', $toggle_highlight[$selected]), $user); + $href = null; + if ($this->getRequest()->getBool('lint')) { + $lint_text = pht('Hide Lint Messages'); + $href = $base_uri->alter('lint', null); + + } else if ($this->lintCommit === null) { + $lint_text = pht('Lint not Available'); + + } else if ($this->lintCommit) { + $lint_text = pht( + 'Switch for %d Lint Message(s)', + count($this->lintMessages)); + $href = $this->getDiffusionRequest()->generateURI(array( + 'action' => 'browse', + 'commit' => $this->lintCommit, + ))->alter('lint', true); + + } else if (!$this->lintMessages) { + $lint_text = pht('0 Lint Messages'); + + } else { + $lint_text = pht('Show %d Lint Message(s)', count($this->lintMessages)); + $href = $base_uri->alter('lint', true); + } + + $lint_button = $this->createViewAction( + $lint_text, + $href, + $user); + + if (!$href) { + $lint_button->setDisabled(true); + } + + $raw_button = $this->createViewAction( pht('View Raw File'), - 'raw', + $base_uri->alter('view', 'raw'), $user, 'file'); @@ -305,23 +377,23 @@ final class DiffusionBrowseFileController extends DiffusionController { ->setUser($user) ->addAction($blame_button) ->addAction($highlight_button) + ->addAction($lint_button) ->addAction($raw_button) ->addAction($edit_button); } private function createViewAction( $localized_text, - $view_mode, + $href, $user, $icon = null) { - $base_uri = $this->getRequest()->getRequestURI(); return id(new PhabricatorActionView()) ->setName($localized_text) ->setIcon($icon) ->setUser($user) ->setRenderAsForm(true) - ->setHref($base_uri->alter('view', $view_mode)); + ->setHref($href); } private function createEditAction() { @@ -482,7 +554,33 @@ final class DiffusionBrowseFileController extends DiffusionController { Javelin::initBehavior('phabricator-oncopy', array()); - $rows = array(); + $engine = null; + $inlines = array(); + if ($this->getRequest()->getBool('lint') && $this->lintMessages) { + $engine = new PhabricatorMarkupEngine(); + $engine->setViewer($user); + + foreach ($this->lintMessages as $message) { + $inline = id(new PhabricatorAuditInlineComment()) + ->setSyntheticAuthor($message['code'].' ('.$message['name'].')') + ->setLineNumber($message['line']) + ->setContent($message['description']); + $inlines[$message['line']][] = $inline; + + $engine->addObject( + $inline, + PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY); + } + + $engine->process(); + require_celerity_resource('differential-changeset-view-css'); + } + + $rows = $this->renderInlines( + idx($inlines, 0, array()), + $needs_blame, + $engine); + foreach ($display as $line) { $line_href = $drequest->generateURI( @@ -654,11 +752,31 @@ final class DiffusionBrowseFileController extends DiffusionController { ), $blame. $line_text); + + $rows = array_merge($rows, $this->renderInlines( + idx($inlines, $line['line'], array()), + $needs_blame, + $engine)); } return $rows; } + private function renderInlines(array $inlines, $needs_blame, $engine) { + $rows = array(); + foreach ($inlines as $inline) { + $inline_view = id(new DifferentialInlineCommentView()) + ->setMarkupEngine($engine) + ->setInlineComment($inline) + ->render(); + $rows[] = + ''. + str_repeat('', ($needs_blame ? 5 : 1)). + ''.$inline_view.''. + ''; + } + return $rows; + } private static function renderRevision(DiffusionRequest $drequest, $revision) { diff --git a/src/applications/diffusion/request/DiffusionRequest.php b/src/applications/diffusion/request/DiffusionRequest.php index 8ec58bc079..f4f28ae44a 100644 --- a/src/applications/diffusion/request/DiffusionRequest.php +++ b/src/applications/diffusion/request/DiffusionRequest.php @@ -203,6 +203,17 @@ abstract class DiffusionRequest { return $this->branch; } + protected function getArcanistBranch() { + return $this->getBranch(); + } + + public function loadBranch() { + return id(new PhabricatorRepositoryBranch())->loadOneWhere( + 'repositoryID = %d AND name = %s', + $this->getRepository()->getID(), + $this->getArcanistBranch()); + } + public function getTagContent() { return $this->tagContent; } diff --git a/src/applications/diffusion/request/DiffusionSvnRequest.php b/src/applications/diffusion/request/DiffusionSvnRequest.php index b305b10fbe..5632633f62 100644 --- a/src/applications/diffusion/request/DiffusionSvnRequest.php +++ b/src/applications/diffusion/request/DiffusionSvnRequest.php @@ -18,6 +18,10 @@ final class DiffusionSvnRequest extends DiffusionRequest { } } + protected function getArcanistBranch() { + return 'svn'; + } + public function getStableCommitName() { if ($this->commit) { return $this->commit; diff --git a/src/infrastructure/internationalization/PhabricatorBaseEnglishTranslation.php b/src/infrastructure/internationalization/PhabricatorBaseEnglishTranslation.php index db785af0e9..914f2d2736 100644 --- a/src/infrastructure/internationalization/PhabricatorBaseEnglishTranslation.php +++ b/src/infrastructure/internationalization/PhabricatorBaseEnglishTranslation.php @@ -150,6 +150,16 @@ abstract class PhabricatorBaseEnglishTranslation '%d Assigned Tasks', ), + 'Show %d Lint Message(s)' => array( + 'Show %d Lint Message', + 'Show %d Lint Messages', + ), + + 'Switch for %d Lint Message(s)' => array( + 'Switch for %d Lint Message', + 'Switch for %d Lint Messages', + ), + ); }