Refactor developer options to specific developer-mode option.

Summary: Refactor options related to verbose error reporting and forcing disk reads into a single developer option.

Test Plan: Run Phabricator with the developer-mode option set and check that errors print stack traces, static assets are always reloaded, etc.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

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

conf/default.conf.php

@@ -798,13 +798,11 @@ return array(
   // behalf, silencing the warning.
   'phabricator.timezone'        => null,
 
-  // When unhandled exceptions occur, stack traces are hidden by default.
-  // You can enable traces for development to make it easier to debug problems.
-  'phabricator.show-stack-traces' => false,
-
-  // Shows an error callout if a page generated PHP errors, warnings or notices.
-  // This makes it harder to miss problems while developing Phabricator.
-  'phabricator.show-error-callout' => false,
+  // Show stack traces when unhandled exceptions occur, force reloading of
+  // static resources (skipping the cache), show an error callout if a page
+  // generated PHP errors, warnings, or notices, force disk reads when
+  // reloading. This option should not be enabled in production.
+  'phabricator.developer-mode' => false,
 
   // When users write comments which have URIs, they'll be automatically linked
   // if the protocol appears in this set. This whitelist is primarily to prevent
@@ -1220,15 +1218,6 @@ return array(
   // unlikely that you need to modify this.
   'celerity.resource-hash' => 'd9455ea150622ee044f7931dabfa52aa',
 
-  // In a development environment, it is desirable to force static resources
-  // (CSS and JS) to be read from disk on every request, so that edits to them
-  // appear when you reload the page even if you haven't updated the resource
-  // maps. This setting ensures requests will be verified against the state on
-  // disk. Generally, you should leave this off in production (caching behavior
-  // and performance improve with it off) but turn it on in development. (These
-  // settings are the defaults.)
-  'celerity.force-disk-reads' => false,
-
   // Minify static resources by removing whitespace and comments. You should
   // enable this in production, but disable it in development.
   'celerity.minify' => false,

conf/development.conf.php

@@ -2,10 +2,7 @@
 
 return array(
 
+  'phabricator.developer-mode'      => true,
   'darkconsole.enabled'             => true,
-  'celerity.force-disk-reads'       => true,
-  'phabricator.show-stack-traces'   => true,
-  'phabricator.show-error-callout'  => true,
-  'celerity.minify'                 => false,
 
 ) + phabricator_read_config_file('default');

src/aphront/AphrontRequest.php

@@ -228,6 +228,26 @@ final class AphrontRequest {
         $more_info = "(This was a web request, {$token_info}.)";
       }
 
+      // Give a more detailed explanation of how to avoid the exception
+      // in developer mode.
+      if (PhabricatorEnv::getEnvConfig('phabricator.developer-mode')) {
+        $more_info = $more_info .
+          "To avoid this error, use phabricator_form() to construct forms. " .
+          "If you are already using phabricator_form(), make sure the form " .
+          "'action' uses a relative URI (i.e., begins with a '/'). Forms " .
+          "using absolute URIs do not include CSRF tokens, to prevent " .
+          "leaking tokens to external sites.\n\n" .
+          "If this page performs writes which do not require CSRF " .
+          "protection (usually, filling caches or logging), you can use " .
+          "AphrontWriteGuard::beginScopedUnguardedWrites() to temporarily " .
+          "bypass CSRF protection while writing. You should use this only " .
+          "for writes which can not be protected with normal CSRF " .
+          "mechanisms.\n\n" .
+          "Some UI elements (like PhabricatorActionListView) also have " .
+          "methods which will allow you to render links as forms (like " .
+          "setRenderAsForm(true)).";
+      }
+
       // This should only be able to happen if you load a form, pull your
       // internet for 6 hours, and then reconnect and immediately submit,
       // but give the user some indication of what happened since the workflow

src/aphront/configuration/AphrontDefaultApplicationConfiguration.php

@@ -235,7 +235,7 @@ class AphrontDefaultApplicationConfiguration
         "schema is up to date.";
     }
 
-    if (PhabricatorEnv::getEnvConfig('phabricator.show-stack-traces')) {
+    if (PhabricatorEnv::getEnvConfig('phabricator.developer-mode')) {
       $trace = $this->renderStackTrace($ex->getTrace(), $user);
     } else {
       $trace = null;

src/applications/config/option/PhabricatorDeveloperConfigOptions.php

@@ -86,48 +86,18 @@ final class PhabricatorDeveloperConfigOptions
             "data to look at eventually). In development, it may be useful to ".
             "set it to 1 in order to debug performance problems.\n\n".
             "NOTE: You must install XHProf for profiling to work.")),
-      $this->newOption('phabricator.show-stack-traces', 'bool', false)
+      $this->newOption('phabricator.developer-mode', 'bool', false)
         ->setBoolOptions(
           array(
-            pht('Show stack traces'),
-            pht('Hide stack traces'),
+            pht('Enable developer mode'),
+            pht('Disable developer mode'),
           ))
-        ->setSummary(pht("Show stack traces when unhandled exceptions occur."))
-        ->setDescription(
-          pht(
-            "When unhandled exceptions occur, stack traces are hidden by ".
-            "default. You can enable traces for development to make it easier ".
-            "to debug problems.")),
-      $this->newOption('phabricator.show-error-callout', 'bool', false)
-        ->setBoolOptions(
-          array(
-            pht('Show error callout'),
-            pht('Hide error callout'),
-          ))
-        ->setSummary(pht("Show error callout."))
-        ->setDescription(
-          pht(
-            "Shows an error callout if a page generated PHP errors, warnings ".
-            "or notices. This makes it harder to miss problems while ".
-            "developing Phabricator. A callout is simply a red error at the ".
-            "top of the page.")),
-      $this->newOption('celerity.force-disk-reads', 'bool', false)
-        ->setBoolOptions(
-          array(
-            pht('Force disk reads'),
-            pht("Don't force disk reads"),
-          ))
-        ->setSummary(pht("Force Celerity to read from disk on every request."))
-        ->setDescription(
-          pht(
-            "In a development environment, it is desirable to force static ".
-            "resources (CSS and JS) to be read from disk on every request, so ".
-            "that edits to them appear when you reload the page even if you ".
-            "haven't updated the resource maps. This setting ensures requests ".
-            "will be verified against the state on disk. Generally, you ".
-            "should leave this off in production (caching behavior and ".
-            "performance improve with it off) but turn it on in development. ".
-            "(These settings are the defaults.)")),
+          ->setSummary(pht("Enable verbose error reporting and disk reads."))
+          ->setDescription(
+            pht(
+              "This option enables verbose error reporting (stack traces, ".
+              "error callouts) and forces disk reads of static assets on ".
+              "every reload.")),
       $this->newOption('celerity.minify', 'bool', false)
         ->setBoolOptions(
           array(

src/infrastructure/celerity/CelerityPhabricatorResourceController.php

@@ -35,7 +35,9 @@ final class CelerityPhabricatorResourceController
 
   protected function buildResourceTransformer() {
     $xformer = new CelerityResourceTransformer();
-    $xformer->setMinify(PhabricatorEnv::getEnvConfig('celerity.minify'));
+    $xformer->setMinify(
+      !PhabricatorEnv::getEnvConfig('phabricator.developer-mode') &&
+      PhabricatorEnv::getEnvConfig('celerity.minify'));
     $xformer->setCelerityMap(CelerityResourceMap::getInstance());
     return $xformer;
   }

src/infrastructure/celerity/CelerityResourceController.php

@@ -35,7 +35,7 @@ abstract class CelerityResourceController extends PhabricatorController {
     }
 
     if (isset($_SERVER['HTTP_IF_MODIFIED_SINCE']) &&
-        !PhabricatorEnv::getEnvConfig('celerity.force-disk-reads')) {
+        !PhabricatorEnv::getEnvConfig('phabricator.developer-mode')) {
       // Return a "304 Not Modified". We don't care about the value of this
       // field since we never change what resource is served by a given URI.
       return $this->makeResponseCacheable(new Aphront304Response());

src/view/page/PhabricatorBarePageView.php

@@ -83,7 +83,9 @@ class PhabricatorBarePageView extends AphrontPageView {
 
       '<script type="text/javascript">'.
         $framebust.
-        'window.__DEV__=1;'.
+        'window.__DEV__='.
+        PhabricatorEnv::getEnvConfig('phabricator.developer-mode') ? '1' : '0'.
+        ';'.
       '</script>',
 
       $response->renderResourcesOfType('css'),

src/view/page/PhabricatorStandardPageView.php

@@ -234,7 +234,7 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView {
     }
 
     $developer_warning = null;
-    if (PhabricatorEnv::getEnvConfig('phabricator.show-error-callout') &&
+    if (PhabricatorEnv::getEnvConfig('phabricator.developer-mode') &&
         DarkConsoleErrorLogPluginAPI::getErrors()) {
       $developer_warning =
         '<div class="aphront-developer-error-callout">'.