revi-archive/phorge-arcanist
1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2025-01-11 07:11:03 +01:00
Code Issues Releases Wiki Activity

Retain newline style when generating diffs

Browse source 
Summary: Builds on D3441, D3440. Instead of exploding on "\r?\n" and then imploding on "\n", retain newline style throughout parsing.

Test Plan:
All unit tests pass (the parser has substantial existing coverage). These new tests pass in the git commit-by-commit test case:

  commit 176a4c2c3fd88b2d598ce41a55d9c3958be9fd2d
  Author: epriestley <git@epriestley.com>
  Date:   Wed Sep 5 14:56:08 2012 -0700

      Convert \r\n newlines to \n newlines.

  commit a73b28e139296d23ade768f2346038318b331f94
  Author: epriestley <git@epriestley.com>
  Date:   Wed Sep 5 14:55:31 2012 -0700

      Add text with \r\n newlines.

  commit 337ccec314075a2bdb4a912ef467d35d04a713e4
  Author: epriestley <git@epriestley.com>
  Date:   Wed Sep 5 14:55:13 2012 -0700

      Convert \n newlines to \r\n newlines.

  commit 6d5e64a4a7a6a036c53b1d087184cb2c70099f2c
  Author: epriestley <git@epriestley.com>
  Date:   Wed Sep 5 14:53:39 2012 -0700

      Remove tabs.

  commit 49395994a1a8a06287e40a3b318be4349e8e0288
  Author: epriestley <git@epriestley.com>
  Date:   Wed Sep 5 14:53:33 2012 -0700

      Add tabs.

  commit a5a53c424f3c2a7e85f6aee35e834c8ec5b3dbe3
  Author: epriestley <git@epriestley.com>
  Date:   Wed Sep 5 14:52:57 2012 -0700

      Add trailing newline.

  commit d53dc614090c6c7d6d023e170877d7f611f18f5a
  Author: epriestley <git@epriestley.com>
  Date:   Wed Sep 5 14:52:41 2012 -0700

      Remove trailing newline.

Previously, the newline-related tests failed.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T866

Differential Revision: https://secure.phabricator.com/D3442
This commit is contained in:
epriestley 2012-10-03 15:32:03 -07:00
parent 383fb42ba8
commit d0425fc238
4 changed files with 77 additions and 54 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

58
src/parser/ArcanistBundle.php
View file

