From ecc32e4d08c4692e3a338e3b93a59fbb49588856 Mon Sep 17 00:00:00 2001 From: tuomaspelkonen Date: Fri, 15 Apr 2011 14:25:23 -0700 Subject: [PATCH] Only table of contents are shown for large diffs by default. Summary: Differential used to only show diffs for the first 100 files. Now all the files are shown in the table of contents and there is a link to the standalone view for every file. The inline diffs can still be seen, if user clicks "Show All Files Inline". Inline comments can also be added in the standalone view, but there is no form to submit them. The revision page must be reloaded to able to submit the inline comment. Test Plan: Changed the limit to three for testing purposes and checked that a diff of mine with 5 files had the links to the standalone views. Made sure that adding a comment in a standalone view worked and that after reloading the revision page the comment was visible. Changed the limit back to 100 and made sure that my diff had all the files inline and that the anchor links were working. Reviewed By: jungejason Reviewers: jungejason CC: epriestley, simpkins, jungejason, tuomaspelkonen Differential Revision: 147 --- .../DifferentialChangesetViewController.php | 1 + .../DifferentialRevisionViewController.php | 13 ++-- .../DifferentialChangesetDetailView.php | 19 +++++- .../view/changesetdetailview/__init__.php | 1 + .../DifferentialDiffTableOfContentsView.php | 62 ++++++++++++++++--- .../view/difftableofcontents/__init__.php | 2 + 6 files changed, 85 insertions(+), 13 deletions(-) diff --git a/src/applications/differential/controller/changesetview/DifferentialChangesetViewController.php b/src/applications/differential/controller/changesetview/DifferentialChangesetViewController.php index f9cd71b188..1c1f09ca22 100644 --- a/src/applications/differential/controller/changesetview/DifferentialChangesetViewController.php +++ b/src/applications/differential/controller/changesetview/DifferentialChangesetViewController.php @@ -169,6 +169,7 @@ class DifferentialChangesetViewController extends DifferentialController { $detail = new DifferentialChangesetDetailView(); $detail->setChangeset($changeset); $detail->appendChild($output); + $detail->setRevisionID($request->getInt('revision_id')); $output = '
'. diff --git a/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php b/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php index 4ca1045f4a..d7d5fa59ec 100644 --- a/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/revisionview/DifferentialRevisionViewController.php @@ -99,19 +99,19 @@ class DifferentialRevisionViewController extends DifferentialController { $warning->setSeverity(AphrontErrorView::SEVERITY_WARNING); $warning->setWidth(AphrontErrorView::WIDTH_WIDE); $warning->appendChild( - "

This diff is very large and affects {$count} files. Only ". - "the first {$limit} files are shown. ". + "

This diff is very large and affects {$count} files. Use ". + "Table of Contents to open files in a standalone view. ". "". phutil_render_tag( 'a', array( 'href' => $request_uri->alter('large', 'true'), ), - 'Show All Files'). + 'Show All Files Inline'). ""); $warning = $warning->render(); - $visible_changesets = array_slice($changesets, 0, $limit, true); + $visible_changesets = array(); } else { $warning = null; $visible_changesets = $changesets; @@ -176,6 +176,9 @@ class DifferentialRevisionViewController extends DifferentialController { $toc_view = new DifferentialDiffTableOfContentsView(); $toc_view->setChangesets($changesets); + $toc_view->setStandaloneViewLink(empty($visible_changesets)); + $toc_view->setVsMap($vs_map); + $toc_view->setRevisionID($revision->getID()); $changeset_view = new DifferentialChangesetListView(); $changeset_view->setChangesets($visible_changesets); @@ -205,8 +208,8 @@ class DifferentialRevisionViewController extends DifferentialController { $revision_detail->render(). $comment_view->render(). $diff_history->render(). - $toc_view->render(). $warning. + $toc_view->render(). $changeset_view->render(). $comment_form->render(). '

', diff --git a/src/applications/differential/view/changesetdetailview/DifferentialChangesetDetailView.php b/src/applications/differential/view/changesetdetailview/DifferentialChangesetDetailView.php index 8f1ac14a27..9a2ca3517c 100644 --- a/src/applications/differential/view/changesetdetailview/DifferentialChangesetDetailView.php +++ b/src/applications/differential/view/changesetdetailview/DifferentialChangesetDetailView.php @@ -20,6 +20,7 @@ class DifferentialChangesetDetailView extends AphrontView { private $changeset; private $buttons = array(); + private $revisionID; public function setChangeset($changeset) { $this->changeset = $changeset; @@ -31,11 +32,20 @@ class DifferentialChangesetDetailView extends AphrontView { return $this; } + public function setRevisionID($revision_id) { + $this->revisionID = $revision_id; + return $this; + } + public function render() { require_celerity_resource('differential-changeset-view-css'); require_celerity_resource('syntax-highlighting-css'); - $edit = false; // TODO + if ($this->revisionID) { + $edit = true; + } else { + $edit = false; + } $changeset = $this->changeset; $class = 'differential-changeset'; @@ -65,6 +75,13 @@ class DifferentialChangesetDetailView extends AphrontView { '
'. $this->renderChildren()); + if ($edit) { + Javelin::initBehavior( + 'differential-edit-inline-comments', array( + 'uri' => '/differential/comment/inline/edit/'.$this->revisionID.'/', + )); + } + return $output; } diff --git a/src/applications/differential/view/changesetdetailview/__init__.php b/src/applications/differential/view/changesetdetailview/__init__.php index 4e50ea0f06..783a68af76 100644 --- a/src/applications/differential/view/changesetdetailview/__init__.php +++ b/src/applications/differential/view/changesetdetailview/__init__.php @@ -7,6 +7,7 @@ phutil_require_module('phabricator', 'infrastructure/celerity/api'); +phutil_require_module('phabricator', 'infrastructure/javelin/api'); phutil_require_module('phabricator', 'infrastructure/javelin/markup'); phutil_require_module('phabricator', 'view/base'); diff --git a/src/applications/differential/view/difftableofcontents/DifferentialDiffTableOfContentsView.php b/src/applications/differential/view/difftableofcontents/DifferentialDiffTableOfContentsView.php index 96adacee77..d67e6bbece 100644 --- a/src/applications/differential/view/difftableofcontents/DifferentialDiffTableOfContentsView.php +++ b/src/applications/differential/view/difftableofcontents/DifferentialDiffTableOfContentsView.php @@ -19,12 +19,31 @@ final class DifferentialDiffTableOfContentsView extends AphrontView { private $changesets = array(); + private $standaloneViewLink = null; + private $renderURI = '/differential/changeset/'; + private $revisionID; public function setChangesets($changesets) { $this->changesets = $changesets; return $this; } + public function setStandaloneViewLink($standalone_view_link) { + $this->standaloneViewLink = $standalone_view_link; + return $this; + } + + public function setVsMap(array $vs_map) { + $this->vsMap = $vs_map; + return $this; + } + + public function setRevisionID($revision_id) { + $this->revisionID = $revision_id; + return $this; + } + + public function render() { require_celerity_resource('differential-core-view-css'); @@ -62,12 +81,40 @@ final class DifferentialDiffTableOfContentsView extends AphrontView { } } } else { - $link = phutil_render_tag( - 'a', - array( - 'href' => '#'.$changeset->getAnchorName(), - ), - phutil_escape_html($display_file)); + + if ($this->standaloneViewLink) { + $id = $changeset->getID(); + if ($id) { + $vs_id = idx($this->vsMap, $id); + } else { + $vs_id = null; + } + $ref = $changeset->getRenderingReference(); + $detail_uri = new PhutilURI($this->renderURI); + $detail_uri->setQueryParams( + array( + 'id' => $ref, + 'vs' => $vs_id, + 'whitespace' => 'TODO', + 'revision_id' => $this->revisionID, + )); + + $link = phutil_render_tag( + 'a', + array( + 'href' => $detail_uri, + 'target' => '_blank', + ), + phutil_escape_html($display_file)); + } else { + $link = phutil_render_tag( + 'a', + array( + 'href' => '#'.$changeset->getAnchorName(), + ), + phutil_escape_html($display_file)); + } + if ($type == DifferentialChangeType::TYPE_MOVE_HERE) { $meta = 'Moved from '.phutil_escape_html($changeset->getOldFile()); } else if ($type == DifferentialChangeType::TYPE_COPY_HERE) { @@ -99,7 +146,8 @@ final class DifferentialDiffTableOfContentsView extends AphrontView { $rows[] = ''. - ''.$char.''. + ''.$char. + ''. ''.$pchar.''. ''.$desc.''. ''.$link.$lines.''. diff --git a/src/applications/differential/view/difftableofcontents/__init__.php b/src/applications/differential/view/difftableofcontents/__init__.php index a29530d370..9d378f861d 100644 --- a/src/applications/differential/view/difftableofcontents/__init__.php +++ b/src/applications/differential/view/difftableofcontents/__init__.php @@ -11,6 +11,8 @@ phutil_require_module('phabricator', 'infrastructure/celerity/api'); phutil_require_module('phabricator', 'view/base'); phutil_require_module('phutil', 'markup'); +phutil_require_module('phutil', 'parser/uri'); +phutil_require_module('phutil', 'utils'); phutil_require_source('DifferentialDiffTableOfContentsView.php');