Page MenuHomePhabricator

D10349.diff
No OneTemporary

D10349.diff

diff --git a/src/applications/diffusion/controller/DiffusionRepositoryEditBranchesController.php b/src/applications/diffusion/controller/DiffusionRepositoryEditBranchesController.php
--- a/src/applications/diffusion/controller/DiffusionRepositoryEditBranchesController.php
+++ b/src/applications/diffusion/controller/DiffusionRepositoryEditBranchesController.php
@@ -43,17 +43,26 @@
$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 @@
->setNewValue($v_autoclose);
}
- id(new PhabricatorRepositoryEditor())
+ $editor = id(new PhabricatorRepositoryEditor())
->setContinueOnNoEffect(true)
->setContentSourceFromRequest($request)
- ->setActor($viewer)
- ->applyTransactions($repository, $xactions);
+ ->setActor($viewer);
+
+ try {
+ $editor->applyTransactions($repository, $xactions);
+ return id(new AphrontRedirectResponse())->setURI($edit_uri);
+ } catch (PhabricatorApplicationTransactionValidationException $ex) {
+ $validation_exception = $ex;
- return id(new AphrontRedirectResponse())->setURI($edit_uri);
+ $e_track = $validation_exception->getShortMessage($type_track);
+ $e_autoclose = $validation_exception->getShortMessage($type_autoclose);
+ }
}
$content = array();
@@ -97,44 +113,107 @@
->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.'.
- "\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.'))
+ 'You can choose a **Default Branch** for viewing this repository.'))
->appendChild(
id(new AphrontFormTextControl())
->setName('default')
->setLabel(pht('Default Branch'))
- ->setValue($v_default)
- ->setCaption(
- pht('Example: %s', phutil_tag('tt', array(), 'develop'))))
+ ->setValue($v_default))
+ ->appendRemarkupInstructions(
+ 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)
- ->setCaption(
- pht('Example: %s', phutil_tag('tt', array(), 'master, develop'))));
+ ->setError($e_track)
+ ->setValue($v_track));
if (!$is_hg) {
$form->appendChild(
- id(new AphrontFormTextControl())
+ id(new AphrontFormTextAreaControl())
->setName('autoclose')
->setLabel(pht('Autoclose Only'))
- ->setValue($v_autoclose)
- ->setCaption(
- pht('Example: %s', phutil_tag('tt', array(), 'master, release'))));
+ ->setError($e_autoclose)
+ ->setValue($v_autoclose));
}
$form->appendChild(
@@ -143,6 +222,7 @@
->addCancelButton($edit_uri));
$form_box = id(new PHUIObjectBoxView())
+ ->setValidationException($validation_exception)
->setHeaderText($title)
->setForm($form);
@@ -156,4 +236,16 @@
));
}
+ 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);
+ }
+
}
diff --git a/src/applications/repository/editor/PhabricatorRepositoryEditor.php b/src/applications/repository/editor/PhabricatorRepositoryEditor.php
--- a/src/applications/repository/editor/PhabricatorRepositoryEditor.php
+++ b/src/applications/repository/editor/PhabricatorRepositoryEditor.php
@@ -325,6 +325,52 @@
$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();
diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php
--- a/src/applications/repository/storage/PhabricatorRepository.php
+++ b/src/applications/repository/storage/PhabricatorRepository.php
@@ -577,16 +577,47 @@
$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])) {
- return false;
+
+ $filter = $this->getDetail($filter_key, array());
+
+ // 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;
}
}
- // By default, all branches pass.
- 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;
+ }
+
+ public static function extractBranchRegexp($pattern) {
+ $matches = null;
+ if (preg_match('/^regexp\\((.*)\\)\z/', $pattern, $matches)) {
+ return $matches[1];
+ }
+ return null;
}
public function shouldTrackBranch($branch) {

File Metadata

Mime Type
text/plain
Expires
Thu, May 9, 8:00 AM (3 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6269097
Default Alt Text
D10349.diff (11 KB)

Event Timeline