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[1-9]\d*)/' => 'DifferentialRevisionOperationController', + 'inlines/(?P[1-9]\d*)/' + => 'DifferentialRevisionInlinesController', ), 'comment/' => array( 'preview/(?P[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,217 @@ +getViewer(); + $id = $request->getURIData('id'); + + $revision = id(new DifferentialRevisionQuery()) + ->withIDs(array($id)) + ->setViewer($viewer) + ->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); + + $curtain = $this->buildCurtain($revision); + + $content = $this->renderInlineThreads($revision, $inlines); + + $header = $this->buildHeader($revision); + + $view = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setCurtain($curtain) + ->setMainColumn($content); + + return $this->newPage() + ->setTitle( + array( + "{$revision_monogram} {$revision_title}", + pht('Inlines'), + )) + ->setCrumbs($crumbs) + ->appendChild($view); + } + + private function buildHeader(DifferentialRevision $revision) { + $viewer = $this->getViewer(); + + return id(new PHUIHeaderView()) + ->setHeader($revision->getTitle()) + ->setUser($viewer) + ->setHeaderIcon('fa-cog'); + } + + private function buildCurtain(DifferentialRevision $revision) { + $viewer = $this->getViewer(); + $curtain = $this->newCurtainView(null); + + $can_edit = PhabricatorPolicyFilter::hasCapability( + $viewer, + $revision, + PhabricatorPolicyCapability::CAN_EDIT); + + $curtain->addAction( + id(new PhabricatorActionView()) + ->setIcon('fa-chevron-left') + ->setHref($revision->getURI()) + ->setName(pht('Back to Revision'))); + + return $curtain; + } + + 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(); + 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 @@ + $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; + } +}