Include symbols in main typeahead

Summary:
  - Include symbols in main typeahead results.
  - Simplify the symbol query a bit and extend PhabricatorOffsetPagedQuery. There was some stuff around language ranking that I got rid of, I think the theory there was that mapping file extensions to languages might not work in general but I think it works fine in practice, and we have more config stuff now around guessing languages and getting the mappings right.
  - Make it easier to debug the typeahead by showing the results in page format for non-ajax requests.
  - When we have too many results, show only the top few of each type.

Depends on D3116, D3117

Test Plan: Used typeahead, got symbols in results. Hit endpoint with non-ajax, got useful debug view.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1569

Differential Revision: https://secure.phabricator.com/D3118

src/__celerity_resource_map__.php

@@ -1462,7 +1462,7 @@ celerity_register_resource_map(array(
   ),
   'javelin-behavior-phabricator-search-typeahead' =>
   array(
-    'uri' => '/res/f552b264/rsrc/js/application/core/behavior-search-typeahead.js',
+    'uri' => '/res/046ab274/rsrc/js/application/core/behavior-search-typeahead.js',
     'type' => 'js',
     'requires' =>
     array(

src/__phutil_library_map__.php

@@ -1477,6 +1477,7 @@ phutil_register_library_map(array(
     'DiffusionSvnRequest' => 'DiffusionRequest',
     'DiffusionSvnTagListQuery' => 'DiffusionTagListQuery',
     'DiffusionSymbolController' => 'DiffusionController',
+    'DiffusionSymbolQuery' => 'PhabricatorOffsetPagedQuery',
     'DiffusionTagListController' => 'DiffusionController',
     'DiffusionTagListQuery' => 'DiffusionQuery',
     'DiffusionTagListView' => 'DiffusionView',

src/applications/diffusion/query/DiffusionSymbolQuery.php

@@ -27,7 +27,7 @@
  *
  * @group diffusion
  */
-final class DiffusionSymbolQuery {
+final class DiffusionSymbolQuery extends PhabricatorOffsetPagedQuery {
 
   private $namePrefix;
   private $name;
@@ -36,8 +36,6 @@ final class DiffusionSymbolQuery {
   private $language;
   private $type;
 
-  private $limit = 20;
-
   private $needPaths;
   private $needArcanistProject;
   private $needRepositories;
@@ -91,15 +89,6 @@ final class DiffusionSymbolQuery {
   }
 
 
-  /**
-   * @task config
-   */
-  public function setLimit($limit) {
-    $this->limit = $limit;
-    return $this;
-  }
-
-
   /**
    * @task config
    */
@@ -145,55 +134,13 @@ final class DiffusionSymbolQuery {
     $symbol = new PhabricatorRepositorySymbol();
     $conn_r = $symbol->establishConnection('r');
 
-    $where = array();
-    if ($this->name) {
-      $where[] = qsprintf(
-        $conn_r,
-        'symbolName = %s',
-        $this->name);
-    }
-
-    if ($this->namePrefix) {
-      $where[] = qsprintf(
-        $conn_r,
-        'symbolName LIKE %>',
-        $this->namePrefix);
-    }
-
-    if ($this->projectIDs) {
-      $where[] = qsprintf(
-        $conn_r,
-        'arcanistProjectID IN (%Ld)',
-        $this->projectIDs);
-    }
-
-    $where = 'WHERE ('.implode(') AND (', $where).')';
-
     $data = queryfx_all(
       $conn_r,
-      'SELECT * FROM %T %Q',
+      'SELECT * FROM %T %Q %Q %Q',
       $symbol->getTableName(),
-      $where);
-
-    // Our ability to match up symbol types and languages probably isn't all
-    // that great, so use them as hints for ranking rather than hard
-    // requirements. TODO: Is this really the right choice?
-    foreach ($data as $key => $row) {
-      $score = 0;
-      if ($this->language && $row['symbolLanguage'] == $this->language) {
-        $score += 2;
-      }
-      if ($this->type && $row['symbolType'] == $this->type) {
-        $score += 1;
-      }
-      $data[$key]['score'] = $score;
-      $data[$key]['id'] = $key;
-    }
-
-    $data = isort($data, 'score');
-    $data = array_reverse($data);
-
-    $data = array_slice($data, 0, $this->limit);
+      $this->buildWhereClause($conn_r),
+      $this->buildOrderClause($conn_r),
+      $this->buildLimitClause($conn_r));
 
     $symbols = $symbol->loadAllFromArray($data);
 
@@ -217,6 +164,54 @@ final class DiffusionSymbolQuery {
 /* -(  Internals  )---------------------------------------------------------- */
 
 
+  /**
+   * @task internal
+   */
+  private function buildOrderClause($conn_r) {
+    return qsprintf(
+      $conn_r,
+      'ORDER BY symbolName ASC');
+  }
+
+
+  /**
+   * @task internal
+   */
+  private function buildWhereClause($conn_r) {
+    $where = array();
+
+    if ($this->name) {
+      $where[] = qsprintf(
+        $conn_r,
+        'symbolName = %s',
+        $this->name);
+    }
+
+    if ($this->namePrefix) {
+      $where[] = qsprintf(
+        $conn_r,
+        'symbolName LIKE %>',
+        $this->namePrefix);
+    }
+
+    if ($this->projectIDs) {
+      $where[] = qsprintf(
+        $conn_r,
+        'arcanistProjectID IN (%Ld)',
+        $this->projectIDs);
+    }
+
+    if ($this->language) {
+      $where[] = qsprintf(
+        $conn_r,
+        'symbolLanguage = %s',
+        $this->language);
+    }
+
+    return $this->formatWhereClause($where);
+  }
+
+
   /**
    * @task internal
    */

src/applications/typeahead/controller/PhabricatorTypeaheadCommonDatasourceController.php

@@ -40,11 +40,13 @@ final class PhabricatorTypeaheadCommonDatasourceController
     $need_upforgrabs = false;
     $need_arcanist_projects = false;
     $need_noproject = false;
+    $need_symbols = false;
     switch ($this->type) {
       case 'mainsearch':
         $need_users = true;
         $need_applications = true;
         $need_rich_data = true;
+        $need_symbols = true;
         break;
       case 'searchowner':
         $need_users = true;
@@ -238,9 +240,70 @@ final class PhabricatorTypeaheadCommonDatasourceController
       }
     }
 
+    if ($need_symbols) {
+      $symbols = id(new DiffusionSymbolQuery())
+        ->setNamePrefix($query)
+        ->setLimit(15)
+        ->needArcanistProjects(true)
+        ->needRepositories(true)
+        ->needPaths(true)
+        ->execute();
+      foreach ($symbols as $symbol) {
+        $lang = $symbol->getSymbolLanguage();
+        $name = $symbol->getSymbolName();
+        $type = $symbol->getSymbolType();
+        $proj = $symbol->getArcanistProject()->getName();
+
+        $results[] = id(new PhabricatorTypeaheadResult())
+          ->setName($name)
+          ->setURI($symbol->getURI())
+          ->setPHID(md5($symbol->getURI())) // Just needs to be unique.
+          ->setDisplayName($symbol->getName())
+          ->setDisplayType(strtoupper($lang).' '.ucwords($type).' ('.$proj.')')
+          ->setPriorityType('symb');
+      }
+    }
+
     $content = mpull($results, 'getWireFormat');
 
+    if ($request->isAjax()) {
       return id(new AphrontAjaxResponse())->setContent($content);
     }
 
+    // If there's a non-Ajax request to this endpoint, show results in a tabular
+    // format to make it easier to debug typeahead output.
+
+    $rows = array();
+    foreach ($results as $result) {
+      $wire = $result->getWireFormat();
+      foreach ($wire as $k => $v) {
+        $wire[$k] = phutil_escape_html($v);
+      }
+      $rows[] = $wire;
+    }
+
+    $table = new AphrontTableView($rows);
+    $table->setHeaders(
+      array(
+        'Name',
+        'URI',
+        'PHID',
+        'Priority',
+        'Display Name',
+        'Display Type',
+        'Image URI',
+        'Priority Type',
+      ));
+
+    $panel = new AphrontPanelView();
+    $panel->setHeader('Typeahead Results');
+    $panel->appendChild($table);
+
+    return $this->buildStandardPageResponse(
+      $panel,
+      array(
+        'title' => 'Typeahead Results',
+      ));
+  }
+
 }

src/applications/typeahead/storage/PhabricatorTypeaheadResult.php

@@ -75,7 +75,7 @@ final class PhabricatorTypeaheadResult {
       $this->priorityString,
       $this->displayName,
       $this->displayType,
-      $this->imageURI,
+      $this->imageURI ? (string)$this->imageURI : null,
       $this->priorityType,
     );
     while (end($data) === null) {

webroot/rsrc/js/application/core/behavior-search-typeahead.js

@@ -53,7 +53,8 @@ JX.behavior('phabricator-search-typeahead', function(config) {
     var type_priority = {
       // TODO: Put jump nav hits like "D123" first.
       'apps' : 2,
-      'user' : 3
+      'user' : 3,
+      'symb' : 4
     };
 
     var tokens = this.tokenize(value);
@@ -85,9 +86,31 @@ JX.behavior('phabricator-search-typeahead', function(config) {
 
       return cmp(u, v);
     });
+
+    // If we have more results than fit, limit each type of result to 3, so
+    // we show 3 applications, then 3 users, etc.
+    var type_count = 0;
+    var current_type = null;
+    for (var ii = 0; ii < list.length; ii++) {
+      if (list.length <= config.limit) {
+        break;
+      }
+      if (list[ii].type != current_type) {
+        current_type = list[ii].type;
+        type_count = 1;
+      } else {
+        type_count++;
+        if (type_count > 3) {
+          list.splice(ii, 1);
+          ii--;
+        }
+      }
+    }
+
   };
 
   datasource.setSortHandler(JX.bind(datasource, sort_handler));
+  datasource.setMaximumResultCount(config.limit);
 
   var typeahead = new JX.Typeahead(JX.$(config.id), JX.$(config.input));
   typeahead.setDatasource(datasource);