refactor diff to not be aware of repository #19

Author: alexandresalome

doc/api/commit.rst

@@ -120,7 +120,6 @@ There are also two other methods for your convenience:
     // The body (rest of the message)
     $commit->getBodyMessage();
 
-
 Diff of a commit
 ----------------
 

doc/api/diff.rst

@@ -61,13 +61,6 @@ Here is an exhaustive list of the *File* object:
 
     $file->getChanges(); // See next chapter
 
-On a *File* object, you can also access old and new contents with:
-
-.. code-block:: php
-
-    echo 'Old content: ', $file->getOldBlob()->getContent();
-    echo 'New content: ', $file->getNewBlob()->getContent();
-
 The FileChange object
 ---------------------
 

src/Gitonomy/Git/Commit.php

@@ -13,6 +13,7 @@
 namespace Gitonomy\Git;
 
 use Gitonomy\Git\Util\StringHelper;
+use Gitonomy\Git\Diff\Diff;
 
 /**
  * Representation of a Git commit.
@@ -156,7 +157,9 @@ private function initialize()
 
     public function getDiff()
     {
-        return new Diff($this->repository, array($this->hash));
+        $args = array('-r', '-p', '-m', '-M', '--no-commit-id', '--full-index', $this->hash);
+
+        return Diff::parse($this->repository->run('diff-tree', $args));
     }
 
     /**

src/Gitonomy/Git/Diff.php renamed to src/Gitonomy/Git/Diff/Diff.php

@@ -10,7 +10,9 @@
  * with this source code in the file LICENSE.
  */
 
-namespace Gitonomy\Git;
+namespace Gitonomy\Git\Diff;
+
+use Gitonomy\Git\Parser\DiffParser;
 
 /**
  * Representation of a diff.
@@ -19,26 +21,11 @@
  */
 class Diff
 {
-    /**
-     * @var Repository
-     */
-    protected $repository;
-
-    /**
-     * @var array
-     */
-    protected $revisions;
-
     /**
      * @var array
      */
     protected $files;
 
-    /**
-     * @var boolean
-     */
-    protected $isTree;
-
     /**
      * @var string
      */
@@ -51,32 +38,28 @@ class Diff
      * @var string     $revision   A string revision, passed to git diff command
      * @var boolean    $isTree     Indicates if revisions are commit-trees to compare
      */
-    public function __construct(Repository $repository, $revisions, $isTree = true)
+    public function __construct(array $files)
     {
-        $this->repository = $repository;
-        $this->revisions  = (array) $revisions;
-        $this->isTree     = $isTree;
+        $this->files = $files;
     }
 
     /**
-     * @return array
+     * @return Diff
      */
-    public function getRevisions()
+    static public function parse($rawDiff)
     {
-        return $this->revisions;
+        $parser = new DiffParser();
+        $parser->parse($rawDiff);
+
+        return new Diff($parser->files);
     }
 
-    protected function initialize()
+    /**
+     * @return array
+     */
+    public function getRevisions()
     {
-        $args = array('-r', '-p', '-m', '-M', '--no-commit-id', '--full-index');
-        $args = array_merge($args, $this->revisions);
-        $this->rawDiff = $this->repository->run($this->isTree ? 'diff-tree' : 'diff', $args);
-
-        $parser = new Parser\DiffParser();
-        $parser->setRepository($this->repository);
-        $parser->parse($this->rawDiff);
-
-        $this->files = $parser->files;
+        return $this->revisions;
     }
 
     /**
@@ -86,8 +69,6 @@ protected function initialize()
      */
     public function getFiles()
     {
-        $this->initialize();
-
         return $this->files;
     }
 
@@ -98,8 +79,20 @@ public function getFiles()
      */
     public function getRawDiff()
     {
-        $this->initialize();
-
         return $this->rawDiff;
     }
+
+    public function toArray()
+    {
+        return array_map(function (File $file) {
+            return $file->toArray();
+        }, $this->files);
+    }
+
+    public static function fromArray(array $array)
+    {
+        return new Diff(array_map(function ($array) {
+            return File::fromArray($array);
+        }, $array));
+    }
 }

src/Gitonomy/Git/Diff/File.php

