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

src/__phutil_library_map__.php

@@ -1138,7 +1138,11 @@ phutil_register_library_map(array(
     '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 @@ phutil_register_library_map(array(
     '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 @@ final class DifferentialChangesetOneUpRenderer
           $inline = $this->buildInlineComment(
             $p['comment'],
             $p['right']);
-          $inline->setBuildScaffolding(false);
 
           $out[] = phutil_tag(
             'tr',
@@ -160,4 +159,9 @@ final class DifferentialChangesetOneUpRenderer
     throw new PhutilMethodNotImplementedException();
   }
 
+  public function getRowScaffoldForInline(PHUIDiffInlineCommentView $view) {
+    return id(new PHUIDiffOneUpInlineCommentRowScaffold())
+      ->addInlineView($view);
+  }
+
 }

src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php

@@ -397,4 +397,9 @@ final class DifferentialChangesetTwoUpRenderer
     return $this->renderChangesetTable($output);
   }
 
+  public function getRowScaffoldForInline(PHUIDiffInlineCommentView $view) {
+    return id(new PHUIDiffTwoUpInlineCommentRowScaffold())
+      ->addInlineView($view);
+  }
+
 }

src/infrastructure/diff/PhabricatorInlineCommentController.php

@@ -114,6 +114,7 @@ abstract class PhabricatorInlineCommentController
 
         $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 @@ abstract class PhabricatorInlineCommentController
     $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,7 +5,6 @@ final class PHUIDiffInlineCommentDetailView
 
   private $inlineComment;
   private $onRight;
-  private $buildScaffolding;
   private $handles;
   private $markupEngine;
   private $editable;
@@ -13,6 +12,10 @@ final class PHUIDiffInlineCommentDetailView
   private $allowReply;
   private $renderer;
 
+  public function getIsOnRight() {
+    return $this->onRight;
+  }
+
   public function setInlineComment(PhabricatorInlineCommentInterface $comment) {
     $this->inlineComment = $comment;
     return $this;
@@ -23,11 +26,6 @@ final class PHUIDiffInlineCommentDetailView
     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 @@ final class PHUIDiffInlineCommentDetailView
           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();
+
+}

src/infrastructure/diff/view/PHUIDiffOneUpInlineCommentRowScaffold.php

@@ -0,0 +1,33 @@
+<?php
+
+/**
+ * Row scaffold for `1up` (unified) changeset views.
+ *
+ * This scaffold is straightforward.
+ */
+final class PHUIDiffOneUpInlineCommentRowScaffold
+  extends PHUIDiffInlineCommentRowScaffold {
+
+  public function render() {
+    $inlines = $this->getInlineViews();
+    if (count($inlines) != 1) {
+      throw new Exception(
+        pht('One-up inline row scaffold must have exactly one inline view!'));
+    }
+    $inline = head($inlines);
+
+    $attrs = array(
+      'colspan' => 3,
+      'class' => 'right3',
+    );
+
+    $cells = array(
+      phutil_tag('th', array()),
+      phutil_tag('th', array()),
+      phutil_tag('td', $attrs, $inline),
+    );
+
+    return phutil_tag('tr', array(), $cells);
+  }
+
+}

src/infrastructure/diff/view/PHUIDiffTwoUpInlineCommentRowScaffold.php

@@ -0,0 +1,72 @@
+<?php
+
+/**
+ * Row scaffold for 2up (side-by-side) changeset views.
+ *
+ * Although this scaffold is normally straightforward, it may also accept
+ * two inline comments and display them adjacently.
+ */
+final class PHUIDiffTwoUpInlineCommentRowScaffold
+  extends PHUIDiffInlineCommentRowScaffold {
+
+  public function render() {
+    $inlines = $this->getInlineViews();
+
+    if (!$inlines) {
+      throw new Exception(
+        pht('Two-up inline row scaffold must have at least one inline view.'));
+    }
+
+    if (count($inlines) > 2) {
+      throw new Exception(
+        pht('Two-up inline row scaffold must have at most two inline views.'));
+    }
+
+    if (count($inlines) == 1) {
+      $inline = head($inlines);
+      if ($inline->getIsOnRight()) {
+        $left_side = null;
+        $right_side = $inline;
+      } else {
+        $left_side = $inline;
+        $right_side = null;
+      }
+    } else {
+      list($u, $v) = $inlines;
+
+      if ($u->getIsOnRight() == $v->getIsOnRight()) {
+        throw new Exception(
+          pht(
+            'Two-up inline row scaffold must have one comment on the left and '.
+            'one comment on the right when showing two comments.'));
+      }
+
+      if ($v->getIsOnRight()) {
+        $left_side = $u;
+        $right_side = $v;
+      } else {
+        $left_side = $v;
+        $right_side = $u;
+      }
+    }
+
+    $left_attrs = array(
+      'class' => 'left',
+    );
+
+    $right_attrs = array(
+      'colspan' => 3,
+      'class' => 'right3',
+    );
+
+    $cells = array(
+      phutil_tag('th', array()),
+      phutil_tag('td', $left_attrs, $left_side),
+      phutil_tag('th', array()),
+      phutil_tag('td', $right_attrs, $right_side),
+    );
+
+    return phutil_tag('tr', array(), $cells);
+  }
+
+}