diff --git a/resources/celerity/map.php b/resources/celerity/map.php index d9c1bcda8d..eef8a2b476 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -11,7 +11,7 @@ return array( 'core.pkg.js' => 'a77025a1', 'darkconsole.pkg.js' => '8ab24e01', 'differential.pkg.css' => 'd8866ed8', - 'differential.pkg.js' => '7b5a4aa4', + 'differential.pkg.js' => '9e55f9f5', 'diffusion.pkg.css' => '591664fa', 'diffusion.pkg.js' => 'bfc0737b', 'maniphest.pkg.css' => '68d4dd3d', @@ -360,18 +360,18 @@ return array( 'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => '82439934', 'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '453c5375', 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => 'd4eecc63', - 'rsrc/js/application/differential/ChangesetViewManager.js' => 'c024db3d', - 'rsrc/js/application/differential/DifferentialInlineCommentEditor.js' => 'f2441746', + 'rsrc/js/application/differential/ChangesetViewManager.js' => 'fce415a0', + 'rsrc/js/application/differential/DifferentialInlineCommentEditor.js' => '6a049cf7', 'rsrc/js/application/differential/behavior-add-reviewers-and-ccs.js' => 'e10f8e18', 'rsrc/js/application/differential/behavior-comment-jump.js' => '4fdb476d', 'rsrc/js/application/differential/behavior-comment-preview.js' => '6932def3', 'rsrc/js/application/differential/behavior-diff-radios.js' => 'e1ff79b1', - 'rsrc/js/application/differential/behavior-dropdown-menus.js' => 'e33d4bc5', + 'rsrc/js/application/differential/behavior-dropdown-menus.js' => '2035b9cb', 'rsrc/js/application/differential/behavior-edit-inline-comments.js' => '65936067', 'rsrc/js/application/differential/behavior-keyboard-nav.js' => '2c426492', 'rsrc/js/application/differential/behavior-populate.js' => 'bdb3e4d0', 'rsrc/js/application/differential/behavior-show-field-details.js' => 'bba9eedf', - 'rsrc/js/application/differential/behavior-show-more.js' => '954d2de0', + 'rsrc/js/application/differential/behavior-show-more.js' => 'c662904a', 'rsrc/js/application/differential/behavior-toggle-files.js' => 'ca3f91eb', 'rsrc/js/application/differential/behavior-user-select.js' => 'a8d8459d', 'rsrc/js/application/diffusion/DiffusionLocateFileSource.js' => 'b42eddc7', @@ -510,7 +510,7 @@ return array( 'aphront-two-column-view-css' => '16ab3ad2', 'aphront-typeahead-control-css' => '0e403212', 'auth-css' => '1e655982', - 'changeset-view-manager' => 'c024db3d', + 'changeset-view-manager' => 'fce415a0', 'config-options-css' => '7fedf08b', 'config-welcome-css' => '6abd79be', 'conpherence-durable-column-view' => '3b836442', @@ -521,7 +521,7 @@ return array( 'conpherence-widget-pane-css' => '3d575438', 'differential-changeset-view-css' => 'b600950c', 'differential-core-view-css' => '7ac3cabc', - 'differential-inline-comment-editor' => 'f2441746', + 'differential-inline-comment-editor' => '6a049cf7', 'differential-results-table-css' => '181aa9d9', 'differential-revision-add-comment-css' => 'c478bcaa', 'differential-revision-comment-css' => '48186045', @@ -569,13 +569,13 @@ return array( 'javelin-behavior-differential-add-reviewers-and-ccs' => 'e10f8e18', 'javelin-behavior-differential-comment-jump' => '4fdb476d', 'javelin-behavior-differential-diff-radios' => 'e1ff79b1', - 'javelin-behavior-differential-dropdown-menus' => 'e33d4bc5', + 'javelin-behavior-differential-dropdown-menus' => '2035b9cb', 'javelin-behavior-differential-edit-inline-comments' => '65936067', 'javelin-behavior-differential-feedback-preview' => '6932def3', 'javelin-behavior-differential-keyboard-navigation' => '2c426492', 'javelin-behavior-differential-populate' => 'bdb3e4d0', 'javelin-behavior-differential-show-field-details' => 'bba9eedf', - 'javelin-behavior-differential-show-more' => '954d2de0', + 'javelin-behavior-differential-show-more' => 'c662904a', 'javelin-behavior-differential-toggle-files' => 'ca3f91eb', 'javelin-behavior-differential-user-select' => 'a8d8459d', 'javelin-behavior-diffusion-commit-branches' => 'bdaf4d04', @@ -944,6 +944,18 @@ return array( 'javelin-dom', 'javelin-reactor-dom', ), + '2035b9cb' => array( + 'javelin-behavior', + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-workflow', + 'phuix-dropdown-menu', + 'phuix-action-list-view', + 'phuix-action-view', + 'phabricator-phtize', + 'changeset-view-manager', + ), '2290aeef' => array( 'javelin-install', 'javelin-dom', @@ -1236,6 +1248,14 @@ return array( '69adf288' => array( 'javelin-install', ), + '6a049cf7' => array( + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-install', + 'javelin-request', + 'javelin-workflow', + ), '6c2b09a2' => array( 'javelin-install', 'javelin-util', @@ -1534,13 +1554,6 @@ return array( 'javelin-resource', 'javelin-routable', ), - '954d2de0' => array( - 'javelin-behavior', - 'javelin-dom', - 'javelin-workflow', - 'javelin-util', - 'javelin-stratcom', - ), '988040b4' => array( 'javelin-install', 'javelin-dom', @@ -1704,16 +1717,6 @@ return array( 'javelin-util', 'phabricator-shaped-request', ), - 'c024db3d' => array( - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-install', - 'javelin-workflow', - 'javelin-router', - 'javelin-behavior-device', - 'javelin-vector', - ), 'c1700f6f' => array( 'javelin-install', 'javelin-util', @@ -1728,6 +1731,14 @@ return array( 'javelin-stratcom', 'javelin-vector', ), + 'c662904a' => array( + 'javelin-behavior', + 'javelin-dom', + 'javelin-workflow', + 'javelin-util', + 'javelin-stratcom', + 'changeset-view-manager', + ), 'c90a04fc' => array( 'javelin-dom', 'javelin-dynval', @@ -1827,18 +1838,6 @@ return array( 'javelin-workflow', 'javelin-vector', ), - 'e33d4bc5' => array( - 'javelin-behavior', - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-workflow', - 'phuix-dropdown-menu', - 'phuix-action-list-view', - 'phuix-action-view', - 'phabricator-phtize', - 'changeset-view-manager', - ), 'e379b58e' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1893,14 +1892,6 @@ return array( 'javelin-install', 'javelin-util', ), - 'f2441746' => array( - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-install', - 'javelin-request', - 'javelin-workflow', - ), 'f24f3253' => array( 'javelin-behavior', 'javelin-dom', @@ -1991,6 +1982,16 @@ return array( 'javelin-dom', 'phortune-credit-card-form', ), + 'fce415a0' => array( + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-install', + 'javelin-workflow', + 'javelin-router', + 'javelin-behavior-device', + 'javelin-vector', + ), 'fe287620' => array( 'javelin-install', 'javelin-dom', diff --git a/src/applications/differential/controller/DifferentialChangesetViewController.php b/src/applications/differential/controller/DifferentialChangesetViewController.php index 33597e0020..59691aa408 100644 --- a/src/applications/differential/controller/DifferentialChangesetViewController.php +++ b/src/applications/differential/controller/DifferentialChangesetViewController.php @@ -154,13 +154,8 @@ final class DifferentialChangesetViewController extends DifferentialController { $parser->setRenderCacheKey($render_cache_key); $parser->setRightSideCommentMapping($right_source, $right_new); $parser->setLeftSideCommentMapping($left_source, $left_new); - $parser->setWhitespaceMode($request->getStr('whitespace')); - $parser->setCharacterEncoding($request->getStr('encoding')); - $parser->setHighlightAs($request->getStr('highlight')); - if ($request->getStr('renderer') == '1up') { - $parser->setRenderer(new DifferentialChangesetOneUpRenderer()); - } + $parser->readParametersFromRequest($request); if ($left && $right) { $parser->setOriginals($left, $right); @@ -235,6 +230,9 @@ final class DifferentialChangesetViewController extends DifferentialController { $detail = id(new DifferentialChangesetDetailView()) ->setUser($this->getViewer()) ->setChangeset($changeset) + ->setRenderingRef($rendering_reference) + ->setRenderURI('/differential/changeset/') + ->setRenderer($parser->getRenderer()->getRendererKey()) ->appendChild($output) ->setVsChangesetID($left_source); @@ -242,11 +240,7 @@ final class DifferentialChangesetViewController extends DifferentialController { 'changesetViewIDs' => array($detail->getID()), )); - Javelin::initBehavior('differential-show-more', array( - 'uri' => '/differential/changeset/', - 'whitespace' => $request->getStr('whitespace'), - )); - + Javelin::initBehavior('differential-show-more'); Javelin::initBehavior('differential-comment-jump', array()); $panel = new DifferentialPrimaryPaneView(); diff --git a/src/applications/differential/parser/DifferentialChangesetParser.php b/src/applications/differential/parser/DifferentialChangesetParser.php index 7d979beddc..cd689153f0 100644 --- a/src/applications/differential/parser/DifferentialChangesetParser.php +++ b/src/applications/differential/parser/DifferentialChangesetParser.php @@ -92,6 +92,44 @@ final class DifferentialChangesetParser { return $this->disableCache; } + public static function getDefaultRendererForViewer(PhabricatorUser $viewer) { + $prefs = $viewer->loadPreferences(); + $pref_unified = PhabricatorUserPreferences::PREFERENCE_DIFF_UNIFIED; + if ($prefs->getPreference($pref_unified) == 'unified') { + return '1up'; + } + return null; + } + + public function readParametersFromRequest(AphrontRequest $request) { + $this->setWhitespaceMode($request->getStr('whitespace')); + $this->setCharacterEncoding($request->getStr('encoding')); + $this->setHighlightAs($request->getStr('highlight')); + + $renderer = null; + + // If the viewer prefers unified diffs, always set the renderer to unified. + // Otherwise, we leave it unspecified and the client will choose a + // renderer based on the screen size. + + if ($request->getStr('renderer')) { + $renderer = $request->getStr('renderer'); + } else { + $renderer = self::getDefaultRendererForViewer($request->getViewer()); + } + + switch ($renderer) { + case '1up': + $this->setRenderer(new DifferentialChangesetOneUpRenderer()); + break; + default: + $this->setRenderer(new DifferentialChangesetTwoUpRenderer()); + break; + } + + return $this; + } + const CACHE_VERSION = 11; const CACHE_MAX_SIZE = 8e6; diff --git a/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php b/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php index b4ef3b640a..b03497560b 100644 --- a/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php @@ -519,7 +519,6 @@ abstract class DifferentialChangesetHTMLRenderer 'sigil' => 'show-more', 'meta' => array( 'type' => ($is_all ? 'all' : null), - 'ref' => $reference, 'range' => $range, ), ), diff --git a/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php b/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php index e3d64f872c..cdefbc303b 100644 --- a/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php @@ -11,6 +11,10 @@ final class DifferentialChangesetOneUpRenderer return 'diff-1up'; } + public function getRendererKey() { + return '1up'; + } + protected function renderColgroup() { return phutil_tag('colgroup', array(), array( phutil_tag('col', array('class' => 'num')), diff --git a/src/applications/differential/render/DifferentialChangesetOneUpTestRenderer.php b/src/applications/differential/render/DifferentialChangesetOneUpTestRenderer.php index b6ea30e51f..a9afb43af8 100644 --- a/src/applications/differential/render/DifferentialChangesetOneUpTestRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetOneUpTestRenderer.php @@ -7,4 +7,8 @@ final class DifferentialChangesetOneUpTestRenderer return true; } + public function getRendererKey() { + return '1up-test'; + } + } diff --git a/src/applications/differential/render/DifferentialChangesetRenderer.php b/src/applications/differential/render/DifferentialChangesetRenderer.php index be440b47e7..09eb27ab47 100644 --- a/src/applications/differential/render/DifferentialChangesetRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetRenderer.php @@ -34,6 +34,8 @@ abstract class DifferentialChangesetRenderer { private $oldFile = false; private $newFile = false; + abstract public function getRendererKey(); + public function setShowEditAndReplyLinks($bool) { $this->showEditAndReplyLinks = $bool; return $this; diff --git a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php index 1129330045..682bcd51e4 100644 --- a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php @@ -11,6 +11,10 @@ final class DifferentialChangesetTwoUpRenderer return 'diff-2up'; } + public function getRendererKey() { + return '2up'; + } + protected function renderColgroup() { return phutil_tag('colgroup', array(), array( phutil_tag('col', array('class' => 'num')), diff --git a/src/applications/differential/render/DifferentialChangesetTwoUpTestRenderer.php b/src/applications/differential/render/DifferentialChangesetTwoUpTestRenderer.php index e144577db9..226758b9fc 100644 --- a/src/applications/differential/render/DifferentialChangesetTwoUpTestRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetTwoUpTestRenderer.php @@ -7,4 +7,8 @@ final class DifferentialChangesetTwoUpTestRenderer return false; } + public function getRendererKey() { + return '2up-test'; + } + } diff --git a/src/applications/differential/view/DifferentialChangesetDetailView.php b/src/applications/differential/view/DifferentialChangesetDetailView.php index 6857fe6336..7723a7be7d 100644 --- a/src/applications/differential/view/DifferentialChangesetDetailView.php +++ b/src/applications/differential/view/DifferentialChangesetDetailView.php @@ -12,6 +12,7 @@ final class DifferentialChangesetDetailView extends AphrontView { private $whitespace; private $renderingRef; private $autoload; + private $renderer; public function setAutoload($autoload) { $this->autoload = $autoload; @@ -69,6 +70,15 @@ final class DifferentialChangesetDetailView extends AphrontView { return $this; } + public function setRenderer($renderer) { + $this->renderer = $renderer; + return $this; + } + + public function getRenderer() { + return $this->renderer; + } + public function getID() { if (!$this->id) { $this->id = celerity_generate_unique_node_id(); @@ -188,19 +198,6 @@ final class DifferentialChangesetDetailView extends AphrontView { $icon = id(new PHUIIconView()) ->setIconFont($display_icon); - $renderer = null; - - // If the viewer prefers unified diffs, always set the renderer to unified. - // Otherwise, we leave it unspecified and the client will choose a - // renderer based on the screen size. - - $viewer = $this->getUser(); - $prefs = $viewer->loadPreferences(); - $pref_unified = PhabricatorUserPreferences::PREFERENCE_DIFF_UNIFIED; - if ($prefs->getPreference($pref_unified) == 'unified') { - $renderer = '1up'; - } - return javelin_tag( 'div', array( @@ -213,7 +210,7 @@ final class DifferentialChangesetDetailView extends AphrontView { 'renderURI' => $this->getRenderURI(), 'whitespace' => $this->getWhitespace(), 'highlight' => null, - 'renderer' => $renderer, + 'renderer' => $this->getRenderer(), 'ref' => $this->getRenderingRef(), 'autoload' => $this->getAutoload(), ), diff --git a/src/applications/differential/view/DifferentialChangesetListView.php b/src/applications/differential/view/DifferentialChangesetListView.php index 4ec0e19a3f..e63c3d613f 100644 --- a/src/applications/differential/view/DifferentialChangesetListView.php +++ b/src/applications/differential/view/DifferentialChangesetListView.php @@ -136,6 +136,9 @@ final class DifferentialChangesetListView extends AphrontView { ), )); + $renderer = DifferentialChangesetParser::getDefaultRendererForViewer( + $this->getUser()); + $output = array(); $ids = array(); foreach ($changesets as $key => $changeset) { @@ -169,6 +172,7 @@ final class DifferentialChangesetListView extends AphrontView { $detail->setRenderURI($this->renderURI); $detail->setWhitespace($this->whitespace); + $detail->setRenderer($renderer); if (isset($this->visibleChangesets[$key])) { $load = 'Loading...'; @@ -205,11 +209,7 @@ final class DifferentialChangesetListView extends AphrontView { 'changesetViewIDs' => $ids, )); - $this->initBehavior('differential-show-more', array( - 'uri' => $this->renderURI, - 'whitespace' => $this->whitespace, - )); - + $this->initBehavior('differential-show-more'); $this->initBehavior('differential-comment-jump', array()); if ($this->inlineURI) { diff --git a/src/applications/phriction/controller/PhrictionDiffController.php b/src/applications/phriction/controller/PhrictionDiffController.php index dea82dcc04..7c4dbc0bc8 100644 --- a/src/applications/phriction/controller/PhrictionDiffController.php +++ b/src/applications/phriction/controller/PhrictionDiffController.php @@ -97,6 +97,8 @@ final class PhrictionDiffController extends PhrictionController { $output = id(new DifferentialChangesetDetailView()) ->setUser($this->getViewer()) ->setChangeset($changeset) + ->setRenderingRef("{$l},{$r}") + ->setRenderURI('/phriction/diff/'.$document->getID().'/') ->appendChild($output); require_celerity_resource('differential-changeset-view-css'); @@ -106,11 +108,7 @@ final class PhrictionDiffController extends PhrictionController { Javelin::initBehavior('differential-populate', array( 'changesetViewIDs' => array($output->getID()), )); - - Javelin::initBehavior('differential-show-more', array( - 'uri' => '/phriction/diff/'.$document->getID().'/', - 'whitespace' => $whitespace_mode, - )); + Javelin::initBehavior('differential-show-more'); $slug = $document->getSlug(); diff --git a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php index a2ffe78639..31881310d0 100644 --- a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php +++ b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php @@ -948,8 +948,8 @@ final class PhabricatorUSEnglishTranslation ), "\xE2\x96\xB2 Show %d Line(s)" => array( - "\xE2\x96\xB2 Show %d Line(s)", - "\xE2\x96\xB2 Show %d Line(s)", + "\xE2\x96\xB2 Show Line", + "\xE2\x96\xB2 Show %d Lines", ), 'Show All %d Line(s)' => array( diff --git a/webroot/rsrc/js/application/differential/ChangesetViewManager.js b/webroot/rsrc/js/application/differential/ChangesetViewManager.js index a18eeda8f1..36f097cf49 100644 --- a/webroot/rsrc/js/application/differential/ChangesetViewManager.js +++ b/webroot/rsrc/js/application/differential/ChangesetViewManager.js @@ -118,25 +118,12 @@ JX.install('ChangesetViewManager', { this._loaded = true; this._sequence++; - var params = { - ref: this._ref, - whitespace: this._whitespace || '', - renderer: this.getRenderer() || '', - highlight: this._highlight || '', - encoding: this._encoding || '' - }; + var params = this._getViewParameters(); var workflow = new JX.Workflow(this._renderURI, params) .setHandler(JX.bind(this, this._onresponse, this._sequence)); - var routable = workflow.getRoutable(); - - routable - .setPriority(500) - .setType('content') - .setKey(this._getRoutableKey()); - - JX.Router.getInstance().queue(routable); + this._startContentWorkflow(workflow); JX.DOM.setContent( this._getContentFrame(), @@ -148,6 +135,95 @@ JX.install('ChangesetViewManager', { return this; }, + /** + * Load missing context in a changeset. + * + * We do this when the user clicks "Show X Lines". We also expand all of + * the missing context when they "Show Entire File". + * + * @param string Line range specification, like "0-40/0-20". + * @param node Row where the context should be rendered after loading. + * @param bool True if this is a bulk load of multiple context blocks. + * @return this + */ + loadContext: function(range, target, bulk) { + var params = this._getViewParameters(); + params.range = range; + + var container = JX.DOM.scry(target, 'td')[0]; + // TODO: pht() + JX.DOM.setContent(container, 'Loading...'); + JX.DOM.alterClass(target, 'differential-show-more-loading', true); + + var workflow = new JX.Workflow(this._renderURI, params) + .setHandler(JX.bind(this, this._oncontext, target)); + + if (bulk) { + // If we're loading a bunch of these because the viewer clicked + // "Show Entire File Content" or similar, use lower-priority requests + // and draw a progress bar. + this._startContentWorkflow(workflow); + } else { + // If this is a single click on a context link, use a higher priority + // load without a chrome change. + workflow.start(); + } + + return this; + }, + + _startContentWorkflow: function(workflow) { + var routable = workflow.getRoutable(); + + routable + .setPriority(500) + .setType('content') + .setKey(this._getRoutableKey()); + + JX.Router.getInstance().queue(routable); + }, + + + /** + * Receive a response to a context request. + */ + _oncontext: function(target, response) { + var table = JX.$H(response.changeset).getNode(); + var root = target.parentNode; + this._moveRows(table, root, target); + root.removeChild(target); + }, + + _moveRows: function(src, dst, before) { + var rows = JX.DOM.scry(src, 'tr'); + for (var ii = 0; ii < rows.length; ii++) { + + // Find the table this