Replace some hsprintf() with phutil_tag() and phutil_tag_div() Depends on D7545.

Test Plan: This is one of the rare moments where unit tests for views would be useful.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, epriestley, aran

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

src/aphront/response/AphrontRedirectResponse.php

@@ -42,18 +42,20 @@ class AphrontRedirectResponse extends AphrontResponse {
       $error->setSeverity(AphrontErrorView::SEVERITY_NOTICE);
       $error->setTitle('Stopped on Redirect');
 
+      $error->appendChild(phutil_tag('p', array(), pht(
+        'You were stopped here because %s is set in your configuration.',
+        phutil_tag('tt', array(), 'debug.stop-on-redirect'))));
+
       $link = phutil_tag(
         'a',
         array(
           'href' => $this->getURI(),
         ),
-        'Continue to: '.$this->getURI());
+        $this->getURI());
 
-      $error->appendChild(hsprintf(
+      $error->appendChild(phutil_tag('p', array(), pht(
-        '<p>You were stopped here because <tt>debug.stop-on-redirect</tt> '.
+        'Continue to: %s',
-        'is set in your configuration.</p>'.
+        $link)));
-        '<p>%s</p>',
-        $link));
 
       $view->appendChild($error);
 

src/infrastructure/celerity/CelerityStaticResourceResponse.php

@@ -196,10 +196,11 @@ final class CelerityStaticResourceResponse {
     if (strpos($data, '<!') !== false) {
       throw new Exception('Literal <! is not allowed inside inline script.');
     }
-    return hsprintf(
+    // We don't use <![CDATA[ ]]> because it is ignored by HTML parsers. We
-      // We don't use <![CDATA[ ]]> because it is ignored by HTML parsers. We
+    // would need to send the document with XHTML content type.
-      // would need to send the document with XHTML content type.
+    return phutil_tag(
-      '<script type="text/javascript">%s</script>',
+      'script',
+      array('type' => 'text/javascript'),
       phutil_safe_html($data));
   }
 

src/infrastructure/diff/PhabricatorInlineCommentController.php

@@ -71,7 +71,8 @@ abstract class PhabricatorInlineCommentController
         $dialog->setTitle('Really delete this comment?');
         $dialog->addHiddenInput('id', $this->getCommentID());
         $dialog->addHiddenInput('op', 'delete');
-        $dialog->appendChild(hsprintf('<p>Delete this inline comment?</p>'));
+        $dialog->appendChild(
+          phutil_tag('p', array(), pht('Delete this inline comment?')));
 
         $dialog->addCancelButton('#');
         $dialog->addSubmitButton('Delete');

src/infrastructure/diff/view/PhabricatorInlineSummaryView.php

@@ -19,12 +19,7 @@ final class PhabricatorInlineSummaryView extends AphrontView {
   }
 
   private function renderHeader() {
-    return phutil_tag(
+    return phutil_tag_div('phabricator-inline-summary', pht('Inline Comments'));
-      'div',
-      array(
-        'class' => 'phabricator-inline-summary',
-      ),
-      'Inline Comments');
   }
 
   private function renderTable() {
@@ -39,7 +34,10 @@ final class PhabricatorInlineSummaryView extends AphrontView {
         }
       }
 
-      $rows[] = hsprintf('<tr><th colspan="3">%s</th></tr>', $group);
+      $rows[] = phutil_tag(
+        'tr',
+        array(),
+        phutil_tag('th', array('colspan' => 3), $group));
 
       foreach ($items as $item) {
 
@@ -80,25 +78,21 @@ final class PhabricatorInlineSummaryView extends AphrontView {
         $where = idx($item, 'where');
 
         $colspan = ($has_where ? null : 2);
-        $rows[] = hsprintf(
+        $rows[] = phutil_tag(
-          '<tr>'.
+          'tr',
-            '<td class="inline-line-number">%s</td>'.
+          array(),
-            '%s'.
+          array(
-            '%s'.
+            phutil_tag('td', array('class' => 'inline-line-number'), $lines),
-          '</tr>',
+            ($has_where
-          $lines,
+              ? phutil_tag('td', array('class' => 'inline-which-diff'), $where)
-          ($has_where
+              : null),
-            ? hsprintf('<td class="inline-which-diff">%s</td>', $where)
+            phutil_tag(
-            : null),
+              'td',
-          phutil_tag(
+              array(
-            'td',
+                'class' => 'inline-summary-content',
-            array(
+                'colspan' => $colspan,
-              'class' => 'inline-summary-content',
+              ),
-              'colspan' => $colspan,
+              phutil_tag_div('phabricator-remarkup', $item['content']))));
-            ),
-            hsprintf(
-              '<div class="phabricator-remarkup">%s</div>',
-              $item['content'])));
       }
     }
 

src/infrastructure/markup/rule/PhabricatorRemarkupRuleYoutube.php

@@ -30,8 +30,8 @@ final class PhabricatorRemarkupRuleYoutube
     }
 
     $youtube_src = 'https://www.youtube.com/embed/'.$v;
