Use phutil_tag in Setup issue stuff

Summary: These are pretty straightforward, they just have a fair amount of instructional text with inline markup.

Test Plan: Added and viewed a UIExample.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T2432

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

src/__phutil_library_map__.php

@@ -1254,6 +1254,7 @@ phutil_register_library_map(array(
     'PhabricatorSetupCheckStorage' => 'applications/config/check/PhabricatorSetupCheckStorage.php',
     'PhabricatorSetupCheckTimezone' => 'applications/config/check/PhabricatorSetupCheckTimezone.php',
     'PhabricatorSetupIssue' => 'applications/config/issue/PhabricatorSetupIssue.php',
+    'PhabricatorSetupIssueExample' => 'applications/uiexample/examples/PhabricatorSetupIssueExample.php',
     'PhabricatorSetupIssueView' => 'applications/config/view/PhabricatorSetupIssueView.php',
     'PhabricatorSlowvoteChoice' => 'applications/slowvote/storage/PhabricatorSlowvoteChoice.php',
     'PhabricatorSlowvoteComment' => 'applications/slowvote/storage/PhabricatorSlowvoteComment.php',
@@ -2652,6 +2653,7 @@ phutil_register_library_map(array(
     'PhabricatorSetupCheckPath' => 'PhabricatorSetupCheck',
     'PhabricatorSetupCheckStorage' => 'PhabricatorSetupCheck',
     'PhabricatorSetupCheckTimezone' => 'PhabricatorSetupCheck',
+    'PhabricatorSetupIssueExample' => 'PhabricatorUIExample',
     'PhabricatorSetupIssueView' => 'AphrontView',
     'PhabricatorSlowvoteChoice' => 'PhabricatorSlowvoteDAO',
     'PhabricatorSlowvoteComment' => 'PhabricatorSlowvoteDAO',

src/applications/config/check/PhabricatorSetupCheckBaseURI.php

@@ -59,9 +59,10 @@ final class PhabricatorSetupCheckBaseURI extends PhabricatorSetupCheck {
       ->setSummary($summary)
       ->setMessage($message)
       ->addCommand(
+        hsprintf(
+          '<tt>phabricator/ $</tt> %s',
           csprintf(
-          '<tt>phabricator/ $</tt> '.
             './bin/config set phabricator.base-uri %s',
-          $base_uri_guess));
+            $base_uri_guess)));
   }
 }

src/applications/config/check/PhabricatorSetupCheckDatabase.php

@@ -79,7 +79,7 @@ final class PhabricatorSetupCheckDatabase extends PhabricatorSetupCheck {
         ->setName(pht('Setup MySQL Schema'))
         ->setMessage($message)
         ->setIsFatal(true)
-        ->addCommand('<tt>phabricator/ $</tt> ./bin/storage upgrade');
+        ->addCommand(hsprintf('<tt>phabricator/ $</tt> ./bin/storage upgrade'));
     }
   }
 }

src/applications/config/view/PhabricatorSetupIssueView.php

