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
This commit is contained in:
epriestley
2011-06-08 12:39:03 -07:00
parent 4d3ef5ee0b
commit 4ec31ef75c
17 changed files with 212 additions and 33 deletions

View File

@@ -344,6 +344,17 @@ return array(
// class names of classes that extend PhutilRemarkupEngineBlockRule // class names of classes that extend PhutilRemarkupEngineBlockRule
'differential.custom-remarkup-block-rules' => null, '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 ------------------------------------------------------------- //
'maniphest.enabled' => true, 'maniphest.enabled' => true,

View File

@@ -154,7 +154,7 @@ celerity_register_resource_map(array(
), ),
'differential-core-view-css' => 'differential-core-view-css' =>
array( array(
'uri' => '/res/d0ae90e5/rsrc/css/application/differential/core.css', 'uri' => '/res/dd6b4ca9/rsrc/css/application/differential/core.css',
'type' => 'css', 'type' => 'css',
'requires' => 'requires' =>
array( array(
@@ -186,7 +186,7 @@ celerity_register_resource_map(array(
), ),
'differential-revision-comment-css' => 'differential-revision-comment-css' =>
array( array(
'uri' => '/res/e3539439/rsrc/css/application/differential/revision-comment.css', 'uri' => '/res/9dcbc5a2/rsrc/css/application/differential/revision-comment.css',
'type' => 'css', 'type' => 'css',
'requires' => 'requires' =>
array( array(
@@ -1114,6 +1114,7 @@ celerity_register_resource_map(array(
'uri' => '/res/pkg/64383b02/core.pkg.css', 'uri' => '/res/pkg/64383b02/core.pkg.css',
'type' => 'css', 'type' => 'css',
), ),
<<<<<<< HEAD
'bb028d56' => 'bb028d56' =>
array ( array (
'name' => 'workflow.pkg.js', 'name' => 'workflow.pkg.js',
@@ -1129,6 +1130,24 @@ celerity_register_resource_map(array(
), ),
'uri' => '/res/pkg/bb028d56/workflow.pkg.js', 'uri' => '/res/pkg/bb028d56/workflow.pkg.js',
'type' => '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' => 'db95a6d0' =>
array ( array (
@@ -1175,6 +1194,7 @@ celerity_register_resource_map(array(
'aphront-table-view-css' => '64383b02', 'aphront-table-view-css' => '64383b02',
'aphront-tokenizer-control-css' => '64383b02', 'aphront-tokenizer-control-css' => '64383b02',
'aphront-typeahead-control-css' => '64383b02', 'aphront-typeahead-control-css' => '64383b02',
<<<<<<< HEAD
'differential-changeset-view-css' => '5dc7e9ec', 'differential-changeset-view-css' => '5dc7e9ec',
'differential-core-view-css' => '5dc7e9ec', 'differential-core-view-css' => '5dc7e9ec',
'differential-revision-add-comment-css' => '5dc7e9ec', 'differential-revision-add-comment-css' => '5dc7e9ec',
@@ -1183,6 +1203,16 @@ celerity_register_resource_map(array(
'differential-revision-detail-css' => '5dc7e9ec', 'differential-revision-detail-css' => '5dc7e9ec',
'differential-revision-history-css' => '5dc7e9ec', 'differential-revision-history-css' => '5dc7e9ec',
'differential-table-of-contents-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', 'diffusion-commit-view-css' => '03ef179e',
'javelin-behavior' => 'db95a6d0', 'javelin-behavior' => 'db95a6d0',
'javelin-behavior-aphront-basic-tokenizer' => '33f413ef', 'javelin-behavior-aphront-basic-tokenizer' => '33f413ef',

View File

@@ -153,6 +153,7 @@ phutil_register_library_map(array(
'DifferentialMail' => 'applications/differential/mail/base', 'DifferentialMail' => 'applications/differential/mail/base',
'DifferentialMarkupEngineFactory' => 'applications/differential/parser/markup', 'DifferentialMarkupEngineFactory' => 'applications/differential/parser/markup',
'DifferentialNewDiffMail' => 'applications/differential/mail/newdiff', 'DifferentialNewDiffMail' => 'applications/differential/mail/newdiff',
'DifferentialPrimaryPaneView' => 'applications/differential/view/primarypane',
'DifferentialReplyHandler' => 'applications/differential/replyhandler', 'DifferentialReplyHandler' => 'applications/differential/replyhandler',
'DifferentialReviewRequestMail' => 'applications/differential/mail/reviewrequest', 'DifferentialReviewRequestMail' => 'applications/differential/mail/reviewrequest',
'DifferentialRevision' => 'applications/differential/storage/revision', 'DifferentialRevision' => 'applications/differential/storage/revision',
@@ -651,6 +652,7 @@ phutil_register_library_map(array(
'DifferentialInlineCommentPreviewController' => 'DifferentialController', 'DifferentialInlineCommentPreviewController' => 'DifferentialController',
'DifferentialInlineCommentView' => 'AphrontView', 'DifferentialInlineCommentView' => 'AphrontView',
'DifferentialNewDiffMail' => 'DifferentialReviewRequestMail', 'DifferentialNewDiffMail' => 'DifferentialReviewRequestMail',
'DifferentialPrimaryPaneView' => 'AphrontView',
'DifferentialReplyHandler' => 'PhabricatorMailReplyHandler', 'DifferentialReplyHandler' => 'PhabricatorMailReplyHandler',
'DifferentialReviewRequestMail' => 'DifferentialMail', 'DifferentialReviewRequestMail' => 'DifferentialMail',
'DifferentialRevision' => 'DifferentialDAO', 'DifferentialRevision' => 'DifferentialDAO',

View File

@@ -229,12 +229,13 @@ class DifferentialChangesetViewController extends DifferentialController {
$detail->setRevisionID($request->getInt('revision_id')); $detail->setRevisionID($request->getInt('revision_id'));
$output = $output =
'<div class="differential-primary-pane">'. id(new DifferentialPrimaryPaneView())
->setLineWidthFromChangesets(array($changeset))
->appendChild(
'<div class="differential-review-stage" '. '<div class="differential-review-stage" '.
'id="differential-review-stage">'. 'id="differential-review-stage">'.
$detail->render(). $detail->render().
'</div>'. '</div>');
'</div>';
return $this->buildStandardPageResponse( return $this->buildStandardPageResponse(
array( array(

View File

@@ -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/diff');
phutil_require_module('phabricator', 'applications/differential/storage/inlinecomment'); phutil_require_module('phabricator', 'applications/differential/storage/inlinecomment');
phutil_require_module('phabricator', 'applications/differential/view/changesetdetailview'); 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', 'applications/phid/handle/data');
phutil_require_module('phabricator', 'infrastructure/javelin/api'); phutil_require_module('phabricator', 'infrastructure/javelin/api');

View File

@@ -117,15 +117,14 @@ class DifferentialDiffViewController extends DifferentialController {
->setRenderingReferences($refs); ->setRenderingReferences($refs);
return $this->buildStandardPageResponse( return $this->buildStandardPageResponse(
'<div class="differential-primary-pane">'. id(new DifferentialPrimaryPaneView())
implode( ->setLineWidthFromChangesets($changesets)
"\n", ->appendChild(
array( array(
$top_panel->render(), $top_panel->render(),
$table_of_contents->render(), $table_of_contents->render(),
$details->render(), $details->render(),
)). )),
'</div>',
array( array(
'title' => 'Diff View', 'title' => 'Diff View',
)); ));

View File

@@ -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/storage/diff');
phutil_require_module('phabricator', 'applications/differential/view/changesetlistview'); phutil_require_module('phabricator', 'applications/differential/view/changesetlistview');
phutil_require_module('phabricator', 'applications/differential/view/difftableofcontents'); 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', 'infrastructure/env');
phutil_require_module('phabricator', 'view/form/base'); phutil_require_module('phabricator', 'view/form/base');
phutil_require_module('phabricator', 'view/form/control/markup'); phutil_require_module('phabricator', 'view/form/control/markup');

View File

@@ -231,15 +231,16 @@ class DifferentialRevisionViewController extends DifferentialController {
$this->updateViewTime($user->getPHID(), $revision->getPHID()); $this->updateViewTime($user->getPHID(), $revision->getPHID());
return $this->buildStandardPageResponse( return $this->buildStandardPageResponse(
'<div class="differential-primary-pane">'. id(new DifferentialPrimaryPaneView())
->setLineWidthFromChangesets($changesets)
->appendChild(
$revision_detail->render(). $revision_detail->render().
$comment_view->render(). $comment_view->render().
$diff_history->render(). $diff_history->render().
$warning. $warning.
$toc_view->render(). $toc_view->render().
$changeset_view->render(). $changeset_view->render().
$comment_form->render(). $comment_form->render()),
'</div>',
array( array(
'title' => $revision->getTitle(), 'title' => $revision->getTitle(),
)); ));

View File

@@ -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/addcomment');
phutil_require_module('phabricator', 'applications/differential/view/changesetlistview'); phutil_require_module('phabricator', 'applications/differential/view/changesetlistview');
phutil_require_module('phabricator', 'applications/differential/view/difftableofcontents'); 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/revisioncommentlist');
phutil_require_module('phabricator', 'applications/differential/view/revisiondetail'); phutil_require_module('phabricator', 'applications/differential/view/revisiondetail');
phutil_require_module('phabricator', 'applications/differential/view/revisionupdatehistory'); phutil_require_module('phabricator', 'applications/differential/view/revisionupdatehistory');

View File

@@ -53,6 +53,8 @@ class DifferentialChangesetParser {
private $renderingReference; private $renderingReference;
private $isSubparser; private $isSubparser;
private $lineWidth = 80;
const CACHE_VERSION = 4; const CACHE_VERSION = 4;
const ATTR_GENERATED = 'attr:generated'; const ATTR_GENERATED = 'attr:generated';
@@ -118,13 +120,26 @@ class DifferentialChangesetParser {
return $this; 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() { private function getRenderCacheKey() {
return $this->renderCacheKey; return $this->renderCacheKey;
} }
public function setChangeset($changeset) { public function setChangeset($changeset) {
$this->changeset = $changeset; $this->changeset = $changeset;
$this->setFilename($changeset->getFilename()); $this->setFilename($changeset->getFilename());
$this->setLineWidth($changeset->getWordWrapWidth());
return $this; return $this;
} }
@@ -646,7 +661,7 @@ class DifferentialChangesetParser {
$text, $text,
$intra[$key]); $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]); $render[$key] = $this->lineWrap($render[$key]);
} }
} }
@@ -669,7 +684,7 @@ class DifferentialChangesetParser {
} else { } else {
++$c; ++$c;
} }
if ($c == 80) { if ($c == $this->lineWidth) {
$ins[] = ($ii + 1); $ins[] = ($ii + 1);
$c = 0; $c = 0;
} }

View File

@@ -169,4 +169,17 @@ class DifferentialChangeset extends DifferentialDAO {
return $path; 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;
}
} }

View File

@@ -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/base');
phutil_require_module('phabricator', 'applications/differential/storage/hunk'); phutil_require_module('phabricator', 'applications/differential/storage/hunk');
phutil_require_module('phabricator', 'applications/repository/constants/repositorytype'); phutil_require_module('phabricator', 'applications/repository/constants/repositorytype');
phutil_require_module('phabricator', 'infrastructure/env');
phutil_require_module('phutil', 'parser/uri'); phutil_require_module('phutil', 'parser/uri');
phutil_require_module('phutil', 'utils'); phutil_require_module('phutil', 'utils');

View File

@@ -117,6 +117,7 @@ final class DifferentialAddCommentView extends AphrontView {
$panel_view->addClass('aphront-panel-flush'); $panel_view->addClass('aphront-panel-flush');
return return
'<div class="differential-add-comment-panel">'.
$panel_view->render(). $panel_view->render().
'<div class="aphront-panel-preview aphront-panel-flush">'. '<div class="aphront-panel-preview aphront-panel-flush">'.
'<div id="comment-preview">'. '<div id="comment-preview">'.
@@ -126,6 +127,7 @@ final class DifferentialAddCommentView extends AphrontView {
'</div>'. '</div>'.
'<div id="inline-comment-preview">'. '<div id="inline-comment-preview">'.
'</div>'. '</div>'.
'</div>'.
'</div>'; '</div>';
} }
} }

View File

@@ -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());
}
}

View File

@@ -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');

View File

@@ -4,8 +4,13 @@
.differential-primary-pane { .differential-primary-pane {
margin: 0 40px; margin: 0 40px;
max-width: 1162px;
padding-bottom: 2em; 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 { .differential-panel {

View File

@@ -9,6 +9,14 @@
border: 1px solid transparent; border: 1px solid transparent;
} }
.differential-comment-list {
max-width: 1162px;
}
.differential-add-comment-panel {
max-width: 1162px;
}
.differential-comment-list .anchor-target { .differential-comment-list .anchor-target {
background-color: #ffffdd; background-color: #ffffdd;
border-color: #ffff00; border-color: #ffff00;