Allow "Track Only" and "Autoclose" to accept regular expressions

Summary: Fixes T2564. See screenshot.

Test Plan:
{F194796}

  - Made a bunch of valid and invalid adjustments here and verified that the branches table showed autoclose state and branches consistent with the settings.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T2564

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

src/applications/diffusion/controller/DiffusionRepositoryEditBranchesController.php

@@ -43,17 +43,26 @@ final class DiffusionRepositoryEditBranchesController
     $edit_uri = $this->getRepositoryControllerURI($repository, 'edit/');
 
     $v_default = $repository->getHumanReadableDetail('default-branch');
-    $v_track = $repository->getHumanReadableDetail(
+    $v_track = $repository->getDetail(
       'branch-filter',
       array());
-    $v_autoclose = $repository->getHumanReadableDetail(
+    $v_track = array_keys($v_track);
+    $v_autoclose = $repository->getDetail(
       'close-commits-filter',
       array());
+    $v_autoclose = array_keys($v_autoclose);
 
+    $e_track = null;
+    $e_autoclose = null;
+
+    $validation_exception = null;
     if ($request->isFormPost()) {
       $v_default = $request->getStr('default');
-      $v_track = $request->getStrList('track');
+
-      $v_autoclose = $request->getStrList('autoclose');
+      $v_track = $this->processBranches($request->getStr('track'));
+      if (!$is_hg) {
+        $v_autoclose = $this->processBranches($request->getStr('autoclose'));
+      }
 
       $xactions = array();
       $template = id(new PhabricatorRepositoryTransaction());
@@ -76,13 +85,20 @@ final class DiffusionRepositoryEditBranchesController
           ->setNewValue($v_autoclose);
       }
 
-      id(new PhabricatorRepositoryEditor())
+      $editor = id(new PhabricatorRepositoryEditor())
         ->setContinueOnNoEffect(true)
         ->setContentSourceFromRequest($request)
-        ->setActor($viewer)
+        ->setActor($viewer);
-        ->applyTransactions($repository, $xactions);
 
+      try {
+        $editor->applyTransactions($repository, $xactions);
         return id(new AphrontRedirectResponse())->setURI($edit_uri);
+      } catch (PhabricatorApplicationTransactionValidationException $ex) {
+        $validation_exception = $ex;
+
+        $e_track = $validation_exception->getShortMessage($type_track);
+        $e_autoclose = $validation_exception->getShortMessage($type_autoclose);
+      }
     }
 
     $content = array();
@@ -97,44 +113,107 @@ final class DiffusionRepositoryEditBranchesController
       ->setObject($repository)
       ->execute();
 
+    $rows = array();
+    $rows[] = array(
+      array(
+        'master',
+      ),
+      pht('Select only master.'),
+    );
+    $rows[] = array(
+      array(
+        'master',
+        'develop',
+        'release',
+      ),
+      pht('Select master, develop, and release.'),
+    );
+    $rows[] = array(
+      array(
+        'master',
+        'regexp(/^release-/)',
+      ),
+      pht('Select master, and all branches which start with "release-".'),
+    );
+    $rows[] = array(
+      array(
+        'regexp(/^(?!temp-)/)',
+      ),
+      pht('Select all branches which do not start with "temp-".'),
+    );
+
+    foreach ($rows as $k => $row) {
+      $rows[$k][0] = phutil_tag(
+        'pre',
+        array(),
+        implode("\n", $row[0]));
+    }
+
+    $example_table = id(new AphrontTableView($rows))
+      ->setHeaders(
+        array(
+          pht('Example'),
+          pht('Effect'),
+        ))
+      ->setColumnClasses(
+        array(
+          '',
+          'wide',
+        ));
+
+    $v_track = implode("\n", $v_track);
+    $v_autoclose = implode("\n", $v_autoclose);
+
     $form = id(new AphrontFormView())
       ->setUser($viewer)
       ->appendRemarkupInstructions(
         pht(
-          'You can choose a **Default Branch** for viewing this repository.'.
+          'You can choose a **Default Branch** for viewing this repository.'))
-          "\n\n".
-          'If you want to import only some branches into Diffusion, you can '.
-          'list them in **Track Only**. Other branches will be ignored. If '.
-          'you do not specify any branches, all branches are tracked.'.
-          "\n\n".
-          'If you have **Autoclose** enabled, Phabricator can close tasks and '.
-          'revisions when corresponding commits are pushed to the repository. '.
-          'If you want to autoclose objects only when commits appear on '.
-          'specific branches, you can list those branches in **Autoclose '.
-          'Only**. By default, all branches autoclose objects.'))
       ->appendChild(
         id(new AphrontFormTextControl())
           ->setName('default')
           ->setLabel(pht('Default Branch'))
-          ->setValue($v_default)
+          ->setValue($v_default))
-          ->setCaption(
+      ->appendRemarkupInstructions(
-            pht('Example: %s', phutil_tag('tt', array(), 'develop'))))
+        pht(
+          'If you want to import only some branches into Diffusion, you can '.
+          'list them in **Track Only**. Other branches will be ignored. If '.
+          'you do not specify any branches, all branches are tracked.'));
+
+    if (!$is_hg) {
+      $form->appendRemarkupInstructions(
+        pht(
+          'If you have **Autoclose** enabled for this repository, Phabricator '.
+          'can close tasks and revisions when corresponding commits are '.
+          'pushed to the repository. If you want to autoclose objects only '.
+          'when commits appear on specific branches, you can list those '.
+          'branches in **Autoclose Only**. By default, all tracked branches '.
+          'will autoclose objects.'));
+    }
+
+    $form
+      ->appendRemarkupInstructions(
+        pht(
+          'When specifying branches, you should enter one branch name per '.
+          'line. You can use regular expressions to match branches by '.
+          'wrapping an expression in `regexp(...)`. For example:'))
       ->appendChild(
-        id(new AphrontFormTextControl())
+        id(new AphrontFormMarkupControl())
+          ->setValue($example_table))
+      ->appendChild(
+        id(new AphrontFormTextAreaControl())
           ->setName('track')
           ->setLabel(pht('Track Only'))
-          ->setValue($v_track)
+          ->setError($e_track)
-          ->setCaption(
+          ->setValue($v_track));
-            pht('Example: %s', phutil_tag('tt', array(), 'master, develop'))));
 
     if (!$is_hg) {
       $form->appendChild(
-        id(new AphrontFormTextControl())
+        id(new AphrontFormTextAreaControl())
           ->setName('autoclose')
           ->setLabel(pht('Autoclose Only'))
-          ->setValue($v_autoclose)
+          ->setError($e_autoclose)
-          ->setCaption(
+          ->setValue($v_autoclose));
-            pht('Example: %s', phutil_tag('tt', array(), 'master, release'))));
     }
 
     $form->appendChild(
@@ -143,6 +222,7 @@ final class DiffusionRepositoryEditBranchesController
         ->addCancelButton($edit_uri));
 
     $form_box = id(new PHUIObjectBoxView())
