From 4bccb1547d6bfe9c4eaa1f5dea93994d488dd26c Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 14 Feb 2018 15:49:00 -0800 Subject: [PATCH] Modularize the "jump nav" behaviors in global search Summary: Depends on D19087. Ref T13079. This still doesn't feel like the most clean, general system in the world, but is a step forward from hard-coded `switch()` stuff. Test Plan: - Jumped to `r`. - Jumped to `a`. - Jumped to `r poe` (multiple results). - Jumped to `r poetry` (one result). - Jumped to `r syzygy` (no results). - Jumped to `p`. - Jumped to `p robot` (multiple results); `p assessment` (one result). - The behavior for `p ` has changed slightly but should be more powerful now (it's consistent with `r `). - Jumped to `s ` and `s ->`. - Jumped to `d`. - Jumped to `f`. - Jumped to `t`. - Jumped to `T123`, `D123`, `@dog`, `PHID-DREV-abcd`, etc. Maniphest Tasks: T13079 Differential Revision: https://secure.phabricator.com/D19088 --- src/__phutil_library_map__.php | 2 - .../DiffusionDatasourceEngineExtension.php | 70 +++++++++ ...ricatorPeopleDatasourceEngineExtension.php | 25 ++++ .../ProjectDatasourceEngineExtension.php | 46 ++++++ .../PhabricatorSearchController.php | 18 +-- .../engine/PhabricatorDatasourceEngine.php | 29 ++++ .../engine/PhabricatorJumpNavHandler.php | 135 ------------------ .../PhabricatorDatasourceEngineExtension.php | 27 +++- ...catorMonogramDatasourceEngineExtension.php | 42 ++++++ 9 files changed, 246 insertions(+), 148 deletions(-) delete mode 100644 src/applications/search/engine/PhabricatorJumpNavHandler.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 771658e32e..22437bc182 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3154,7 +3154,6 @@ phutil_register_library_map(array( 'PhabricatorJSONExportFormat' => 'infrastructure/export/format/PhabricatorJSONExportFormat.php', 'PhabricatorJavelinLinter' => 'infrastructure/lint/linter/PhabricatorJavelinLinter.php', 'PhabricatorJiraIssueHasObjectEdgeType' => 'applications/doorkeeper/edge/PhabricatorJiraIssueHasObjectEdgeType.php', - 'PhabricatorJumpNavHandler' => 'applications/search/engine/PhabricatorJumpNavHandler.php', 'PhabricatorKeyValueDatabaseCache' => 'applications/cache/PhabricatorKeyValueDatabaseCache.php', 'PhabricatorKeyValueSerializingCacheProxy' => 'applications/cache/PhabricatorKeyValueSerializingCacheProxy.php', 'PhabricatorKeyboardRemarkupRule' => 'infrastructure/markup/rule/PhabricatorKeyboardRemarkupRule.php', @@ -8708,7 +8707,6 @@ phutil_register_library_map(array( 'PhabricatorJSONExportFormat' => 'PhabricatorExportFormat', 'PhabricatorJavelinLinter' => 'ArcanistLinter', 'PhabricatorJiraIssueHasObjectEdgeType' => 'PhabricatorEdgeType', - 'PhabricatorJumpNavHandler' => 'Phobject', 'PhabricatorKeyValueDatabaseCache' => 'PhutilKeyValueCache', 'PhabricatorKeyValueSerializingCacheProxy' => 'PhutilKeyValueCacheProxy', 'PhabricatorKeyboardRemarkupRule' => 'PhutilRemarkupRule', diff --git a/src/applications/diffusion/engineextension/DiffusionDatasourceEngineExtension.php b/src/applications/diffusion/engineextension/DiffusionDatasourceEngineExtension.php index fdfe2d02e1..9678b627b1 100644 --- a/src/applications/diffusion/engineextension/DiffusionDatasourceEngineExtension.php +++ b/src/applications/diffusion/engineextension/DiffusionDatasourceEngineExtension.php @@ -9,4 +9,74 @@ final class DiffusionDatasourceEngineExtension new DiffusionSymbolDatasource(), ); } + + public function newJumpURI($query) { + $viewer = $this->getViewer(); + + // Send "r" to Diffusion. + if (preg_match('/^r\z/i', $query)) { + return '/diffusion/'; + } + + // Send "a" to the commit list ("Audit"). + if (preg_match('/^a\z/i', $query)) { + return '/diffusion/commit/'; + } + + // Send "r " to a search for a matching repository. + $matches = null; + if (preg_match('/^r\s+(.+)\z/i', $query, $matches)) { + $raw_query = $matches[1]; + + $engine = id(new PhabricatorRepository()) + ->newFerretEngine(); + + $compiler = id(new PhutilSearchQueryCompiler()) + ->setEnableFunctions(true); + + $raw_tokens = $compiler->newTokens($raw_query); + + $fulltext_tokens = array(); + foreach ($raw_tokens as $raw_token) { + $fulltext_token = id(new PhabricatorFulltextToken()) + ->setToken($raw_token); + $fulltext_tokens[] = $fulltext_token; + } + + $repositories = id(new PhabricatorRepositoryQuery()) + ->setViewer($viewer) + ->withFerretConstraint($engine, $fulltext_tokens) + ->execute(); + if (count($repositories) == 1) { + // Just one match, jump to repository. + return head($repositories)->getURI(); + } else { + // More than one match, jump to search. + return urisprintf( + '/diffusion/?order=relevance&query=%s#R', + $raw_query); + } + } + + // Send "s " to a symbol search. + $matches = null; + if (preg_match('/^s\s+(.+)\z/i', $query, $matches)) { + $symbol = $matches[1]; + + $parts = null; + if (preg_match('/(.*)(?:\\.|::|->)(.*)/', $symbol, $parts)) { + return urisprintf( + '/diffusion/symbol/%s/?jump=true&context=%s', + $parts[2], + $parts[1]); + } else { + return urisprintf( + '/diffusion/symbol/%s/?jump=true', + $symbol); + } + } + + return null; + } + } diff --git a/src/applications/people/engineextension/PhabricatorPeopleDatasourceEngineExtension.php b/src/applications/people/engineextension/PhabricatorPeopleDatasourceEngineExtension.php index b9de70b99d..dc8599554e 100644 --- a/src/applications/people/engineextension/PhabricatorPeopleDatasourceEngineExtension.php +++ b/src/applications/people/engineextension/PhabricatorPeopleDatasourceEngineExtension.php @@ -8,4 +8,29 @@ final class PhabricatorPeopleDatasourceEngineExtension new PhabricatorPeopleDatasource(), ); } + + public function newJumpURI($query) { + $viewer = $this->getViewer(); + + // Send "u" to the user directory. + if (preg_match('/^u\z/i', $query)) { + return '/people/'; + } + + // Send "u " to the user's profile page. + $matches = null; + if (preg_match('/^u\s+(.+)\z/i', $query, $matches)) { + $raw_query = $matches[1]; + + // TODO: We could test that this is a valid username and jump to + // a search in the user directory if it isn't. + + return urisprintf( + '/p/%s/', + $raw_query); + } + + return null; + } + } diff --git a/src/applications/project/engineextension/ProjectDatasourceEngineExtension.php b/src/applications/project/engineextension/ProjectDatasourceEngineExtension.php index 6500db9b62..b566d58f93 100644 --- a/src/applications/project/engineextension/ProjectDatasourceEngineExtension.php +++ b/src/applications/project/engineextension/ProjectDatasourceEngineExtension.php @@ -8,4 +8,50 @@ final class ProjectDatasourceEngineExtension new PhabricatorProjectDatasource(), ); } + + public function newJumpURI($query) { + $viewer = $this->getViewer(); + + // Send "p" to Projects. + if (preg_match('/^p\z/i', $query)) { + return '/diffusion/'; + } + + // Send "p " to a search for similar projects. + $matches = null; + if (preg_match('/^p\s+(.+)\z/i', $query, $matches)) { + $raw_query = $matches[1]; + + $engine = id(new PhabricatorProject()) + ->newFerretEngine(); + + $compiler = id(new PhutilSearchQueryCompiler()) + ->setEnableFunctions(true); + + $raw_tokens = $compiler->newTokens($raw_query); + + $fulltext_tokens = array(); + foreach ($raw_tokens as $raw_token) { + $fulltext_token = id(new PhabricatorFulltextToken()) + ->setToken($raw_token); + $fulltext_tokens[] = $fulltext_token; + } + + $projects = id(new PhabricatorProjectQuery()) + ->setViewer($viewer) + ->withFerretConstraint($engine, $fulltext_tokens) + ->execute(); + if (count($projects) == 1) { + // Just one match, jump to project. + return head($projects)->getURI(); + } else { + // More than one match, jump to search. + return urisprintf( + '/project/?order=relevance&query=%s#R', + $raw_query); + } + } + + return null; + } } diff --git a/src/applications/search/controller/PhabricatorSearchController.php b/src/applications/search/controller/PhabricatorSearchController.php index a1f6fd68b5..b3d3ab18fa 100644 --- a/src/applications/search/controller/PhabricatorSearchController.php +++ b/src/applications/search/controller/PhabricatorSearchController.php @@ -11,13 +11,15 @@ final class PhabricatorSearchController public function handleRequest(AphrontRequest $request) { $viewer = $this->getViewer(); + $query = $request->getStr('query'); - if ($request->getStr('jump') != 'no') { - $response = PhabricatorJumpNavHandler::getJumpResponse( - $viewer, - $request->getStr('query')); - if ($response) { - return $response; + if ($request->getStr('jump') != 'no' && strlen($query)) { + $jump_uri = id(new PhabricatorDatasourceEngine()) + ->setViewer($viewer) + ->newJumpURI($query); + + if ($jump_uri !== null) { + return id(new AphrontRedirectResponse())->setURI($jump_uri); } } @@ -29,7 +31,7 @@ final class PhabricatorSearchController if ($request->getBool('search:primary')) { // If there's no query, just take the user to advanced search. - if (!strlen($request->getStr('query'))) { + if (!strlen($query)) { $advanced_uri = '/search/query/advanced/'; return id(new AphrontRedirectResponse())->setURI($advanced_uri); } @@ -71,7 +73,7 @@ final class PhabricatorSearchController // Add the user's query, then save this as a new saved query and send // the user to the results page. - $saved->setParameter('query', $request->getStr('query')); + $saved->setParameter('query', $query); $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); try { diff --git a/src/applications/search/engine/PhabricatorDatasourceEngine.php b/src/applications/search/engine/PhabricatorDatasourceEngine.php index 2f8103ce12..62304d26aa 100644 --- a/src/applications/search/engine/PhabricatorDatasourceEngine.php +++ b/src/applications/search/engine/PhabricatorDatasourceEngine.php @@ -2,7 +2,36 @@ final class PhabricatorDatasourceEngine extends Phobject { + private $viewer; + + public function setViewer(PhabricatorUser $viewer) { + $this->viewer = $viewer; + return $this; + } + + public function getViewer() { + return $this->viewer; + } + public function getAllQuickSearchDatasources() { return PhabricatorDatasourceEngineExtension::getAllQuickSearchDatasources(); } + + public function newJumpURI($query) { + $viewer = $this->getViewer(); + $extensions = PhabricatorDatasourceEngineExtension::getAllExtensions(); + + foreach ($extensions as $extension) { + $jump_uri = id(clone $extension) + ->setViewer($viewer) + ->newJumpURI($query); + + if ($jump_uri !== null) { + return $jump_uri; + } + } + + return null; + } + } diff --git a/src/applications/search/engine/PhabricatorJumpNavHandler.php b/src/applications/search/engine/PhabricatorJumpNavHandler.php deleted file mode 100644 index ddade36d84..0000000000 --- a/src/applications/search/engine/PhabricatorJumpNavHandler.php +++ /dev/null @@ -1,135 +0,0 @@ - 'uri:/diffusion/commit/', - '/^f$/i' => 'uri:/feed/', - '/^d$/i' => 'uri:/differential/', - '/^r$/i' => 'uri:/diffusion/', - '/^t$/i' => 'uri:/maniphest/', - '/^p$/i' => 'uri:/project/', - '/^u$/i' => 'uri:/people/', - '/^p\s+(.+)$/i' => 'project', - '/^u\s+(\S+)$/i' => 'user', - '/^(?:s)\s+(\S+)/i' => 'find-symbol', - '/^r\s+(.+)$/i' => 'find-repository', - ); - - foreach ($patterns as $pattern => $effect) { - $matches = null; - if (preg_match($pattern, $jump, $matches)) { - if (!strncmp($effect, 'uri:', 4)) { - return id(new AphrontRedirectResponse()) - ->setURI(substr($effect, 4)); - } else { - switch ($effect) { - case 'user': - return id(new AphrontRedirectResponse()) - ->setURI('/p/'.$matches[1].'/'); - case 'project': - $project = self::findCloselyNamedProject($matches[1]); - if ($project) { - return id(new AphrontRedirectResponse()) - ->setURI('/project/view/'.$project->getID().'/'); - } else { - $jump = $matches[1]; - } - break; - case 'find-symbol': - $context = ''; - $symbol = $matches[1]; - $parts = array(); - if (preg_match('/(.*)(?:\\.|::|->)(.*)/', $symbol, $parts)) { - $context = '&context='.phutil_escape_uri($parts[1]); - $symbol = $parts[2]; - } - return id(new AphrontRedirectResponse()) - ->setURI("/diffusion/symbol/$symbol/?jump=true$context"); - case 'find-repository': - $raw_query = $matches[1]; - - $engine = id(new PhabricatorRepository()) - ->newFerretEngine(); - - $compiler = id(new PhutilSearchQueryCompiler()) - ->setEnableFunctions(true); - - $raw_tokens = $compiler->newTokens($raw_query); - - $fulltext_tokens = array(); - foreach ($raw_tokens as $raw_token) { - $fulltext_token = id(new PhabricatorFulltextToken()) - ->setToken($raw_token); - $fulltext_tokens[] = $fulltext_token; - } - - $repositories = id(new PhabricatorRepositoryQuery()) - ->setViewer($viewer) - ->withFerretConstraint($engine, $fulltext_tokens) - ->execute(); - if (count($repositories) == 1) { - // Just one match, jump to repository. - $uri = head($repositories)->getURI(); - } else { - // More than one match, jump to search. - $uri = urisprintf( - '/diffusion/?order=name&query=%s', - $raw_query); - } - return id(new AphrontRedirectResponse())->setURI($uri); - default: - throw new Exception(pht("Unknown jump effect '%s'!", $effect)); - } - } - } - } - - // If none of the patterns matched, look for an object by name. - $objects = id(new PhabricatorObjectQuery()) - ->setViewer($viewer) - ->withNames(array($jump)) - ->execute(); - - if (count($objects) == 1) { - $handle = id(new PhabricatorHandleQuery()) - ->setViewer($viewer) - ->withPHIDs(mpull($objects, 'getPHID')) - ->executeOne(); - - return id(new AphrontRedirectResponse())->setURI($handle->getURI()); - } - - return null; - } - - private static function findCloselyNamedProject($name) { - $project = id(new PhabricatorProject())->loadOneWhere( - 'name = %s', - $name); - if ($project) { - return $project; - } else { // no exact match, try a fuzzy match - $projects = id(new PhabricatorProject())->loadAllWhere( - 'name LIKE %~', - $name); - if ($projects) { - $min_name_length = PHP_INT_MAX; - $best_project = null; - foreach ($projects as $project) { - $name_length = strlen($project->getName()); - if ($name_length <= $min_name_length) { - $min_name_length = $name_length; - $best_project = $project; - } - } - return $best_project; - } else { - return null; - } - } - } -} diff --git a/src/applications/search/engineextension/PhabricatorDatasourceEngineExtension.php b/src/applications/search/engineextension/PhabricatorDatasourceEngineExtension.php index 50758bc49d..be015526da 100644 --- a/src/applications/search/engineextension/PhabricatorDatasourceEngineExtension.php +++ b/src/applications/search/engineextension/PhabricatorDatasourceEngineExtension.php @@ -2,12 +2,33 @@ abstract class PhabricatorDatasourceEngineExtension extends Phobject { - abstract public function newQuickSearchDatasources(); + private $viewer; - final public static function getAllQuickSearchDatasources() { - $extensions = id(new PhutilClassMapQuery()) + final public function setViewer(PhabricatorUser $viewer) { + $this->viewer = $viewer; + return $this; + } + + final public function getViewer() { + return $this->viewer; + } + + public function newQuickSearchDatasources() { + return array(); + } + + public function newJumpURI($query) { + return null; + } + + final public static function getAllExtensions() { + return id(new PhutilClassMapQuery()) ->setAncestorClass(__CLASS__) ->execute(); + } + + final public static function getAllQuickSearchDatasources() { + $extensions = self::getAllExtensions(); $datasources = array(); foreach ($extensions as $extension) { diff --git a/src/applications/typeahead/engineextension/PhabricatorMonogramDatasourceEngineExtension.php b/src/applications/typeahead/engineextension/PhabricatorMonogramDatasourceEngineExtension.php index 694236cae4..ec34538dd9 100644 --- a/src/applications/typeahead/engineextension/PhabricatorMonogramDatasourceEngineExtension.php +++ b/src/applications/typeahead/engineextension/PhabricatorMonogramDatasourceEngineExtension.php @@ -8,4 +8,46 @@ final class PhabricatorMonogramDatasourceEngineExtension new PhabricatorTypeaheadMonogramDatasource(), ); } + + public function newJumpURI($query) { + $viewer = $this->getViewer(); + + // These first few rules are sort of random but don't fit anywhere else + // today and don't feel worth adding separate extensions for. + + // Send "f" to Feed. + if (preg_match('/^f\z/i', $query)) { + return '/feed/'; + } + + // Send "d" to Differential. + if (preg_match('/^d\z/i', $query)) { + return '/differential/'; + } + + // Send "t" to Maniphest. + if (preg_match('/^t\z/i', $query)) { + return '/maniphest/'; + } + + // Otherwise, if the user entered an object name, jump to that object. + $objects = id(new PhabricatorObjectQuery()) + ->setViewer($viewer) + ->withNames(array($query)) + ->execute(); + if (count($objects) == 1) { + $object = head($objects); + $object_phid = $object->getPHID(); + + $handles = $viewer->loadHandles(array($object_phid)); + $handle = $handles[$object_phid]; + + if ($handle->isComplete()) { + return $handle->getURI(); + } + } + + return null; + } + }