Fix arc diff / arc patch to handle files with missing trailing newlines properly

Summary:
  - When you try to "arc patch" a change which adds or removes a trailing newline or alters a file with a trailing newline near the newline, the patch fails because we fail to reconstuct the "\ No newline at end of file" line.
  - We don't currently record enough information about these diffs to reconstruct them with a sensible amount of effort. Store the "\ No newline at end of file" line instead of just a flag.
  - There are some special rules for these lines; implement them.

NOTE: This causes some display glitching in Differential, but nothing major; I'll fix that in a followup patch.

Test Plan:
  - Created a diff which added a trailing newline, removed a trailing newline, and altered a file without a trailing newline near the end of the file.
  - Verified that the git version of this patch and the 'arc export --git' version of this patch are (essentially) identical.
  - Verified that "arc diff" + "arc patch" can apply these changes.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, epriestley

Maniphest Tasks: T533

Differential Revision: https://secure.phabricator.com/D1810

src/parser/bundle/ArcanistBundle.php

@@ -381,15 +381,15 @@ final class ArcanistBundle {
     return $this->changes;
   }
 
-  private function breakHunkIntoSmallHunks(ArcanistDiffHunk $hunk) {
+  private function breakHunkIntoSmallHunks(ArcanistDiffHunk $base_hunk) {
     $context = 3;
 
     $results = array();
-    $lines = explode("\n", $hunk->getCorpus());
+    $lines = explode("\n", $base_hunk->getCorpus());
     $n = count($lines);
 
-    $old_offset = $hunk->getOldOffset();
-    $new_offset = $hunk->getNewOffset();
+    $old_offset = $base_hunk->getOldOffset();
+    $new_offset = $base_hunk->getNewOffset();
 
     $ii = 0;
     $jj = 0;
@@ -414,6 +414,7 @@ final class ArcanistBundle {
 
       $old_lines = 0;
       $new_lines = 0;
+      $hunk_adjust = 0;
       $last_change = $jj;
       $break_here = null;
       for (; $jj < $n; ++$jj) {
@@ -437,9 +438,14 @@ final class ArcanistBundle {
         } else {
           $break_here = null;
           $last_change = $jj;
-          if ($lines[$jj][0] == '-') {
+
+          if ($lines[$jj][0] == '\\') {
+            // When we have a "\ No newline at end of file" line, it does not
+            // contribute to either hunk length.
+            ++$hunk_adjust;
+          } else if ($lines[$jj][0] == '-') {
             ++$old_lines;
-          } else {
+          } else if ($lines[$jj][0] == '+') {
             ++$new_lines;
           }
         }
@@ -450,12 +456,13 @@ final class ArcanistBundle {
       }
 
       $hunk_length = min($jj, $n) - $hunk_start;
+      $count_length = ($hunk_length - $hunk_adjust);
 
       $hunk = new ArcanistDiffHunk();
       $hunk->setOldOffset($old_offset + $hunk_start - $ii);
       $hunk->setNewOffset($new_offset + $hunk_start - $ii);
-      $hunk->setOldLength($hunk_length - $new_lines);
-      $hunk->setNewLength($hunk_length - $old_lines);
+      $hunk->setOldLength($count_length - $new_lines);
+      $hunk->setNewLength($count_length - $old_lines);
 
       $corpus = array_slice($lines, $hunk_start, $hunk_length);
       $corpus = implode("\n", $corpus);
@@ -507,7 +514,23 @@ final class ArcanistBundle {
         $n_len = $small_hunk->getNewLength();
         $corpus = $small_hunk->getCorpus();
 
-        $result[] = "@@ -{$o_off},{$o_len} +{$n_off},{$n_len} @@";
+        // NOTE: If the length is 1 it can be omitted. Since git does this,
+        // we also do it so that "arc export --git" diffs are as similar to
+        // real git diffs as possible, which helps debug issues.
+
+        if ($o_len == 1) {
+          $o_head = "{$o_off}";
+        } else {
+          $o_head = "{$o_off},{$o_len}";
+        }
+
+        if ($n_len == 1) {
+          $n_head = "{$n_off}";
+        } else {
+          $n_head = "{$n_off},{$n_len}";
+        }
+
+        $result[] = "@@ -{$o_head} +{$n_head} @@";
         $result[] = $corpus;
       }
     }

src/parser/diff/ArcanistDiffParser.php

@@ -793,8 +793,10 @@ final class ArcanistDiffParser {
                 "Expected '\ No newline at end of file'.");
             }
             if ($new_len) {
+              $real[] = $line;
               $hunk->setIsMissingOldNewline(true);
             } else {
+              $real[] = $line;
               $hunk->setIsMissingNewNewline(true);
             }
             if (!$new_len) {