mirror of
https://we.phorge.it/source/phorge.git
synced 2024-09-19 16:58:48 +02:00
Adds an option to allow sending unified diff contexts in differential mails.
This commit is contained in:
Manuel Klimek
Manuel Klimek
parent
df3162584e
commit
17217fda28
5 changed files with 301 additions and 2 deletions
|
|
@ -382,6 +382,13 @@ return array(
|
|||
// patches are sent in. Valid options are 'unified' (like diff -u) or 'git'.
|
||||
'metamta.differential.patch-format' => 'unified',
|
||||
|
||||
// Enables a different format for comments in differential emails.
|
||||
// Differential will create unified diffs around the comment, which
|
||||
// will give enough context for people who are only viewing the
|
||||
// reviews in email to understand what is going on. The context will
|
||||
// be created based on the range of the comment.
|
||||
'metamta.differential.unified-comment-context' => true,
|
||||
|
||||
// Prefix prepended to mail sent by Diffusion.
|
||||
'metamta.diffusion.subject-prefix' => '[Diffusion]',
|
||||
|
||||
|
|
|
|||
|
|
@ -216,6 +216,7 @@ phutil_register_library_map(array(
|
|||
'DifferentialBranchFieldSpecification' => 'applications/differential/field/specification/DifferentialBranchFieldSpecification.php',
|
||||
'DifferentialCCWelcomeMail' => 'applications/differential/mail/DifferentialCCWelcomeMail.php',
|
||||
'DifferentialCCsFieldSpecification' => 'applications/differential/field/specification/DifferentialCCsFieldSpecification.php',
|
||||
'DifferentialChangeSetTestCase' => 'applications/differential/storage/__tests__/DifferentialChangesetTestCase.php',
|
||||
'DifferentialChangeType' => 'applications/differential/constants/DifferentialChangeType.php',
|
||||
'DifferentialChangeset' => 'applications/differential/storage/DifferentialChangeset.php',
|
||||
'DifferentialChangesetDetailView' => 'applications/differential/view/DifferentialChangesetDetailView.php',
|
||||
|
|
@ -1381,6 +1382,7 @@ phutil_register_library_map(array(
|
|||
'DifferentialBranchFieldSpecification' => 'DifferentialFieldSpecification',
|
||||
'DifferentialCCWelcomeMail' => 'DifferentialReviewRequestMail',
|
||||
'DifferentialCCsFieldSpecification' => 'DifferentialFieldSpecification',
|
||||
'DifferentialChangeSetTestCase' => 'PhabricatorTestCase',
|
||||
'DifferentialChangeset' => 'DifferentialDAO',
|
||||
'DifferentialChangesetDetailView' => 'AphrontView',
|
||||
'DifferentialChangesetListView' => 'AphrontView',
|
||||
|
|
|
|||
|
|
@ -165,8 +165,21 @@ final class DifferentialCommentMail extends DifferentialMail {
|
|||
} else {
|
||||
$range = $start;
|
||||
}
|
||||
$content = $inline->getContent();
|
||||
$body[] = $this->formatText("{$file}:{$range} {$content}");
|
||||
|
||||
if (!PhabricatorEnv::getEnvConfig(
|
||||
'metamta.differential.unified-comment-context', false)) {
|
||||
$body[] = $this->formatText("{$file}:{$range} {$content}");
|
||||
} else {
|
||||
$body[] = "Comment at: " . $file . ":" . $range;
|
||||
|
||||
$changeset->attachHunks($changeset->loadHunks());
|
||||
$body[] = $changeset->makeContextDiff($inline, 1);
|
||||
$body[] = null;
|
||||
|
||||
$content = $inline->getContent();
|
||||
$body[] = $content;
|
||||
$body[] = null;
|
||||
}
|
||||
}
|
||||
$body[] = null;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -215,4 +215,88 @@ final class DifferentialChangeset extends DifferentialDAO {
|
|||
return false;
|
||||
}
|
||||
|
||||
public function makeContextDiff($inline, $add_context) {
|
||||
$context = array();
|
||||
$debug = False;
|
||||
if ($debug) {
|
||||
$context[] = 'Inline: '.$inline->getIsNewFile().' '.
|
||||
$inline->getLineNumber().' '.$inline->getLineLength();
|
||||
foreach ($this->getHunks() as $hunk) {
|
||||
$context[] = 'hunk: '.$hunk->getOldOffset().'-'.
|
||||
$hunk->getOldLen().'; '.$hunk->getNewOffset().'-'.$hunk->getNewLen();
|
||||
$context[] = $hunk->getChanges();
|
||||
}
|
||||
}
|
||||
|
||||
if ($inline->getIsNewFile()) {
|
||||
$prefix = '+';
|
||||
} else {
|
||||
$prefix = '-';
|
||||
}
|
||||
foreach ($this->getHunks() as $hunk) {
|
||||
if ($inline->getIsNewFile()) {
|
||||
$offset = $hunk->getNewOffset();
|
||||
$length = $hunk->getNewLen();
|
||||
} else {
|
||||
$offset = $hunk->getOldOffset();
|
||||
$length = $hunk->getOldLen();
|
||||
}
|
||||
$start = $inline->getLineNumber() - $offset;
|
||||
$end = $start + $inline->getLineLength();
|
||||
// We need to go in if $start == $length, because the last line
|
||||
// might be a "\No newline at end of file" marker, which we want
|
||||
// to show if the additional context is > 0.
|
||||
if ($start <= $length && $end >= 0) {
|
||||
$start = $start - $add_context;
|
||||
$end = $end + $add_context;
|
||||
$hunk_content = array();
|
||||
$hunk_pos = array( "-" => 0, "+" => 0 );
|
||||
$hunk_offset = array( "-" => NULL, "+" => NULL );
|
||||
$hunk_last = array( "-" => NULL, "+" => NULL );
|
||||
foreach (explode("\n", $hunk->getChanges()) as $line) {
|
||||
$inCommon = strncmp($line, " ", 1) === 0;
|
||||
$inOld = strncmp($line, "-", 1) === 0 || $inCommon;
|
||||
$inNew = strncmp($line, "+", 1) === 0 || $inCommon;
|
||||
$inSelected = strncmp($line, $prefix, 1) === 0;
|
||||
$skip = !$inSelected && !$inCommon;
|
||||
if ($hunk_pos[$prefix] <= $end) {
|
||||
if ($start <= $hunk_pos[$prefix]) {
|
||||
if (!$skip || ($hunk_pos[$prefix] != $start &&
|
||||
$hunk_pos[$prefix] != $end)) {
|
||||
if ($inOld) {
|
||||
if ($hunk_offset["-"] === NULL)
|
||||
$hunk_offset["-"] = $hunk_pos["-"];
|
||||
$hunk_last["-"] = $hunk_pos["-"];
|
||||
}
|
||||
if ($inNew) {
|
||||
if ($hunk_offset["+"] === NULL)
|
||||
$hunk_offset["+"] = $hunk_pos["+"];
|
||||
$hunk_last["+"] = $hunk_pos["+"];
|
||||
}
|
||||
|
||||
$hunk_content[] = $line;
|
||||
}
|
||||
}
|
||||
if ($inOld) ++$hunk_pos["-"];
|
||||
if ($inNew) ++$hunk_pos["+"];
|
||||
}
|
||||
}
|
||||
if ($hunk_offset["-"] !== NULL || $hunk_offset["+"] !== NULL) {
|
||||
$header = "@@";
|
||||
if ($hunk_offset["-"] !== NULL) {
|
||||
$header .= " -" . ($hunk->getOldOffset() + $hunk_offset["-"]) .
|
||||
"," . ($hunk_last["-"]-$hunk_offset["-"]+1);
|
||||
}
|
||||
if ($hunk_offset["+"] !== NULL) {
|
||||
$header .= " +" . ($hunk->getNewOffset() + $hunk_offset["+"]) .
|
||||
"," . ($hunk_last["+"]-$hunk_offset["+"]+1);
|
||||
}
|
||||
$header .= " @@";
|
||||
$context[] = $header;
|
||||
$context[] = implode("\n", $hunk_content);
|
||||
}
|
||||
}
|
||||
}
|
||||
return implode("\n", $context);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,193 @@
|
|||
<?php
|
||||
|
||||
final class DifferentialChangeSetTestCase extends PhabricatorTestCase {
|
||||
private function createComment() {
|
||||
$comment = new DifferentialInlineComment();
|
||||
return $comment;
|
||||
}
|
||||
// $line: 1 based
|
||||
// $length: 0 based (0 meaning 1 line)
|
||||
private function createNewComment($line, $length) {
|
||||
$comment = $this->createComment();
|
||||
$comment->setIsNewFile(True);
|
||||
$comment->setLineNumber($line);
|
||||
$comment->setLineLength($length);
|
||||
return $comment;
|
||||
}
|
||||
// $line: 1 based
|
||||
// $length: 0 based (0 meaning 1 line)
|
||||
private function createOldComment($line, $length) {
|
||||
$comment = $this->createComment();
|
||||
$comment->setIsNewFile(False);
|
||||
$comment->setLineNumber($line);
|
||||
$comment->setLineLength($length);
|
||||
return $comment;
|
||||
}
|
||||
private function createHunk($oldOffset, $oldLen, $newOffset, $newLen, $changes) {
|
||||
$hunk = new DifferentialHunk();
|
||||
$hunk->setOldOffset($oldOffset);
|
||||
$hunk->setOldLen($oldLen);
|
||||
$hunk->setNewOffset($newOffset);
|
||||
$hunk->setNewLen($newLen);
|
||||
$hunk->setChanges($changes);
|
||||
return $hunk;
|
||||
}
|
||||
private function createChange($hunks) {
|
||||
$change = new DifferentialChangeset();
|
||||
$change->attachHunks($hunks);
|
||||
return $change;
|
||||
}
|
||||
// Returns a change that consists of a single hunk, starting at line 1.
|
||||
private function createSingleChange($old_lines, $new_lines, $changes) {
|
||||
return $this->createChange(array(
|
||||
0 => $this->createHunk(1, $old_lines, 1, $new_lines, $changes),
|
||||
));
|
||||
}
|
||||
|
||||
public function testOneLineOldComment() {
|
||||
$change = $this->createSingleChange(1, 0, "-a");
|
||||
$context = $change->makeContextDiff($this->createOldComment(1, 0), 0);
|
||||
$this->assertEqual("@@ -1,1 @@\n-a", $context);
|
||||
}
|
||||
|
||||
public function testOneLineNewComment() {
|
||||
$change = $this->createSingleChange(0, 1, "+a");
|
||||
$context = $change->makeContextDiff($this->createNewComment(1, 0), 0);
|
||||
$this->assertEqual("@@ +1,1 @@\n+a", $context);
|
||||
}
|
||||
|
||||
public function testCannotFindContext() {
|
||||
$change = $this->createSingleChange(0, 1, "+a");
|
||||
$context = $change->makeContextDiff($this->createNewComment(2, 0), 0);
|
||||
$this->assertEqual("", $context);
|
||||
}
|
||||
|
||||
public function testOverlapFromStartOfHunk() {
|
||||
$change = $this->createChange(array(
|
||||
0 => $this->createHunk(23, 2, 42, 2, " 1\n 2"),
|
||||
));
|
||||
$context = $change->makeContextDiff($this->createNewComment(41, 1), 0);
|
||||
$this->assertEqual("@@ -23,1 +42,1 @@\n 1", $context);
|
||||
}
|
||||
|
||||
public function testOverlapAfterEndOfHunk() {
|
||||
$change = $this->createChange(array(
|
||||
0 => $this->createHunk(23, 2, 42, 2, " 1\n 2"),
|
||||
));
|
||||
$context = $change->makeContextDiff($this->createNewComment(43, 1), 0);
|
||||
$this->assertEqual("@@ -24,1 +43,1 @@\n 2", $context);
|
||||
}
|
||||
|
||||
public function testInclusionOfNewFileInOldCommentFromStart() {
|
||||
$change = $this->createSingleChange(2, 3,
|
||||
"+n1\n".
|
||||
" e1/2\n".
|
||||
"-o2\n".
|
||||
"+n3\n");
|
||||
$context = $change->makeContextDiff($this->createOldComment(1, 1), 0);
|
||||
$this->assertEqual(
|
||||
"@@ -1,2 +2,1 @@\n".
|
||||
" e1/2\n".
|
||||
"-o2", $context);
|
||||
}
|
||||
|
||||
public function testInclusionOfOldFileInNewCommentFromStart() {
|
||||
$change = $this->createSingleChange(2, 2,
|
||||
"-o1\n".
|
||||
" e2/1\n".
|
||||
"-o3\n".
|
||||
"+n2\n");
|
||||
$context = $change->makeContextDiff($this->createNewComment(1, 1), 0);
|
||||
$this->assertEqual(
|
||||
"@@ -2,1 +1,2 @@\n".
|
||||
" e2/1\n".
|
||||
"+n2", $context);
|
||||
}
|
||||
|
||||
public function testNoNewlineAtEndOfFile() {
|
||||
$change = $this->createSingleChange(0, 1,
|
||||
"+a\n".
|
||||
"\\No newline at end of file");
|
||||
// Note that this only works with additional context.
|
||||
$context = $change->makeContextDiff($this->createNewComment(2, 0), 1);
|
||||
$this->assertEqual(
|
||||
"@@ +1,1 @@\n".
|
||||
"+a\n".
|
||||
"\\No newline at end of file", $context);
|
||||
}
|
||||
|
||||
public function testMultiLineNewComment() {
|
||||
$change = $this->createSingleChange(7, 7,
|
||||
" e1\n".
|
||||
" e2\n".
|
||||
"-o3\n".
|
||||
"-o4\n".
|
||||
"+n3\n".
|
||||
" e5/4\n".
|
||||
" e6/5\n".
|
||||
"+n6\n".
|
||||
" e7\n");
|
||||
$context = $change->makeContextDiff($this->createNewComment(2, 4), 0);
|
||||
$this->assertEqual(
|
||||
"@@ -2,5 +2,5 @@\n".
|
||||
" e2\n".
|
||||
"-o3\n".
|
||||
"-o4\n".
|
||||
"+n3\n".
|
||||
" e5/4\n".
|
||||
" e6/5\n".
|
||||
"+n6", $context);
|
||||
}
|
||||
|
||||
public function testMultiLineOldComment() {
|
||||
$change = $this->createSingleChange(7, 7,
|
||||
" e1\n".
|
||||
" e2\n".
|
||||
"-o3\n".
|
||||
"-o4\n".
|
||||
"+n3\n".
|
||||
" e5/4\n".
|
||||
" e6/5\n".
|
||||
"+n6\n".
|
||||
" e7\n");
|
||||
$context = $change->makeContextDiff($this->createOldComment(2, 4), 0);
|
||||
$this->assertEqual(
|
||||
"@@ -2,5 +2,4 @@\n".
|
||||
" e2\n".
|
||||
"-o3\n".
|
||||
"-o4\n".
|
||||
"+n3\n".
|
||||
" e5/4\n".
|
||||
" e6/5", $context);
|
||||
}
|
||||
|
||||
public function testInclusionOfNewFileInOldCommentFromStartWithContext() {
|
||||
$change = $this->createSingleChange(2, 3,
|
||||
"+n1\n".
|
||||
" e1/2\n".
|
||||
"-o2\n".
|
||||
"+n3\n");
|
||||
$context = $change->makeContextDiff($this->createOldComment(1, 1), 1);
|
||||
$this->assertEqual(
|
||||
"@@ -1,2 +1,2 @@\n".
|
||||
"+n1\n".
|
||||
" e1/2\n".
|
||||
"-o2", $context);
|
||||
}
|
||||
|
||||
public function testInclusionOfOldFileInNewCommentFromStartWithContext() {
|
||||
$change = $this->createSingleChange(2, 2,
|
||||
"-o1\n".
|
||||
" e2/1\n".
|
||||
"-o3\n".
|
||||
"+n2\n");
|
||||
$context = $change->makeContextDiff($this->createNewComment(1, 1), 1);
|
||||
$this->assertEqual(
|
||||
"@@ -1,3 +1,2 @@\n".
|
||||
"-o1\n".
|
||||
" e2/1\n".
|
||||
"-o3\n".
|
||||
"+n2", $context);
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
Reference in a new issue