Page MenuHomePhabricator
Files F15383420

D18112.id43576.diff
No OneTemporary
Actions

D18112.id43576.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -547,6 +547,7 @@
'DifferentialRevisionHeraldField' => 'applications/differential/herald/DifferentialRevisionHeraldField.php',
'DifferentialRevisionHeraldFieldGroup' => 'applications/differential/herald/DifferentialRevisionHeraldFieldGroup.php',
'DifferentialRevisionIDCommitMessageField' => 'applications/differential/field/DifferentialRevisionIDCommitMessageField.php',
+ 'DifferentialRevisionInlinesController' => 'applications/differential/controller/DifferentialRevisionInlinesController.php',
'DifferentialRevisionLandController' => 'applications/differential/controller/DifferentialRevisionLandController.php',
'DifferentialRevisionListController' => 'applications/differential/controller/DifferentialRevisionListController.php',
'DifferentialRevisionListView' => 'applications/differential/view/DifferentialRevisionListView.php',
@@ -1737,6 +1738,7 @@
'PHUIDiffInlineCommentTableScaffold' => 'infrastructure/diff/view/PHUIDiffInlineCommentTableScaffold.php',
'PHUIDiffInlineCommentUndoView' => 'infrastructure/diff/view/PHUIDiffInlineCommentUndoView.php',
'PHUIDiffInlineCommentView' => 'infrastructure/diff/view/PHUIDiffInlineCommentView.php',
+ 'PHUIDiffInlineThreader' => 'infrastructure/diff/view/PHUIDiffInlineThreader.php',
'PHUIDiffOneUpInlineCommentRowScaffold' => 'infrastructure/diff/view/PHUIDiffOneUpInlineCommentRowScaffold.php',
'PHUIDiffRevealIconView' => 'infrastructure/diff/view/PHUIDiffRevealIconView.php',
'PHUIDiffTableOfContentsItemView' => 'infrastructure/diff/view/PHUIDiffTableOfContentsItemView.php',
@@ -5515,6 +5517,7 @@
'DifferentialRevisionHeraldField' => 'HeraldField',
'DifferentialRevisionHeraldFieldGroup' => 'HeraldFieldGroup',
'DifferentialRevisionIDCommitMessageField' => 'DifferentialCommitMessageField',
+ 'DifferentialRevisionInlinesController' => 'DifferentialController',
'DifferentialRevisionLandController' => 'DifferentialController',
'DifferentialRevisionListController' => 'DifferentialController',
'DifferentialRevisionListView' => 'AphrontView',
@@ -6870,6 +6873,7 @@
'PHUIDiffInlineCommentTableScaffold' => 'AphrontView',
'PHUIDiffInlineCommentUndoView' => 'PHUIDiffInlineCommentView',
'PHUIDiffInlineCommentView' => 'AphrontView',
+ 'PHUIDiffInlineThreader' => 'Phobject',
'PHUIDiffOneUpInlineCommentRowScaffold' => 'PHUIDiffInlineCommentRowScaffold',
'PHUIDiffRevealIconView' => 'AphrontView',
'PHUIDiffTableOfContentsItemView' => 'AphrontView',
diff --git a/src/applications/base/controller/PhabricatorController.php b/src/applications/base/controller/PhabricatorController.php
--- a/src/applications/base/controller/PhabricatorController.php
+++ b/src/applications/base/controller/PhabricatorController.php
@@ -483,8 +483,10 @@
// NOTE: Applications (objects of class PhabricatorApplication) can't
// currently be set here, although they don't need any of the extensions
// anyway. This should probably work differently than it does, though.
- if ($object instanceof PhabricatorLiskDAO) {
- $action_list->setObject($object);
+ if ($object) {
+ if ($object instanceof PhabricatorLiskDAO) {
+ $action_list->setObject($object);
+ }
}
$curtain = id(new PHUICurtainView())
diff --git a/src/applications/differential/application/PhabricatorDifferentialApplication.php b/src/applications/differential/application/PhabricatorDifferentialApplication.php
--- a/src/applications/differential/application/PhabricatorDifferentialApplication.php
+++ b/src/applications/differential/application/PhabricatorDifferentialApplication.php
@@ -77,6 +77,8 @@
=> 'DifferentialDiffCreateController',
'operation/(?P<id>[1-9]\d*)/'
=> 'DifferentialRevisionOperationController',
+ 'inlines/(?P<id>[1-9]\d*)/'
+ => 'DifferentialRevisionInlinesController',
),
'comment/' => array(
'preview/(?P<id>[1-9]\d*)/' => 'DifferentialCommentPreviewController',
diff --git a/src/applications/differential/controller/DifferentialRevisionInlinesController.php b/src/applications/differential/controller/DifferentialRevisionInlinesController.php
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/controller/DifferentialRevisionInlinesController.php
@@ -0,0 +1,325 @@
+<?php
+
+final class DifferentialRevisionInlinesController
+ extends DifferentialController {
+
+ public function shouldAllowPublic() {
+ return true;
+ }
+
+ public function handleRequest(AphrontRequest $request) {
+ $viewer = $this->getViewer();
+ $id = $request->getURIData('id');
+
+ $revision = id(new DifferentialRevisionQuery())
+ ->withIDs(array($id))
+ ->setViewer($viewer)
+ ->needDiffIDs(true)
+ ->executeOne();
+ if (!$revision) {
+ return new Aphront404Response();
+ }
+
+ $revision_monogram = $revision->getMonogram();
+ $revision_uri = $revision->getURI();
+ $revision_title = $revision->getTitle();
+
+ $query = id(new DifferentialInlineCommentQuery())
+ ->setViewer($viewer)
+ ->needHidden(true)
+ ->withRevisionPHIDs(array($revision->getPHID()));
+ $inlines = $query->execute();
+
+ $crumbs = $this->buildApplicationCrumbs();
+ $crumbs->addTextCrumb($revision_monogram, $revision_uri);
+ $crumbs->addTextCrumb(pht('Inline Comments'));
+ $crumbs->setBorder(true);
+
+ $content = $this->renderInlineTable($revision, $inlines);
+ $header = $this->buildHeader($revision);
+
+ $view = id(new PHUITwoColumnView())
+ ->setHeader($header)
+ ->setFooter($content);
+
+ return $this->newPage()
+ ->setTitle(
+ array(
+ "{$revision_monogram} {$revision_title}",
+ pht('Inlines'),
+ ))
+ ->setCrumbs($crumbs)
+ ->appendChild($view);
+ }
+
+ private function buildHeader(DifferentialRevision $revision) {
+ $viewer = $this->getViewer();
+
+ $button = id(new PHUIButtonView())
+ ->setTag('a')
+ ->setIcon('fa-chevron-left')
+ ->setHref($revision->getURI())
+ ->setText(pht('Back to Revision'));
+
+ return id(new PHUIHeaderView())
+ ->setHeader($revision->getTitle())
+ ->setUser($viewer)
+ ->setHeaderIcon('fa-cog')
+ ->addActionLink($button);
+ }
+
+ private function renderInlineTable(
+ DifferentialRevision $revision,
+ array $inlines) {
+
+ $viewer = $this->getViewer();
+ $inlines = id(new PHUIDiffInlineThreader())
+ ->reorderAndThreadCommments($inlines);
+
+ $handle_phids = array();
+ $changeset_ids = array();
+ foreach ($inlines as $inline) {
+ $handle_phids[] = $inline->getAuthorPHID();
+ $changeset_ids[] = $inline->getChangesetID();
+ }
+ $handles = $viewer->loadHandles($handle_phids);
+ $handles = iterator_to_array($handles);
+
+ if ($changeset_ids) {
+ $changesets = id(new DifferentialChangesetQuery())
+ ->setViewer($viewer)
+ ->withIDs($changeset_ids)
+ ->execute();
+ $changesets = mpull($changesets, null, 'getID');
+ } else {
+ $changesets = array();
+ }
+
+ $current_changeset = head($revision->getDiffIDs());
+
+ $rows = array();
+ foreach ($inlines as $inline) {
+ $status_icons = array();
+
+ $c_id = $inline->getChangesetID();
+ $d_id = $changesets[$c_id]->getDiffID();
+
+ if ($d_id == $current_changeset) {
+ $diff_id = phutil_tag('strong', array(), pht('Current'));
+ } else {
+ $diff_id = pht('Diff %d', $d_id);
+ }
+
+ $reviewer = $handles[$inline->getAuthorPHID()]->renderLink();
+ $datetime = 0;
+
+ $comment_href = $revision->getURI().'#inline-'.$inline->getID();
+ $comment = phutil_tag(
+ 'a',
+ array(
+ 'href' => $comment_href,
+ ),
+ $inline->getContent());
+
+ $state = $inline->getFixedState();
+ if ($state == PhabricatorInlineCommentInterface::STATE_DONE) {
+ $status_icons[] = id(new PHUIIconView())
+ ->setIcon('fa-check green')
+ ->addClass('mmr');
+ } else if ($inline->getReplyToCommentPHID() &&
+ $inline->getAuthorPHID() == $revision->getAuthorPHID()) {
+ $status_icons[] = id(new PHUIIconView())
+ ->setIcon('fa-commenting-o blue')
+ ->addClass('mmr');
+ } else {
+ $status_icons[] = id(new PHUIIconView())
+ ->setIcon('fa-circle-o grey')
+ ->addClass('mmr');
+ }
+
+
+ if ($inline->getReplyToCommentPHID()) {
+ $reply_icon = id(new PHUIIconView())
+ ->setIcon('fa-reply mmr darkgrey');
+ $comment = array($reply_icon, $comment);
+ }
+
+ // TODO: These both fatal?
+ // $datetime = $inline->getDateModified();
+ // $datetime = $inline->getDateCreated();
+
+ $rows[] = array(
+ $diff_id,
+ $status_icons,
+ $reviewer,
+ AphrontTableView::renderSingleDisplayLine($comment),
+ $datetime,
+ );
+ }
+
+ $table = new AphrontTableView($rows);
+ $table->setHeaders(
+ array(
+ pht('Diff'),
+ pht('Status'),
+ pht('Reviewer'),
+ pht('Comment'),
+ pht('Created'),
+ ));
+ $table->setColumnClasses(
+ array(
+ '',
+ '',
+ '',
+ 'wide',
+ 'right',
+ ));
+ $table->setColumnVisibility(
+ array(
+ true,
+ true,
+ true,
+ true,
+ true,
+ ));
+
+ return id(new PHUIObjectBoxView())
+ ->setHeaderText(pht('Inline Comments'))
+ ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY)
+ ->setTable($table);
+ }
+
+ private function renderInlineThreads(
+ DifferentialRevision $revision,
+ array $inlines) {
+
+ $viewer = $this->getViewer();
+
+ $inlines = id(new PHUIDiffInlineThreader())
+ ->reorderAndThreadCommments($inlines);
+
+ $engine = id(new PhabricatorMarkupEngine())
+ ->setViewer($viewer);
+
+ $groups = array();
+ $group = array();
+ $handle_phids = array();
+ $changeset_ids = array();
+ foreach ($inlines as $inline) {
+ if ($inline->getReplyToCommentPHID() === null) {
+ if ($group) {
+ $groups[] = $group;
+ }
+ $group = array();
+ }
+ $group[] = $inline;
+
+ $engine->addObject(
+ $inline,
+ PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY);
+
+ $handle_phids[] = $inline->getAuthorPHID();
+ $changeset_ids[] = $inline->getChangesetID();
+ }
+
+ $engine->process();
+
+ if ($group) {
+ $groups[] = $group;
+ }
+
+ foreach ($groups as $key => $group) {
+ $groups[$key] = array(
+ 'inlines' => $group,
+ );
+ }
+
+ $handles = $viewer->loadHandles($handle_phids);
+ $handles = iterator_to_array($handles);
+
+ if ($changeset_ids) {
+ $changesets = id(new DifferentialChangesetQuery())
+ ->setViewer($viewer)
+ ->needHunks(true)
+ ->withIDs($changeset_ids)
+ ->execute();
+ $changesets = mpull($changesets, null, 'getID');
+ } else {
+ $changesets = array();
+ }
+
+ $out = array();
+ foreach ($groups as $key => $group) {
+ $thread = array();
+
+ $head = head($group['inlines']);
+ $changeset = idx($changesets, $head->getChangesetID());
+ if (!$changeset) {
+ continue;
+ }
+
+ $context = $this->renderContext($changeset, $head);
+
+ foreach ($group['inlines'] as $comment) {
+ $view = id(new PHUIDiffInlineCommentDetailView())
+ ->setViewer($viewer)
+ ->setInlineComment($comment)
+ ->setHandles($handles)
+ ->setEditable(false)
+ ->setAllowReply(false)
+ ->setCanMarkDone(false)
+ ->setMarkupEngine($engine)
+ ->setObjectOwnerPHID($revision->getAuthorPHID());
+
+ $thread[] = phutil_tag(
+ 'div',
+ array(
+ 'style' => 'padding: 8px;',
+ ),
+ $view);
+ }
+
+ $out[] = id(new PHUIObjectBoxView())
+ ->setHeaderText($changeset->getFilename())
+ ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY)
+ ->appendChild($context)
+ ->appendChild($thread);
+ }
+
+ return $out;
+ }
+
+ private function renderContext(
+ DifferentialChangeset $changeset,
+ $comment) {
+
+ $is_new = $comment->getIsNewFile();
+ $start = $comment->getLineNumber();
+ $length = $comment->getLineLength();
+
+ $viewer = $this->getViewer();
+ $engine = new PhabricatorMarkupEngine();
+
+ if ($is_new) {
+ $offset_mode = 'new';
+ } else {
+ $offset_mode = 'old';
+ }
+
+ $parser = id(new DifferentialChangesetParser())
+ ->setUser($viewer)
+ ->setChangeset($changeset)
+ ->setMarkupEngine($engine);
+
+ $renderer = new DifferentialChangesetOneUpRenderer();
+ $parser->setRenderer($renderer);
+
+ $context = 1;
+
+ return $parser->render(
+ $start - $context,
+ $length + 1 + ($context * 2),
+ array());
+ }
+
+}
diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php
--- a/src/applications/differential/controller/DifferentialRevisionViewController.php
+++ b/src/applications/differential/controller/DifferentialRevisionViewController.php
@@ -576,6 +576,12 @@
$curtain->addAction(
id(new PhabricatorActionView())
+ ->setIcon('fa-indent')
+ ->setHref("/differential/revision/inlines/{$revision_id}/")
+ ->setName(pht('List Inline Comments')));
+
+ $curtain->addAction(
+ id(new PhabricatorActionView())
->setIcon('fa-upload')
->setHref("/differential/revision/update/{$revision_id}/")
->setName(pht('Update Diff'))
diff --git a/src/applications/differential/parser/DifferentialChangesetParser.php b/src/applications/differential/parser/DifferentialChangesetParser.php
--- a/src/applications/differential/parser/DifferentialChangesetParser.php
+++ b/src/applications/differential/parser/DifferentialChangesetParser.php
@@ -1045,7 +1045,8 @@
}
}
- $this->comments = $this->reorderAndThreadComments($this->comments);
+ $this->comments = id(new PHUIDiffInlineThreader())
+ ->reorderAndThreadCommments($this->comments);
foreach ($this->comments as $comment) {
$final = $comment->getLineNumber() +
@@ -1617,68 +1618,6 @@
return array($old_back, $new_back);
}
- private function reorderAndThreadComments(array $comments) {
- $comments = msort($comments, 'getID');
-
- // Build an empty map of all the comments we actually have. If a comment
- // is a reply but the parent has gone missing, we don't want it to vanish
- // completely.
- $comment_phids = mpull($comments, 'getPHID');
- $replies = array_fill_keys($comment_phids, array());
-
- // Now, remove all comments which are replies, leaving only the top-level
- // comments.
- foreach ($comments as $key => $comment) {
- $reply_phid = $comment->getReplyToCommentPHID();
- if (isset($replies[$reply_phid])) {
- $replies[$reply_phid][] = $comment;
- unset($comments[$key]);
- }
- }
-
- // For each top level comment, add the comment, then add any replies
- // to it. Do this recursively so threads are shown in threaded order.
- $results = array();
- foreach ($comments as $comment) {
- $results[] = $comment;
- $phid = $comment->getPHID();
- $descendants = $this->getInlineReplies($replies, $phid, 1);
- foreach ($descendants as $descendant) {
- $results[] = $descendant;
- }
- }
-
- // If we have anything left, they were cyclic references. Just dump
- // them in a the end. This should be impossible, but users are very
- // creative.
- foreach ($replies as $phid => $comments) {
- foreach ($comments as $comment) {
- $results[] = $comment;
- }
- }
-
- return $results;
- }
-
- private function getInlineReplies(array &$replies, $phid, $depth) {
- $comments = idx($replies, $phid, array());
- unset($replies[$phid]);
-
- $results = array();
- foreach ($comments as $comment) {
- $results[] = $comment;
- $descendants = $this->getInlineReplies(
- $replies,
- $comment->getPHID(),
- $depth + 1);
- foreach ($descendants as $descendant) {
- $results[] = $descendant;
- }
- }
-
- return $results;
- }
-
private function getOffset(array $map, $line) {
if (!$map) {
return null;
diff --git a/src/infrastructure/diff/view/PHUIDiffInlineThreader.php b/src/infrastructure/diff/view/PHUIDiffInlineThreader.php
new file mode 100644
--- /dev/null
+++ b/src/infrastructure/diff/view/PHUIDiffInlineThreader.php
@@ -0,0 +1,66 @@
+<?php
+
+final class PHUIDiffInlineThreader extends Phobject {
+
+ public function reorderAndThreadCommments(array $comments) {
+ $comments = msort($comments, 'getID');
+
+ // Build an empty map of all the comments we actually have. If a comment
+ // is a reply but the parent has gone missing, we don't want it to vanish
+ // completely.
+ $comment_phids = mpull($comments, 'getPHID');
+ $replies = array_fill_keys($comment_phids, array());
+
+ // Now, remove all comments which are replies, leaving only the top-level
+ // comments.
+ foreach ($comments as $key => $comment) {
+ $reply_phid = $comment->getReplyToCommentPHID();
+ if (isset($replies[$reply_phid])) {
+ $replies[$reply_phid][] = $comment;
+ unset($comments[$key]);
+ }
+ }
+
+ // For each top level comment, add the comment, then add any replies
+ // to it. Do this recursively so threads are shown in threaded order.
+ $results = array();
+ foreach ($comments as $comment) {
+ $results[] = $comment;
+ $phid = $comment->getPHID();
+ $descendants = $this->getInlineReplies($replies, $phid, 1);
+ foreach ($descendants as $descendant) {
+ $results[] = $descendant;
+ }
+ }
+
+ // If we have anything left, they were cyclic references. Just dump
+ // them in a the end. This should be impossible, but users are very
+ // creative.
+ foreach ($replies as $phid => $comments) {
+ foreach ($comments as $comment) {
+ $results[] = $comment;
+ }
+ }
+
+ return $results;
+ }
+
+ private function getInlineReplies(array &$replies, $phid, $depth) {
+ $comments = idx($replies, $phid, array());
+ unset($replies[$phid]);
+
+ $results = array();
+ foreach ($comments as $comment) {
+ $results[] = $comment;
+ $descendants = $this->getInlineReplies(
+ $replies,
+ $comment->getPHID(),
+ $depth + 1);
+ foreach ($descendants as $descendant) {
+ $results[] = $descendant;
+ }
+ }
+
+ return $results;
+ }
+}

File Metadata

Mime Type
text/plain
Expires
Sat, Mar 15, 4:38 PM (3 d, 14 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7698107
Default Alt Text
D18112.id43576.diff (18 KB)

Event Timeline

Phabricator · Built by Phacility