-    $iframe = hsprintf(
+    $iframe = phutil_tag_div(
-      '<div class="embedded-youtube-video">%s</div>',
+      'embedded-youtube-video',
       phutil_tag(
         'iframe',
         array(

src/view/control/AphrontTableView.php

@@ -272,10 +272,13 @@ final class AphrontTableView extends AphrontView {
       }
     } else {
       $colspan = max(count(array_filter($visibility)), 1);
-      $table[] = hsprintf(
+      $table[] = phutil_tag(
-        '<tr class="no-data"><td colspan="%s">%s</td></tr>',
+        'tr',
-        $colspan,
+        array('class' => 'no-data'),
-        coalesce($this->noDataString, pht('No data available.')));
+        phutil_tag(
+          'td',
+          array('colspan' => $colspan),
+          coalesce($this->noDataString, pht('No data available.'))));
     }
 
     $table_class = 'aphront-table-view';
@@ -287,7 +290,7 @@ final class AphrontTableView extends AphrontView {
     }
 
     $html = phutil_tag('table', array('class' => $table_class), $table);
-    return hsprintf('<div class="aphront-table-wrap">%s</div>', $html);
+    return phutil_tag_div('aphront-table-wrap', $html);
   }
 
   public static function renderSingleDisplayLine($line) {

src/view/control/PhabricatorObjectSelectorDialog.php

@@ -117,22 +117,19 @@ final class PhabricatorObjectSelectorDialog {
         'action' => $this->submitURI,
         'id'     => $search_id,
       ),
-      hsprintf(
+      phutil_tag(
-        '<table class="phabricator-object-selector-search">
+        'table',
-          <tr>
+        array('class' => 'phabricator-object-selector-search'),
-            <td class="phabricator-object-selector-search-filter">%s</td>
+        phutil_tag('tr', array(), array(
-            <td class="phabricator-object-selector-search-text">%s</td>
+          phutil_tag(
-          </tr>
+            'td',
-        </table>',
+            array('class' => 'phabricator-object-selector-search-filter'),
-        phutil_tag(
+            phutil_tag('select', array('id' => $filter_id), $options)),
-          'select',
+          phutil_tag(
-            array('id' => $filter_id),
+            'td',
-          $options),
+            array('class' => 'phabricator-object-selector-search-text'),
-        phutil_tag(
+            phutil_tag('input', array('id' => $query_id, 'type' => 'text'))),
-          'input',
+        ))));
-            array(
-              'id' => $query_id,
-              'type' => 'text'))));
 
     $result_box = phutil_tag(
       'div',
@@ -142,17 +139,15 @@ final class PhabricatorObjectSelectorDialog {
       ),
       '');
 
-    $attached_box = hsprintf(
+    $attached_box = phutil_tag_div(
-      '<div class="phabricator-object-selector-current">'.
+      'phabricator-object-selector-current',
-        '<div class="phabricator-object-selector-currently-attached">'.
+      phutil_tag_div(
-          '<div class="phabricator-object-selector-header">%s</div>'.
+        'phabricator-object-selector-currently-attached',
-          '<div id="%s"></div>'.
+        array(
-          '%s'.
+          phutil_tag_div('phabricator-object-selector-header', $this->header),
-        '</div>'.
+          phutil_tag('div', array('id' => $current_id)),
-      '</div>',
+          $instructions,
-      $this->header,
+        )));
-      $current_id,
-      $instructions);
 
     $dialog = new AphrontDialogView();
     $dialog

src/view/form/control/AphrontFormCheckboxControl.php

@@ -38,10 +38,10 @@ final class AphrontFormCheckboxControl extends AphrontFormControl {
           'for' => $id,
         ),
         $box['label']);
