From 726a4912bd7ed3957133394cfdb4b65dc22cf060 Mon Sep 17 00:00:00 2001 From: vrana Date: Thu, 8 Nov 2012 15:14:44 -0800 Subject: [PATCH] Allow filtering by lint code in Diffusion Test Plan: /diffusion/ARC/lint/master/src/, clicked on count link. /diffusion/ARC/browse/master/src/difference/?lint=XHP9, clicked on file name. /diffusion/ARC/browse/master/src/difference/ArcanistDiffUtils.php?lint=, verified that all messages are displayed. /diffusion/ARC/browse/master/src/difference/ArcanistDiffUtils.php?lint=XHP9. /diffusion/ARC/browse/master/src/difference/ArcanistDiffUtils.php?lint=TXT3, verified that 0 messages are displayed. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Differential Revision: https://secure.phabricator.com/D3929 --- .../DiffusionBrowseFileController.php | 25 +++++++++++----- .../controller/DiffusionController.php | 4 ++- .../controller/DiffusionDiffController.php | 4 ++- .../controller/DiffusionLintController.php | 20 +++++++++++-- .../diffusion/request/DiffusionRequest.php | 20 ++++++++++++- .../view/DiffusionBrowseTableView.php | 29 ++++++++++++++----- .../PhabricatorBaseEnglishTranslation.php | 5 ++++ 7 files changed, 87 insertions(+), 20 deletions(-) diff --git a/src/applications/diffusion/controller/DiffusionBrowseFileController.php b/src/applications/diffusion/controller/DiffusionBrowseFileController.php index 551a210828..f89543e50e 100644 --- a/src/applications/diffusion/controller/DiffusionBrowseFileController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseFileController.php @@ -147,11 +147,22 @@ final class DiffusionBrowseFileController extends DiffusionController { $this->lintCommit = $branch->getLintCommit(); } + $conn = id(new PhabricatorRepository())->establishConnection('r'); + + $where = ''; + if ($drequest->getLint()) { + $where = qsprintf( + $conn, + 'AND code = %s', + $drequest->getLint()); + } + $this->lintMessages = queryfx_all( - id(new PhabricatorRepository())->establishConnection('r'), - 'SELECT * FROM %T WHERE branchID = %d AND path = %s', + $conn, + 'SELECT * FROM %T WHERE branchID = %d %Q AND path = %s', PhabricatorRepository::TABLE_LINTMESSAGE, $branch->getID(), + $where, '/'.$drequest->getPath()); } @@ -331,8 +342,8 @@ final class DiffusionBrowseFileController extends DiffusionController { $href = null; - if ($this->getRequest()->getBool('lint')) { - $lint_text = pht('Hide Lint Messages'); + if ($this->getRequest()->getStr('lint') !== null) { + $lint_text = pht('Hide %d Lint Messages', count($this->lintMessages)); $href = $base_uri->alter('lint', null); } else if ($this->lintCommit === null) { @@ -345,14 +356,14 @@ final class DiffusionBrowseFileController extends DiffusionController { $href = $this->getDiffusionRequest()->generateURI(array( 'action' => 'browse', 'commit' => $this->lintCommit, - ))->alter('lint', true); + ))->alter('lint', ''); } 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); + $href = $base_uri->alter('lint', ''); } $lint_button = $this->createViewAction( @@ -556,7 +567,7 @@ final class DiffusionBrowseFileController extends DiffusionController { $engine = null; $inlines = array(); - if ($this->getRequest()->getBool('lint') && $this->lintMessages) { + if ($this->getRequest()->getStr('lint') !== null && $this->lintMessages) { $engine = new PhabricatorMarkupEngine(); $engine->setViewer($user); diff --git a/src/applications/diffusion/controller/DiffusionController.php b/src/applications/diffusion/controller/DiffusionController.php index 243e986bd4..fa07dd1669 100644 --- a/src/applications/diffusion/controller/DiffusionController.php +++ b/src/applications/diffusion/controller/DiffusionController.php @@ -6,7 +6,9 @@ abstract class DiffusionController extends PhabricatorController { public function willProcessRequest(array $data) { if (isset($data['callsign'])) { - $drequest = DiffusionRequest::newFromAphrontRequestDictionary($data); + $drequest = DiffusionRequest::newFromAphrontRequestDictionary( + $data, + $this->getRequest()); $this->diffusionRequest = $drequest; } } diff --git a/src/applications/diffusion/controller/DiffusionDiffController.php b/src/applications/diffusion/controller/DiffusionDiffController.php index fc8bc03ab9..5d129cd274 100644 --- a/src/applications/diffusion/controller/DiffusionDiffController.php +++ b/src/applications/diffusion/controller/DiffusionDiffController.php @@ -6,7 +6,9 @@ final class DiffusionDiffController extends DiffusionController { $data = $data + array( 'dblob' => $this->getRequest()->getStr('ref'), ); - $drequest = DiffusionRequest::newFromAphrontRequestDictionary($data); + $drequest = DiffusionRequest::newFromAphrontRequestDictionary( + $data, + $this->getRequest()); $this->diffusionRequest = $drequest; } diff --git a/src/applications/diffusion/controller/DiffusionLintController.php b/src/applications/diffusion/controller/DiffusionLintController.php index 587b6b342d..0dc0a784af 100644 --- a/src/applications/diffusion/controller/DiffusionLintController.php +++ b/src/applications/diffusion/controller/DiffusionLintController.php @@ -4,6 +4,8 @@ final class DiffusionLintController extends DiffusionController { public function processRequest() { + $drequest = $this->getDiffusionRequest(); + $codes = $this->loadLintCodes(); $codes = array_reverse(isort($codes, 'n')); @@ -11,7 +13,13 @@ final class DiffusionLintController extends DiffusionController { foreach ($codes as $code) { $rows[] = array( $code['n'], - $code['files'], + hsprintf( + '%s', + $drequest->generateURI(array( + 'action' => 'browse', + 'lint' => $code['code'], + )), + $code['files']), phutil_escape_html(ArcanistLintSeverity::getStringForSeverity( $code['maxSeverity'])), phutil_escape_html($code['code']), @@ -28,6 +36,14 @@ final class DiffusionLintController extends DiffusionController { 'Code', 'Name', 'Example', + )) + ->setColumnClasses(array( + 'n', + 'n', + '', + 'pri', + '', + '', )); $content = array(); @@ -50,7 +66,7 @@ final class DiffusionLintController extends DiffusionController { $nav, array('title' => array( 'Lint', - $this->getDiffusionRequest()->getRepository()->getCallsign(), + $drequest->getRepository()->getCallsign(), ))); } diff --git a/src/applications/diffusion/request/DiffusionRequest.php b/src/applications/diffusion/request/DiffusionRequest.php index 9efd98b07d..94e35fcb97 100644 --- a/src/applications/diffusion/request/DiffusionRequest.php +++ b/src/applications/diffusion/request/DiffusionRequest.php @@ -20,6 +20,7 @@ abstract class DiffusionRequest { protected $branch; protected $commitType = 'commit'; protected $tagContent; + protected $lint; protected $repository; protected $repositoryCommit; @@ -80,7 +81,10 @@ abstract class DiffusionRequest { * @return DiffusionRequest New request object. * @task new */ - final public static function newFromAphrontRequestDictionary(array $data) { + final public static function newFromAphrontRequestDictionary( + array $data, + AphrontRequest $request) { + $callsign = phutil_unescape_uri_path_component(idx($data, 'callsign')); $object = self::newFromCallsign($callsign); @@ -88,6 +92,7 @@ abstract class DiffusionRequest { $parsed = self::parseRequestBlob(idx($data, 'dblob'), $use_branches); $object->initializeFromDictionary($parsed); + $object->lint = $request->getStr('lint'); return $object; } @@ -203,6 +208,10 @@ abstract class DiffusionRequest { return $this->branch; } + public function getLint() { + return $this->lint; + } + protected function getArcanistBranch() { return $this->getBranch(); } @@ -303,6 +312,7 @@ abstract class DiffusionRequest { 'path' => $this->getPath(), 'branch' => $this->getBranch(), 'commit' => $default_commit, + 'lint' => $this->getLint(), ); foreach ($defaults as $key => $val) { if (!isset($params[$key])) { // Overwrite NULL. @@ -324,6 +334,7 @@ abstract class DiffusionRequest { * - `path` Optional, path to file. * - `commit` Optional, commit identifier. * - `line` Optional, line range. + * - `lint` Optional, lint code. * - `params` Optional, query parameters. * * The function generates the specified URI and returns it. @@ -442,6 +453,13 @@ abstract class DiffusionRequest { } $uri = new PhutilURI($uri); + + if (isset($params['lint'])) { + $params['params'] = idx($params, 'params', array()) + array( + 'lint' => $params['lint'], + ); + } + if (idx($params, 'params')) { $uri->setQueryParams($params['params']); } diff --git a/src/applications/diffusion/view/DiffusionBrowseTableView.php b/src/applications/diffusion/view/DiffusionBrowseTableView.php index de846f2478..17a8924f59 100644 --- a/src/applications/diffusion/view/DiffusionBrowseTableView.php +++ b/src/applications/diffusion/view/DiffusionBrowseTableView.php @@ -81,11 +81,12 @@ final class DiffusionBrowseTableView extends DiffusionView { $lint = self::loadLintMessagesCount($drequest); if ($lint !== null) { - $return['lint'] = phutil_render_tag( - 'a', - array( - 'href' => $drequest->generateURI(array('action' => 'lint')), - ), + $return['lint'] = hsprintf( + '%s', + $drequest->generateURI(array( + 'action' => 'lint', + 'lint' => '', + )), number_format($lint)); } @@ -98,12 +99,23 @@ final class DiffusionBrowseTableView extends DiffusionView { return null; } + $conn = $drequest->getRepository()->establishConnection('r'); + + $where = ''; + if ($drequest->getLint()) { + $where = qsprintf( + $conn, + 'AND code = %s', + $drequest->getLint()); + } + $like = (substr($drequest->getPath(), -1) == '/' ? 'LIKE %>' : '= %s'); return head(queryfx_one( - $drequest->getRepository()->establishConnection('r'), - 'SELECT COUNT(*) FROM %T WHERE branchID = %d AND path '.$like, + $conn, + 'SELECT COUNT(*) FROM %T WHERE branchID = %d %Q AND path '.$like, PhabricatorRepository::TABLE_LINTMESSAGE, $branch->getID(), + $where, '/'.$drequest->getPath())); } @@ -226,6 +238,7 @@ final class DiffusionBrowseTableView extends DiffusionView { $branch = $this->getDiffusionRequest()->loadBranch(); $show_lint = ($branch && $branch->getLintCommit()); + $lint = $request->getLint(); $view = new AphrontTableView($rows); $view->setHeaders( @@ -233,7 +246,7 @@ final class DiffusionBrowseTableView extends DiffusionView { 'History', 'Edit', 'Path', - 'Lint', + ($lint ? phutil_escape_html($lint) : 'Lint'), 'Modified', 'Date', 'Time', diff --git a/src/infrastructure/internationalization/PhabricatorBaseEnglishTranslation.php b/src/infrastructure/internationalization/PhabricatorBaseEnglishTranslation.php index 914f2d2736..eb2fb02e7e 100644 --- a/src/infrastructure/internationalization/PhabricatorBaseEnglishTranslation.php +++ b/src/infrastructure/internationalization/PhabricatorBaseEnglishTranslation.php @@ -155,6 +155,11 @@ abstract class PhabricatorBaseEnglishTranslation 'Show %d Lint Messages', ), + 'Hide %d Lint Message(s)' => array( + 'Hide %d Lint Message', + 'Hide %d Lint Messages', + ), + 'Switch for %d Lint Message(s)' => array( 'Switch for %d Lint Message', 'Switch for %d Lint Messages',