From 0a7973488f5e69779d9d227a50361826aa614660 Mon Sep 17 00:00:00 2001
From: vrana <jakubv@fb.com>
Date: Wed, 27 Jun 2012 10:44:29 -0700
Subject: [PATCH] Simplify DifferentialHunk::getAddedLines()

Summary: I will also need `getRemovedLines()` so refactor this first.

Test Plan:
New test case.
Viewed uncached diff.
Verified that the only callsite of `getAddedLines()` trims lines.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2875
---
 src/__phutil_library_map__.php                |  2 +
 .../differential/storage/DifferentialHunk.php | 65 ++++++++-----------
 .../__tests__/DifferentialHunkTestCase.php    | 53 +++++++++++++++
 .../storage/__tests__/hunk/basic.diff         |  8 +++
 .../storage/__tests__/hunk/new.txt            |  6 ++
 .../storage/__tests__/hunk/newline.diff       |  3 +
 .../storage/__tests__/hunk/old.txt            |  5 ++
 7 files changed, 103 insertions(+), 39 deletions(-)
 create mode 100644 src/applications/differential/storage/__tests__/DifferentialHunkTestCase.php
 create mode 100644 src/applications/differential/storage/__tests__/hunk/basic.diff
 create mode 100644 src/applications/differential/storage/__tests__/hunk/new.txt
 create mode 100644 src/applications/differential/storage/__tests__/hunk/newline.diff
 create mode 100644 src/applications/differential/storage/__tests__/hunk/old.txt

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 9920d554e6..f6a625a657 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -264,6 +264,7 @@ phutil_register_library_map(array(
     'DifferentialGitSVNIDFieldSpecification' => 'applications/differential/field/specification/DifferentialGitSVNIDFieldSpecification.php',
     'DifferentialHostFieldSpecification' => 'applications/differential/field/specification/DifferentialHostFieldSpecification.php',
     'DifferentialHunk' => 'applications/differential/storage/DifferentialHunk.php',
+    'DifferentialHunkTestCase' => 'applications/differential/storage/__tests__/DifferentialHunkTestCase.php',
     'DifferentialInlineComment' => 'applications/differential/storage/DifferentialInlineComment.php',
     'DifferentialInlineCommentEditController' => 'applications/differential/controller/DifferentialInlineCommentEditController.php',
     'DifferentialInlineCommentEditView' => 'applications/differential/view/DifferentialInlineCommentEditView.php',
@@ -1326,6 +1327,7 @@ phutil_register_library_map(array(
     'DifferentialGitSVNIDFieldSpecification' => 'DifferentialFieldSpecification',
     'DifferentialHostFieldSpecification' => 'DifferentialFieldSpecification',
     'DifferentialHunk' => 'DifferentialDAO',
+    'DifferentialHunkTestCase' => 'ArcanistPhutilTestCase',
     'DifferentialInlineComment' =>
     array(
       0 => 'DifferentialDAO',
diff --git a/src/applications/differential/storage/DifferentialHunk.php b/src/applications/differential/storage/DifferentialHunk.php
index 7892afdcab..f9f2567960 100644
--- a/src/applications/differential/storage/DifferentialHunk.php
+++ b/src/applications/differential/storage/DifferentialHunk.php
@@ -26,35 +26,22 @@ final class DifferentialHunk extends DifferentialDAO {
   protected $newLen;
 
   public function getAddedLines() {
-    $lines = array();
-    $n = $this->newOffset;
-    foreach (explode("\n", $this->changes) as $diff_line) {
-      if ($diff_line == '' || $diff_line[0] == '\\') {
-        continue;
-      }
-      if ($diff_line[0] == '+') {
-        $lines[$n] = (string)substr($diff_line, 1); // substr('+', 1) === false
-      }
-      if ($diff_line[0] != '-') {
-        $n++;
-      }
-    }
-    return $lines;
+    return $this->makeContent($include = '+');
   }
 
   public function makeNewFile() {
-    return $this->makeContent($exclude = '-');
+    return implode('', $this->makeContent($include = ' +'));
   }
 
   public function makeOldFile() {
-    return $this->makeContent($exclude = '+');
+    return implode('', $this->makeContent($include = ' -'));
   }
 
   public function makeChanges() {
-    return $this->makeContent($exclude = ' ');
+    return implode('', $this->makeContent($include = '-+'));
   }
 
-  final private function makeContent($exclude) {
+  final private function makeContent($include) {
     $results = array();
     $lines = explode("\n", $this->changes);
 
@@ -74,32 +61,32 @@ final class DifferentialHunk extends DifferentialDAO {
     //    + x
     //    \ No newline at end of file
 
-
+    $n = (strpos($include, '+') !== false ?
+      $this->newOffset :
+      $this->oldOffset);
     $use_next_newline = false;
-    $has_newline = true;
     foreach ($lines as $line) {
-      if (isset($line[0])) {
-        if ($line[0] == $exclude) {
-          $use_next_newline = false;
-          continue;
-        }
-        if ($line[0] == '\\') {
-          if ($use_next_newline) {
-            $has_newline = false;
-          }
-          continue;
-        }
+      if (!isset($line[0])) {
+        continue;
+      }
+
+      if ($line[0] == '\\') {
+        if ($use_next_newline) {
+          $results[last_key($results)] = rtrim(end($results), "\n");
+        }
+      } else if (strpos($include, $line[0]) === false) {
+        $use_next_newline = false;
+      } else {
+        $use_next_newline = true;
+        $results[$n] = substr($line, 1)."\n";
+      }
+
+      if ($line[0] == ' ' || strpos($include, $line[0]) !== false) {
+        $n++;
       }
-      $use_next_newline = true;
-      $results[] = substr($line, 1);
     }
 
-    $possible_newline = '';
-    if ($has_newline) {
-      $possible_newline = "\n";
-    }
-
-    return implode("\n", $results).$possible_newline;
+    return $results;
   }
 
 }
diff --git a/src/applications/differential/storage/__tests__/DifferentialHunkTestCase.php b/src/applications/differential/storage/__tests__/DifferentialHunkTestCase.php
new file mode 100644
index 0000000000..331c58a640
--- /dev/null
+++ b/src/applications/differential/storage/__tests__/DifferentialHunkTestCase.php
@@ -0,0 +1,53 @@
+<?php
+
+/*
+ * Copyright 2012 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+final class DifferentialHunkTestCase extends ArcanistPhutilTestCase {
+
+  public function testMakeChanges() {
+    $root = dirname(__FILE__).'/hunk/';
+
+    $hunk = new DifferentialHunk();
+    $hunk->setChanges(Filesystem::readFile($root.'basic.diff'));
+    $hunk->setOldOffset(1);
+    $hunk->setNewOffset(11);
+
+    $old = Filesystem::readFile($root.'old.txt');
+    $this->assertEqual($old, $hunk->makeOldFile());
+
+    $new = Filesystem::readFile($root.'new.txt');
+    $this->assertEqual($new, $hunk->makeNewFile());
+
+    $added = array(
+      12 => "1 quack\n",
+      13 => "1 quack\n",
+      16 => "5 drake\n",
+    );
+    $this->assertEqual($added, $hunk->getAddedLines());
+
+    $hunk = new DifferentialHunk();
+    $hunk->setChanges(Filesystem::readFile($root.'newline.diff'));
+    $hunk->setOldOffset(1);
+    $hunk->setNewOffset(11);
+
+    $this->assertEqual("a\n", $hunk->makeOldFile());
+    $this->assertEqual("a", $hunk->makeNewFile());
+    $this->assertEqual(array(11 => "a"), $hunk->getAddedLines());
+
+  }
+
+}
diff --git a/src/applications/differential/storage/__tests__/hunk/basic.diff b/src/applications/differential/storage/__tests__/hunk/basic.diff
new file mode 100644
index 0000000000..54038f3ddc
--- /dev/null
+++ b/src/applications/differential/storage/__tests__/hunk/basic.diff
@@ -0,0 +1,8 @@
+ 1 duck
++1 quack
++1 quack
+ 2 duck
+-3 duck
+ 4 duck
+-5 duck
++5 drake
diff --git a/src/applications/differential/storage/__tests__/hunk/new.txt b/src/applications/differential/storage/__tests__/hunk/new.txt
new file mode 100644
index 0000000000..85a58e2107
--- /dev/null
+++ b/src/applications/differential/storage/__tests__/hunk/new.txt
@@ -0,0 +1,6 @@
+1 duck
+1 quack
+1 quack
+2 duck
+4 duck
+5 drake
diff --git a/src/applications/differential/storage/__tests__/hunk/newline.diff b/src/applications/differential/storage/__tests__/hunk/newline.diff
new file mode 100644
index 0000000000..dbd57a4493
--- /dev/null
+++ b/src/applications/differential/storage/__tests__/hunk/newline.diff
@@ -0,0 +1,3 @@
+-a
++a
+\ No newline at end of file
diff --git a/src/applications/differential/storage/__tests__/hunk/old.txt b/src/applications/differential/storage/__tests__/hunk/old.txt
new file mode 100644
index 0000000000..cbf0bc9ac4
--- /dev/null
+++ b/src/applications/differential/storage/__tests__/hunk/old.txt
@@ -0,0 +1,5 @@
+1 duck
+2 duck
+3 duck
+4 duck
+5 duck