Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F13209953
D11970.id28856.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
18 KB
Referenced Files
None
Subscribers
None
D11970.id28856.diff
View Options
diff --git a/src/applications/differential/__tests__/DifferentialParseRenderTestCase.php b/src/applications/differential/__tests__/DifferentialParseRenderTestCase.php
--- a/src/applications/differential/__tests__/DifferentialParseRenderTestCase.php
+++ b/src/applications/differential/__tests__/DifferentialParseRenderTestCase.php
@@ -2,8 +2,12 @@
final class DifferentialParseRenderTestCase extends PhabricatorTestCase {
+ private function getTestDataDirectory() {
+ return dirname(__FILE__).'/data/';
+ }
+
public function testParseRender() {
- $dir = dirname(__FILE__).'/data/';
+ $dir = $this->getTestDataDirectory();
foreach (Filesystem::listDirectory($dir, $show_hidden = false) as $file) {
if (!preg_match('/\.diff$/', $file)) {
continue;
@@ -22,46 +26,94 @@
}
foreach (array('one', 'two') as $type) {
- $parser = $this->buildChangesetParser($type, $data, $file);
- $actual = $parser->render(null, null, array());
+ $this->runParser($type, $data, $file, 'expect');
+ $this->runParser($type, $data, $file, 'unshielded');
+ $this->runParser($type, $data, $file, 'whitespace');
+ }
+ }
+ }
- $expect = Filesystem::readFile($dir.$file.'.'.$type.'.expect');
- $this->assertEqual($expect, (string)$actual, $file.'.'.$type);
+ private function runParser($type, $data, $file, $extension) {
+ $dir = $this->getTestDataDirectory();
+ $test_file = $dir.$file.'.'.$type.'.'.$extension;
+ if (!Filesystem::pathExists($test_file)) {
+ return;
+ }
+
+ $unshielded = false;
+ $whitespace = false;
+ switch ($extension) {
+ case 'unshielded':
+ $unshielded = true;
+ break;
+ case 'whitespace';
+ $unshielded = true;
+ $whitespace = true;
+ break;
+ }
+
+ $parsers = $this->buildChangesetParsers($type, $data, $file);
+ $actual = $this->renderParsers($parsers, $unshielded, $whitespace);
+ $expect = Filesystem::readFile($test_file);
+
+ $this->assertEqual($expect, $actual, basename($test_file));
+ }
+
+ private function renderParsers(array $parsers, $unshield, $whitespace) {
+ $result = array();
+ foreach ($parsers as $parser) {
+ if ($unshield) {
+ $s_range = 0;
+ $e_range = 0xFFFF;
+ } else {
+ $s_range = null;
+ $e_range = null;
}
+
+ if ($whitespace) {
+ $parser->setWhitespaceMode(
+ DifferentialChangesetParser::WHITESPACE_SHOW_ALL);
+ }
+
+ $result[] = $parser->render($s_range, $e_range, array());
}
+ return implode(str_repeat('~', 80)."\n", $result);
}
- private function buildChangesetParser($type, $data, $file) {
+ private function buildChangesetParsers($type, $data, $file) {
$parser = new ArcanistDiffParser();
$changes = $parser->parseDiff($data);
$diff = DifferentialDiff::newFromRawChanges(
PhabricatorUser::getOmnipotentUser(),
$changes);
- if (count($diff->getChangesets()) !== 1) {
- throw new Exception("Expected one changeset: {$file}");
- }
- $changeset = head($diff->getChangesets());
+ $changesets = $diff->getChangesets();
$engine = new PhabricatorMarkupEngine();
$engine->setViewer(new PhabricatorUser());
- $cparser = new DifferentialChangesetParser();
- $cparser->setUser(new PhabricatorUser());
- $cparser->setDisableCache(true);
- $cparser->setChangeset($changeset);
- $cparser->setMarkupEngine($engine);
-
- if ($type == 'one') {
- $cparser->setRenderer(new DifferentialChangesetOneUpTestRenderer());
- } else if ($type == 'two') {
- $cparser->setRenderer(new DifferentialChangesetTwoUpTestRenderer());
- } else {
- throw new Exception("Unknown renderer type '{$type}'!");
+ $parsers = array();
+ foreach ($changesets as $changeset) {
+ $cparser = new DifferentialChangesetParser();
+ $cparser->setUser(new PhabricatorUser());
+ $cparser->setDisableCache(true);
+ $cparser->setChangeset($changeset);
+ $cparser->setMarkupEngine($engine);
+
+ if ($type == 'one') {
+ $cparser->setRenderer(new DifferentialChangesetOneUpTestRenderer());
+ } else if ($type == 'two') {
+ $cparser->setRenderer(new DifferentialChangesetTwoUpTestRenderer());
+ } else {
+ throw new Exception(
+ pht('Unknown renderer type "%s"!', $type));
+ }
+
+ $parsers[] = $cparser;
}
- return $cparser;
+ return $parsers;
}
}
diff --git a/src/applications/differential/__tests__/data/fruit.diff.one.expect b/src/applications/differential/__tests__/data/fruit.diff.one.expect
--- a/src/applications/differential/__tests__/data/fruit.diff.one.expect
+++ b/src/applications/differential/__tests__/data/fruit.diff.one.expect
@@ -1,12 +1,14 @@
CTYPE 1 1 (unforced)
-
+fruit
-
-N 1 + apple~
-N 2 + banana~
-N 3 + cherry~
-N 4 + date~
-N 5 + elderberry~
-N 6 + fig~
-N 7 + grape~
-N 8 + honeydew~
-N 9 + ~
+N 1 + apple\n~
+N 2 + banana\n~
+N 3 + cherry\n~
+N 4 + date\n~
+N 5 + elderberry\n~
+N 6 + fig\n~
+N 7 + grape\n~
+N 8 + honeydew\n~
+N 9 + \n~
+O X <EMPTY>
diff --git a/src/applications/differential/__tests__/data/fruit.diff.two.expect b/src/applications/differential/__tests__/data/fruit.diff.two.expect
--- a/src/applications/differential/__tests__/data/fruit.diff.two.expect
+++ b/src/applications/differential/__tests__/data/fruit.diff.two.expect
@@ -1,21 +1,22 @@
CTYPE 1 1 (unforced)
-
+fruit
-
O - . ~
-N 1 + apple~
+N 1 + apple\n~
O - . ~
-N 2 + banana~
+N 2 + banana\n~
O - . ~
-N 3 + cherry~
+N 3 + cherry\n~
O - . ~
-N 4 + date~
+N 4 + date\n~
O - . ~
-N 5 + elderberry~
+N 5 + elderberry\n~
O - . ~
-N 6 + fig~
+N 6 + fig\n~
O - . ~
-N 7 + grape~
+N 7 + grape\n~
O - . ~
-N 8 + honeydew~
+N 8 + honeydew\n~
O - . ~
-N 9 + ~
+N 9 + \n~
diff --git a/src/applications/differential/__tests__/data/generated.diff b/src/applications/differential/__tests__/data/generated.diff
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/__tests__/data/generated.diff
@@ -0,0 +1,10 @@
+diff --git a/GENERATED b/GENERATED
+index 5dcff7f..eff82ef 100644
+--- a/GENERATED
++++ b/GENERATED
+@@ -1,4 +1,4 @@
+ @generated
+
+-This is a generated file.
++This is a generated file, full of generated code.
+
diff --git a/src/applications/differential/__tests__/data/generated.diff.one.expect b/src/applications/differential/__tests__/data/generated.diff.one.expect
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/__tests__/data/generated.diff.one.expect
@@ -0,0 +1,5 @@
+CTYPE 2 1 (unforced)
+GENERATED
+GENERATED
+-
+SHIELD (default) This file contains generated code, which does not normally need to be reviewed.
diff --git a/src/applications/differential/__tests__/data/generated.diff.one.unshielded b/src/applications/differential/__tests__/data/generated.diff.one.unshielded
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/__tests__/data/generated.diff.one.unshielded
@@ -0,0 +1,6 @@
+N 1 . @generated\n~
+O 2 - \n~
+N 2 + \n~
+O 3 - This is a generated file.\n~
+N 3 + This is a generated file{(, full of generated code)}.\n~
+N 4 . \n~
diff --git a/src/applications/differential/__tests__/data/generated.diff.two.expect b/src/applications/differential/__tests__/data/generated.diff.two.expect
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/__tests__/data/generated.diff.two.expect
@@ -0,0 +1,5 @@
+CTYPE 2 1 (unforced)
+GENERATED
+GENERATED
+-
+SHIELD (default) This file contains generated code, which does not normally need to be reviewed.
diff --git a/src/applications/differential/__tests__/data/generated.diff.two.unshielded b/src/applications/differential/__tests__/data/generated.diff.two.unshielded
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/__tests__/data/generated.diff.two.unshielded
@@ -0,0 +1,8 @@
+O 1 . @generated\n~
+N 1 . @generated\n~
+O 2 - \n~
+N 2 + \n~
+O 3 - This is a generated file.\n~
+N 3 + This is a generated file{(, full of generated code)}.\n~
+O 4 . \n~
+N 4 . \n~
diff --git a/src/applications/differential/__tests__/data/move-unedited.diff b/src/applications/differential/__tests__/data/move-unedited.diff
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/__tests__/data/move-unedited.diff
@@ -0,0 +1,4 @@
+diff --git a/SRC b/DST
+similarity index 100%
+rename from SRC
+rename to DST
diff --git a/src/applications/differential/__tests__/data/move-unedited.diff.one.expect b/src/applications/differential/__tests__/data/move-unedited.diff.one.expect
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/__tests__/data/move-unedited.diff.one.expect
@@ -0,0 +1,10 @@
+CTYPE 6 1 (unforced)
+SRC
+DST
+-
+SHIELD (none) The contents of this file were not changed.
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+CTYPE 4 1 (forced)
+-
+SRC
+DST
diff --git a/src/applications/differential/__tests__/data/move-unedited.diff.two.expect b/src/applications/differential/__tests__/data/move-unedited.diff.two.expect
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/__tests__/data/move-unedited.diff.two.expect
@@ -0,0 +1,10 @@
+CTYPE 6 1 (unforced)
+SRC
+DST
+-
+SHIELD (none) The contents of this file were not changed.
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+CTYPE 4 1 (forced)
+-
+SRC
+DST
diff --git a/src/applications/differential/__tests__/data/move.diff b/src/applications/differential/__tests__/data/move.diff
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/__tests__/data/move.diff
@@ -0,0 +1,12 @@
+diff --git a/SRC b/DST
+similarity index 57%
+rename from SRC
+rename to DST
+index 5d5c76f..f7bd789 100644
+--- a/SRC
++++ b/DST
+@@ -1,3 +1,3 @@
+ Apples
+-Bananas
++Blueberries
+ Cherries
diff --git a/src/applications/differential/__tests__/data/move.diff.one.expect b/src/applications/differential/__tests__/data/move.diff.one.expect
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/__tests__/data/move.diff.one.expect
@@ -0,0 +1,13 @@
+CTYPE 6 1 (unforced)
+SRC
+DST
+-
+N 1 . Apples\n~
+O 2 - B{(anana)}s\n~
+N 2 + B{(lueberrie)}s\n~
+N 3 . Cherries\n~
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+CTYPE 4 1 (forced)
+-
+SRC
+DST
diff --git a/src/applications/differential/__tests__/data/move.diff.two.expect b/src/applications/differential/__tests__/data/move.diff.two.expect
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/__tests__/data/move.diff.two.expect
@@ -0,0 +1,15 @@
+CTYPE 6 1 (unforced)
+SRC
+DST
+-
+O 1 . Apples\n~
+N 1 . Apples\n~
+O 2 - B{(anana)}s\n~
+N 2 + B{(lueberrie)}s\n~
+O 3 . Cherries\n~
+N 3 . Cherries\n~
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+CTYPE 4 1 (forced)
+-
+SRC
+DST
diff --git a/src/applications/differential/__tests__/data/whitespace.diff b/src/applications/differential/__tests__/data/whitespace.diff
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/__tests__/data/whitespace.diff
@@ -0,0 +1,7 @@
+diff --git a/WHITESPACE b/WHITESPACE
+index 4505d51..bca7deb 100644
+--- a/WHITESPACE
++++ b/WHITESPACE
+@@ -1 +1 @@
+--=[-Rocket-Ship>
++ -=[-Rocket-Ship>
diff --git a/src/applications/differential/__tests__/data/whitespace.diff.one.expect b/src/applications/differential/__tests__/data/whitespace.diff.one.expect
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/__tests__/data/whitespace.diff.one.expect
@@ -0,0 +1,5 @@
+CTYPE 2 1 (unforced)
+WHITESPACE
+WHITESPACE
+-
+SHIELD (whitespace) This file was changed only by adding or removing whitespace.
diff --git a/src/applications/differential/__tests__/data/whitespace.diff.one.whitespace b/src/applications/differential/__tests__/data/whitespace.diff.one.whitespace
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/__tests__/data/whitespace.diff.one.whitespace
@@ -0,0 +1,2 @@
+O 1 - -=[-Rocket-Ship>\n~
+N 1 + {( )}-=[-Rocket-Ship>\n~
diff --git a/src/applications/differential/__tests__/data/whitespace.diff.two.expect b/src/applications/differential/__tests__/data/whitespace.diff.two.expect
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/__tests__/data/whitespace.diff.two.expect
@@ -0,0 +1,5 @@
+CTYPE 2 1 (unforced)
+WHITESPACE
+WHITESPACE
+-
+SHIELD (whitespace) This file was changed only by adding or removing whitespace.
diff --git a/src/applications/differential/__tests__/data/whitespace.diff.two.whitespace b/src/applications/differential/__tests__/data/whitespace.diff.two.whitespace
new file mode 100644
--- /dev/null
+++ b/src/applications/differential/__tests__/data/whitespace.diff.two.whitespace
@@ -0,0 +1,2 @@
+O 1 - -=[-Rocket-Ship>\n~
+N 1 + {( )}-=[-Rocket-Ship>\n~
diff --git a/src/applications/differential/parser/DifferentialChangesetParser.php b/src/applications/differential/parser/DifferentialChangesetParser.php
--- a/src/applications/differential/parser/DifferentialChangesetParser.php
+++ b/src/applications/differential/parser/DifferentialChangesetParser.php
@@ -612,11 +612,6 @@
$moveaway = false;
$changetype = $this->changeset->getChangeType();
if ($changetype == DifferentialChangeType::TYPE_MOVE_AWAY) {
- // sometimes we show moved files as unchanged, sometimes deleted,
- // and sometimes inconsistent with what actually happened at the
- // destination of the move. Rather than make a false claim,
- // omit the 'not changed' notice if this is the source of a move
- $unchanged = true;
$moveaway = true;
}
@@ -776,6 +771,16 @@
pht(
'This file contains generated code, which does not normally '.
'need to be reviewed.'));
+ } else if ($this->isMoveAway()) {
+ // We put an empty shield on these files. Normally, they do not have
+ // any diff content anyway. However, if they come through `arc`, they
+ // may have content. We don't want to show it (it's not useful) and
+ // we bailed out of fully processing it earlier anyway.
+
+ // We could show a message like "this file was moved", but we show
+ // that as a change header anyway, so it would be redundant. Instead,
+ // just render an empty shield to skip rendering the diff body.
+ $shield = '';
} else if ($this->isUnchanged()) {
$type = 'text';
if (!$rows) {
@@ -791,8 +796,6 @@
$shield = $renderer->renderShield(
pht('The contents of this file were not changed.'),
$type);
- } else if ($this->isMoveAway()) {
- $shield = null;
} else if ($this->isWhitespaceOnly()) {
$shield = $renderer->renderShield(
pht('This file was changed only by adding or removing whitespace.'),
@@ -809,7 +812,7 @@
}
}
- if ($shield) {
+ if ($shield !== null) {
return $renderer->renderChangesetTable($shield);
}
diff --git a/src/applications/differential/parser/DifferentialHunkParser.php b/src/applications/differential/parser/DifferentialHunkParser.php
--- a/src/applications/differential/parser/DifferentialHunkParser.php
+++ b/src/applications/differential/parser/DifferentialHunkParser.php
@@ -96,7 +96,7 @@
public function setOldLineTypeMap(array $map) {
$lines = $this->getOldLines();
foreach ($lines as $key => $data) {
- $lines[$key]['type'] = $map[$data['line']];
+ $lines[$key]['type'] = idx($map, $data['line']);
}
$this->oldLines = $lines;
return $this;
@@ -117,7 +117,7 @@
public function setNewLineTypeMap(array $map) {
$lines = $this->getNewLines();
foreach ($lines as $key => $data) {
- $lines[$key]['type'] = $map[$data['line']];
+ $lines[$key]['type'] = idx($map, $data['line']);
}
$this->newLines = $lines;
return $this;
diff --git a/src/applications/differential/render/DifferentialChangesetTestRenderer.php b/src/applications/differential/render/DifferentialChangesetTestRenderer.php
--- a/src/applications/differential/render/DifferentialChangesetTestRenderer.php
+++ b/src/applications/differential/render/DifferentialChangesetTestRenderer.php
@@ -7,13 +7,16 @@
$changeset = $this->getChangeset();
$old = nonempty($changeset->getOldFile(), '-');
+ $current = nonempty($changeset->getFilename(), '-');
$away = nonempty(implode(', ', $changeset->getAwayPaths()), '-');
+
$ctype = $changeset->getChangeType();
$ftype = $changeset->getFileType();
$force = ($force ? '(forced)' : '(unforced)');
return "CTYPE {$ctype} {$ftype} {$force}\n".
"{$old}\n".
+ "{$current}\n".
"{$away}\n";
}
@@ -47,19 +50,50 @@
$out = array();
+ $any_old = false;
+ $any_new = false;
$primitives = $this->buildPrimitives($range_start, $range_len);
foreach ($primitives as $p) {
$type = $p['type'];
switch ($type) {
case 'old':
case 'new':
+ if ($type == 'old') {
+ $any_old = true;
+ }
+ if ($type == 'new') {
+ $any_new = true;
+ }
$num = nonempty($p['line'], '-');
$render = $p['render'];
$htype = nonempty($p['htype'], '.');
// TODO: This should probably happen earlier, whenever we deal with
// \r and \t normalization?
- $render = rtrim($render, "\r\n");
+ $render = str_replace(
+ array(
+ "\r",
+ "\n",
+ ),
+ array(
+ '\\r',
+ '\\n',
+ ),
+ $render);
+
+ $render = str_replace(
+ array(
+ '<span class="bright">',
+ '</span>',
+ ),
+ array(
+ '{(',
+ ')}',
+ ),
+ $render);
+
+ $render = html_entity_decode($render);
+
$t = ($type == 'old') ? 'O' : 'N';
$out[] = "{$t} {$num} {$htype} {$render}~";
@@ -70,6 +104,14 @@
}
}
+ if (!$any_old) {
+ $out[] = 'O X <EMPTY>';
+ }
+
+ if (!$any_new) {
+ $out[] = 'N X <EMPTY>';
+ }
+
$out = implode("\n", $out)."\n";
return $out;
}
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Sat, May 18, 3:02 AM (2 w, 13 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6299387
Default Alt Text
D11970.id28856.diff (18 KB)
Attached To
Mode
D11970: Expand Differential test coverage to include moves, shields, and more
Attached
Detach File
Event Timeline
Log In to Comment