Begin separating inline comment scaffolding from other renderers

Summary:
Ref T2009. Inline comments have "scaffolding", which is basically some empty table cells/rows around them to get the layout correct.

The scaffolding depends on the renderer, since the cells are different for side-by-side vs unified diffs.

This is currently duplicated all over the place:

  - Edit view has 1up/2up.
  - Detail view has 1up/2up.
  - 1up renderer has 1up.
  - 2up renderer has four separate copies of the 2up logic.

These all have subtle differences, which are mostly bugs. Start making the scaffolding more composable so we can get rid of that mess.

Test Plan: Added, edited, and removed inline comments on unified and side-by-side diffs.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T2009

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

Author: epriestley

src/__phutil_library_map__.php

@@ -1138,7 +1138,11 @@
     'PHUICrumbsView' => 'view/phui/PHUICrumbsView.php',
     'PHUIDiffInlineCommentDetailView' => 'infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php',
     'PHUIDiffInlineCommentEditView' => 'infrastructure/diff/view/PHUIDiffInlineCommentEditView.php',
+    'PHUIDiffInlineCommentRowScaffold' => 'infrastructure/diff/view/PHUIDiffInlineCommentRowScaffold.php',
+    'PHUIDiffInlineCommentTableScaffold' => 'infrastructure/diff/view/PHUIDiffInlineCommentTableScaffold.php',
     'PHUIDiffInlineCommentView' => 'infrastructure/diff/view/PHUIDiffInlineCommentView.php',
+    'PHUIDiffOneUpInlineCommentRowScaffold' => 'infrastructure/diff/view/PHUIDiffOneUpInlineCommentRowScaffold.php',
+    'PHUIDiffTwoUpInlineCommentRowScaffold' => 'infrastructure/diff/view/PHUIDiffTwoUpInlineCommentRowScaffold.php',
     'PHUIDocumentExample' => 'applications/uiexample/examples/PHUIDocumentExample.php',
     'PHUIDocumentView' => 'view/phui/PHUIDocumentView.php',
     'PHUIFeedStoryExample' => 'applications/uiexample/examples/PHUIFeedStoryExample.php',
@@ -4367,7 +4371,11 @@
     'PHUICrumbsView' => 'AphrontView',
     'PHUIDiffInlineCommentDetailView' => 'PHUIDiffInlineCommentView',
     'PHUIDiffInlineCommentEditView' => 'PHUIDiffInlineCommentView',
+    'PHUIDiffInlineCommentRowScaffold' => 'AphrontView',
+    'PHUIDiffInlineCommentTableScaffold' => 'AphrontView',
     'PHUIDiffInlineCommentView' => 'AphrontView',
+    'PHUIDiffOneUpInlineCommentRowScaffold' => 'PHUIDiffInlineCommentRowScaffold',
+    'PHUIDiffTwoUpInlineCommentRowScaffold' => 'PHUIDiffInlineCommentRowScaffold',
     'PHUIDocumentExample' => 'PhabricatorUIExample',
     'PHUIDocumentView' => 'AphrontTagView',
     'PHUIFeedStoryExample' => 'PhabricatorUIExample',

src/applications/differential/render/DifferentialChangesetHTMLRenderer.php

