1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-09-20 09:18:48 +02:00

Distinguish between ported-forward and ported-backward comments

Summary:
Ref T7447. This might be overkill, but I want to over-explain things until we have more confidence that this is rarely confusing.

NOTE: I'm playing it a bit fast and loose with `setIsGhost()` (passing a dictionary) because making API changes requires changing the interface and Diffusion, which is a pain. I'll clean this up at the end once the interface is more final. This is well-contained for now.

Test Plan:
  - Viewed "base vs 2" in a diff with 3 diffs, saw some "older comments" and some "newer comments".
  - Hovered the tags for an explanation of comment spookiness.

{F377703}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7447

Differential Revision: https://secure.phabricator.com/D12490
This commit is contained in:
epriestley 2015-04-21 06:16:03 -07:00
parent aa04e97de7
commit fe774e68e2
2 changed files with 31 additions and 5 deletions

View file

@ -368,8 +368,10 @@ final class DifferentialChangesetViewController extends DifferentialController {
if ($changeset_id >= $first_new_id) { if ($changeset_id >= $first_new_id) {
$name_map = $name_map_new; $name_map = $name_map_new;
$is_new = true;
} else { } else {
$name_map = $name_map_old; $name_map = $name_map_old;
$is_new = false;
} }
$filename = $changeset->getFilename(); $filename = $changeset->getFilename();
@ -377,6 +379,15 @@ final class DifferentialChangesetViewController extends DifferentialController {
// This changeset is on a file with the same name as the current // This changeset is on a file with the same name as the current
// changeset, so we're going to port it forward or backward. // changeset, so we're going to port it forward or backward.
$target_id = $name_map[$filename]; $target_id = $name_map[$filename];
if ($is_new) {
$reason = pht(
'This comment was made on a file with the same name, but '.
'in a newer diff.');
} else {
$reason = pht(
'This comment was made on a file with the same name, but '.
'in an older diff.');
}
} }
// If we found a changeset to port this comment to, bring it forward // If we found a changeset to port this comment to, bring it forward
@ -385,7 +396,11 @@ final class DifferentialChangesetViewController extends DifferentialController {
$inline $inline
->makeEphemeral(true) ->makeEphemeral(true)
->setChangesetID($target_id) ->setChangesetID($target_id)
->setIsGhost(true); ->setIsGhost(
array(
'new' => $is_new,
'reason' => $reason,
));
$results[] = $inline; $results[] = $inline;
} }

View file

@ -123,14 +123,25 @@ final class PHUIDiffInlineCommentDetailView
} }
$ghost_tag = null; $ghost_tag = null;
if ($inline->getIsGhost()) { $ghost = $inline->getIsGhost();
// TODO: This could also be a newer comment, not necessarily an older if ($ghost) {
// comment. if ($ghost['new']) {
$ghost_text = pht('Newer Comment');
} else {
$ghost_text = pht('Older Comment');
}
$ghost_tag = id(new PHUITagView()) $ghost_tag = id(new PHUITagView())
->setType(PHUITagView::TYPE_SHADE) ->setType(PHUITagView::TYPE_SHADE)
->setName(pht('Old Comment')) ->setName($ghost_text)
->setSlimShady(true) ->setSlimShady(true)
->setShade(PHUITagView::COLOR_BLUE) ->setShade(PHUITagView::COLOR_BLUE)
->addSigil('has-tooltip')
->setMetadata(
array(
'tip' => $ghost['reason'],
'size' => 300,
))
->addClass('mml'); ->addClass('mml');
$classes[] = 'inline-comment-ghost'; $classes[] = 'inline-comment-ghost';
} }