Return no results from grep repository queries on error

Summary: Fixes T7852. Although `1` could also indicate other kinds of problems, assume it means "no results".

Test Plan: Searched for nonsense strings in Git and Mercurial. Searched for valid strings in Git and Mercurial.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T7852

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

src/applications/diffusion/conduit/DiffusionQueryPathsConduitAPIMethod.php

@@ -44,7 +44,6 @@ final class DiffusionQueryPathsConduitAPIMethod
       $commit,
       $path);
 
-
     $lines = id(new LinesOfALargeExecFuture($future))->setDelimiter("\0");
     return $this->filterResults($lines, $request);
   }
@@ -86,16 +85,15 @@ final class DiffusionQueryPathsConduitAPIMethod
     $results = array();
     $count = 0;
     foreach ($lines as $line) {
-      if (!$pattern || preg_match($pattern, $line)) {
-        if ($count >= $offset) {
-          $results[] = $line;
-        }
+      if (strlen($pattern) && !preg_match($pattern, $line)) {
+        continue;
+      }
 
-        $count++;
+      $results[] = $line;
+      $count++;
 
-        if ($limit && ($count >= ($offset + $limit))) {
-          break;
-        }
+      if ($limit && ($count >= ($offset + $limit))) {
+        break;
       }
     }
 

src/applications/diffusion/conduit/DiffusionSearchQueryConduitAPIMethod.php

@@ -25,18 +25,19 @@ final class DiffusionSearchQueryConduitAPIMethod
     );
   }
 
-  protected function defineCustomErrorTypes() {
-    return array(
-      'ERR-GREP-COMMAND' => pht('Grep command failed.'),
-    );
-  }
-
   protected function getResult(ConduitAPIRequest $request) {
     try {
       $results = parent::getResult($request);
     } catch (CommandException $ex) {
-      throw id(new ConduitException('ERR-GREP-COMMAND'))
-        ->setErrorDescription($ex->getStderr());
+      $err = $ex->getError();
+
+      if ($err === 1) {
+        // `git grep` and `hg grep` exit with 1 if there are no matches;
+        // assume we just didn't get any hits.
+        return array();
+      }
+
+      throw $ex;
     }
 
     $offset = $request->getValue('offset');
@@ -63,6 +64,7 @@ final class DiffusionSearchQueryConduitAPIMethod
 
     $binary_pattern = '/Binary file [^:]*:(.+) matches/';
     $lines = new LinesOfALargeExecFuture($future);
+
     foreach ($lines as $line) {
       $result = null;
       if (preg_match('/[^:]*:(.+)\0(.+)\0(.*)/', $line, $result)) {

src/applications/diffusion/controller/DiffusionBrowseController.php

@@ -351,19 +351,16 @@ final class DiffusionBrowseController extends DiffusionController {
   }
 
   private function renderSearchResults() {
+    $request = $this->getRequest();
+
     $drequest = $this->getDiffusionRequest();
     $repository = $drequest->getRepository();
     $results = array();
 
-    $limit = 100;
-    $page = $this->getRequest()->getInt('page', 0);
-    $pager = new PHUIPagerView();
-    $pager->setPageSize($limit);
-    $pager->setOffset($page);
-    $pager->setURI($this->getRequest()->getRequestURI(), 'page');
+    $pager = id(new PHUIPagerView())
+      ->readFromRequest($request);
 
     $search_mode = null;
-
     switch ($repository->getVersionControlSystem()) {
       case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
         $results = array();
@@ -371,32 +368,31 @@ final class DiffusionBrowseController extends DiffusionController {
       default:
         if (strlen($this->getRequest()->getStr('grep'))) {
           $search_mode = 'grep';
-          $query_string = $this->getRequest()->getStr('grep');
+          $query_string = $request->getStr('grep');
           $results = $this->callConduitWithDiffusionRequest(
             'diffusion.searchquery',
             array(
               'grep' => $query_string,
               'commit' => $drequest->getStableCommit(),
               'path' => $drequest->getPath(),
-              'limit' => $limit + 1,
-              'offset' => $page,
+              'limit' => $pager->getPageSize() + 1,
+              'offset' => $pager->getOffset(),
             ));
         } else { // Filename search.
           $search_mode = 'find';
-          $query_string = $this->getRequest()->getStr('find');
+          $query_string = $request->getStr('find');
           $results = $this->callConduitWithDiffusionRequest(
             'diffusion.querypaths',
             array(
               'pattern' => $query_string,
               'commit' => $drequest->getStableCommit(),
               'path' => $drequest->getPath(),
-              'limit' => $limit + 1,
-              'offset' => $page,
+              'limit' => $pager->getPageSize() + 1,
+              'offset' => $pager->getOffset(),
             ));
         }
         break;
     }
-
     $results = $pager->sliceResults($results);
 
     if ($search_mode == 'grep') {