-      $rows[] = hsprintf(
+      $rows[] = phutil_tag('tr', array(), array(
-        '<tr><td>%s</td><th>%s</th></tr>',
+        phutil_tag('td', array(), $checkbox),
-        $checkbox,
+        phutil_tag('th', array(), $label)
-        $label);
+      ));
     }
     return phutil_tag(
       'table',

src/view/form/control/AphrontFormRadioButtonControl.php

@@ -51,15 +51,15 @@ final class AphrontFormRadioButtonControl extends AphrontFormControl {
         $button['label']);
 
       if ($button['caption']) {
-        $label = hsprintf(
+        $label = array(
-          '%s<div class="aphront-form-radio-caption">%s</div>',
           $label,
-          $button['caption']);
+          phutil_tag_div('aphront-form-radio-caption', $button['caption']),
+        );
       }
-      $rows[] = hsprintf(
+      $rows[] = phutil_tag('tr', array(), array(
-        '<tr><td>%s</td><th>%s</th></tr>',
+        phutil_tag('td', array(), $radio),
-        $radio,
+        phutil_tag('th', array(), $label),
-        $label);
+      ));
     }
 
     return phutil_tag(

src/view/layout/AphrontContextBarView.php

@@ -15,16 +15,15 @@ final class AphrontContextBarView extends AphrontView {
 
     require_celerity_resource('aphront-contextbar-view-css');
 
-    return hsprintf(
+    return phutil_tag_div(
-      '<div class="aphront-contextbar-view">'.
+      'aphront-contextbar-view',
-        '<div class="aphront-contextbar-core">'.
+      array(
-          '<div class="aphront-contextbar-buttons">%s</div>'.
+        phutil_tag_div('aphront-contextbar-core', array(
-          '<div class="aphront-contextbar-content">%s</div>'.
+          phutil_tag_div('aphront-contextbar-buttons', $view->render()),
-        '</div>'.
+          phutil_tag_div('aphront-contextbar-content', $this->renderChildren()),
-        '<div style="clear: both;"></div>'.
+        )),
-      '</div>',
+        phutil_tag('div', array('style' => 'clear: both;')),
-      $view->render(),
+      ));
-      $this->renderChildren());
   }
 
 }

src/view/layout/AphrontPanelView.php

