From 79218b6e470db11e95c6702070a126fc594a97fc Mon Sep 17 00:00:00 2001 From: vrana Date: Tue, 24 Jan 2012 00:37:50 -0800 Subject: [PATCH] Factor out DifferentialDiffTableOfContentsView::renderChangesetLink() Summary: Links from lint errors for large diffs don't work. This diff adds TODO for it because I am not sure how to do it. Move of changeset links rendering to a separate method would be still useful. Test Plan: Display ToC of large diff, verify link. Repeat for small diff. Reviewers: tuomaspelkonen, epriestley Reviewed By: epriestley CC: aran, epriestley Differential Revision: https://secure.phabricator.com/D1476 --- .../DifferentialLintFieldSpecification.php | 2 + .../DifferentialDiffTableOfContentsView.php | 69 +++++++++---------- 2 files changed, 35 insertions(+), 36 deletions(-) diff --git a/src/applications/differential/field/specification/lint/DifferentialLintFieldSpecification.php b/src/applications/differential/field/specification/lint/DifferentialLintFieldSpecification.php index 318cae1036..431f260db7 100644 --- a/src/applications/differential/field/specification/lint/DifferentialLintFieldSpecification.php +++ b/src/applications/differential/field/specification/lint/DifferentialLintFieldSpecification.php @@ -56,6 +56,8 @@ final class DifferentialLintFieldSpecification $line_link = phutil_escape_html($line); if (isset($path_changesets[$path])) { + // TODO: Create standalone links for large diffs. Logic is in + // DifferentialDiffTableOfContentsView::renderChangesetLink(). $line_link = phutil_render_tag( 'a', array( diff --git a/src/applications/differential/view/difftableofcontents/DifferentialDiffTableOfContentsView.php b/src/applications/differential/view/difftableofcontents/DifferentialDiffTableOfContentsView.php index 0c5ba187cb..ce1180c3dd 100644 --- a/src/applications/differential/view/difftableofcontents/DifferentialDiffTableOfContentsView.php +++ b/src/applications/differential/view/difftableofcontents/DifferentialDiffTableOfContentsView.php @@ -58,44 +58,9 @@ final class DifferentialDiffTableOfContentsView extends AphrontView { $changesets = $this->changesets; foreach ($changesets as $changeset) { - $file = $changeset->getFilename(); - $display_file = $changeset->getDisplayFilename(); - $type = $changeset->getChangeType(); $ftype = $changeset->getFileType(); - - if ($this->standaloneViewLink) { - $id = $changeset->getID(); - if ($id) { - $vs_id = idx($this->vsMap, $id); - } else { - $vs_id = null; - } - - $ref = $vs_id ? $id.'/'.$vs_id : $id; - $detail_uri = new PhutilURI($this->renderURI); - $detail_uri->setQueryParams( - array( - 'ref' => $ref, - 'whitespace' => $this->whitespace, - '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)); - } + $link = $this->renderChangesetLink($changeset); if (DifferentialChangeType::isOldLocationChangeType($type)) { $away = $changeset->getAwayPaths(); @@ -170,4 +135,36 @@ final class DifferentialDiffTableOfContentsView extends AphrontView { ''. ''; } + + private function renderChangesetLink(DifferentialChangeset $changeset) { + $display_file = $changeset->getDisplayFilename(); + + if ($this->standaloneViewLink) { + $id = $changeset->getID(); + $vs_id = idx($this->vsMap, $id); + + $ref = $vs_id ? $id.'/'.$vs_id : $id; + $detail_uri = new PhutilURI($this->renderURI); + $detail_uri->setQueryParams( + array( + 'ref' => $ref, + 'whitespace' => $this->whitespace, + 'revision_id' => $this->revisionID, + )); + + return phutil_render_tag( + 'a', + array( + 'href' => $detail_uri, + 'target' => '_blank', + ), + phutil_escape_html($display_file)); + } + return phutil_render_tag( + 'a', + array( + 'href' => '#'.$changeset->getAnchorName(), + ), + phutil_escape_html($display_file)); + } }