From d43dec1d12883c647d18b9357e96bcefc4f375d3 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 4 Jan 2012 07:35:52 -0800
Subject: [PATCH] Make it harder to miss errors and warnings while developing
 Phabricator

Summary:
If a page generates warnings or errors, you only get a little red dot in
DarkConsole which is hard to see. DarkConsole is also fairly big and there are
plenty of reasons not to leave it open all the time.

Instead, unconditionally show a big message to developers if there are errors or
warnings.

We could make this more sophisticated eventually, but the value is just that you
see it.

Test Plan: Browsed pages with and without warnings, got the right banner state.

Reviewers: nh, btrahan, jungejason

Reviewed By: btrahan

CC: aran, btrahan

Maniphest Tasks: T734

Differential Revision: https://secure.phabricator.com/D1307
---
 conf/default.conf.php                               |  6 +++++-
 conf/development.conf.php                           |  9 +++++----
 .../page/standard/PhabricatorStandardPageView.php   | 13 ++++++++++++-
 src/view/page/standard/__init__.php                 |  1 +
 .../css/application/base/standard-page-view.css     |  9 +++++++++
 5 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/conf/default.conf.php b/conf/default.conf.php
index 8a3003eb43..62ce6b2b35 100644
--- a/conf/default.conf.php
+++ b/conf/default.conf.php
@@ -1,7 +1,7 @@
 <?php
 
 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -427,6 +427,10 @@ return array(
   // 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,
+
   // When users write comments which have URIs, they'll be automaticaly linked
   // if the protocol appears in this set. This whitelist is primarily to prevent
   // security issues like javascript:// URIs.
diff --git a/conf/development.conf.php b/conf/development.conf.php
index 34edf822a1..963e6a88b2 100644
--- a/conf/development.conf.php
+++ b/conf/development.conf.php
@@ -1,7 +1,7 @@
 <?php
 
 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -18,8 +18,9 @@
 
 return array(
 
-  'darkconsole.enabled'           => true,
-  'celerity.force-disk-reads'     => true,
-  'phabricator.show-stack-traces' => true,
+  'darkconsole.enabled'             => true,
+  'celerity.force-disk-reads'       => true,
+  'phabricator.show-stack-traces'   => true,
+  'phabricator.show-error-callout'  => true,
 
 ) + phabricator_read_config_file('default');
diff --git a/src/view/page/standard/PhabricatorStandardPageView.php b/src/view/page/standard/PhabricatorStandardPageView.php
index a68baf7d92..b8859dbf28 100644
--- a/src/view/page/standard/PhabricatorStandardPageView.php
+++ b/src/view/page/standard/PhabricatorStandardPageView.php
@@ -1,7 +1,7 @@
 <?php
 
 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -364,8 +364,19 @@ class PhabricatorStandardPageView extends AphrontPageView {
         '</div>';
     }
 
+    $developer_warning = null;
+    if (PhabricatorEnv::getEnvConfig('phabricator.show-error-callout') &&
+        DarkConsoleErrorLogPluginAPI::getErrors()) {
+      $developer_warning =
+        '<div class="aphront-developer-error-callout">'.
+          'This page raised PHP errors. Find them in DarkConsole '.
+          'or the error log.'.
+        '</div>';
+    }
+
     return
       ($console ? '<darkconsole />' : null).
+      $developer_warning.
       '<div class="phabricator-standard-page '.$admin_class.'">'.
         $header_chrome.
         $this->bodyContent.
diff --git a/src/view/page/standard/__init__.php b/src/view/page/standard/__init__.php
index 6acc29d440..f07d67355c 100644
--- a/src/view/page/standard/__init__.php
+++ b/src/view/page/standard/__init__.php
@@ -6,6 +6,7 @@
 
 
 
+phutil_require_module('phabricator', 'aphront/console/plugin/errorlog/api');
 phutil_require_module('phabricator', 'aphront/request');
 phutil_require_module('phabricator', 'applications/people/storage/preferences');
 phutil_require_module('phabricator', 'infrastructure/celerity/api');
diff --git a/webroot/rsrc/css/application/base/standard-page-view.css b/webroot/rsrc/css/application/base/standard-page-view.css
index 6f117d88dd..fc2235fa10 100644
--- a/webroot/rsrc/css/application/base/standard-page-view.css
+++ b/webroot/rsrc/css/application/base/standard-page-view.css
@@ -179,3 +179,12 @@ td.phabricator-login-details {
   font-family:        "Menlo", "Consolas", "Monaco", monospace;
   font-size:          10px;
 }
+
+.aphront-developer-error-callout {
+  padding: 2em;
+  background: #aa0000;
+  color: white;
+  text-align: center;
+  font-size: 11px;
+  font-family: "Verdana";
+}