Render default/current settings in the same format they'll be edited

Summary:
Fixes T4773. For config settings of type `list<string>`, `set`, or `list<regex>`, the "defaults" table and "examples" aren't always in the same format you should actually use when changing the setting.

This is pretty confusing. Instead, always show the settings in the desired format. For example, if the user should enter a newline-separated list, show them a newline separated list.

Test Plan:
  - Grepped for `list<string>`, `list<regex>`, and `'set'`; verified all the config had the right example format (most already did).
  - Viewed config settings of various kinds, including custom settings.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4773

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

src/applications/config/controller/PhabricatorConfigEditController.php

@@ -94,7 +94,14 @@ final class PhabricatorConfigEditController
         }
       }
     } else {
-      $display_value = $this->getDisplayValue($option, $config_entry);
+      if ($config_entry->getIsDeleted()) {
+        $display_value = null;
+      } else {
+        $display_value = $this->getDisplayValue(
+          $option,
+          $config_entry,
+          $config_entry->getValue());
+      }
     }
 
     $form = new AphrontFormView();
@@ -186,7 +193,7 @@ final class PhabricatorConfigEditController
       $form->appendChild(
         id(new AphrontFormMarkupControl())
           ->setLabel(pht('Default'))
-          ->setValue($this->renderDefaults($option)));
+          ->setValue($this->renderDefaults($option, $config_entry)));
     }
 
     $title = pht('Edit %s', $this->key);
@@ -348,17 +355,16 @@ final class PhabricatorConfigEditController
 
   private function getDisplayValue(
     PhabricatorConfigOption $option,
-    PhabricatorConfigEntry $entry) {
+    PhabricatorConfigEntry $entry,
-
+    $value) {
-    if ($entry->getIsDeleted()) {
-      return null;
-    }
 
     if ($option->isCustomType()) {
-      return $option->getCustomObject()->getDisplayValue($option, $entry);
+      return $option->getCustomObject()->getDisplayValue(
+        $option,
+        $entry,
+        $value);
     } else {
       $type = $option->getType();
-      $value = $entry->getValue();
       switch ($type) {
         case 'int':
         case 'string':
@@ -497,7 +503,10 @@ final class PhabricatorConfigEditController
       $table);
   }
 
-  private function renderDefaults(PhabricatorConfigOption $option) {
+  private function renderDefaults(
+    PhabricatorConfigOption $option,
+    PhabricatorConfigEntry $entry) {
+
     $stack = PhabricatorEnv::getConfigSourceStack();
     $stack = $stack->getStack();
 
@@ -515,7 +524,9 @@ final class PhabricatorConfigEditController
       if (!array_key_exists($option->getKey(), $value)) {
         $value = phutil_tag('em', array(), pht('(empty)'));
       } else {
-        $value = PhabricatorConfigJSON::prettyPrintJSON(
+        $value = $this->getDisplayValue(
+          $option,
+          $entry,
           $value[$option->getKey()]);
       }
 

src/applications/config/custom/PhabricatorConfigJSONOptionType.php

@@ -34,18 +34,6 @@ abstract class PhabricatorConfigJSONOptionType
     return array($e_value, $errors, $storage_value, $display_value);
   }
 
-  public function getDisplayValue(
-    PhabricatorConfigOption $option,
-    PhabricatorConfigEntry $entry) {
-    $value = $entry->getValue();
-    if (!$value) {
-      return '';
-    }
-
-    $json = new PhutilJSON();
-    return $json->encodeFormatted($value);
-  }
-
   public function renderControl(
     PhabricatorConfigOption $option,
     $display_value,

src/applications/config/custom/PhabricatorConfigOptionType.php

@@ -20,8 +20,16 @@ abstract class PhabricatorConfigOptionType {
 
   public function getDisplayValue(
     PhabricatorConfigOption $option,
-    PhabricatorConfigEntry $entry) {
+    PhabricatorConfigEntry $entry,
-    return $entry->getValue();
+    $value) {
+
+    if (is_array($value)) {
+      $json = new PhutilJSON();
+      return $json->encodeFormatted($value);
+    } else {
+      return $value;
+    }
+
   }
 
   public function renderControl(

src/applications/config/option/PhabricatorCoreConfigOptions.php

@@ -59,8 +59,8 @@ final class PhabricatorCoreConfigOptions
               "won't work. The major use case for this is moving installs ".
               "across domains."))
         ->addExample(
-          '["http://phabricator2.example.com/", '.
+          "http://phabricator2.example.com/\n".
-            '"http://phabricator3.example.com/"]',
+          "http://phabricator3.example.com/",
           pht('Valid Setting')),
       $this->newOption('phabricator.timezone', 'string', null)
         ->setSummary(

src/applications/config/option/PhabricatorSecurityConfigOptions.php

@@ -124,8 +124,7 @@ final class PhabricatorSecurityConfigOptions
             "automatically linked if the protocol appears in this set. This ".
             "whitelist is primarily to prevent security issues like ".
             "javascript:// URIs."))
-        ->addExample(
+        ->addExample("http\nhttps", pht('Valid Setting'))
-          '{"http": true, "https": true"}', pht('Valid Setting'))
         ->setLocked(true),
       $this->newOption(
         'uri.allowed-editor-protocols',

src/infrastructure/celerity/management/CelerityManagementMapWorkflow.php

@@ -196,7 +196,7 @@ final class CelerityManagementMapWorkflow
    *
    * @param string Resource name.
    * @param string Resource data.
-   * @return pair<string|null, list<string>|nul> The `@provides` symbol and the
+   * @return pair<string|null, list<string>|null> The `@provides` symbol and the
    *    list of `@requires` symbols. If the resource is not part of the
    *    dependency graph, both are null.
    */