From 4ec31ef75c3b78760fc7e904f503815d03e3bdaa Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 8 Jun 2011 12:39:03 -0700
Subject: [PATCH] Provide basic capabilities to make Differential column width
 flexible

Summary:
- Make wrap width settable in PHP.
  - Dynamically generate max-width based on configurable maximum width.
  - Constrain non-diff elements to standard width.
  - Provide a configuration setting.

Test Plan:
Set various things to 100 / 120, as far as I could tell everything seemed to
render sensibly? This should have no effect on 80-col changes.

Reviewed By: jdperlow
Reviewers: jdperlow, tuomaspelkonen, jungejason, aran
CC: aran, jdperlow
Differential Revision: 413
---
 conf/default.conf.php                         | 11 +++
 src/__celerity_resource_map__.php             | 34 ++++++++-
 src/__phutil_library_map__.php                |  2 +
 .../DifferentialChangesetViewController.php   | 13 ++--
 .../controller/changesetview/__init__.php     |  1 +
 .../DifferentialDiffViewController.php        |  9 +--
 .../controller/diffview/__init__.php          |  1 +
 .../DifferentialRevisionViewController.php    | 19 ++---
 .../controller/revisionview/__init__.php      |  1 +
 .../changeset/DifferentialChangesetParser.php | 19 ++++-
 .../changeset/DifferentialChangeset.php       | 13 ++++
 .../storage/changeset/__init__.php            |  1 +
 .../addcomment/DifferentialAddCommentView.php | 18 +++--
 .../DifferentialPrimaryPaneView.php           | 74 +++++++++++++++++++
 .../view/primarypane/__init__.php             | 14 ++++
 .../css/application/differential/core.css     |  7 +-
 .../differential/revision-comment.css         |  8 ++
 17 files changed, 212 insertions(+), 33 deletions(-)
 create mode 100644 src/applications/differential/view/primarypane/DifferentialPrimaryPaneView.php
 create mode 100644 src/applications/differential/view/primarypane/__init__.php