@@ -16,7 +16,8 @@ final class PhabricatorSetupIssueView extends AphrontView {
   public function render() {
     $issue = $this->getIssue();
 
-    $description = phutil_tag(
+    $description = array();
+    $description[] = phutil_tag(
       'div',
       array(
         'class' => 'setup-issue-instructions',
@@ -25,25 +26,25 @@ final class PhabricatorSetupIssueView extends AphrontView {
 
     $configs = $issue->getPHPConfig();
     if ($configs) {
-      $description .= $this->renderPHPConfig($configs);
+      $description[] = $this->renderPHPConfig($configs);
     }
 
     $configs = $issue->getPhabricatorConfig();
     if ($configs) {
-      $description .= $this->renderPhabricatorConfig($configs);
+      $description[] = $this->renderPhabricatorConfig($configs);
     }
 
     $commands = $issue->getCommands();
     if ($commands) {
       $run_these = pht("Run these %d command(s):", count($commands));
-      $description .= phutil_tag(
+      $description[] = phutil_tag(
         'div',
         array(
           'class' => 'setup-issue-config',
         ),
         array(
-          phutil_render_tag('p', array(), $run_these),
-          phutil_render_tag('pre', array(), implode("\n", $commands)),
+          phutil_tag('p', array(), $run_these),
+          phutil_tag('pre', array(), array_interleave("\n", $commands)),
         ));
     }
 
@@ -52,11 +53,11 @@ final class PhabricatorSetupIssueView extends AphrontView {
       $install_these = pht(
         "Install these %d PHP extension(s):", count($extensions));
 
-      $install_info = pht(
+      $install_info = phutil_safe_html(pht(
         "You can usually install a PHP extension using <tt>apt-get</tt> or ".
         "<tt>yum</tt>. Common package names are ".
         "<tt>php-<em>extname</em></tt> or <tt>php5-<em>extname</em></tt>. ".
-        "Try commands like these:");
+        "Try commands like these:"));
 
       // TODO: We should do a better job of detecting how to install extensions
       // on the current system.
@@ -65,6 +66,7 @@ final class PhabricatorSetupIssueView extends AphrontView {
         "$ sudo yum install php-<em>extname</em>       # Red Hat / Derivatives",
       );
       $install_commands = implode("\n", $install_commands);
+      $install_commands = phutil_safe_html($install_commands);
 
       $fallback_info = pht(
         "If those commands don't work, try Google. The process of installing ".
@@ -75,19 +77,20 @@ final class PhabricatorSetupIssueView extends AphrontView {
       $restart_info = pht(
         "After installing new PHP extensions, <strong>restart your webserver ".
         "for the changes to take effect</strong>.");
+      $restart_info = phutil_safe_html($restart_info);
 
-      $description .= phutil_tag(
+      $description[] = phutil_tag(
         'div',
         array(
           'class' => 'setup-issue-config',
         ),
         array(
-          phutil_render_tag('p', array(), $install_these),
-          phutil_render_tag('pre', array(), implode("\n", $extensions)),
-          phutil_render_tag('p', array(), $install_info),
-          phutil_render_tag('pre', array(), $install_commands),
-          phutil_render_tag('p', array(), $fallback_info),
-          phutil_render_tag('p', array(), $restart_info),
+          phutil_tag('p', array(), $install_these),
+          phutil_tag('pre', array(), implode("\n", $extensions)),
+          phutil_tag('p', array(), $install_info),
+          phutil_tag('pre', array(), $install_commands),
+          phutil_tag('p', array(), $fallback_info),
+          phutil_tag('p', array(), $restart_info),
         ));
 
     }
@@ -111,7 +114,12 @@ final class PhabricatorSetupIssueView extends AphrontView {
       array(
         'class' => 'setup-issue',
       ),
-      array($name, $description, $next));
+      $this->renderHTMLView(
+        array(
+          $name,
+          $description,
+          $next,
+        )));
   }
 
   private function renderPhabricatorConfig(array $configs) {
@@ -124,32 +132,11 @@ final class PhabricatorSetupIssueView extends AphrontView {
         "The current Phabricator configuration has these %d value(s):",
         count($configs)));
 
-    $table = array();
+    $dict = array();
     foreach ($configs as $key) {
-      $table[] = '<tr>';
-      $table[] = '<th>'.phutil_escape_html($key).'</th>';
-
-      $value = PhabricatorEnv::getUnrepairedEnvConfig($key);
-      if ($value === null) {
-        $value = '<em>null</em>';
-      } else if ($value === false) {
-        $value = '<em>false</em>';
-      } else if ($value === true) {
-        $value = '<em>true</em>';
-      } else {
-        $value = phutil_escape_html(
-          PhabricatorConfigJSON::prettyPrintJSON($value));
+      $dict[$key] = PhabricatorEnv::getUnrepairedEnvConfig($key);
     }
-
-      $table[] = '<td>'.$value.'</td>';
-      $table[] = '</tr>';
-    }
-
-    $table = phutil_tag(
-      'table',
-      array(
-      ),
-      new PhutilSafeHTML(implode("\n", $table)));
+    $table = $this->renderValueTable($dict);
 
     $options = PhabricatorApplicationConfigOptions::loadAllOptions();
 
@@ -164,12 +151,11 @@ final class PhabricatorSetupIssueView extends AphrontView {
 
       $update = array();
       foreach ($configs as $key) {
-        $cmd = '<tt>phabricator/ $</tt> ./bin/config set '.
-                phutil_escape_html($key).' '.
-               '<em>value</em>';
-        $update[] = $cmd;
+        $update[] = hsprintf(
+          '<tt>phabricator/ $</tt> ./bin/config set %s <em>value</em>',
+          $key);
       }
-      $update = phutil_render_tag('pre', array(), implode("\n", $update));
+      $update = phutil_tag('pre', array(), array_interleave("\n", $update));
     } else {
       $update = array();
       foreach ($configs as $config) {
@@ -182,11 +168,11 @@ final class PhabricatorSetupIssueView extends AphrontView {
             'href' => '/config/edit/'.$config.'/?issue='.$issue->getIssueKey(),
           ),
           pht('Edit %s', $config));
-        $update[] = '<li>'.$link.'</li>';
+        $update[] = phutil_tag('li', array(), $link);
       }
       if ($update) {
-        $update = '<ul>'.implode("\n", $update).'</ul>';
-        $update_info = phutil_render_tag(
+        $update = phutil_tag('ul', array(), $update);
+        $update_info = phutil_tag(
           'p',
           array(),
           pht("You can update these %d value(s) here:", count($configs)));
@@ -196,12 +182,12 @@ final class PhabricatorSetupIssueView extends AphrontView {
       }
     }
 
-    return phutil_render_tag(
+    return phutil_tag(
       'div',
       array(
         'class' => 'setup-issue-config',
       ),
-      self::renderSingleView(
+      self::renderHTMLView(
         array(
           $table_info,
           $table,
@@ -218,32 +204,12 @@ final class PhabricatorSetupIssueView extends AphrontView {
         "The current PHP configuration has these %d value(s):",
         count($configs)));
 
-    $table = array();
+    $dict = array();
     foreach ($configs as $key) {
-      $table[] = '<tr>';
-      $table[] = '<th>'.phutil_escape_html($key).'</th>';
-
-      $value = ini_get($key);
-      if ($value === null) {
-        $value = '<em>null</em>';
-      } else if ($value === false) {
-        $value = '<em>false</em>';
-      } else if ($value === true) {
-        $value = '<em>true</em>';
-      } else if ($value === '') {
-        $value = '<em>(empty string)</em>';
-      } else {
-        $value = phutil_escape_html($value);
+      $dict[$key] = ini_get($key);
     }
 
-      $table[] = '<td>'.$value.'</td>';
-      $table[] = '</tr>';
-    }
-
-    $table = phutil_tag(
-      'table',
-      array(),
-      new PhutilSafeHTML(implode("\n", $table)));
+    $table = $this->renderValueTable($dict);
 
     ob_start();
       phpinfo();
@@ -270,41 +236,42 @@ final class PhabricatorSetupIssueView extends AphrontView {
       }
     }
 
+    $info = array();
     if (!$ini_loc) {
-      $info = phutil_tag(
+      $info[] = phutil_tag(
         'p',
         array(),
         pht(
           "To update these %d value(s), edit your PHP configuration file.",
           count($configs)));
     } else {
-      $info = phutil_tag(
+      $info[] = phutil_tag(
         'p',
         array(),
         pht(
           "To update these %d value(s), edit your PHP configuration file, ".
           "located here:",
           count($configs)));
-      $info .= phutil_tag(
+      $info[] = phutil_tag(
         'pre',
         array(),
         $ini_loc);
     }
 
     if ($more_loc) {
-      $info .= phutil_tag(
+      $info[] = phutil_tag(
         'p',
         array(),
         pht(
           "PHP also loaded these configuration file(s):",
           count($more_loc)));
-      $info .= phutil_tag(
+      $info[] = phutil_tag(
         'pre',
         array(),
         implode("\n", $more_loc));
     }
 
-    $info .= phutil_tag(
+    $info[] = phutil_tag(
       'p',
       array(),
       phutil_safe_html(pht(
@@ -312,7 +279,7 @@ final class PhabricatorSetupIssueView extends AphrontView {
         '<a href="%s">PHP Documentation</a>.',
         'http://php.net/manual/ini.list.php')));
 
-    $info .= phutil_tag(
+    $info[] = phutil_tag(
       'p',
       array(),
       phutil_safe_html(pht(
@@ -324,7 +291,38 @@ final class PhabricatorSetupIssueView extends AphrontView {
       array(
         'class' => 'setup-issue-config',
       ),
-      array($table_info, $table, $info));
+      $this->renderHTMLView(
+        array(
+          $table_info,
+          $table,
+          $info,
+        )));
+  }
+
+  private function renderValueTable(array $dict) {
+    $rows = array();
+    foreach ($dict as $key => $value) {
+      $cols = array(
+        phutil_tag('th', array(), $key),
+        phutil_tag('td', array(), $this->renderValueForDisplay($value)),
+      );
+      $rows[] = phutil_tag('tr', array(), $cols);
+    }
+    return phutil_tag('table', array(), $rows);
+  }
+
+  private function renderValueForDisplay($value) {
+    if ($value === null) {
+      return phutil_tag('em', array(), 'null');
+    } else if ($value === false) {
+      return phutil_tag('em', array(), 'false');
+    } else if ($value === true) {
+      return phutil_tag('em', array(), 'true');
+    } else if ($value === '') {
+      return phutil_tag('em', array(), 'empty string');
+    } else {
+      return PhabricatorConfigJSON::prettyPrintJSON($value);
+    }
   }
 
 }

src/applications/uiexample/examples/PhabricatorSetupIssueExample.php

@@ -0,0 +1,38 @@
+<?php
+
+final class PhabricatorSetupIssueExample extends PhabricatorUIExample {
+
+  public function getName() {
+    return 'Setup Issue';
+  }
+
+  public function getDescription() {
+    return 'Setup errors and warnings.';
+  }
+
+  public function renderExample() {
+    $request = $this->getRequest();
+    $user = $request->getUser();
+
+    $issue = id(new PhabricatorSetupIssue())
+      ->setShortName(pht('Short Name'))
+      ->setName(pht('Name'))
+      ->setSummary(pht('Summary'))
+      ->setMessage(pht('Message'))
+      ->setIssueKey('example.key')
+      ->addCommand('$ # Add Command')
+      ->addCommand(hsprintf('<tt>$</tt> %s', '$ ls -1 > /dev/null'))
+      ->addPHPConfig('php.config.example')
+      ->addPhabricatorConfig('test.value')
+      ->addPHPExtension('libexample');
+
+    // NOTE: Since setup issues may be rendered before we can build the page
+    // chrome, they don't explicitly include resources.
+    require_celerity_resource('setup-issue-css');
+
+    $view = id(new PhabricatorSetupIssueView())
+      ->setIssue($issue);
+
+    return $view;
+  }
+}