diff --git a/src/applications/differential/controller/DifferentialChangesetViewController.php b/src/applications/differential/controller/DifferentialChangesetViewController.php index af9eeea505..fc0536902c 100644 --- a/src/applications/differential/controller/DifferentialChangesetViewController.php +++ b/src/applications/differential/controller/DifferentialChangesetViewController.php @@ -1,565 +1,382 @@ getViewer(); $rendering_reference = $request->getStr('ref'); $parts = explode('/', $rendering_reference); if (count($parts) == 2) { list($id, $vs) = $parts; } else { $id = $parts[0]; $vs = 0; } $id = (int)$id; $vs = (int)$vs; $load_ids = array($id); if ($vs && ($vs != -1)) { $load_ids[] = $vs; } $changesets = id(new DifferentialChangesetQuery()) ->setViewer($viewer) ->withIDs($load_ids) ->needHunks(true) ->execute(); $changesets = mpull($changesets, null, 'getID'); $changeset = idx($changesets, $id); if (!$changeset) { return new Aphront404Response(); } $vs_changeset = null; if ($vs && ($vs != -1)) { $vs_changeset = idx($changesets, $vs); if (!$vs_changeset) { return new Aphront404Response(); } } $view = $request->getStr('view'); if ($view) { $phid = idx($changeset->getMetadata(), "$view:binary-phid"); if ($phid) { return id(new AphrontRedirectResponse())->setURI("/file/info/$phid/"); } switch ($view) { case 'new': return $this->buildRawFileResponse($changeset, $is_new = true); case 'old': if ($vs_changeset) { return $this->buildRawFileResponse($vs_changeset, $is_new = true); } return $this->buildRawFileResponse($changeset, $is_new = false); default: return new Aphront400Response(); } } + $old = array(); + $new = array(); if (!$vs) { $right = $changeset; $left = null; $right_source = $right->getID(); $right_new = true; $left_source = $right->getID(); $left_new = false; $render_cache_key = $right->getID(); + + $old[] = $changeset; + $new[] = $changeset; } else if ($vs == -1) { $right = null; $left = $changeset; $right_source = $left->getID(); $right_new = false; $left_source = $left->getID(); $left_new = true; $render_cache_key = null; + + $old[] = $changeset; + $new[] = $changeset; } else { $right = $changeset; $left = $vs_changeset; $right_source = $right->getID(); $right_new = true; $left_source = $left->getID(); $left_new = true; $render_cache_key = null; + + $new[] = $left; + $new[] = $right; } if ($left) { $left_data = $left->makeNewFile(); if ($right) { $right_data = $right->makeNewFile(); } else { $right_data = $left->makeOldFile(); } $engine = new PhabricatorDifferenceEngine(); $synthetic = $engine->generateChangesetFromFileContent( $left_data, $right_data); $choice = clone nonempty($left, $right); $choice->attachHunks($synthetic->getHunks()); $changeset = $choice; } $coverage = null; if ($right && $right->getDiffID()) { $unit = id(new DifferentialDiffProperty())->loadOneWhere( 'diffID = %d AND name = %s', $right->getDiffID(), 'arc:unit'); if ($unit) { $coverage = array(); foreach ($unit->getData() as $result) { $result_coverage = idx($result, 'coverage'); if (!$result_coverage) { continue; } $file_coverage = idx($result_coverage, $right->getFileName()); if (!$file_coverage) { continue; } $coverage[] = $file_coverage; } $coverage = ArcanistUnitTestResult::mergeCoverage($coverage); } } $spec = $request->getStr('range'); list($range_s, $range_e, $mask) = DifferentialChangesetParser::parseRangeSpecification($spec); $parser = id(new DifferentialChangesetParser()) ->setCoverage($coverage) ->setChangeset($changeset) ->setRenderingReference($rendering_reference) ->setRenderCacheKey($render_cache_key) ->setRightSideCommentMapping($right_source, $right_new) ->setLeftSideCommentMapping($left_source, $left_new); $parser->readParametersFromRequest($request); if ($left && $right) { $parser->setOriginals($left, $right); } $diff = $changeset->getDiff(); $revision_id = $diff->getRevisionID(); $can_mark = false; $object_owner_phid = null; if ($revision_id) { $revision = id(new DifferentialRevisionQuery()) ->setViewer($viewer) ->withIDs(array($revision_id)) ->executeOne(); if ($revision) { $can_mark = ($revision->getAuthorPHID() == $viewer->getPHID()); $object_owner_phid = $revision->getAuthorPHID(); } } // Load both left-side and right-side inline comments. if ($revision) { - $inlines = $this->loadInlineComments( - $revision, - array(nonempty($left, $right)), - $left_new, - array($right), - $right_new); + $query = id(new DifferentialInlineCommentQuery()) + ->setViewer($viewer) + ->withRevisionPHIDs(array($revision->getPHID())); + $inlines = $query->execute(); + $inlines = $query->adjustInlinesForChangesets($inlines, $old, $new); } else { $inlines = array(); } if ($left_new) { $inlines = array_merge( $inlines, $this->buildLintInlineComments($left)); } if ($right_new) { $inlines = array_merge( $inlines, $this->buildLintInlineComments($right)); } $phids = array(); foreach ($inlines as $inline) { $parser->parseInlineComment($inline); if ($inline->getAuthorPHID()) { $phids[$inline->getAuthorPHID()] = true; } } $phids = array_keys($phids); $handles = $this->loadViewerHandles($phids); $parser->setHandles($handles); $engine = new PhabricatorMarkupEngine(); $engine->setViewer($viewer); foreach ($inlines as $inline) { $engine->addObject( $inline, PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY); } $engine->process(); $parser ->setUser($viewer) ->setMarkupEngine($engine) ->setShowEditAndReplyLinks(true) ->setCanMarkDone($can_mark) ->setObjectOwnerPHID($object_owner_phid) ->setRange($range_s, $range_e) ->setMask($mask); if ($request->isAjax()) { $mcov = $parser->renderModifiedCoverage(); $coverage = array( 'differential-mcoverage-'.md5($changeset->getFilename()) => $mcov, ); return id(new PhabricatorChangesetResponse()) ->setRenderedChangeset($parser->renderChangeset()) ->setCoverage($coverage) ->setUndoTemplates($parser->getRenderer()->renderUndoTemplates()); } $detail = id(new DifferentialChangesetListView()) ->setUser($this->getViewer()) ->setChangesets(array($changeset)) ->setVisibleChangesets(array($changeset)) ->setRenderingReferences(array($rendering_reference)) ->setRenderURI('/differential/changeset/') ->setDiff($diff) ->setTitle(pht('Standalone View')) ->setParser($parser); if ($revision_id) { $detail->setInlineCommentControllerURI( '/differential/comment/inline/edit/'.$revision_id.'/'); } $crumbs = $this->buildApplicationCrumbs(); if ($revision_id) { $crumbs->addTextCrumb('D'.$revision_id, '/D'.$revision_id); } $diff_id = $diff->getID(); if ($diff_id) { $crumbs->addTextCrumb( pht('Diff %d', $diff_id), $this->getApplicationURI('diff/'.$diff_id)); } $crumbs->addTextCrumb($changeset->getDisplayFilename()); return $this->buildApplicationPage( array( $crumbs, $detail, ), array( 'title' => pht('Changeset View'), 'device' => false, )); } - private function loadInlineComments( - DifferentialRevision $revision, - array $left, - $left_new, - array $right, - $right_new) { - - assert_instances_of($left, 'DifferentialChangeset'); - assert_instances_of($right, 'DifferentialChangeset'); - - $viewer = $this->getViewer(); - $all = array_merge($left, $right); - - $inlines = id(new DifferentialInlineCommentQuery()) - ->setViewer($viewer) - ->withRevisionPHIDs(array($revision->getPHID())) - ->execute(); - - $changeset_ids = mpull($inlines, 'getChangesetID'); - $changeset_ids = array_unique($changeset_ids); - if ($changeset_ids) { - $changesets = id(new DifferentialChangesetQuery()) - ->setViewer($viewer) - ->withIDs($changeset_ids) - ->execute(); - $changesets = mpull($changesets, null, 'getID'); - } else { - $changesets = array(); - } - $changesets += mpull($all, null, 'getID'); - - $id_map = array(); - foreach ($all as $changeset) { - $id_map[$changeset->getID()] = $changeset->getID(); - } - - // Generate filename maps for older and newer comments. If we're bringing - // an older comment forward in a diff-of-diffs, we want to put it on the - // left side of the screen, not the right side. Both sides are "new" files - // with the same name, so they're both appropriate targets, but the left - // is a better target conceptually for users because it's more consistent - // with the rest of the UI, which shows old information on the left and - // new information on the right. - $move_here = DifferentialChangeType::TYPE_MOVE_HERE; - - $name_map_old = array(); - $name_map_new = array(); - $move_map = array(); - foreach ($all as $changeset) { - $changeset_id = $changeset->getID(); - - $filenames = array(); - $filenames[] = $changeset->getFilename(); - - // If this is the target of a move, also map comments on the old filename - // to this changeset. - if ($changeset->getChangeType() == $move_here) { - $old_file = $changeset->getOldFile(); - $filenames[] = $old_file; - $move_map[$changeset_id][$old_file] = true; - } - - foreach ($filenames as $filename) { - // We update the old map only if we don't already have an entry (oldest - // changeset persists). - if (empty($name_map_old[$filename])) { - $name_map_old[$filename] = $changeset_id; - } - - // We always update the new map (newest changeset overwrites). - $name_map_new[$changeset->getFilename()] = $changeset_id; - } - } - - // Find the smallest "new" changeset ID. We'll consider everything - // larger than this to be "newer", and everything smaller to be "older". - $first_new_id = min(mpull($right, 'getID')); - - $results = array(); - foreach ($inlines as $inline) { - $changeset_id = $inline->getChangesetID(); - if (isset($id_map[$changeset_id])) { - // This inline is legitimately on one of the current changesets, so - // we can include it in the result set unmodified. - $results[] = $inline; - continue; - } - - $changeset = idx($changesets, $changeset_id); - if (!$changeset) { - // Just discard this inline, as it has bogus data. - continue; - } - - $target_id = null; - - if ($changeset_id >= $first_new_id) { - $name_map = $name_map_new; - $is_new = true; - } else { - $name_map = $name_map_old; - $is_new = false; - } - - $filename = $changeset->getFilename(); - if (isset($name_map[$filename])) { - // This changeset is on a file with the same name as the current - // changeset, so we're going to port it forward or backward. - $target_id = $name_map[$filename]; - - $is_move = isset($move_map[$target_id][$filename]); - if ($is_new) { - if ($is_move) { - $reason = pht( - 'This comment was made on a file with the same name as the '. - 'file this file was moved from, but in a newer diff.'); - } else { - $reason = pht( - 'This comment was made on a file with the same name, but '. - 'in a newer diff.'); - } - } else { - if ($is_move) { - $reason = pht( - 'This comment was made on a file with the same name as the '. - 'file this file was moved from, but in an older diff.'); - } else { - $reason = pht( - 'This comment was made on a file with the same name, but '. - 'in an older diff.'); - } - } - } - - - // If we didn't find a target and this change is the target of a move, - // look for a match against the old filename. - if (!$target_id) { - if ($changeset->getChangeType() == $move_here) { - $filename = $changeset->getOldFile(); - if (isset($name_map[$filename])) { - $target_id = $name_map[$filename]; - if ($is_new) { - $reason = pht( - 'This comment was made on a file which this file was moved '. - 'to, but in a newer diff.'); - } else { - $reason = pht( - 'This comment was made on a file which this file was moved '. - 'to, but in an older diff.'); - } - } - } - } - - - // If we found a changeset to port this comment to, bring it forward - // or backward and mark it. - if ($target_id) { - $inline - ->makeEphemeral(true) - ->setChangesetID($target_id) - ->setIsGhost( - array( - 'new' => $is_new, - 'reason' => $reason, - )); - - $results[] = $inline; - } - } - - // Filter out the inlines we ported forward which won't be visible because - // they appear on the wrong side of a file. - $keep_map = array(); - foreach ($left as $changeset) { - $keep_map[$changeset->getID()][(int)$left_new] = true; - } - foreach ($right as $changeset) { - $keep_map[$changeset->getID()][(int)$right_new] = true; - } - foreach ($results as $key => $inline) { - $is_new = (int)$inline->getIsNewFile(); - $changeset_id = $inline->getChangesetID(); - if (!isset($keep_map[$changeset_id][$is_new])) { - unset($results[$key]); - continue; - } - } - - return $results; - } - private function buildRawFileResponse( DifferentialChangeset $changeset, $is_new) { $viewer = $this->getViewer(); if ($is_new) { $key = 'raw:new:phid'; } else { $key = 'raw:old:phid'; } $metadata = $changeset->getMetadata(); $file = null; $phid = idx($metadata, $key); if ($phid) { $file = id(new PhabricatorFileQuery()) ->setViewer($viewer) ->withPHIDs(array($phid)) ->execute(); if ($file) { $file = head($file); } } if (!$file) { // This is just building a cache of the changeset content in the file // tool, and is safe to run on a read pathway. $unguard = AphrontWriteGuard::beginScopedUnguardedWrites(); if ($is_new) { $data = $changeset->makeNewFile(); } else { $data = $changeset->makeOldFile(); } $file = PhabricatorFile::newFromFileData( $data, array( 'name' => $changeset->getFilename(), 'mime-type' => 'text/plain', )); $metadata[$key] = $file->getPHID(); $changeset->setMetadata($metadata); $changeset->save(); unset($unguard); } return $file->getRedirectResponse(); } private function buildLintInlineComments($changeset) { $lint = id(new DifferentialDiffProperty())->loadOneWhere( 'diffID = %d AND name = %s', $changeset->getDiffID(), 'arc:lint'); if (!$lint) { return array(); } $lint = $lint->getData(); $inlines = array(); foreach ($lint as $msg) { if ($msg['path'] != $changeset->getFilename()) { continue; } $inline = new DifferentialInlineComment(); $inline->setChangesetID($changeset->getID()); $inline->setIsNewFile(1); $inline->setSyntheticAuthor('Lint: '.$msg['name']); $inline->setLineNumber($msg['line']); $inline->setLineLength(0); $inline->setContent('%%%'.$msg['description'].'%%%'); $inlines[] = $inline; } return $inlines; } } diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index 6905d03cea..215a455ef7 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -1,954 +1,967 @@ revisionID = $data['id']; } public function processRequest() { $request = $this->getRequest(); $user = $request->getUser(); $viewer_is_anonymous = !$user->isLoggedIn(); $revision = id(new DifferentialRevisionQuery()) ->withIDs(array($this->revisionID)) ->setViewer($request->getUser()) ->needRelationships(true) ->needReviewerStatus(true) ->needReviewerAuthority(true) ->executeOne(); if (!$revision) { return new Aphront404Response(); } $diffs = id(new DifferentialDiffQuery()) ->setViewer($request->getUser()) ->withRevisionIDs(array($this->revisionID)) ->needArcanistProjects(true) ->execute(); $diffs = array_reverse($diffs, $preserve_keys = true); if (!$diffs) { throw new Exception( 'This revision has no diffs. Something has gone quite wrong.'); } $revision->attachActiveDiff(last($diffs)); $diff_vs = $request->getInt('vs'); $target_id = $request->getInt('id'); $target = idx($diffs, $target_id, end($diffs)); $target_manual = $target; if (!$target_id) { foreach ($diffs as $diff) { if ($diff->getCreationMethod() != 'commit') { $target_manual = $diff; } } } if (empty($diffs[$diff_vs])) { $diff_vs = null; } $repository = null; $repository_phid = $target->getRepositoryPHID(); if ($repository_phid) { if ($repository_phid == $revision->getRepositoryPHID()) { $repository = $revision->getRepository(); } else { $repository = id(new PhabricatorRepositoryQuery()) ->setViewer($user) ->withPHIDs(array($repository_phid)) ->executeOne(); } } list($changesets, $vs_map, $vs_changesets, $rendering_references) = $this->loadChangesetsAndVsMap( $target, idx($diffs, $diff_vs), $repository); if ($request->getExists('download')) { return $this->buildRawDiffResponse( $revision, $changesets, $vs_changesets, $vs_map, $repository); } $props = id(new DifferentialDiffProperty())->loadAllWhere( 'diffID = %d', $target_manual->getID()); $props = mpull($props, 'getData', 'getName'); - $all_changesets = $changesets; - $inlines = $revision->loadInlineComments($all_changesets, $user); - $object_phids = array_merge( $revision->getReviewers(), $revision->getCCPHIDs(), $revision->loadCommitPHIDs(), array( $revision->getAuthorPHID(), $user->getPHID(), )); foreach ($revision->getAttached() as $type => $phids) { foreach ($phids as $phid => $info) { $object_phids[] = $phid; } } $field_list = PhabricatorCustomField::getObjectFields( $revision, PhabricatorCustomField::ROLE_VIEW); $field_list->setViewer($user); $field_list->readFieldsFromStorage($revision); $warning_handle_map = array(); foreach ($field_list->getFields() as $key => $field) { $req = $field->getRequiredHandlePHIDsForRevisionHeaderWarnings(); foreach ($req as $phid) { $warning_handle_map[$key][] = $phid; $object_phids[] = $phid; } } $handles = $this->loadViewerHandles($object_phids); $request_uri = $request->getRequestURI(); $limit = 100; $large = $request->getStr('large'); if (count($changesets) > $limit && !$large) { $count = count($changesets); $warning = new PHUIInfoView(); $warning->setTitle('Very Large Diff'); $warning->setSeverity(PHUIInfoView::SEVERITY_WARNING); $warning->appendChild(hsprintf( '%s %s', pht( 'This diff is very large and affects %s files. '. 'You may load each file individually or ', new PhutilNumber($count)), phutil_tag( 'a', array( 'class' => 'button grey', 'href' => $request_uri ->alter('large', 'true') ->setFragment('toc'), ), pht('Show All Files Inline')))); $warning = $warning->render(); - $my_inlines = id(new DifferentialInlineCommentQuery()) + $map = $vs_map; + if (!$map) { + $map = array_fill_keys(array_keys($changesets), 0); + } + + $old = array(); + $new = array(); + foreach ($map as $id => $vs) { + if ($vs <= 0) { + $old[] = $id; + $new[] = $id; + } else { + $new[] = $id; + $new[] = $vs; + } + } + $old = array_select_keys($changesets, $old); + $new = array_select_keys($changesets, $new); + + $query = id(new DifferentialInlineCommentQuery()) ->setViewer($user) - ->withDrafts(true) - ->withAuthorPHIDs(array($user->getPHID())) - ->withRevisionPHIDs(array($revision->getPHID())) - ->execute(); + ->withRevisionPHIDs(array($revision->getPHID())); + $inlines = $query->execute(); + $inlines = $query->adjustInlinesForChangesets($inlines, $old, $new); $visible_changesets = array(); - foreach ($inlines + $my_inlines as $inline) { + foreach ($inlines as $inline) { $changeset_id = $inline->getChangesetID(); if (isset($changesets[$changeset_id])) { $visible_changesets[$changeset_id] = $changesets[$changeset_id]; } } if (!empty($props['arc:lint'])) { $changeset_paths = mpull($changesets, null, 'getFilename'); foreach ($props['arc:lint'] as $lint) { $changeset = idx($changeset_paths, $lint['path']); if ($changeset) { $visible_changesets[$changeset->getID()] = $changeset; } } } } else { $warning = null; $visible_changesets = $changesets; } // TODO: This should be in a DiffQuery or similar. $need_props = array(); foreach ($field_list->getFields() as $field) { foreach ($field->getRequiredDiffPropertiesForRevisionView() as $prop) { $need_props[$prop] = $prop; } } if ($need_props) { $prop_diff = $revision->getActiveDiff(); $load_props = id(new DifferentialDiffProperty())->loadAllWhere( 'diffID = %d AND name IN (%Ls)', $prop_diff->getID(), $need_props); $load_props = mpull($load_props, 'getData', 'getName'); foreach ($need_props as $need) { $prop_diff->attachProperty($need, idx($load_props, $need)); } } $commit_hashes = mpull($diffs, 'getSourceControlBaseRevision'); $local_commits = idx($props, 'local:commits', array()); foreach ($local_commits as $local_commit) { $commit_hashes[] = idx($local_commit, 'tree'); $commit_hashes[] = idx($local_commit, 'local'); } $commit_hashes = array_unique(array_filter($commit_hashes)); if ($commit_hashes) { $commits_for_links = id(new DiffusionCommitQuery()) ->setViewer($user) ->withIdentifiers($commit_hashes) ->execute(); $commits_for_links = mpull( $commits_for_links, null, 'getCommitIdentifier'); } else { $commits_for_links = array(); } $revision_detail = id(new DifferentialRevisionDetailView()) ->setUser($user) ->setRevision($revision) ->setDiff(end($diffs)) ->setCustomFields($field_list) ->setURI($request->getRequestURI()); $actions = $this->getRevisionActions($revision); $whitespace = $request->getStr( 'whitespace', DifferentialChangesetParser::WHITESPACE_IGNORE_MOST); $arc_project = $target->getArcanistProject(); if ($arc_project) { list($symbol_indexes, $project_phids) = $this->buildSymbolIndexes( $arc_project, $visible_changesets); } else { $symbol_indexes = array(); $project_phids = null; } $revision_detail->setActions($actions); $revision_detail->setUser($user); $revision_detail_box = $revision_detail->render(); $revision_warnings = $this->buildRevisionWarnings( $revision, $field_list, $warning_handle_map, $handles); if ($revision_warnings) { $revision_warnings = id(new PHUIInfoView()) ->setSeverity(PHUIInfoView::SEVERITY_WARNING) ->setErrors($revision_warnings); $revision_detail_box->setInfoView($revision_warnings); } $comment_view = $this->buildTransactions( $revision, $diff_vs ? $diffs[$diff_vs] : $target, - $target, - $all_changesets); + $target); if (!$viewer_is_anonymous) { $comment_view->setQuoteRef('D'.$revision->getID()); $comment_view->setQuoteTargetID('comment-content'); } $wrap_id = celerity_generate_unique_node_id(); $comment_view = phutil_tag( 'div', array( 'id' => $wrap_id, ), $comment_view); if ($arc_project) { Javelin::initBehavior( 'repository-crossreference', array( 'section' => $wrap_id, 'projects' => $project_phids, )); } $changeset_view = new DifferentialChangesetListView(); $changeset_view->setChangesets($changesets); $changeset_view->setVisibleChangesets($visible_changesets); if (!$viewer_is_anonymous) { $changeset_view->setInlineCommentControllerURI( '/differential/comment/inline/edit/'.$revision->getID().'/'); } $changeset_view->setStandaloneURI('/differential/changeset/'); $changeset_view->setRawFileURIs( '/differential/changeset/?view=old', '/differential/changeset/?view=new'); $changeset_view->setUser($user); $changeset_view->setDiff($target); $changeset_view->setRenderingReferences($rendering_references); $changeset_view->setVsMap($vs_map); $changeset_view->setWhitespace($whitespace); if ($repository) { $changeset_view->setRepository($repository); } $changeset_view->setSymbolIndexes($symbol_indexes); $changeset_view->setTitle('Diff '.$target->getID()); $diff_history = id(new DifferentialRevisionUpdateHistoryView()) ->setUser($user) ->setDiffs($diffs) ->setSelectedVersusDiffID($diff_vs) ->setSelectedDiffID($target->getID()) ->setSelectedWhitespace($whitespace) ->setCommitsForLinks($commits_for_links); $local_view = id(new DifferentialLocalCommitsView()) ->setUser($user) ->setLocalCommits(idx($props, 'local:commits')) ->setCommitsForLinks($commits_for_links); if ($repository) { $other_revisions = $this->loadOtherRevisions( $changesets, $target, $repository); } else { $other_revisions = array(); } $other_view = null; if ($other_revisions) { $other_view = $this->renderOtherRevisions($other_revisions); } $toc_view = new DifferentialDiffTableOfContentsView(); $toc_view->setChangesets($changesets); $toc_view->setVisibleChangesets($visible_changesets); $toc_view->setRenderingReferences($rendering_references); $toc_view->setUnitTestData(idx($props, 'arc:unit', array())); if ($repository) { $toc_view->setRepository($repository); } $toc_view->setDiff($target); $toc_view->setUser($user); $toc_view->setRevisionID($revision->getID()); $toc_view->setWhitespace($whitespace); $comment_form = null; if (!$viewer_is_anonymous) { $draft = id(new PhabricatorDraft())->loadOneWhere( 'authorPHID = %s AND draftKey = %s', $user->getPHID(), 'differential-comment-'.$revision->getID()); $reviewers = array(); $ccs = array(); if ($draft) { $reviewers = idx($draft->getMetadata(), 'reviewers', array()); $ccs = idx($draft->getMetadata(), 'ccs', array()); if ($reviewers || $ccs) { $handles = $this->loadViewerHandles(array_merge($reviewers, $ccs)); $reviewers = array_select_keys($handles, $reviewers); $ccs = array_select_keys($handles, $ccs); } } $comment_form = new DifferentialAddCommentView(); $comment_form->setRevision($revision); $review_warnings = array(); foreach ($field_list->getFields() as $field) { $review_warnings[] = $field->getWarningsForDetailView(); } $review_warnings = array_mergev($review_warnings); if ($review_warnings) { $review_warnings_panel = id(new PHUIInfoView()) ->setSeverity(PHUIInfoView::SEVERITY_WARNING) ->setErrors($review_warnings); $comment_form->setInfoView($review_warnings_panel); } $comment_form->setActions($this->getRevisionCommentActions($revision)); $action_uri = $this->getApplicationURI( 'comment/save/'.$revision->getID().'/'); $comment_form->setActionURI($action_uri); $comment_form->setUser($user); $comment_form->setDraft($draft); $comment_form->setReviewers(mpull($reviewers, 'getFullName', 'getPHID')); $comment_form->setCCs(mpull($ccs, 'getFullName', 'getPHID')); // TODO: This just makes the "Z" key work. Generalize this and remove // it at some point. $comment_form = phutil_tag( 'div', array( 'class' => 'differential-add-comment-panel', ), $comment_form); } $pane_id = celerity_generate_unique_node_id(); Javelin::initBehavior( 'differential-keyboard-navigation', array( 'haunt' => $pane_id, )); Javelin::initBehavior('differential-user-select'); $page_pane = id(new DifferentialPrimaryPaneView()) ->setID($pane_id) ->appendChild($comment_view); $signatures = DifferentialRequiredSignaturesField::loadForRevision( $revision); $missing_signatures = false; foreach ($signatures as $phid => $signed) { if (!$signed) { $missing_signatures = true; } } if ($missing_signatures) { $signature_message = id(new PHUIInfoView()) ->setErrors( array( array( phutil_tag('strong', array(), pht('Content Hidden:')), ' ', pht( 'The content of this revision is hidden until the author has '. 'signed all of the required legal agreements.'), ), )); $page_pane->appendChild($signature_message); } else { $page_pane->appendChild( array( $diff_history, $warning, $local_view, $toc_view, $other_view, $changeset_view, )); } if ($comment_form) { $page_pane->appendChild($comment_form); } else { // TODO: For now, just use this to get "Login to Comment". $page_pane->appendChild( id(new PhabricatorApplicationTransactionCommentView()) ->setUser($user) ->setRequestURI($request->getRequestURI())); } $object_id = 'D'.$revision->getID(); $content = array( $revision_detail_box, $page_pane, ); $crumbs = $this->buildApplicationCrumbs(); $crumbs->addTextCrumb($object_id, '/'.$object_id); $prefs = $user->loadPreferences(); $pref_filetree = PhabricatorUserPreferences::PREFERENCE_DIFF_FILETREE; if ($prefs->getPreference($pref_filetree)) { $collapsed = $prefs->getPreference( PhabricatorUserPreferences::PREFERENCE_NAV_COLLAPSED, false); $nav = id(new DifferentialChangesetFileTreeSideNavBuilder()) ->setTitle('D'.$revision->getID()) ->setBaseURI(new PhutilURI('/D'.$revision->getID())) ->setCollapsed((bool)$collapsed) ->build($changesets); $nav->appendChild($content); $nav->setCrumbs($crumbs); $content = $nav; } else { array_unshift($content, $crumbs); } return $this->buildApplicationPage( $content, array( 'title' => $object_id.' '.$revision->getTitle(), 'pageObjects' => array($revision->getPHID()), )); } private function getRevisionActions(DifferentialRevision $revision) { $viewer = $this->getRequest()->getUser(); $revision_id = $revision->getID(); $revision_phid = $revision->getPHID(); $can_edit = PhabricatorPolicyFilter::hasCapability( $viewer, $revision, PhabricatorPolicyCapability::CAN_EDIT); $actions = array(); $actions[] = id(new PhabricatorActionView()) ->setIcon('fa-pencil') ->setHref("/differential/revision/edit/{$revision_id}/") ->setName(pht('Edit Revision')) ->setDisabled(!$can_edit) ->setWorkflow(!$can_edit); $actions[] = id(new PhabricatorActionView()) ->setIcon('fa-upload') ->setHref("/differential/revision/update/{$revision_id}/") ->setName(pht('Update Diff')) ->setDisabled(!$can_edit) ->setWorkflow(!$can_edit); $this->requireResource('phabricator-object-selector-css'); $this->requireResource('javelin-behavior-phabricator-object-selector'); $actions[] = id(new PhabricatorActionView()) ->setIcon('fa-link') ->setName(pht('Edit Dependencies')) ->setHref("/search/attach/{$revision_phid}/DREV/dependencies/") ->setWorkflow(true) ->setDisabled(!$can_edit); $maniphest = 'PhabricatorManiphestApplication'; if (PhabricatorApplication::isClassInstalled($maniphest)) { $actions[] = id(new PhabricatorActionView()) ->setIcon('fa-anchor') ->setName(pht('Edit Maniphest Tasks')) ->setHref("/search/attach/{$revision_phid}/TASK/") ->setWorkflow(true) ->setDisabled(!$can_edit); } $request_uri = $this->getRequest()->getRequestURI(); $actions[] = id(new PhabricatorActionView()) ->setIcon('fa-download') ->setName(pht('Download Raw Diff')) ->setHref($request_uri->alter('download', 'true')); return $actions; } private function getRevisionCommentActions(DifferentialRevision $revision) { $actions = array( DifferentialAction::ACTION_COMMENT => true, ); $viewer = $this->getRequest()->getUser(); $viewer_phid = $viewer->getPHID(); $viewer_is_owner = ($viewer_phid == $revision->getAuthorPHID()); $viewer_is_reviewer = in_array($viewer_phid, $revision->getReviewers()); $status = $revision->getStatus(); $viewer_has_accepted = false; $viewer_has_rejected = false; $status_accepted = DifferentialReviewerStatus::STATUS_ACCEPTED; $status_rejected = DifferentialReviewerStatus::STATUS_REJECTED; foreach ($revision->getReviewerStatus() as $reviewer) { if ($reviewer->getReviewerPHID() == $viewer_phid) { if ($reviewer->getStatus() == $status_accepted) { $viewer_has_accepted = true; } if ($reviewer->getStatus() == $status_rejected) { $viewer_has_rejected = true; } break; } } $allow_self_accept = PhabricatorEnv::getEnvConfig( 'differential.allow-self-accept'); $always_allow_abandon = PhabricatorEnv::getEnvConfig( 'differential.always-allow-abandon'); $always_allow_close = PhabricatorEnv::getEnvConfig( 'differential.always-allow-close'); $allow_reopen = PhabricatorEnv::getEnvConfig( 'differential.allow-reopen'); if ($viewer_is_owner) { switch ($status) { case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW: $actions[DifferentialAction::ACTION_ACCEPT] = $allow_self_accept; $actions[DifferentialAction::ACTION_ABANDON] = true; $actions[DifferentialAction::ACTION_RETHINK] = true; break; case ArcanistDifferentialRevisionStatus::NEEDS_REVISION: case ArcanistDifferentialRevisionStatus::CHANGES_PLANNED: $actions[DifferentialAction::ACTION_ACCEPT] = $allow_self_accept; $actions[DifferentialAction::ACTION_ABANDON] = true; $actions[DifferentialAction::ACTION_REQUEST] = true; break; case ArcanistDifferentialRevisionStatus::ACCEPTED: $actions[DifferentialAction::ACTION_ABANDON] = true; $actions[DifferentialAction::ACTION_REQUEST] = true; $actions[DifferentialAction::ACTION_RETHINK] = true; $actions[DifferentialAction::ACTION_CLOSE] = true; break; case ArcanistDifferentialRevisionStatus::CLOSED: break; case ArcanistDifferentialRevisionStatus::ABANDONED: $actions[DifferentialAction::ACTION_RECLAIM] = true; break; } } else { switch ($status) { case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW: $actions[DifferentialAction::ACTION_ABANDON] = $always_allow_abandon; $actions[DifferentialAction::ACTION_ACCEPT] = true; $actions[DifferentialAction::ACTION_REJECT] = true; $actions[DifferentialAction::ACTION_RESIGN] = $viewer_is_reviewer; break; case ArcanistDifferentialRevisionStatus::NEEDS_REVISION: case ArcanistDifferentialRevisionStatus::CHANGES_PLANNED: $actions[DifferentialAction::ACTION_ABANDON] = $always_allow_abandon; $actions[DifferentialAction::ACTION_ACCEPT] = true; $actions[DifferentialAction::ACTION_REJECT] = !$viewer_has_rejected; $actions[DifferentialAction::ACTION_RESIGN] = $viewer_is_reviewer; break; case ArcanistDifferentialRevisionStatus::ACCEPTED: $actions[DifferentialAction::ACTION_ABANDON] = $always_allow_abandon; $actions[DifferentialAction::ACTION_ACCEPT] = !$viewer_has_accepted; $actions[DifferentialAction::ACTION_REJECT] = true; $actions[DifferentialAction::ACTION_RESIGN] = $viewer_is_reviewer; break; case ArcanistDifferentialRevisionStatus::CLOSED: case ArcanistDifferentialRevisionStatus::ABANDONED: break; } if ($status != ArcanistDifferentialRevisionStatus::CLOSED) { $actions[DifferentialAction::ACTION_CLAIM] = true; $actions[DifferentialAction::ACTION_CLOSE] = $always_allow_close; } } $actions[DifferentialAction::ACTION_ADDREVIEWERS] = true; $actions[DifferentialAction::ACTION_ADDCCS] = true; $actions[DifferentialAction::ACTION_REOPEN] = $allow_reopen && ($status == ArcanistDifferentialRevisionStatus::CLOSED); $actions = array_keys(array_filter($actions)); $actions_dict = array(); foreach ($actions as $action) { $actions_dict[$action] = DifferentialAction::getActionVerb($action); } return $actions_dict; } private function loadChangesetsAndVsMap( DifferentialDiff $target, DifferentialDiff $diff_vs = null, PhabricatorRepository $repository = null) { $load_diffs = array($target); if ($diff_vs) { $load_diffs[] = $diff_vs; } $raw_changesets = id(new DifferentialChangesetQuery()) ->setViewer($this->getRequest()->getUser()) ->withDiffs($load_diffs) ->execute(); $changeset_groups = mgroup($raw_changesets, 'getDiffID'); $changesets = idx($changeset_groups, $target->getID(), array()); $changesets = mpull($changesets, null, 'getID'); $refs = array(); $vs_map = array(); $vs_changesets = array(); if ($diff_vs) { $vs_id = $diff_vs->getID(); $vs_changesets_path_map = array(); foreach (idx($changeset_groups, $vs_id, array()) as $changeset) { $path = $changeset->getAbsoluteRepositoryPath($repository, $diff_vs); $vs_changesets_path_map[$path] = $changeset; $vs_changesets[$changeset->getID()] = $changeset; } foreach ($changesets as $key => $changeset) { $path = $changeset->getAbsoluteRepositoryPath($repository, $target); if (isset($vs_changesets_path_map[$path])) { $vs_map[$changeset->getID()] = $vs_changesets_path_map[$path]->getID(); $refs[$changeset->getID()] = $changeset->getID().'/'.$vs_changesets_path_map[$path]->getID(); unset($vs_changesets_path_map[$path]); } else { $refs[$changeset->getID()] = $changeset->getID(); } } foreach ($vs_changesets_path_map as $path => $changeset) { $changesets[$changeset->getID()] = $changeset; $vs_map[$changeset->getID()] = -1; $refs[$changeset->getID()] = $changeset->getID().'/-1'; } } else { foreach ($changesets as $changeset) { $refs[$changeset->getID()] = $changeset->getID(); } } $changesets = msort($changesets, 'getSortKey'); return array($changesets, $vs_map, $vs_changesets, $refs); } private function buildSymbolIndexes( PhabricatorRepositoryArcanistProject $arc_project, array $visible_changesets) { assert_instances_of($visible_changesets, 'DifferentialChangeset'); $engine = PhabricatorSyntaxHighlighter::newEngine(); $langs = $arc_project->getSymbolIndexLanguages(); if (!$langs) { return array(array(), array()); } $symbol_indexes = array(); $project_phids = array_merge( array($arc_project->getPHID()), nonempty($arc_project->getSymbolIndexProjects(), array())); $indexed_langs = array_fill_keys($langs, true); foreach ($visible_changesets as $key => $changeset) { $lang = $engine->getLanguageFromFilename($changeset->getFilename()); if (isset($indexed_langs[$lang])) { $symbol_indexes[$key] = array( 'lang' => $lang, 'projects' => $project_phids, ); } } return array($symbol_indexes, $project_phids); } private function loadOtherRevisions( array $changesets, DifferentialDiff $target, PhabricatorRepository $repository) { assert_instances_of($changesets, 'DifferentialChangeset'); $paths = array(); foreach ($changesets as $changeset) { $paths[] = $changeset->getAbsoluteRepositoryPath( $repository, $target); } if (!$paths) { return array(); } $path_map = id(new DiffusionPathIDQuery($paths))->loadPathIDs(); if (!$path_map) { return array(); } $recent = (PhabricatorTime::getNow() - phutil_units('30 days in seconds')); $query = id(new DifferentialRevisionQuery()) ->setViewer($this->getRequest()->getUser()) ->withStatus(DifferentialRevisionQuery::STATUS_OPEN) ->withUpdatedEpochBetween($recent, null) ->setOrder(DifferentialRevisionQuery::ORDER_MODIFIED) ->setLimit(10) ->needFlags(true) ->needDrafts(true) ->needRelationships(true); foreach ($path_map as $path => $path_id) { $query->withPath($repository->getID(), $path_id); } $results = $query->execute(); // Strip out *this* revision. foreach ($results as $key => $result) { if ($result->getID() == $this->revisionID) { unset($results[$key]); } } return $results; } private function renderOtherRevisions(array $revisions) { assert_instances_of($revisions, 'DifferentialRevision'); $viewer = $this->getViewer(); $header = id(new PHUIHeaderView()) ->setHeader(pht('Similar Open Revisions')) ->setSubheader( pht('Recently updated open revisions affecting the same files.')); $view = id(new DifferentialRevisionListView()) ->setHeader($header) ->setRevisions($revisions) ->setUser($viewer); $phids = $view->getRequiredHandlePHIDs(); $handles = $this->loadViewerHandles($phids); $view->setHandles($handles); return $view; } /** * Note this code is somewhat similar to the buildPatch method in * @{class:DifferentialReviewRequestMail}. * * @return @{class:AphrontRedirectResponse} */ private function buildRawDiffResponse( DifferentialRevision $revision, array $changesets, array $vs_changesets, array $vs_map, PhabricatorRepository $repository = null) { assert_instances_of($changesets, 'DifferentialChangeset'); assert_instances_of($vs_changesets, 'DifferentialChangeset'); $viewer = $this->getRequest()->getUser(); id(new DifferentialHunkQuery()) ->setViewer($viewer) ->withChangesets($changesets) ->needAttachToChangesets(true) ->execute(); $diff = new DifferentialDiff(); $diff->attachChangesets($changesets); $raw_changes = $diff->buildChangesList(); $changes = array(); foreach ($raw_changes as $changedict) { $changes[] = ArcanistDiffChange::newFromDictionary($changedict); } $loader = id(new PhabricatorFileBundleLoader()) ->setViewer($viewer); $bundle = ArcanistBundle::newFromChanges($changes); $bundle->setLoadFileDataCallback(array($loader, 'loadFileData')); $vcs = $repository ? $repository->getVersionControlSystem() : null; switch ($vcs) { case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL: $raw_diff = $bundle->toGitPatch(); break; case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: default: $raw_diff = $bundle->toUnifiedDiff(); break; } $request_uri = $this->getRequest()->getRequestURI(); // this ends up being something like // D123.diff // or the verbose // D123.vs123.id123.whitespaceignore-all.diff // lame but nice to include these options $file_name = ltrim($request_uri->getPath(), '/').'.'; foreach ($request_uri->getQueryParams() as $key => $value) { if ($key == 'download') { continue; } $file_name .= $key.$value.'.'; } $file_name .= 'diff'; $file = PhabricatorFile::buildFromFileDataOrHash( $raw_diff, array( 'name' => $file_name, 'ttl' => (60 * 60 * 24), 'viewPolicy' => PhabricatorPolicies::POLICY_NOONE, )); $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); $file->attachToObject($revision->getPHID()); unset($unguarded); return $file->getRedirectResponse(); } private function buildTransactions( DifferentialRevision $revision, DifferentialDiff $left_diff, - DifferentialDiff $right_diff, - array $changesets) { + DifferentialDiff $right_diff) { $timeline = $this->buildTransactionTimeline( $revision, new DifferentialTransactionQuery(), $engine = null, array( 'left' => $left_diff->getID(), 'right' => $right_diff->getID(), )); return $timeline; } private function buildRevisionWarnings( DifferentialRevision $revision, PhabricatorCustomFieldList $field_list, array $warning_handle_map, array $handles) { $warnings = array(); foreach ($field_list->getFields() as $key => $field) { $phids = idx($warning_handle_map, $key, array()); $field_handles = array_select_keys($handles, $phids); $field_warnings = $field->getWarningsForRevisionHeader($field_handles); foreach ($field_warnings as $warning) { $warnings[] = $warning; } } return $warnings; } } diff --git a/src/applications/differential/query/DifferentialInlineCommentQuery.php b/src/applications/differential/query/DifferentialInlineCommentQuery.php index 0a1a22c986..7ef6f0a1ca 100644 --- a/src/applications/differential/query/DifferentialInlineCommentQuery.php +++ b/src/applications/differential/query/DifferentialInlineCommentQuery.php @@ -1,140 +1,328 @@ viewer = $viewer; return $this; } public function getViewer() { return $this->viewer; } public function withIDs(array $ids) { $this->ids = $ids; return $this; } public function withPHIDs(array $phids) { $this->phids = $phids; return $this; } public function withDrafts($drafts) { $this->drafts = $drafts; return $this; } public function withAuthorPHIDs(array $author_phids) { $this->authorPHIDs = $author_phids; return $this; } public function withRevisionPHIDs(array $revision_phids) { $this->revisionPHIDs = $revision_phids; return $this; } public function withDeletedDrafts($deleted_drafts) { $this->deletedDrafts = $deleted_drafts; return $this; } public function execute() { $table = new DifferentialTransactionComment(); $conn_r = $table->establishConnection('r'); $data = queryfx_all( $conn_r, 'SELECT * FROM %T %Q %Q', $table->getTableName(), $this->buildWhereClause($conn_r), $this->buildLimitClause($conn_r)); $comments = $table->loadAllFromArray($data); foreach ($comments as $key => $value) { $comments[$key] = DifferentialInlineComment::newFromModernComment( $value); } return $comments; } public function executeOne() { // TODO: Remove when this query moves to PolicyAware. return head($this->execute()); } protected function buildWhereClause(AphrontDatabaseConnection $conn_r) { $where = array(); // Only find inline comments. $where[] = qsprintf( $conn_r, 'changesetID IS NOT NULL'); if ($this->ids !== null) { $where[] = qsprintf( $conn_r, 'id IN (%Ld)', $this->ids); } if ($this->phids !== null) { $where[] = qsprintf( $conn_r, 'phid IN (%Ls)', $this->phids); } if ($this->revisionPHIDs !== null) { $where[] = qsprintf( $conn_r, 'revisionPHID IN (%Ls)', $this->revisionPHIDs); } if ($this->drafts === null) { if ($this->deletedDrafts) { $where[] = qsprintf( $conn_r, '(authorPHID = %s) OR (transactionPHID IS NOT NULL)', $this->getViewer()->getPHID()); } else { $where[] = qsprintf( $conn_r, '(authorPHID = %s AND isDeleted = 0) OR (transactionPHID IS NOT NULL)', $this->getViewer()->getPHID()); } } else if ($this->drafts) { $where[] = qsprintf( $conn_r, '(authorPHID = %s AND isDeleted = 0) AND (transactionPHID IS NULL)', $this->getViewer()->getPHID()); } else { $where[] = qsprintf( $conn_r, 'transactionPHID IS NOT NULL'); } return $this->formatWhereClause($where); } + public function adjustInlinesForChangesets( + array $inlines, + array $old, + array $new) { + + assert_instances_of($inlines, 'DifferentialInlineComment'); + assert_instances_of($old, 'DifferentialChangeset'); + assert_instances_of($new, 'DifferentialChangeset'); + + $viewer = $this->getViewer(); + $all = array_merge($old, $new); + + $changeset_ids = mpull($inlines, 'getChangesetID'); + $changeset_ids = array_unique($changeset_ids); + if ($changeset_ids) { + $changesets = id(new DifferentialChangesetQuery()) + ->setViewer($viewer) + ->withIDs($changeset_ids) + ->execute(); + $changesets = mpull($changesets, null, 'getID'); + } else { + $changesets = array(); + } + $changesets += mpull($all, null, 'getID'); + + $id_map = array(); + foreach ($all as $changeset) { + $id_map[$changeset->getID()] = $changeset->getID(); + } + + // Generate filename maps for older and newer comments. If we're bringing + // an older comment forward in a diff-of-diffs, we want to put it on the + // left side of the screen, not the right side. Both sides are "new" files + // with the same name, so they're both appropriate targets, but the left + // is a better target conceptually for users because it's more consistent + // with the rest of the UI, which shows old information on the left and + // new information on the right. + $move_here = DifferentialChangeType::TYPE_MOVE_HERE; + + $name_map_old = array(); + $name_map_new = array(); + $move_map = array(); + foreach ($all as $changeset) { + $changeset_id = $changeset->getID(); + + $filenames = array(); + $filenames[] = $changeset->getFilename(); + + // If this is the target of a move, also map comments on the old filename + // to this changeset. + if ($changeset->getChangeType() == $move_here) { + $old_file = $changeset->getOldFile(); + $filenames[] = $old_file; + $move_map[$changeset_id][$old_file] = true; + } + + foreach ($filenames as $filename) { + // We update the old map only if we don't already have an entry (oldest + // changeset persists). + if (empty($name_map_old[$filename])) { + $name_map_old[$filename] = $changeset_id; + } + + // We always update the new map (newest changeset overwrites). + $name_map_new[$changeset->getFilename()] = $changeset_id; + } + } + + // Find the smallest "new" changeset ID. We'll consider everything + // larger than this to be "newer", and everything smaller to be "older". + $first_new_id = min(mpull($new, 'getID')); + + $results = array(); + foreach ($inlines as $inline) { + $changeset_id = $inline->getChangesetID(); + if (isset($id_map[$changeset_id])) { + // This inline is legitimately on one of the current changesets, so + // we can include it in the result set unmodified. + $results[] = $inline; + continue; + } + + $changeset = idx($changesets, $changeset_id); + if (!$changeset) { + // Just discard this inline, as it has bogus data. + continue; + } + + $target_id = null; + + if ($changeset_id >= $first_new_id) { + $name_map = $name_map_new; + $is_new = true; + } else { + $name_map = $name_map_old; + $is_new = false; + } + + $filename = $changeset->getFilename(); + if (isset($name_map[$filename])) { + // This changeset is on a file with the same name as the current + // changeset, so we're going to port it forward or backward. + $target_id = $name_map[$filename]; + + $is_move = isset($move_map[$target_id][$filename]); + if ($is_new) { + if ($is_move) { + $reason = pht( + 'This comment was made on a file with the same name as the '. + 'file this file was moved from, but in a newer diff.'); + } else { + $reason = pht( + 'This comment was made on a file with the same name, but '. + 'in a newer diff.'); + } + } else { + if ($is_move) { + $reason = pht( + 'This comment was made on a file with the same name as the '. + 'file this file was moved from, but in an older diff.'); + } else { + $reason = pht( + 'This comment was made on a file with the same name, but '. + 'in an older diff.'); + } + } + } + + + // If we didn't find a target and this change is the target of a move, + // look for a match against the old filename. + if (!$target_id) { + if ($changeset->getChangeType() == $move_here) { + $filename = $changeset->getOldFile(); + if (isset($name_map[$filename])) { + $target_id = $name_map[$filename]; + if ($is_new) { + $reason = pht( + 'This comment was made on a file which this file was moved '. + 'to, but in a newer diff.'); + } else { + $reason = pht( + 'This comment was made on a file which this file was moved '. + 'to, but in an older diff.'); + } + } + } + } + + + // If we found a changeset to port this comment to, bring it forward + // or backward and mark it. + if ($target_id) { + $inline + ->makeEphemeral(true) + ->setChangesetID($target_id) + ->setIsGhost( + array( + 'new' => $is_new, + 'reason' => $reason, + )); + + $results[] = $inline; + } + } + + // Filter out the inlines we ported forward which won't be visible because + // they appear on the wrong side of a file. + $keep_map = array(); + foreach ($old as $changeset) { + $keep_map[$changeset->getID()][0] = true; + } + foreach ($new as $changeset) { + $keep_map[$changeset->getID()][1] = true; + } + + foreach ($results as $key => $inline) { + $is_new = (int)$inline->getIsNewFile(); + $changeset_id = $inline->getChangesetID(); + if (!isset($keep_map[$changeset_id][$is_new])) { + unset($results[$key]); + continue; + } + } + + return $results; + } + }