@@ -19,11 +19,6 @@
  */
 class File
 {
-    /**
-     * @var Repository
-     */
-    protected $repository;
-
     /**
      * @var string
      */
@@ -67,9 +62,8 @@ class File
     /**
      * Instanciates a new Diff File object.
      */
-    public function __construct(Repository $repository, $oldName, $newName, $oldMode, $newMode, $oldIndex, $newIndex, $isBinary)
+    public function __construct($oldName, $newName, $oldMode, $newMode, $oldIndex, $newIndex, $isBinary)
     {
-        $this->repository = $repository;
         $this->oldName  = $oldName;
         $this->newName  = $newName;
         $this->oldMode  = $oldMode;
@@ -191,32 +185,40 @@ public function getNewIndex()
         return $this->newIndex;
     }
 
-    public function getOldBlob()
-    {
-        if (null === $this->oldIndex) {
-            throw new \LogicException('Can\'t instanciate Blob: no old index');
-        }
-
-        return $this->repository->getBlob($this->oldIndex);
-    }
-
-    public function getNewBlob()
+    public function isBinary()
     {
-        if (null === $this->newIndex) {
-            throw new \LogicException('Can\'t instanciate Blob: no old index');
-        }
-
-        return $this->repository->getBlob($this->newIndex);
+        return $this->isBinary;
     }
 
     public function getChanges()
     {
         return $this->changes;
     }
 
-    public function isBinary()
+    public function toArray()
     {
-        return $this->isBinary;
+        return array(
+            'old_name' => $this->oldName,
+            'new_name' => $this->newName,
+            'old_mode' => $this->oldMode,
+            'new_mode' => $this->newMode,
+            'old_index' => $this->oldIndex,
+            'new_index' => $this->newIndex,
+            'is_binary' => $this->isBinary,
+            'changes' => array_map(function (FileChange $change) {
+                return $change->toArray();
+            }, $this->changes)
+        );
     }
 
+    public static function fromArray(array $array)
+    {
+        $file = new File($array['old_name'], $array['new_name'], $array['old_mode'], $array['new_mode'], $array['old_index'], $array['new_index'], $array['is_binary']);
+
+        foreach ($array['changes'] as $change) {
+            $file->addChange(FileChange::fromArray($change));
+        }
+
+        return $file;
+    }
 }

src/Gitonomy/Git/Diff/FileChange.php

@@ -69,4 +69,20 @@ public function getLines()
     {
         return $this->lines;
     }
+
+    public function toArray()
+    {
+        return array(
+            'range_old_start' => $this->rangeOldStart,
+            'range_old_count' => $this->rangeOldCount,
+            'range_new_start' => $this->rangeNewStart,
+            'range_new_count' => $this->rangeNewCount,
+            'lines' => $this->lines,
+        );
+    }
+
+    public static function fromArray(array $array)
+    {
+        return new FileChange($array['range_old_start'], $array['range_old_count'], $array['range_new_start'], $array['range_new_count'], $array['lines']);
+    }
 }

src/Gitonomy/Git/Parser/DiffParser.php

@@ -19,19 +19,9 @@
 class DiffParser extends ParserBase
 {
     public $files;
-    protected $repository;
-
-    public function setRepository(Repository $repository)
-    {
-        $this->repository = $repository;
-    }
 
     protected function doParse()
     {
-        if (null === $this->repository) {
-            throw new \RuntimeException('Can\'t work without Repository');
-        }
-
         $this->files = array();
 
         while (!$this->isFinished()) {
@@ -103,7 +93,7 @@ protected function doParse()
             $newName  = $newName === '/dev/null' ? null : substr($newName, 2);
             $oldIndex = preg_match('/^0+$/', $oldIndex) ? null : $oldIndex;
             $newIndex = preg_match('/^0+$/', $newIndex) ? null : $newIndex;
-            $file = new File($this->repository, $oldName, $newName, $oldMode, $newMode, $oldIndex, $newIndex, $isBinary);
+            $file = new File($oldName, $newName, $oldMode, $newMode, $oldIndex, $newIndex, $isBinary);
 
             // 5. Diff
             while ($this->expects('@@ ')) {

src/Gitonomy/Git/Parser/ParserBase.php

@@ -82,7 +82,7 @@ protected function consumeHash()
     protected function consumeRegexp($regexp)
     {
         if (!preg_match($regexp.'A', $this->content, $vars, null, $this->cursor)) {
-            throw new \RuntimeException('No match for regexp '.$regexp);
+            throw new \RuntimeException('No match for regexp '.$regexp.' Upcoming: '.substr($this->content, $this->cursor, 30));
         }
 
         $this->cursor += strlen($vars[0]);

src/Gitonomy/Git/Repository.php

@@ -17,6 +17,7 @@
 use Psr\Log\LoggerInterface;
 
 use Gitonomy\Git\Exception\RuntimeException;
+use Gitonomy\Git\Diff\Diff;
 
 /**
  * Git repository object.
@@ -280,7 +281,9 @@ public function getLog($revisions = null, $paths = null, $offset = null, $limit
      */
     public function getDiff($revision)
     {
-        return new Diff($this, $revision);
+        $args = array('-r', '-p', '-m', '-M', '--no-commit-id', '--full-index', $revision);
+
+        return new Diff($this->run('diff', $args));
     }
 
     /**

tests/Gitonomy/Git/Tests/CommitTest.php

@@ -13,7 +13,7 @@
 namespace Gitonomy\Git\Tests;
 
 use Gitonomy\Git\Commit;
-use Gitonomy\Git\Diff;
+use Gitonomy\Git\Diff\Diff;
 
 class CommitTest extends AbstractTest
 {
@@ -27,7 +27,6 @@ public function testGetDiff($repository)
         $diff = $commit->getDiff();
 
         $this->assertTrue($diff instanceof Diff, "getDiff() returns a Diff object");
-        $this->assertEquals(array($commit->getHash()), $diff->getRevisions(), "getDiff() revision is correct");
     }
 
     /**