@@ -3,7 +3,20 @@
 abstract class DifferentialChangesetHTMLRenderer
   extends DifferentialChangesetRenderer {
 
+  public static function getHTMLRendererByKey($key) {
+    switch ($key) {
+      case '1up':
+        return new DifferentialChangesetOneUpRenderer();
+      case '2up':
+      default:
+        return new DifferentialChangesetTwoUpRenderer();
+    }
+    throw new Exception(pht('Unknown HTML renderer "%s"!', $key));
+  }
+
   abstract protected function getRendererTableClass();
+  abstract public function getRowScaffoldForInline(
+    PHUIDiffInlineCommentView $view);
 
   protected function renderChangeTypeHeader($force) {
     $changeset = $this->getChangeset();

src/applications/differential/render/DifferentialChangesetOneUpRenderer.php

@@ -95,7 +95,6 @@ public function renderTextChange(
           $inline = $this->buildInlineComment(
             $p['comment'],
             $p['right']);
-          $inline->setBuildScaffolding(false);
 
           $out[] = phutil_tag(
             'tr',
@@ -160,4 +159,9 @@ public function renderFileChange(
     throw new PhutilMethodNotImplementedException();
   }
 
+  public function getRowScaffoldForInline(PHUIDiffInlineCommentView $view) {
+    return id(new PHUIDiffOneUpInlineCommentRowScaffold())
+      ->addInlineView($view);
+  }
+
 }

src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php

@@ -397,4 +397,9 @@ public function renderFileChange($old_file = null,
     return $this->renderChangesetTable($output);
   }
 
+  public function getRowScaffoldForInline(PHUIDiffInlineCommentView $view) {
+    return id(new PHUIDiffTwoUpInlineCommentRowScaffold())
+      ->addInlineView($view);
+  }
+
 }

src/infrastructure/diff/PhabricatorInlineCommentController.php

@@ -114,6 +114,7 @@ public function processRequest() {
 
         $edit_dialog->addHiddenInput('id', $this->getCommentID());
         $edit_dialog->addHiddenInput('op', 'edit');
+        $edit_dialog->addHiddenInput('renderer', $this->getRenderer());
 
         $edit_dialog->appendChild(
           $this->renderTextArea(
@@ -236,11 +237,16 @@ private function buildRenderedCommentResponse(
     $view = id(new PHUIDiffInlineCommentDetailView())
       ->setInlineComment($inline)
       ->setOnRight($on_right)
-      ->setBuildScaffolding(true)
       ->setMarkupEngine($engine)
       ->setHandles($handles)
-      ->setEditable(true)
-      ->setRenderer($this->getRenderer());
+      ->setEditable(true);
+
+    $renderer = DifferentialChangesetHTMLRenderer::getHTMLRendererByKey(
+      $this->getRenderer());
+
+    $view = $renderer->getRowScaffoldForInline($view);
+    $view = id(new PHUIDiffInlineCommentTableScaffold())
+      ->addRowScaffold($view);
 
     return id(new AphrontAjaxResponse())
       ->setContent(

src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php

@@ -5,14 +5,17 @@ final class PHUIDiffInlineCommentDetailView
 
   private $inlineComment;
   private $onRight;
-  private $buildScaffolding;
   private $handles;
   private $markupEngine;
   private $editable;
   private $preview;
   private $allowReply;
   private $renderer;
 
+  public function getIsOnRight() {
+    return $this->onRight;
+  }
+
   public function setInlineComment(PhabricatorInlineCommentInterface $comment) {
     $this->inlineComment = $comment;
     return $this;
@@ -23,11 +26,6 @@ public function setOnRight($on_right) {
     return $this;
   }
 
-  public function setBuildScaffolding($scaffold) {
-    $this->buildScaffolding = $scaffold;
-    return $this;
-  }
-
   public function setHandles(array $handles) {
     assert_instances_of($handles, 'PhabricatorObjectHandle');
     $this->handles = $handles;
@@ -254,40 +252,7 @@ public function render() {
           phutil_tag_div('phabricator-remarkup', $content)),
       ));
 
-    return $this->scaffoldMarkup($markup);
-  }
-
-  private function scaffoldMarkup($markup) {
-    if (!$this->buildScaffolding) {
-      return $markup;
-    }
-
-    if ($this->renderer == '1up') {
-      $cells = array(
-        phutil_tag('th', array()),
-        phutil_tag('th', array()),
-        phutil_tag(
-          'td',
-          array('colspan' => 3, 'class' => 'right3'),
-          $markup),
-      );
-    } else {
-      $left_markup = !$this->onRight ? $markup : '';
-      $right_markup = $this->onRight ? $markup : '';
-
-      $cells = array(
-        phutil_tag('th', array()),
-        phutil_tag('td', array('class' => 'left'), $left_markup),
-        phutil_tag('th', array()),
-        phutil_tag(
-          'td',
-          array('colspan' => 3, 'class' => 'right3'),
-          $right_markup),
-      );
-    }
-
-    $row = phutil_tag('tr', array(), $cells);
-    return phutil_tag('table', array(), $row);
+    return $markup;
   }
 
 }

src/infrastructure/diff/view/PHUIDiffInlineCommentEditView.php

@@ -11,6 +11,10 @@ final class PHUIDiffInlineCommentEditView
   private $length;
   private $renderer;
 
+  public function getIsOnRight() {
+    return $this->onRight;
+  }
+
   public function setRenderer($renderer) {
     $this->renderer = $renderer;
     return $this;

src/infrastructure/diff/view/PHUIDiffInlineCommentRowScaffold.php

@@ -0,0 +1,23 @@
+<?php
+
+/**
+ * Wraps an inline comment in a table row.
+ *
+ * Inline comments need different wrapping cells when shown in unified vs
+ * side-by-side diffs, as the two tables have different layouts. This wraps
+ * an inline comment element in an appropriate table row.
+ */
+abstract class PHUIDiffInlineCommentRowScaffold extends AphrontView {
+
+  private $views = array();
+
+  public function getInlineViews() {
+    return $this->views;
+  }
+
+  public function addInlineView(PHUIDiffInlineCommentView $view) {
+    $this->views[] = $view;
+    return $this;
+  }
+
+}

src/infrastructure/diff/view/PHUIDiffInlineCommentTableScaffold.php

@@ -0,0 +1,22 @@
+<?php
+
+/**
+ * Wraps an inline comment row scaffold in a table.
+ *
+ * This scaffold is used to ship inlines over the wire to the client, so they
+ * arrive in a form that's easy to mainipulate (a valid table node).
+ */
+final class PHUIDiffInlineCommentTableScaffold extends AphrontView {
+
+  private $rows = array();
+
+  public function addRowScaffold(PHUIDiffInlineCommentRowScaffold $row) {
+    $this->rows[] = $row;
+    return $this;
+  }
+
+  public function render() {
+    return phutil_tag('table', array(), $this->rows);
+  }
+
+}

src/infrastructure/diff/view/PHUIDiffInlineCommentView.php

@@ -1,3 +1,7 @@
 <?php
 
-abstract class PHUIDiffInlineCommentView extends AphrontView {}
+abstract class PHUIDiffInlineCommentView extends AphrontView {
+
+  abstract public function getIsOnRight();
+
+}