diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php index d3fb3637fc..20e3e7a568 100644 --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -776,10 +776,26 @@ final class DifferentialTransactionEditor $body = parent::buildMailBody($object, $xactions); + $type_inline = DifferentialTransaction::TYPE_INLINE; + + $inlines = array(); + foreach ($xactions as $xaction) { + if ($xaction->getTransactionType() == $type_inline) { + $inlines[] = $xaction; + } + } + + if ($inlines) { + $body->addTextSection( + pht('INLINE COMMENTS'), + $this->renderInlineCommentsForMail($object, $inlines)); + } + $body->addTextSection( pht('REVISION DETAIL'), PhabricatorEnv::getProductionURI('/D'.$object->getID())); + return $body; } @@ -797,4 +813,76 @@ final class DifferentialTransactionEditor return parent::extractFilePHIDsFromCustomTransaction($object, $xaction); } + private function renderInlineCommentsForMail( + PhabricatorLiskDAO $object, + array $inlines) { + + $context_key = 'metamta.differential.unified-comment-context'; + $show_context = PhabricatorEnv::getEnvConfig($context_key); + + $changeset_ids = array(); + foreach ($inlines as $inline) { + $id = $inline->getComment()->getChangesetID(); + $changeset_ids[$id] = $id; + } + + // TODO: We should write a proper Query class for this eventually. + $changesets = id(new DifferentialChangeset())->loadAllWhere( + 'id IN (%Ld)', + $changeset_ids); + if ($show_context) { + $hunk_parser = new DifferentialHunkParser(); + foreach ($changesets as $changeset) { + $changeset->attachHunks($changeset->loadHunks()); + } + } + + $inline_groups = DifferentialTransactionComment::sortAndGroupInlines( + $inlines, + $changesets); + + $result = array(); + foreach ($inline_groups as $changeset_id => $group) { + $changeset = idx($changesets, $changeset_id); + if (!$changeset) { + continue; + } + + foreach ($group as $inline) { + $comment = $inline->getComment(); + $file = $changeset->getFilename(); + $start = $comment->getLineNumber(); + $len = $comment->getLineLength(); + if ($len) { + $range = $start.'-'.($start + $len); + } else { + $range = $start; + } + + $inline_content = $comment->getContent(); + + if (!$show_context) { + $result[] = "{$file}:{$range} {$inline_content}"; + } else { + $result[] = "================"; + $result[] = "Comment at: " . $file . ":" . $range; + $result[] = $hunk_parser->makeContextDiff( + $changeset->getHunks(), + $comment->getIsNewFile(), + $comment->getLineNumber(), + $comment->getLineLength(), + 1); + $result[] = "----------------"; + + $result[] = $inline_content; + $result[] = null; + } + } + } + + return implode("\n", $result); + } + + + } diff --git a/src/applications/differential/storage/DifferentialTransaction.php b/src/applications/differential/storage/DifferentialTransaction.php index cd82ab1737..80e2b914ee 100644 --- a/src/applications/differential/storage/DifferentialTransaction.php +++ b/src/applications/differential/storage/DifferentialTransaction.php @@ -41,6 +41,36 @@ final class DifferentialTransaction extends PhabricatorApplicationTransaction { return false; } + public function shouldHideForMail(array $xactions) { + switch ($this->getTransactionType()) { + case self::TYPE_INLINE: + // Hide inlines when rendering mail transactions if any other + // transaction type exists. + foreach ($xactions as $xaction) { + if ($xaction->getTransactionType() != self::TYPE_INLINE) { + return true; + } + } + + // If only inline transactions exist, we just render the first one. + return ($this !== head($xactions)); + } + + return $this->shouldHide(); + } + + public function getBodyForMail() { + switch ($this->getTransactionType()) { + case self::TYPE_INLINE: + // Don't render inlines into the mail body; they render into a special + // section immediately after the body instead. + return null; + } + + return parent::getBodyForMail(); + } + + public function getTitle() { $author_phid = $this->getAuthorPHID(); $author_handle = $this->renderHandleLink($author_phid); diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index adc83a2819..a04daf4123 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -1592,13 +1592,18 @@ abstract class PhabricatorApplicationTransactionEditor $comments = array(); foreach ($xactions as $xaction) { - if ($xaction->shouldHideForMail()) { + if ($xaction->shouldHideForMail($xactions)) { continue; } - $headers[] = id(clone $xaction)->setRenderingTarget('text')->getTitle(); - $comment = $xaction->getComment(); - if ($comment && strlen($comment->getContent())) { - $comments[] = $comment->getContent(); + + $header = $xaction->getTitleForMail(); + if ($header !== null) { + $headers[] = $header; + } + + $comment = $xaction->getBodyForMail(); + if ($comment !== null) { + $comments[] = $comment; } } diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php index bef24bed2b..1d777a3872 100644 --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -318,10 +318,22 @@ abstract class PhabricatorApplicationTransaction return false; } - public function shouldHideForMail() { + public function shouldHideForMail(array $xactions) { return $this->shouldHide(); } + public function getTitleForMail() { + return id(clone $this)->setRenderingTarget('text')->getTitle(); + } + + public function getBodyForMail() { + $comment = $this->getComment(); + if ($comment && strlen($comment->getContent())) { + return $comment->getContent(); + } + return null; + } + public function getNoEffectDescription() { switch ($this->getTransactionType()) {