@ -226,7 +226,9 @@ final class ArcanistBundle {
}
$result[] = 'Index: '.$index_path;
$result[] = PHP_EOL;
$result[] = str_repeat('=', 67);
$result[] = PHP_EOL;
if ($old_path === null) {
$old_path = '/dev/null';
@ -247,8 +249,8 @@ final class ArcanistBundle {
$old_path = $cur_path;
}
$result[] = '--- '.$old_path;
$result[] = '+++ '.$cur_path;
$result[] = '--- '.$old_path.PHP_EOL;
$result[] = '+++ '.$cur_path.PHP_EOL;
$result[] = $hunk_changes;
}
@ -257,7 +259,7 @@ final class ArcanistBundle {
return '';
}
$diff = implode("\n", $result)."\n";
$diff = implode('', $result);
return $this->convertNonUTF8Diff($diff);
}
@ -383,45 +385,45 @@ final class ArcanistBundle {
$cur_target = 'b/'.$cur_path;
}
$result[] = "diff --git {$old_index} {$cur_index}";
$result[] = "diff --git {$old_index} {$cur_index}".PHP_EOL;
if ($type == ArcanistDiffChangeType::TYPE_ADD) {
$result[] = "new file mode {$new_mode}";
$result[] = "new file mode {$new_mode}".PHP_EOL;
}
if ($type == ArcanistDiffChangeType::TYPE_COPY_HERE ||
$type == ArcanistDiffChangeType::TYPE_MOVE_HERE ||
$type == ArcanistDiffChangeType::TYPE_COPY_AWAY) {
if ($old_mode !== $new_mode) {
$result[] = "old mode {$old_mode}";
$result[] = "new mode {$new_mode}";
$result[] = "old mode {$old_mode}".PHP_EOL;
$result[] = "new mode {$new_mode}".PHP_EOL;
}
}
if ($type == ArcanistDiffChangeType::TYPE_COPY_HERE) {
$result[] = "copy from {$old_path}";
$result[] = "copy to {$cur_path}";
$result[] = "copy from {$old_path}".PHP_EOL;
$result[] = "copy to {$cur_path}".PHP_EOL;
} else if ($type == ArcanistDiffChangeType::TYPE_MOVE_HERE) {
$result[] = "rename from {$old_path}";
$result[] = "rename to {$cur_path}";
$result[] = "rename from {$old_path}".PHP_EOL;
$result[] = "rename to {$cur_path}".PHP_EOL;
} else if ($type == ArcanistDiffChangeType::TYPE_DELETE ||
$type == ArcanistDiffChangeType::TYPE_MULTICOPY) {
$old_mode = idx($change->getOldProperties(), 'unix:filemode');
if ($old_mode) {
$result[] = "deleted file mode {$old_mode}";
$result[] = "deleted file mode {$old_mode}".PHP_EOL;
}
}
if ($change_body) {
if (!$is_binary) {
$result[] = "--- {$old_target}";
$result[] = "+++ {$cur_target}";
$result[] = "--- {$old_target}".PHP_EOL;
$result[] = "+++ {$cur_target}".PHP_EOL;
}
$result[] = $change_body;
}
}
$diff = implode("\n", $result)."\n";
$diff = implode('', $result).PHP_EOL;
return $this->convertNonUTF8Diff($diff);
}
@ -440,7 +442,7 @@ final class ArcanistBundle {
$context = 3;
$results = array();
$lines = explode("\n", $base_hunk->getCorpus());
$lines = phutil_split_lines($base_hunk->getCorpus());
$n = count($lines);
$old_offset = $base_hunk->getOldOffset();
@ -530,7 +532,7 @@ final class ArcanistBundle {
$hunk->setNewLength($count_length - $old_lines);
$corpus = array_slice($lines, $hunk_start, $hunk_length);
$corpus = implode("\n", $corpus);
$corpus = implode('', $corpus);
$hunk->setCorpus($corpus);
$results[] = $hunk;
@ -596,11 +598,11 @@ final class ArcanistBundle {
$n_head = "{$n_off},{$n_len}";
}
$result[] = "@@ -{$o_head} +{$n_head} @@";
$result[] = "@@ -{$o_head} +{$n_head} @@".PHP_EOL;
$result[] = $corpus;
}
}
return implode("\n", $result);
return implode('', $result);
}
public function setLoadFileDataCallback($callback) {
@ -656,16 +658,16 @@ final class ArcanistBundle {
}
$content = array();
$content[] = "index {$old_sha1}..{$new_sha1}";
$content[] = "GIT binary patch";
$content[] = "index {$old_sha1}..{$new_sha1}".PHP_EOL;
$content[] = "GIT binary patch".PHP_EOL;
$content[] = "literal {$new_length}";
$content[] = $this->emitBinaryDiffBody($new_data);
$content[] = "literal {$new_length}".PHP_EOL;
$content[] = $this->emitBinaryDiffBody($new_data).PHP_EOL;
$content[] = "literal {$old_length}";
$content[] = $this->emitBinaryDiffBody($old_data);
$content[] = "literal {$old_length}".PHP_EOL;
$content[] = $this->emitBinaryDiffBody($old_data).PHP_EOL;
return implode("\n", $content);
return implode('', $content);
}
private function emitBinaryDiffBody($data) {
@ -691,11 +693,9 @@ final class ArcanistBundle {
$buf .= chr($len - 26 + ord('a') - 1);
}
$buf .= self::encodeBase85($line);
$buf .= "\n";
$buf .= PHP_EOL;
}
$buf .= "\n";
return $buf;
}

71
src/parser/ArcanistDiffParser.php
View file