+      ->setValidationException($validation_exception)
       ->setHeaderText($title)
       ->setForm($form);
 
@@ -156,4 +236,16 @@ final class DiffusionRepositoryEditBranchesController
       ));
   }
 
+  private function processBranches($string) {
+    $lines = phutil_split_lines($string, $retain_endings = false);
+    foreach ($lines as $key => $line) {
+      $lines[$key] = trim($line);
+      if (!strlen($lines[$key])) {
+        unset($lines[$key]);
+      }
+    }
+
+    return array_values($lines);
+  }
+
 }

src/applications/repository/editor/PhabricatorRepositoryEditor.php

@@ -325,6 +325,52 @@ final class PhabricatorRepositoryEditor
     $errors = parent::validateTransaction($object, $type, $xactions);
 
     switch ($type) {
+      case PhabricatorRepositoryTransaction::TYPE_AUTOCLOSE:
+      case PhabricatorRepositoryTransaction::TYPE_TRACK_ONLY:
+        foreach ($xactions as $xaction) {
+          foreach ($xaction->getNewValue() as $pattern) {
+            // Check for invalid regular expressions.
+            $regexp = PhabricatorRepository::extractBranchRegexp($pattern);
+            if ($regexp !== null) {
+              $ok = @preg_match($regexp, '');
+              if ($ok === false) {
+                $error = new PhabricatorApplicationTransactionValidationError(
+                  $type,
+                  pht('Invalid'),
+                  pht(
+                    'Expression "%s" is not a valid regular expression. Note '.
+                    'that you must include delimiters.',
+                    $regexp),
+                  $xaction);
+                $errors[] = $error;
+                continue;
+              }
+            }
+
+            // Check for formatting mistakes like `regex(...)` instead of
+            // `regexp(...)`.
+            $matches = null;
+            if (preg_match('/^([^(]+)\\(.*\\)\z/', $pattern, $matches)) {
+              switch ($matches[1]) {
+                case 'regexp':
+                  break;
+                default:
+                  $error = new PhabricatorApplicationTransactionValidationError(
+                    $type,
+                    pht('Invalid'),
+                    pht(
+                      'Matching function "%s(...)" is not recognized. Valid '.
+                      'functions are: regexp(...).',
+                      $matches[1]),
+                    $xaction);
+                  $errors[] = $error;
+                  break;
+              }
+            }
+          }
+        }
+        break;
+
       case PhabricatorRepositoryTransaction::TYPE_REMOTE_URI:
         foreach ($xactions as $xaction) {
           $new_uri = $xaction->getNewValue();

src/applications/repository/storage/PhabricatorRepository.php

@@ -577,16 +577,47 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
     $is_git = ($vcs == PhabricatorRepositoryType::REPOSITORY_TYPE_GIT);
 
     $use_filter = ($is_git);
+    if (!$use_filter) {
+      // If this VCS doesn't use filters, pass everything through.
+      return true;
+    }
+
 
-    if ($use_filter) {
     $filter = $this->getDetail($filter_key, array());
-      if ($filter && empty($filter[$branch])) {
+
+    // If there's no filter set, let everything through.
+    if (!$filter) {
+      return true;
+    }
+
+    // If this branch isn't literally named `regexp(...)`, and it's in the
+    // filter list, let it through.
+    if (isset($filter[$branch])) {
+      if (self::extractBranchRegexp($branch) === null) {
+        return true;
+      }
+    }
+
+    // If the branch matches a regexp, let it through.
+    foreach ($filter as $pattern => $ignored) {
+      $regexp = self::extractBranchRegexp($pattern);
+      if ($regexp !== null) {
+        if (preg_match($regexp, $branch)) {
+          return true;
+        }
+      }
+    }
+
+    // Nothing matched, so filter this branch out.
     return false;
   }
-    }
 
-    // By default, all branches pass.
+  public static function extractBranchRegexp($pattern) {
-    return true;
+    $matches = null;
+    if (preg_match('/^regexp\\((.*)\\)\z/', $pattern, $matches)) {
+      return $matches[1];
+    }
+    return null;
   }
 
   public function shouldTrackBranch($branch) {