Page MenuHomePhabricator
Files F15165838

D18157.diff
No OneTemporary
Actions

D18157.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -3746,6 +3746,7 @@
'PhabricatorRecaptchaConfigOptions' => 'applications/config/option/PhabricatorRecaptchaConfigOptions.php',
'PhabricatorRedirectController' => 'applications/base/controller/PhabricatorRedirectController.php',
'PhabricatorRefreshCSRFController' => 'applications/auth/controller/PhabricatorRefreshCSRFController.php',
+ 'PhabricatorRegexListConfigType' => 'applications/config/type/PhabricatorRegexListConfigType.php',
'PhabricatorRegistrationProfile' => 'applications/people/storage/PhabricatorRegistrationProfile.php',
'PhabricatorReleephApplication' => 'applications/releeph/application/PhabricatorReleephApplication.php',
'PhabricatorReleephApplicationConfigOptions' => 'applications/releeph/config/PhabricatorReleephApplicationConfigOptions.php',
@@ -4071,6 +4072,7 @@
'PhabricatorStorageSchemaSpec' => 'infrastructure/storage/schema/PhabricatorStorageSchemaSpec.php',
'PhabricatorStorageSetupCheck' => 'applications/config/check/PhabricatorStorageSetupCheck.php',
'PhabricatorStringConfigType' => 'applications/config/type/PhabricatorStringConfigType.php',
+ 'PhabricatorStringListConfigType' => 'applications/config/type/PhabricatorStringListConfigType.php',
'PhabricatorStringListEditField' => 'applications/transactions/editfield/PhabricatorStringListEditField.php',
'PhabricatorStringSetting' => 'applications/settings/setting/PhabricatorStringSetting.php',
'PhabricatorSubmitEditField' => 'applications/transactions/editfield/PhabricatorSubmitEditField.php',
@@ -4133,6 +4135,7 @@
'PhabricatorTextAreaEditField' => 'applications/transactions/editfield/PhabricatorTextAreaEditField.php',
'PhabricatorTextConfigType' => 'applications/config/type/PhabricatorTextConfigType.php',
'PhabricatorTextEditField' => 'applications/transactions/editfield/PhabricatorTextEditField.php',
+ 'PhabricatorTextListConfigType' => 'applications/config/type/PhabricatorTextListConfigType.php',
'PhabricatorTime' => 'infrastructure/time/PhabricatorTime.php',
'PhabricatorTimeFormatSetting' => 'applications/settings/setting/PhabricatorTimeFormatSetting.php',
'PhabricatorTimeGuard' => 'infrastructure/time/PhabricatorTimeGuard.php',
@@ -9207,6 +9210,7 @@
'PhabricatorRecaptchaConfigOptions' => 'PhabricatorApplicationConfigOptions',
'PhabricatorRedirectController' => 'PhabricatorController',
'PhabricatorRefreshCSRFController' => 'PhabricatorAuthController',
+ 'PhabricatorRegexListConfigType' => 'PhabricatorTextListConfigType',
'PhabricatorRegistrationProfile' => 'Phobject',
'PhabricatorReleephApplication' => 'PhabricatorApplication',
'PhabricatorReleephApplicationConfigOptions' => 'PhabricatorApplicationConfigOptions',
@@ -9609,6 +9613,7 @@
'PhabricatorStorageSchemaSpec' => 'PhabricatorConfigSchemaSpec',
'PhabricatorStorageSetupCheck' => 'PhabricatorSetupCheck',
'PhabricatorStringConfigType' => 'PhabricatorTextConfigType',
+ 'PhabricatorStringListConfigType' => 'PhabricatorTextListConfigType',
'PhabricatorStringListEditField' => 'PhabricatorEditField',
'PhabricatorStringSetting' => 'PhabricatorSetting',
'PhabricatorSubmitEditField' => 'PhabricatorEditField',
@@ -9670,6 +9675,7 @@
'PhabricatorTextAreaEditField' => 'PhabricatorEditField',
'PhabricatorTextConfigType' => 'PhabricatorConfigType',
'PhabricatorTextEditField' => 'PhabricatorEditField',
+ 'PhabricatorTextListConfigType' => 'PhabricatorTextConfigType',
'PhabricatorTime' => 'Phobject',
'PhabricatorTimeFormatSetting' => 'PhabricatorSelectSetting',
'PhabricatorTimeGuard' => 'Phobject',
diff --git a/src/applications/config/controller/PhabricatorConfigEditController.php b/src/applications/config/controller/PhabricatorConfigEditController.php
--- a/src/applications/config/controller/PhabricatorConfigEditController.php
+++ b/src/applications/config/controller/PhabricatorConfigEditController.php
@@ -347,20 +347,6 @@
$set_value = null;
switch ($type) {
- case 'list<string>':
- case 'list<regex>':
- $set_value = phutil_split_lines(
- $request->getStr('value'),
- $retain_endings = false);
-
- foreach ($set_value as $key => $v) {
- if (!strlen($v)) {
- unset($set_value[$key]);
- }
- }
- $set_value = array_values($set_value);
-
- break;
case 'set':
$set_value = array_fill_keys($request->getStrList('value'), true);
break;
@@ -441,9 +427,6 @@
return $value;
case 'bool':
return $value ? 'true' : 'false';
- case 'list<string>':
- case 'list<regex>':
- return implode("\n", nonempty($value, array()));
case 'set':
return implode("\n", nonempty(array_keys($value), array()));
default:
@@ -497,11 +480,6 @@
$control = id(new AphrontFormSelectControl())
->setOptions($names);
break;
- case 'list<string>':
- case 'list<regex>':
- $control = id(new AphrontFormTextAreaControl())
- ->setCaption(pht('Separate values with newlines.'));
- break;
case 'set':
$control = id(new AphrontFormTextAreaControl())
->setCaption(pht('Separate values with newlines or commas.'));
diff --git a/src/applications/config/management/PhabricatorConfigManagementSetWorkflow.php b/src/applications/config/management/PhabricatorConfigManagementSetWorkflow.php
--- a/src/applications/config/management/PhabricatorConfigManagementSetWorkflow.php
+++ b/src/applications/config/management/PhabricatorConfigManagementSetWorkflow.php
@@ -65,6 +65,7 @@
$value = $type->newValueFromCommandLineValue(
$option,
$value);
+ $type->validateStoredValue($option, $value);
} catch (PhabricatorConfigValidationException $ex) {
throw new PhutilArgumentUsageException($ex->getMessage());
}
@@ -109,26 +110,10 @@
$command);
break;
default:
- if (preg_match('/^list</', $type)) {
- $command = csprintf(
- './bin/config set %R %s',
- $key,
- '["a", "b", "c"]');
-
- $message = sprintf(
- "%s\n\n %s\n",
- pht(
- 'Config key "%s" is of type "%s". Specify it in JSON. '.
- 'For example:',
- $key,
- $type),
- $command);
- } else {
- $message = pht(
- 'Config key "%s" is of type "%s". Specify it in JSON.',
- $key,
- $type);
- }
+ $message = pht(
+ 'Config key "%s" is of type "%s". Specify it in JSON.',
+ $key,
+ $type);
break;
}
throw new PhutilArgumentUsageException($message);
diff --git a/src/applications/config/option/PhabricatorApplicationConfigOptions.php b/src/applications/config/option/PhabricatorApplicationConfigOptions.php
--- a/src/applications/config/option/PhabricatorApplicationConfigOptions.php
+++ b/src/applications/config/option/PhabricatorApplicationConfigOptions.php
@@ -85,60 +85,6 @@
}
}
break;
- case 'list<regex>':
- $valid = true;
- if (!is_array($value)) {
- throw new PhabricatorConfigValidationException(
- pht(
- "Option '%s' must be a list of regular expressions, but value ".
- "is not an array.",
- $option->getKey()));
- }
- if ($value && array_keys($value) != range(0, count($value) - 1)) {
- throw new PhabricatorConfigValidationException(
- pht(
- "Option '%s' must be a list of regular expressions, but the ".
- "value is a map with unnatural keys.",
- $option->getKey()));
- }
- foreach ($value as $v) {
- $ok = @preg_match($v, '');
- if ($ok === false) {
- throw new PhabricatorConfigValidationException(
- pht(
- "Option '%s' must be a list of regular expressions, but the ".
- "value '%s' is not a valid regular expression.",
- $option->getKey(),
- $v));
- }
- }
- break;
- case 'list<string>':
- $valid = true;
- if (!is_array($value)) {
- throw new PhabricatorConfigValidationException(
- pht(
- "Option '%s' must be a list of strings, but value is not ".
- "an array.",
- $option->getKey()));
- }
- if ($value && array_keys($value) != range(0, count($value) - 1)) {
- throw new PhabricatorConfigValidationException(
- pht(
- "Option '%s' must be a list of strings, but the value is a ".
- "map with unnatural keys.",
- $option->getKey()));
- }
- foreach ($value as $v) {
- if (!is_string($v)) {
- throw new PhabricatorConfigValidationException(
- pht(
- "Option '%s' must be a list of strings, but it contains one ".
- "or more non-strings.",
- $option->getKey()));
- }
- }
- break;
case 'wild':
default:
break;
diff --git a/src/applications/config/type/PhabricatorRegexListConfigType.php b/src/applications/config/type/PhabricatorRegexListConfigType.php
new file mode 100644
--- /dev/null
+++ b/src/applications/config/type/PhabricatorRegexListConfigType.php
@@ -0,0 +1,24 @@
+<?php
+
+final class PhabricatorRegexListConfigType
+ extends PhabricatorTextListConfigType {
+
+ const TYPEKEY = 'list<regex>';
+
+ protected function validateStoredItem(
+ PhabricatorConfigOption $option,
+ $value) {
+
+ $ok = @preg_match($value, '');
+ if ($ok === false) {
+ throw $this->newException(
+ pht(
+ 'Option "%s" is of type "%s" and must be set to a list of valid '.
+ 'regular expressions, but "%s" is not a valid regular expression.',
+ $option->getKey(),
+ $this->getTypeKey(),
+ $value));
+ }
+ }
+
+}
diff --git a/src/applications/config/type/PhabricatorStringListConfigType.php b/src/applications/config/type/PhabricatorStringListConfigType.php
new file mode 100644
--- /dev/null
+++ b/src/applications/config/type/PhabricatorStringListConfigType.php
@@ -0,0 +1,8 @@
+<?php
+
+final class PhabricatorStringListConfigType
+ extends PhabricatorTextListConfigType {
+
+ const TYPEKEY = 'list<string>';
+
+}
diff --git a/src/applications/config/type/PhabricatorTextListConfigType.php b/src/applications/config/type/PhabricatorTextListConfigType.php
new file mode 100644
--- /dev/null
+++ b/src/applications/config/type/PhabricatorTextListConfigType.php
@@ -0,0 +1,98 @@
+<?php
+
+abstract class PhabricatorTextListConfigType
+ extends PhabricatorTextConfigType {
+
+ protected function newControl(PhabricatorConfigOption $option) {
+ return id(new AphrontFormTextAreaControl())
+ ->setCaption(pht('Separate values with newlines.'));
+ }
+
+ protected function newCanonicalValue(
+ PhabricatorConfigOption $option,
+ $value) {
+
+ $value = phutil_split_lines($value, $retain_endings = false);
+ foreach ($value as $k => $v) {
+ if (!strlen($v)) {
+ unset($value[$k]);
+ }
+ }
+
+ return array_values($value);
+ }
+
+ public function newValueFromCommandLineValue(
+ PhabricatorConfigOption $option,
+ $value) {
+
+ try {
+ $value = phutil_json_decode($value);
+ } catch (Exception $ex) {
+ throw $this->newException(
+ pht(
+ 'Option "%s" is of type "%s", but the value you provided is not a '.
+ 'valid JSON list. When setting a list option from the command '.
+ 'line, specify the value in JSON. You may need to quote the '.
+ 'value for your shell (for example: \'["a", "b", ...]\').',
+ $option->getKey(),
+ $this->getTypeKey()));
+ }
+
+ return $value;
+ }
+
+ public function newDisplayValue(
+ PhabricatorConfigOption $option,
+ $value) {
+ return implode("\n", $value);
+ }
+
+ public function validateStoredValue(
+ PhabricatorConfigOption $option,
+ $value) {
+
+ if (!is_array($value)) {
+ throw $this->newException(
+ pht(
+ 'Option "%s" is of type "%s", but the configured value is not '.
+ 'a list.',
+ $option->getKey(),
+ $this->getTypeKey()));
+ }
+
+ $expect_key = 0;
+ foreach ($value as $k => $v) {
+ if (!is_string($v)) {
+ throw $this->newException(
+ pht(
+ 'Option "%s" is of type "%s", but the item at index "%s" of the '.
+ 'list is not a string.',
+ $option->getKey(),
+ $this->getTypeKey(),
+ $k));
+ }
+
+ // Make sure this is a list with keys "0, 1, 2, ...", not a map with
+ // arbitrary keys.
+ if ($k != $expect_key) {
+ throw $this->newException(
+ pht(
+ 'Option "%s" is of type "%s", but the value is not a list: it '.
+ 'is a map with unnatural or sparse keys.',
+ $option->getKey(),
+ $this->getTypeKey()));
+ }
+ $expect_key++;
+
+ $this->validateStoredItem($option, $v);
+ }
+ }
+
+ protected function validateStoredItem(
+ PhabricatorConfigOption $option,
+ $value) {
+ return;
+ }
+
+}

File Metadata

Mime Type
text/plain
Expires
Wed, Feb 19, 4:09 PM (21 h, 12 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7170160
Default Alt Text
D18157.diff (13 KB)

Event Timeline

Phabricator · Built by Phacility