@ -235,10 +235,11 @@ final class ArcanistDiffParser {
'(?P<type>diff -r) (?P<hgrev>[a-f0-9]+) (?:-r [a-f0-9]+ )?(?P<cur>.+)',
);
$line = $this->getLine();
$line = $this->getLineTrimmed();
$match = null;
$ok = $this->tryMatchHeader($patterns, $line, $match);
$failed_parse = false;
if (!$ok && $this->isFirstNonEmptyLine()) {
// 'hg export' command creates so called "extended diff" that
// contains some meta information and comment at the beginning
@ -250,13 +251,19 @@ final class ArcanistDiffParser {
if (!$this->tryMatchHeader($patterns, $line, $match)) {
// Restore line before guessing to display correct error.
$this->restoreLine();
$this->didFailParse(
"Expected a hunk header, like 'Index: /path/to/file.ext' (svn), ".
"'Property changes on: /path/to/file.ext' (svn properties), ".
"'commit 59bcc3ad6775562f845953cf01624225' (git show), ".
"'diff --git' (git diff), '--- filename' (unified diff), or " .
"'diff -r' (hg diff or patch).");
$failed_parse = true;
}
} else if (!$ok) {
$failed_parse = true;
}
if ($failed_parse) {
$this->didFailParse(
"Expected a hunk header, like 'Index: /path/to/file.ext' (svn), ".
"'Property changes on: /path/to/file.ext' (svn properties), ".
"'commit 59bcc3ad6775562f845953cf01624225' (git show), ".
"'diff --git' (git diff), '--- filename' (unified diff), or " .
"'diff -r' (hg diff or patch).");
}
if (isset($match['type'])) {
@ -361,15 +368,17 @@ final class ArcanistDiffParser {
$this->didFailParse("Expected 'Date:'.");
}
while (($line = $this->nextLine()) !== null) {
while (($line = $this->nextLineTrimmed()) !== null) {
if (strlen($line) && $line[0] != ' ') {
break;
}
// Strip leading spaces from Git commit messages.
$message[] = substr($line, 4);
// Strip leading spaces from Git commit messages. Note that empty lines
// are represented as just "\n"; don't touch those.
$message[] = preg_replace('/^ /', '', $this->getLine());
}
$message = rtrim(implode("\n", $message));
$message = rtrim(implode('', $message), "\r\n");
$change->setMetadata('message', $message);
}
@ -472,8 +481,8 @@ final class ArcanistDiffParser {
$line = $this->nextLine();
}
$old = rtrim(implode("\n", $old));
$new = rtrim(implode("\n", $new));
$old = rtrim(implode('', $old));
$new = rtrim(implode('', $new));
if (!strlen($old)) {
$old = null;
@ -754,7 +763,7 @@ final class ArcanistDiffParser {
$this->didFailParse("Expected 'literal NNNN' to start git binary patch.");
}
do {
$line = $this->nextLine();
$line = $this->nextLineTrimmed();
if ($line === '' || $line === null) {
// Some versions of Mercurial apparently omit the terminal newline,
// although it's unclear if Git will ever do this. In either case,
@ -800,7 +809,7 @@ final class ArcanistDiffParser {
$all_changes = array();
do {
$hunk = new ArcanistDiffHunk();
$line = $this->getLine();
$line = $this->getLineTrimmed();
$real = array();
// In the case where only one line is changed, the length is omitted.
@ -896,6 +905,8 @@ final class ArcanistDiffParser {
--$new_len;
$real[] = $line;
break;
case "\r":
case "\n":
case '~':
$advance = true;
break 2;
@ -908,7 +919,7 @@ final class ArcanistDiffParser {
$this->didFailParse("Found the wrong number of hunk lines.");
}
$corpus = implode("\n", $real);
$corpus = implode('', $real);
$is_binary = false;
if ($this->detectBinaryFiles) {
@ -1007,13 +1018,7 @@ final class ArcanistDiffParser {
$text = preg_replace('/'.$ansi_color_pattern.'/', '', $text);
}
// TODO: This is a hack for SVN + Windows. Eventually, we should retain line
// endings and preserve them through the patch lifecycle or something along
// those lines.
// NOTE: On Windows, a diff may have \n delimited lines (because the file
// uses \n) but \r\n delimited blocks. Split on both.
$this->text = preg_split('/\r?\n/', $text);
$this->text = phutil_split_lines($text);
$this->line = 0;
}
@ -1027,11 +1032,27 @@ final class ArcanistDiffParser {
return null;
}
protected function getLineTrimmed() {
$line = $this->getLine();
if ($line !== null) {
$line = trim($line, "\r\n");
}
return $line;
}
protected function nextLine() {
$this->line++;
return $this->getLine();
}
protected function nextLineTrimmed() {
$line = $this->nextLine();
if ($line !== null) {
$line = trim($line, "\r\n");
}
return $line;
}
protected function nextNonemptyLine() {
while (($line = $this->nextLine()) !== null) {
if (strlen(trim($line)) !== 0) {
@ -1079,14 +1100,14 @@ final class ArcanistDiffParser {
}
protected function didFailParse($message) {
$context = 3;
$context = 5;
$min = max(0, $this->line - $context);
$max = min($this->line + $context, count($this->text) - 1);
$context = '';
for ($ii = $min; $ii <= $max; $ii++) {
$context .= sprintf(
"%8.8s %6.6s %s\n",
"%8.8s %6.6s %s",
($ii == $this->line) ? '>>> ' : '',
$ii + 1,
$this->text[$ii]);

2
src/parser/__tests__/ArcanistDiffParserTestCase.php
View file

@ -93,6 +93,7 @@ final class ArcanistDiffParserTestCase extends ArcanistTestCase {
%%
%%
%%%
EOCORPUS;
$corpus_1 = <<<EOCORPUS
%%%%%
@ -100,6 +101,7 @@ EOCORPUS;
{$there_is_a_literal_trailing_space_here}
-!
+! quack
EOCORPUS;
$this->assertEqual(
$corpus_0,

BIN
src/parser/__tests__/bundle.git.tgz
View file

Binary file not shown.