@@ -69,25 +69,20 @@ final class AphrontPanelView extends AphrontView {
     }
 
     if ($this->caption !== null) {
-      $caption = phutil_tag(
+      $caption = phutil_tag_div('aphront-panel-view-caption', $this->caption);
-        'div',
-        array('class' => 'aphront-panel-view-caption'),
-        $this->caption);
     } else {
       $caption = null;
     }
 
     $buttons = null;
     if ($this->buttons) {
-      $buttons = hsprintf(
+      $buttons = phutil_tag_div(
-        '<div class="aphront-panel-view-buttons">%s</div>',
+        'aphront-panel-view-buttons',
         phutil_implode_html(" ", $this->buttons));
     }
-    $header_elements = hsprintf(
+    $header_elements = phutil_tag_div(
-      '<div class="aphront-panel-header">%s%s%s</div>',
+      'aphront-panel-header',
-      $buttons,
+      array($buttons, $header, $caption));
-      $header,
-      $caption);
 
     $table = phutil_implode_html('', $this->renderChildren());
 

src/view/layout/PhabricatorSourceCodeView.php

@@ -116,11 +116,8 @@ final class PhabricatorSourceCodeView extends AphrontView {
     $classes[] = 'remarkup-code';
     $classes[] = 'PhabricatorMonospaced';
 
-    return phutil_tag(
+    return phutil_tag_div(
-      'div',
+      'phabricator-source-code-container',
-      array(
-        'class' => 'phabricator-source-code-container',
-      ),
       javelin_tag(
         'table',
         array(

src/view/layout/PhabricatorTransactionView.php

@@ -62,6 +62,10 @@ final class PhabricatorTransactionView extends AphrontView {
 
     $transaction_id = $this->anchorName ? 'anchor-'.$this->anchorName : null;
 
+    $header = phutil_tag_div(
+      'phabricator-transaction-header',
+      array($info, $actions));
+
     return phutil_tag(
       'div',
       array(
@@ -69,15 +73,9 @@ final class PhabricatorTransactionView extends AphrontView {
         'id'    => $transaction_id,
         'style' => $style,
       ),
-      hsprintf(
+      phutil_tag_div(
-        '<div class="phabricator-transaction-detail %s">'.
+        'phabricator-transaction-detail '.$classes,
-          '<div class="phabricator-transaction-header">%s%s</div>'.
+        array($header, $content)));
-          '%s'.
-        '</div>',
-        $classes,
-        $info,
-        $actions,
-        $content));
 
   }
 
@@ -118,8 +116,9 @@ final class PhabricatorTransactionView extends AphrontView {
 
     $info = phutil_implode_html(" \xC2\xB7 ", $info);
 
-    return hsprintf(
+    return phutil_tag(
-      '<span class="phabricator-transaction-info">%s</span>',
+      'span',
+      array('class' => 'phabricator-transaction-info'),
       $info);
   }
 
@@ -139,9 +138,8 @@ final class PhabricatorTransactionView extends AphrontView {
     if (!$this->hasChildren()) {
       return null;
     }
-    return phutil_tag(
+    return phutil_tag_div(
-      'div',
+      'phabricator-transaction-content',
-      array('class' => 'phabricator-transaction-content'),
       $this->renderChildren());
   }
 

src/view/page/AphrontRequestFailureView.php

@@ -13,15 +13,15 @@ final class AphrontRequestFailureView extends AphrontView {
   final public function render() {
     require_celerity_resource('aphront-request-failure-view-css');
 
-    return hsprintf(
+    $head = phutil_tag_div(
-      '<div class="aphront-request-failure-view">'.
+      'aphront-request-failure-head',
-        '<div class="aphront-request-failure-head">'.
+      phutil_tag('h1', array(), $this->header));
-          '<h1>%s</h1>'.
+
-        '</div>'.
+    $body = phutil_tag_div(
-        '<div class="aphront-request-failure-body">%s</div>'.
+      'aphront-request-failure-body',
-      '</div>',
-      $this->header,
       $this->renderChildren());
+
+    return phutil_tag_div('aphront-request-failure-view', array($head, $body));
   }
 
 }

src/view/page/PhabricatorStandardPageView.php

@@ -298,11 +298,8 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView {
     $developer_warning = null;
     if (PhabricatorEnv::getEnvConfig('phabricator.developer-mode') &&
         DarkConsoleErrorLogPluginAPI::getErrors()) {
-      $developer_warning = phutil_tag(
+      $developer_warning = phutil_tag_div(
-        'div',
+        'aphront-developer-error-callout',
-        array(
-          'class' => 'aphront-developer-error-callout',
-        ),
         pht(
           'This page raised PHP errors. Find them in DarkConsole '.
           'or the error log.'));
@@ -313,11 +310,8 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView {
     if ($user && $user->getIsAdmin()) {
       $open = PhabricatorSetupCheck::getOpenSetupIssueCount();
       if ($open) {
-        $setup_warning = phutil_tag(
+        $setup_warning = phutil_tag_div(
-          'div',
+          'setup-warning-callout',
-          array(
-            'class' => 'setup-warning-callout',
-          ),
           phutil_tag(
             'a',
             array(
@@ -334,16 +328,16 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView {
           'id' => 'base-page',
           'class' => 'phabricator-standard-page',
         ),
-        hsprintf(
+        array(
-          '%s%s%s'.
+          $developer_warning,
-          '<div class="phabricator-standard-page-body">'.
+          $setup_warning,
-            '%s%s<div style="clear: both;"></div>'.
+          $header_chrome,
-          '</div>',
+          phutil_tag_div('phabricator-standard-page-body', array(
-        $developer_warning,
+            ($console ? hsprintf('<darkconsole />') : null),
-        $setup_warning,
+            parent::getBody(),
-        $header_chrome,
+            phutil_tag('div', array('style' => 'clear: both;')),
-        ($console ? hsprintf('<darkconsole />') : null),
+          )),
-        parent::getBody()));
+        ));
   }
 
   protected function getTail() {

src/view/page/menu/PhabricatorMainMenuSearchView.php

@@ -66,13 +66,12 @@ final class PhabricatorMainMenuSearchView extends AphrontView {
         'action' => '/search/',
         'method' => 'POST',
       ),
-      hsprintf(
+      phutil_tag_div('phabricator-main-menu-search-container', array(
-        '<div class="phabricator-main-menu-search-container">'.
-          '%s<button>Search</button>%s%s'.
-        '</div>',
         $input,
+        phutil_tag('button', array(), pht('Search')),
         $scope_input,
-        $target));
+        $target,
+      )));
 
     return $form;
   }

src/view/widget/bars/AphrontProgressBarView.php

@@ -38,18 +38,15 @@ final class AphrontProgressBarView extends AphrontBarView {
 
     $color = $this->getColor();
 
-    return phutil_tag(
+    return phutil_tag_div(
-      'div',
+      "aphront-bar progress color-{$color}",
-      array(
-        'class' => "aphront-bar progress color-{$color}",
-      ),
       array(
         phutil_tag(
           'div',
           array('title' => $this->alt),
           phutil_tag(
             'div',
-            array('style' => hsprintf("width: %dpx;", $width)),
+            array('style' => "width: {$width}px;"),
             '')),
         phutil_tag(
           'span',

src/view/widget/hovercard/PhabricatorHovercardView.php

@@ -92,22 +92,15 @@ final class PhabricatorHovercardView extends AphrontView {
       $body_title = $handle->getFullName();
     }
 
-    $body[] = phutil_tag(
+    $body[] = phutil_tag_div('phabricator-hovercard-body-header', $body_title);
-      'div',
-        array(
-          'class' => 'phabricator-hovercard-body-header'
-          ),
-        $body_title);
 
     foreach ($this->fields as $field) {
-      $item = hsprintf('<strong>%s:</strong> <span>%s</span>',
+      $item = array(
-        $field['label'], $field['value']);
+        phutil_tag('strong', array(), $field['label']),
-      $body[] = phutil_tag(
+        ' ',
-        'div',
+        phutil_tag('span', array(), $field['value']),
-          array(
+      );
-            'class' => 'phabricator-hovercard-body-item'
+      $body[] = phutil_tag_div('phabricator-hovercard-body-item', $item);
-            ),
-          $item);
     }
 
     if ($handle->getImageURI()) {
@@ -147,40 +140,22 @@ final class PhabricatorHovercardView extends AphrontView {
 
     $tail = null;
     if ($buttons) {
-      $tail = phutil_tag('div',
+      $tail = phutil_tag_div('phabricator-hovercard-tail', $buttons);
-        array('class' => 'phabricator-hovercard-tail'),
-        $buttons);
     }
 
     // Assemble container
     // TODO: Add color support
-    $content = hsprintf(
+    $hovercard = phutil_tag_div(
-      '%s%s%s',
+      'phabricator-hovercard-container',
-      phutil_tag('div',
-        array(
-          'class' => 'phabricator-hovercard-head'
-        ),
-        $header),
-      phutil_tag('div',
-        array(
-          'class' => 'phabricator-hovercard-body'
-        ),
-        $body),
-      $tail);
-
-    $hovercard = phutil_tag("div",
       array(
-        "class" => "phabricator-hovercard-container",
+        phutil_tag_div('phabricator-hovercard-head', $header),
-      ),
+        phutil_tag_div('phabricator-hovercard-body', $body),
-      $content);
+        $tail,
+      ));
 
     // Wrap for thick border
     // and later the tip at the bottom
-    return phutil_tag('div',
+    return phutil_tag_div('phabricator-hovercard-wrapper', $hovercard);
-      array(
-        'class' => 'phabricator-hovercard-wrapper',
-      ),
-      $hovercard);
   }
 
 }