diff --git a/conf/default.conf.php b/conf/default.conf.php
index 5a8fa92fda..40f2f718a8 100644
--- a/conf/default.conf.php
+++ b/conf/default.conf.php
@@ -344,6 +344,17 @@ return array(
   // class names of classes that extend PhutilRemarkupEngineBlockRule
   'differential.custom-remarkup-block-rules' => null,
 
+  // Set display word-wrap widths for Differential. Specify a dictionary of
+  // regular expressions mapping to column widths. The filename will be matched
+  // against each regexp in order until one matches. The default configuration
+  // uses a width of 100 for Java and 80 for other languages. Note that 80 is
+  // the greatest column width of all time. Changes here will not be immediately
+  // reflected in old revisions unless you purge the render cache.
+  'differential.wordwrap' => array(
+    '/\.java$/' => 100,
+    '/.*/'      => 80,
+  ),
+
 // -- Maniphest ------------------------------------------------------------- //
 
   'maniphest.enabled' => true,
diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php
index d28440099d..b5916df4eb 100644
--- a/src/__celerity_resource_map__.php
+++ b/src/__celerity_resource_map__.php
@@ -154,7 +154,7 @@ celerity_register_resource_map(array(
   ),
   'differential-core-view-css' =>
   array(
-    'uri' => '/res/d0ae90e5/rsrc/css/application/differential/core.css',
+    'uri' => '/res/dd6b4ca9/rsrc/css/application/differential/core.css',
     'type' => 'css',
     'requires' =>
     array(
@@ -186,7 +186,7 @@ celerity_register_resource_map(array(
   ),
   'differential-revision-comment-css' =>
   array(
-    'uri' => '/res/e3539439/rsrc/css/application/differential/revision-comment.css',
+    'uri' => '/res/9dcbc5a2/rsrc/css/application/differential/revision-comment.css',
     'type' => 'css',
     'requires' =>
     array(
@@ -1114,6 +1114,7 @@ celerity_register_resource_map(array(
       'uri' => '/res/pkg/64383b02/core.pkg.css',
       'type' => 'css',
     ),
+<<<<<<< HEAD
     'bb028d56' =>
     array (
       'name' => 'workflow.pkg.js',
@@ -1129,6 +1130,24 @@ celerity_register_resource_map(array(
       ),
       'uri' => '/res/pkg/bb028d56/workflow.pkg.js',
       'type' => 'js',
+=======
+    '9b16dd9e' =>
+    array (
+      'name' => 'differential.pkg.css',
+      'symbols' =>
+      array (
+        0 => 'differential-core-view-css',
+        1 => 'differential-changeset-view-css',
+        2 => 'differential-revision-detail-css',
+        3 => 'differential-revision-history-css',
+        4 => 'differential-table-of-contents-css',
+        5 => 'differential-revision-comment-css',
+        6 => 'differential-revision-add-comment-css',
+        7 => 'differential-revision-comment-list-css',
+      ),
+      'uri' => '/res/pkg/9b16dd9e/differential.pkg.css',
+      'type' => 'css',
+>>>>>>> Provide basic capabilities to make Differential column width flexible
     ),
     'db95a6d0' =>
     array (
@@ -1175,6 +1194,7 @@ celerity_register_resource_map(array(
     'aphront-table-view-css' => '64383b02',
     'aphront-tokenizer-control-css' => '64383b02',
     'aphront-typeahead-control-css' => '64383b02',
+<<<<<<< HEAD
     'differential-changeset-view-css' => '5dc7e9ec',
     'differential-core-view-css' => '5dc7e9ec',
     'differential-revision-add-comment-css' => '5dc7e9ec',
@@ -1183,6 +1203,16 @@ celerity_register_resource_map(array(
     'differential-revision-detail-css' => '5dc7e9ec',
     'differential-revision-history-css' => '5dc7e9ec',
     'differential-table-of-contents-css' => '5dc7e9ec',
+=======
+    'differential-changeset-view-css' => '9b16dd9e',
+    'differential-core-view-css' => '9b16dd9e',
+    'differential-revision-add-comment-css' => '9b16dd9e',
+    'differential-revision-comment-css' => '9b16dd9e',
+    'differential-revision-comment-list-css' => '9b16dd9e',
+    'differential-revision-detail-css' => '9b16dd9e',
+    'differential-revision-history-css' => '9b16dd9e',
+    'differential-table-of-contents-css' => '9b16dd9e',
+>>>>>>> Provide basic capabilities to make Differential column width flexible
     'diffusion-commit-view-css' => '03ef179e',
     'javelin-behavior' => 'db95a6d0',
     'javelin-behavior-aphront-basic-tokenizer' => '33f413ef',
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 195693c8b5..87d0852b5c 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -153,6 +153,7 @@ phutil_register_library_map(array(
     'DifferentialMail' => 'applications/differential/mail/base',
     'DifferentialMarkupEngineFactory' => 'applications/differential/parser/markup',
     'DifferentialNewDiffMail' => 'applications/differential/mail/newdiff',
+    'DifferentialPrimaryPaneView' => 'applications/differential/view/primarypane',
     'DifferentialReplyHandler' => 'applications/differential/replyhandler',
     'DifferentialReviewRequestMail' => 'applications/differential/mail/reviewrequest',
     'DifferentialRevision' => 'applications/differential/storage/revision',
@@ -651,6 +652,7 @@ phutil_register_library_map(array(
     'DifferentialInlineCommentPreviewController' => 'DifferentialController',
     'DifferentialInlineCommentView' => 'AphrontView',
     'DifferentialNewDiffMail' => 'DifferentialReviewRequestMail',
+    'DifferentialPrimaryPaneView' => 'AphrontView',
     'DifferentialReplyHandler' => 'PhabricatorMailReplyHandler',
     'DifferentialReviewRequestMail' => 'DifferentialMail',
     'DifferentialRevision' => 'DifferentialDAO',
diff --git a/src/applications/differential/controller/changesetview/DifferentialChangesetViewController.php b/src/applications/differential/controller/changesetview/DifferentialChangesetViewController.php
index 647143ebf6..d85059d0e1 100644
--- a/src/applications/differential/controller/changesetview/DifferentialChangesetViewController.php
+++ b/src/applications/differential/controller/changesetview/DifferentialChangesetViewController.php
@@ -229,12 +229,13 @@ class DifferentialChangesetViewController extends DifferentialController {
     $detail->setRevisionID($request->getInt('revision_id'));
 
     $output =
-      '<div class="differential-primary-pane">'.
-        '<div class="differential-review-stage" '.
-          'id="differential-review-stage">'.
-          $detail->render().
-        '</div>'.
-      '</div>';
+      id(new DifferentialPrimaryPaneView())
+        ->setLineWidthFromChangesets(array($changeset))
+        ->appendChild(
+          '<div class="differential-review-stage" '.
+            'id="differential-review-stage">'.
+            $detail->render().
+          '</div>');
 
     return $this->buildStandardPageResponse(
       array(
diff --git a/src/applications/differential/controller/changesetview/__init__.php b/src/applications/differential/controller/changesetview/__init__.php
index e3d0b1b139..1dee351113 100644
--- a/src/applications/differential/controller/changesetview/__init__.php
+++ b/src/applications/differential/controller/changesetview/__init__.php
@@ -19,6 +19,7 @@ phutil_require_module('phabricator', 'applications/differential/storage/changese
 phutil_require_module('phabricator', 'applications/differential/storage/diff');
 phutil_require_module('phabricator', 'applications/differential/storage/inlinecomment');
 phutil_require_module('phabricator', 'applications/differential/view/changesetdetailview');
+phutil_require_module('phabricator', 'applications/differential/view/primarypane');
 phutil_require_module('phabricator', 'applications/phid/handle/data');
 phutil_require_module('phabricator', 'infrastructure/javelin/api');
 
diff --git a/src/applications/differential/controller/diffview/DifferentialDiffViewController.php b/src/applications/differential/controller/diffview/DifferentialDiffViewController.php
index bbf8d69319..6a81b7d756 100644
--- a/src/applications/differential/controller/diffview/DifferentialDiffViewController.php
+++ b/src/applications/differential/controller/diffview/DifferentialDiffViewController.php
@@ -117,15 +117,14 @@ class DifferentialDiffViewController extends DifferentialController {
       ->setRenderingReferences($refs);
 
     return $this->buildStandardPageResponse(
-      '<div class="differential-primary-pane">'.
-        implode(
-          "\n",
+      id(new DifferentialPrimaryPaneView())
+        ->setLineWidthFromChangesets($changesets)
+        ->appendChild(
           array(
             $top_panel->render(),
             $table_of_contents->render(),
             $details->render(),
-          )).
-      '</div>',
+          )),
       array(
         'title' => 'Diff View',
       ));
diff --git a/src/applications/differential/controller/diffview/__init__.php b/src/applications/differential/controller/diffview/__init__.php
index ab06276b86..e724d51ec3 100644
--- a/src/applications/differential/controller/diffview/__init__.php
+++ b/src/applications/differential/controller/diffview/__init__.php
@@ -12,6 +12,7 @@ phutil_require_module('phabricator', 'applications/differential/data/revisionlis
 phutil_require_module('phabricator', 'applications/differential/storage/diff');
 phutil_require_module('phabricator', 'applications/differential/view/changesetlistview');
 phutil_require_module('phabricator', 'applications/differential/view/difftableofcontents');
+phutil_require_module('phabricator', 'applications/differential/view/primarypane');
 phutil_require_module('phabricator', 'infrastructure/env');
 phutil_require_module('phabricator', 'view/form/base');
 phutil_require_module('phabricator', 'view/form/control/markup');
diff --git a/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php b/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php
index a1e56400e9..7fdf3427be 100644
--- a/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php
+++ b/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php
@@ -231,15 +231,16 @@ class DifferentialRevisionViewController extends DifferentialController {
     $this->updateViewTime($user->getPHID(), $revision->getPHID());
 
     return $this->buildStandardPageResponse(
-      '<div class="differential-primary-pane">'.
-        $revision_detail->render().
-        $comment_view->render().
-        $diff_history->render().
-        $warning.
-        $toc_view->render().
-        $changeset_view->render().
-        $comment_form->render().
-      '</div>',
+      id(new DifferentialPrimaryPaneView())
+        ->setLineWidthFromChangesets($changesets)
+        ->appendChild(
+          $revision_detail->render().
+          $comment_view->render().
+          $diff_history->render().
+          $warning.
+          $toc_view->render().
+          $changeset_view->render().
+          $comment_form->render()),
       array(
         'title' => $revision->getTitle(),
       ));
diff --git a/src/applications/differential/controller/revisionview/__init__.php b/src/applications/differential/controller/revisionview/__init__.php
index cbc1f3627c..aff41d24a8 100644
--- a/src/applications/differential/controller/revisionview/__init__.php
+++ b/src/applications/differential/controller/revisionview/__init__.php
@@ -22,6 +22,7 @@ phutil_require_module('phabricator', 'applications/differential/storage/viewtime
 phutil_require_module('phabricator', 'applications/differential/view/addcomment');
 phutil_require_module('phabricator', 'applications/differential/view/changesetlistview');
 phutil_require_module('phabricator', 'applications/differential/view/difftableofcontents');
+phutil_require_module('phabricator', 'applications/differential/view/primarypane');
 phutil_require_module('phabricator', 'applications/differential/view/revisioncommentlist');
 phutil_require_module('phabricator', 'applications/differential/view/revisiondetail');
 phutil_require_module('phabricator', 'applications/differential/view/revisionupdatehistory');
diff --git a/src/applications/differential/parser/changeset/DifferentialChangesetParser.php b/src/applications/differential/parser/changeset/DifferentialChangesetParser.php
index b3a52ead5e..25a2859b69 100644
--- a/src/applications/differential/parser/changeset/DifferentialChangesetParser.php
+++ b/src/applications/differential/parser/changeset/DifferentialChangesetParser.php
@@ -53,6 +53,8 @@ class DifferentialChangesetParser {
   private $renderingReference;
   private $isSubparser;
 
+  private $lineWidth = 80;
+
   const CACHE_VERSION = 4;
 
   const ATTR_GENERATED  = 'attr:generated';
@@ -118,13 +120,26 @@ class DifferentialChangesetParser {
     return $this;
   }
 
+  /**
+   * Set the character width at which lines will be wrapped. Defaults to 80.
+   *
+   * @param   int Hard-wrap line-width for diff display.
+   * @return this
+   */
+  public function setLineWidth($width) {
+    $this->lineWidth = $width;
+    return $this;
+  }
+
   private function getRenderCacheKey() {
     return $this->renderCacheKey;
   }
 
   public function setChangeset($changeset) {
     $this->changeset = $changeset;
+
     $this->setFilename($changeset->getFilename());
+    $this->setLineWidth($changeset->getWordWrapWidth());
 
     return $this;
   }
@@ -646,7 +661,7 @@ class DifferentialChangesetParser {
           $text,
           $intra[$key]);
       }
-      if (isset($corpus[$key]) && strlen($corpus[$key]) > 80) {
+      if (isset($corpus[$key]) && strlen($corpus[$key]) > $this->lineWidth) {
         $render[$key] = $this->lineWrap($render[$key]);
       }
     }
@@ -669,7 +684,7 @@ class DifferentialChangesetParser {
       } else {
         ++$c;
       }
-      if ($c == 80) {
+      if ($c == $this->lineWidth) {
         $ins[] = ($ii + 1);
         $c = 0;
       }
diff --git a/src/applications/differential/storage/changeset/DifferentialChangeset.php b/src/applications/differential/storage/changeset/DifferentialChangeset.php
index 3e87d18ee6..e0ba4facbd 100644
--- a/src/applications/differential/storage/changeset/DifferentialChangeset.php
+++ b/src/applications/differential/storage/changeset/DifferentialChangeset.php
@@ -169,4 +169,17 @@ class DifferentialChangeset extends DifferentialDAO {
     return $path;
   }
 
+  /**
+   * Retreive the configured wordwrap width for this changeset.
+   */
+  public function getWordWrapWidth() {
+    $config = PhabricatorEnv::getEnvConfig('differential.wordwrap');
+    foreach ($config as $regexp => $width) {
+      if (preg_match($regexp, $this->getFileName())) {
+        return $width;
+      }
+    }
+    return 80;
+  }
+
 }
diff --git a/src/applications/differential/storage/changeset/__init__.php b/src/applications/differential/storage/changeset/__init__.php
index da4cbf09ba..4c6a24d692 100644
--- a/src/applications/differential/storage/changeset/__init__.php
+++ b/src/applications/differential/storage/changeset/__init__.php
@@ -10,6 +10,7 @@ phutil_require_module('phabricator', 'applications/differential/constants/change
 phutil_require_module('phabricator', 'applications/differential/storage/base');
 phutil_require_module('phabricator', 'applications/differential/storage/hunk');
 phutil_require_module('phabricator', 'applications/repository/constants/repositorytype');
+phutil_require_module('phabricator', 'infrastructure/env');
 
 phutil_require_module('phutil', 'parser/uri');
 phutil_require_module('phutil', 'utils');
diff --git a/src/applications/differential/view/addcomment/DifferentialAddCommentView.php b/src/applications/differential/view/addcomment/DifferentialAddCommentView.php
index 731c8ff3d9..30d582a30c 100644
--- a/src/applications/differential/view/addcomment/DifferentialAddCommentView.php
+++ b/src/applications/differential/view/addcomment/DifferentialAddCommentView.php
@@ -117,14 +117,16 @@ final class DifferentialAddCommentView extends AphrontView {
     $panel_view->addClass('aphront-panel-flush');
 
     return
-      $panel_view->render().
-      '<div class="aphront-panel-preview aphront-panel-flush">'.
-        '<div id="comment-preview">'.
-          '<span class="aphront-panel-preview-loading-text">'.
-            'Loading comment preview...'.
-          '</span>'.
-        '</div>'.
-        '<div id="inline-comment-preview">'.
+      '<div class="differential-add-comment-panel">'.
+        $panel_view->render().
+        '<div class="aphront-panel-preview aphront-panel-flush">'.
+          '<div id="comment-preview">'.
+            '<span class="aphront-panel-preview-loading-text">'.
+              'Loading comment preview...'.
+            '</span>'.
+          '</div>'.
+          '<div id="inline-comment-preview">'.
+          '</div>'.
         '</div>'.
       '</div>';
   }
diff --git a/src/applications/differential/view/primarypane/DifferentialPrimaryPaneView.php b/src/applications/differential/view/primarypane/DifferentialPrimaryPaneView.php
new file mode 100644
index 0000000000..2ec78b164c
--- /dev/null
+++ b/src/applications/differential/view/primarypane/DifferentialPrimaryPaneView.php
@@ -0,0 +1,74 @@
+<?php
+
+/*
+ * Copyright 2011 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+final class DifferentialPrimaryPaneView extends AphrontView {
+
+  private $lineWidth = 80;
+
+  public function setLineWidth($width) {
+    $this->lineWidth = $width;
+    return $this;
+  }
+
+  public function setLineWidthFromChangesets(array $changesets) {
+    if (empty($changesets)) {
+      return;
+    }
+
+    $max = 1;
+    foreach ($changesets as $changeset) {
+      $max = max($max, $changeset->getWordWrapWidth());
+    }
+    $this->setLineWidth($max);
+
+    return $this;
+  }
+
+  public function render() {
+
+    // This is chosen somewhat arbitrarily so the math works out correctly
+    // for 80 columns and sets it to the preexisting width (1162px). It may
+    // need some tweaking, but when lineWidth = 80, the computed pixel width
+    // should be 1162px or something along those lines.
+
+    // Width of the constant-width elements (like line numbers, padding,
+    // and borders).
+    $const = 148;
+    $width = ceil(((1162 - $const) / 80) * $this->lineWidth) + $const;
+    $width = max(1162, $width);
+
+    // Override the 'td' width rule with a more specific, inline style tag.
+    // TODO: move this to <head> somehow.
+    $td_width = ceil((88 / 80) * $this->lineWidth);
+    $style_tag = phutil_render_tag(
+      'style',
+      array(
+        'type' => 'text/css',
+      ),
+      ".differential-diff td { width: {$td_width}ex; }");
+
+    return phutil_render_tag(
+      'div',
+      array(
+        'class' => 'differential-primary-pane',
+        'style' => "max-width: {$width}px",
+      ),
+      $style_tag.$this->renderChildren());
+  }
+
+}
diff --git a/src/applications/differential/view/primarypane/__init__.php b/src/applications/differential/view/primarypane/__init__.php
new file mode 100644
index 0000000000..ec28cedc88
--- /dev/null
+++ b/src/applications/differential/view/primarypane/__init__.php
@@ -0,0 +1,14 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'view/base');
+
+phutil_require_module('phutil', 'markup');
+
+
+phutil_require_source('DifferentialPrimaryPaneView.php');
diff --git a/webroot/rsrc/css/application/differential/core.css b/webroot/rsrc/css/application/differential/core.css
index 5f2e7423d5..574d850f9e 100644
--- a/webroot/rsrc/css/application/differential/core.css
+++ b/webroot/rsrc/css/application/differential/core.css
@@ -4,8 +4,13 @@
 
 .differential-primary-pane {
   margin:           0 40px;
-  max-width:        1162px;
   padding-bottom:   2em;
+
+  /**
+   * This is potentially overridden by a 'style' attribute which selects a
+   * different width based on the maximum character width of the diff.
+   */
+  max-width:        1162px;
 }
 
 .differential-panel {
diff --git a/webroot/rsrc/css/application/differential/revision-comment.css b/webroot/rsrc/css/application/differential/revision-comment.css
index d45ba4f554..c5bd8c01c4 100644
--- a/webroot/rsrc/css/application/differential/revision-comment.css
+++ b/webroot/rsrc/css/application/differential/revision-comment.css
@@ -9,6 +9,14 @@
   border: 1px solid transparent;
 }
 
+.differential-comment-list {
+  max-width: 1162px;
+}
+
+.differential-add-comment-panel {
+  max-width: 1162px;
+}
+
 .differential-comment-list .anchor-target {
   background-color: #ffffdd;
   border-color: #ffff00;