Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15500465
D12484.id29971.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
10 KB
Referenced Files
None
Subscribers
None
D12484.id29971.diff
View Options
diff --git a/src/applications/audit/storage/PhabricatorAuditInlineComment.php b/src/applications/audit/storage/PhabricatorAuditInlineComment.php
--- a/src/applications/audit/storage/PhabricatorAuditInlineComment.php
+++ b/src/applications/audit/storage/PhabricatorAuditInlineComment.php
@@ -5,6 +5,7 @@
private $proxy;
private $syntheticAuthor;
+ private $isGhost;
public function __construct() {
$this->proxy = new PhabricatorAuditTransactionComment();
@@ -308,6 +309,15 @@
return $this->proxy->getFixedState();
}
+ public function setIsGhost($is_ghost) {
+ $this->isGhost = $is_ghost;
+ return $this;
+ }
+
+ public function getIsGhost() {
+ return $this->isGhost;
+ }
+
/* -( PhabricatorMarkupInterface Implementation )-------------------------- */
diff --git a/src/applications/differential/controller/DifferentialChangesetViewController.php b/src/applications/differential/controller/DifferentialChangesetViewController.php
--- a/src/applications/differential/controller/DifferentialChangesetViewController.php
+++ b/src/applications/differential/controller/DifferentialChangesetViewController.php
@@ -9,8 +9,6 @@
public function handleRequest(AphrontRequest $request) {
$viewer = $this->getViewer();
- $author_phid = $viewer->getPHID();
-
$rendering_reference = $request->getStr('ref');
$parts = explode('/', $rendering_reference);
if (count($parts) == 2) {
@@ -161,10 +159,33 @@
$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.
- $inlines = $this->loadInlineComments(
- array($left_source, $right_source),
- $author_phid);
+ if ($revision) {
+ $inlines = $this->loadInlineComments(
+ $revision,
+ nonempty($left, $right),
+ $left_new,
+ $right,
+ $right_new);
+ } else {
+ $inlines = array();
+ }
if ($left_new) {
$inlines = array_merge(
@@ -201,22 +222,6 @@
$engine->process();
- $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();
- }
- }
-
$parser
->setUser($viewer)
->setMarkupEngine($engine)
@@ -280,16 +285,82 @@
));
}
- private function loadInlineComments(array $changeset_ids, $author_phid) {
- $changeset_ids = array_unique(array_filter($changeset_ids));
- if (!$changeset_ids) {
- return;
- }
+ private function loadInlineComments(
+ DifferentialRevision $revision,
+ DifferentialChangeset $left,
+ $left_new,
+ DifferentialChangeset $right,
+ $right_new) {
+
+ $viewer = $this->getViewer();
+ $all = array($left, $right);
- return id(new DifferentialInlineCommentQuery())
- ->setViewer($this->getViewer())
- ->withChangesetIDs($changeset_ids)
+ $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(
+ $left->getID() => $left->getID(),
+ $right->getID() => $right->getID(),
+ );
+
+ $name_map = array(
+ $left->getFilename() => $left->getID(),
+ $right->getFilename() => $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 with bogus data.
+ continue;
+ }
+
+ $target_id = null;
+
+ $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];
+ }
+
+ // 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(true);
+
+ $results[] = $inline;
+ }
+ }
+
+ return $results;
}
private function buildRawFileResponse(
diff --git a/src/applications/differential/query/DifferentialInlineCommentQuery.php b/src/applications/differential/query/DifferentialInlineCommentQuery.php
--- a/src/applications/differential/query/DifferentialInlineCommentQuery.php
+++ b/src/applications/differential/query/DifferentialInlineCommentQuery.php
@@ -13,7 +13,6 @@
private $phids;
private $drafts;
private $authorPHIDs;
- private $changesetIDs;
private $revisionPHIDs;
private $deletedDrafts;
@@ -46,11 +45,6 @@
return $this;
}
- public function withChangesetIDs(array $ids) {
- $this->changesetIDs = $ids;
- return $this;
- }
-
public function withRevisionPHIDs(array $revision_phids) {
$this->revisionPHIDs = $revision_phids;
return $this;
@@ -116,13 +110,6 @@
$this->revisionPHIDs);
}
- if ($this->changesetIDs !== null) {
- $where[] = qsprintf(
- $conn_r,
- 'changesetID IN (%Ld)',
- $this->changesetIDs);
- }
-
if ($this->drafts === null) {
if ($this->deletedDrafts) {
$where[] = qsprintf(
diff --git a/src/applications/differential/storage/DifferentialInlineComment.php b/src/applications/differential/storage/DifferentialInlineComment.php
--- a/src/applications/differential/storage/DifferentialInlineComment.php
+++ b/src/applications/differential/storage/DifferentialInlineComment.php
@@ -5,6 +5,7 @@
private $proxy;
private $syntheticAuthor;
+ private $isGhost;
public function __construct() {
$this->proxy = new DifferentialTransactionComment();
@@ -225,6 +226,20 @@
return $this->proxy->getFixedState();
}
+ public function setIsGhost($is_ghost) {
+ $this->isGhost = $is_ghost;
+ return $this;
+ }
+
+ public function getIsGhost() {
+ return $this->isGhost;
+ }
+
+ public function makeEphemeral() {
+ $this->proxy->makeEphemeral();
+ return $this;
+ }
+
/* -( PhabricatorMarkupInterface Implementation )-------------------------- */
diff --git a/src/infrastructure/diff/interface/PhabricatorInlineCommentInterface.php b/src/infrastructure/diff/interface/PhabricatorInlineCommentInterface.php
--- a/src/infrastructure/diff/interface/PhabricatorInlineCommentInterface.php
+++ b/src/infrastructure/diff/interface/PhabricatorInlineCommentInterface.php
@@ -54,4 +54,7 @@
public function save();
public function delete();
+ public function setIsGhost($is_ghost);
+ public function getIsGhost();
+
}
diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php
--- a/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php
+++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php
@@ -122,6 +122,19 @@
->addClass('mml inline-draft-text');
}
+ $ghost_tag = null;
+ if ($inline->getIsGhost()) {
+ // TODO: This could also be a newer comment, not necessarily an older
+ // comment.
+ $ghost_tag = id(new PHUITagView())
+ ->setType(PHUITagView::TYPE_SHADE)
+ ->setName(pht('Old Comment'))
+ ->setSlimShady(true)
+ ->setShade(PHUITagView::COLOR_BLUE)
+ ->addClass('mml');
+ $classes[] = 'inline-comment-ghost';
+ }
+
// I think this is unused
if ($inline->getHasReplies()) {
$classes[] = 'inline-comment-has-reply';
@@ -351,6 +364,7 @@
$author,
$author_owner,
$draft_text,
+ $ghost_tag,
));
$group_right = phutil_tag(
diff --git a/webroot/rsrc/css/application/differential/phui-inline-comment.css b/webroot/rsrc/css/application/differential/phui-inline-comment.css
--- a/webroot/rsrc/css/application/differential/phui-inline-comment.css
+++ b/webroot/rsrc/css/application/differential/phui-inline-comment.css
@@ -132,6 +132,32 @@
padding-bottom: 4px;
}
+/* - Ghost Comment ------------------------------------------------------------
+
+ Comments from older or newer versions of the changeset.
+
+*/
+
+.differential-inline-comment.inline-comment-ghost {
+ border: 1px solid {$lightgreyborder};
+ opacity: 0.75;
+}
+
+.differential-inline-comment.inline-comment-ghost
+ .differential-inline-comment-head {
+ border-bottom: 1px solid {$lightgreyborder};
+ background-color: {$lightgreybackground};
+}
+
+.differential-inline-comment.inline-comment-ghost
+ .button.simple {
+ border-color: {$lightgreyborder};
+}
+
+.differential-inline-comment.inline-comment-ghost
+ .button.simple .phui-icon-view {
+ color: {$lightgreytext};
+}
/* - New/Edit Inline Comment --------------------------------------------------
@@ -350,7 +376,7 @@
}
.differential-inline-comment.inline-state-is-draft .inline-draft-text {
- display: inline-block;
+ display: inline-block;
}
.inline-state-is-draft .differential-inline-done-label {
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Mon, Apr 14, 6:01 PM (1 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7661574
Default Alt Text
D12484.id29971.diff (10 KB)
Attached To
Mode
D12484: Port comments through time and space in the common/best case
Attached
Detach File
Event Timeline
Log In to Comment