Skip to content

Commit 64d8e79

Browse files
Patrick-BeuksPatrick-Beuks
authored
fix error with empty index by including raw information in diff (#229)
* fix error with empty index by including raw information in diff * fix style error
1 parent 16214e3 commit 64d8e79

File tree

6 files changed

+140
-6
lines changed

6 files changed

+140
-6
lines changed

‎src/Gitonomy/Git/Commit.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public function setData(array $data)
6262
*/
6363
public function getDiff()
6464
{
65-
$args = ['-r', '-p', '-m', '-M', '--no-commit-id', '--full-index', $this->revision];
65+
$args = ['-r', '-p', '--raw', '-m', '-M', '--no-commit-id', '--full-index', $this->revision];
6666

6767
$diff = Diff::parse($this->repository->run('diff-tree', $args));
6868
$diff->setRepository($this->repository);

‎src/Gitonomy/Git/Diff/File.php

+8
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,10 @@ public function getOldBlob()
278278
throw new \LogicException('Can\'t return old Blob on a creation');
279279
}
280280

281+
if ($this->oldIndex === '') {
282+
throw new \RuntimeException('Index is missing to return Blob object.');
283+
}
284+
281285
return $this->repository->getBlob($this->oldIndex);
282286
}
283287

@@ -291,6 +295,10 @@ public function getNewBlob()
291295
throw new \LogicException('Can\'t return new Blob on a deletion');
292296
}
293297

298+
if ($this->newIndex === '') {
299+
throw new \RuntimeException('Index is missing to return Blob object.');
300+
}
301+
294302
return $this->repository->getBlob($this->newIndex);
295303
}
296304
}

‎src/Gitonomy/Git/Parser/DiffParser.php

+19-2
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,31 @@ class DiffParser extends ParserBase
2222
protected function doParse()
2323
{
2424
$this->files = [];
25+
$indexes = [];
26+
// Diff contains raw information
27+
if (str_starts_with($this->content, ':')) {
28+
while ($this->expects(':')) {
29+
$this->consumeRegexp('/\d{6} \d{6} /');
30+
$oldIndex = $this->consumeShortHash();
31+
$this->consume(' ');
32+
$newIndex = $this->consumeShortHash();
33+
$this->consumeTo("\n");
34+
$this->consumeNewLine();
35+
$indexes[] = [$oldIndex, $newIndex];
36+
}
37+
$this->consumeNewLine();
38+
} elseif (!$this->isFinished()) {
39+
trigger_error('Using Diff::parse without raw information is deprecated. See https://github.com/gitonomy/gitlib/issues/227.', E_USER_DEPRECATED);
40+
}
2541

42+
$fileIndex = 0;
2643
while (!$this->isFinished()) {
2744
// 1. title
2845
$vars = $this->consumeRegexp("/diff --git \"?(a\/.*?)\"? \"?(b\/.*?)\"?\n/");
2946
$oldName = $vars[1];
3047
$newName = $vars[2];
31-
$oldIndex = null;
32-
$newIndex = null;
48+
$oldIndex = isset($indexes[$fileIndex]) ? $indexes[$fileIndex][0] : null;
49+
$newIndex = isset($indexes[$fileIndex]) ? $indexes[$fileIndex][1] : null;
3350
$oldMode = null;
3451
$newMode = null;
3552

‎src/Gitonomy/Git/Repository.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ public function getDiff($revisions)
416416
$revisions = new RevisionList($this, $revisions);
417417
}
418418

419-
$args = array_merge(['-r', '-p', '-m', '-M', '--no-commit-id', '--full-index'], $revisions->getAsTextArray());
419+
$args = array_merge(['-r', '-p', '--raw', '-m', '-M', '--no-commit-id', '--full-index'], $revisions->getAsTextArray());
420420

421421
$diff = Diff::parse($this->run('diff', $args));
422422
$diff->setRepository($this);

‎src/Gitonomy/Git/WorkingCopy.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,15 @@ public function getUntrackedFiles()
5050

5151
public function getDiffPending()
5252
{
53-
$diff = Diff::parse($this->run('diff', ['-r', '-p', '-m', '-M', '--full-index']));
53+
$diff = Diff::parse($this->run('diff', ['-r', '-p', '--raw', '-m', '-M', '--full-index']));
5454
$diff->setRepository($this->repository);
5555

5656
return $diff;
5757
}
5858

5959
public function getDiffStaged()
6060
{
61-
$diff = Diff::parse($this->run('diff', ['-r', '-p', '-m', '-M', '--full-index', '--staged']));
61+
$diff = Diff::parse($this->run('diff', ['-r', '-p', '--raw', '-m', '-M', '--full-index', '--staged']));
6262
$diff->setRepository($this->repository);
6363

6464
return $diff;

‎tests/Gitonomy/Git/Tests/DiffTest.php

+109
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
namespace Gitonomy\Git\Tests;
1414

1515
use Gitonomy\Git\Diff\Diff;
16+
use Gitonomy\Git\Diff\File;
17+
use Gitonomy\Git\Repository;
1618

1719
class DiffTest extends AbstractTest
1820
{
@@ -151,6 +153,113 @@ public function testWorksWithUmlauts($repository)
151153
$this->assertSame('file_with_umlauts_\303\244\303\266\303\274', $files[0]->getNewName());
152154
}
153155

156+
public function testDeleteFileWithoutRaw()
157+
{
158+
$deprecationCalled = false;
159+
$self = $this;
160+
set_error_handler(function (int $errno, string $errstr) use ($self, &$deprecationCalled): void {
161+
$deprecationCalled = true;
162+
$self->assertSame('Using Diff::parse without raw information is deprecated. See https://github.com/gitonomy/gitlib/issues/227.', $errstr);
163+
}, E_USER_DEPRECATED);
164+
165+
$diff = Diff::parse(<<<'DIFF'
166+
diff --git a/test b/test
167+
deleted file mode 100644
168+
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..0000000000000000000000000000000000000000
169+
170+
DIFF);
171+
$firstFile = $diff->getFiles()[0];
172+
173+
restore_exception_handler();
174+
175+
$this->assertTrue($deprecationCalled);
176+
$this->assertFalse($firstFile->isCreation());
177+
// TODO: Enable after #226 is merged
178+
//$this->assertTrue($firstFile->isDeletion());
179+
//$this->assertFalse($firstFile->isChangeMode());
180+
$this->assertSame('e69de29bb2d1d6434b8b29ae775ad8c2e48c5391', $firstFile->getOldIndex());
181+
$this->assertNull($firstFile->getNewIndex());
182+
}
183+
184+
public function testModeChangeFileWithoutRaw()
185+
{
186+
$deprecationCalled = false;
187+
$self = $this;
188+
set_error_handler(function (int $errno, string $errstr) use ($self, &$deprecationCalled): void {
189+
$deprecationCalled = true;
190+
$self->assertSame('Using Diff::parse without raw information is deprecated. See https://github.com/gitonomy/gitlib/issues/227.', $errstr);
191+
}, E_USER_DEPRECATED);
192+
193+
$diff = Diff::parse(<<<'DIFF'
194+
diff --git a/a.out b/a.out
195+
old mode 100755
196+
new mode 100644
197+
198+
DIFF);
199+
$firstFile = $diff->getFiles()[0];
200+
201+
restore_exception_handler();
202+
203+
$this->assertTrue($deprecationCalled);
204+
$this->assertFalse($firstFile->isCreation());
205+
$this->assertFalse($firstFile->isDeletion());
206+
$this->assertTrue($firstFile->isChangeMode());
207+
$this->assertSame('', $firstFile->getOldIndex());
208+
$this->assertSame('', $firstFile->getNewIndex());
209+
}
210+
211+
public function testModeChangeFileWithRaw()
212+
{
213+
$deprecationCalled = false;
214+
set_error_handler(function (int $errno, string $errstr) use (&$deprecationCalled): void {
215+
$deprecationCalled = true;
216+
}, E_USER_DEPRECATED);
217+
218+
$diff = Diff::parse(<<<'DIFF'
219+
:100755 100644 d1af4b23d0cc9313e5b2d3ef2fb9696c94afaa81 d1af4b23d0cc9313e5b2d3ef2fb9696c94afaa81 M a.out
220+
221+
diff --git a/a.out b/a.out
222+
old mode 100755
223+
new mode 100644
224+
225+
DIFF);
226+
$firstFile = $diff->getFiles()[0];
227+
228+
restore_exception_handler();
229+
230+
$this->assertFalse($deprecationCalled);
231+
$this->assertFalse($firstFile->isCreation());
232+
$this->assertFalse($firstFile->isDeletion());
233+
$this->assertTrue($firstFile->isChangeMode());
234+
$this->assertSame('d1af4b23d0cc9313e5b2d3ef2fb9696c94afaa81', $firstFile->getOldIndex());
235+
$this->assertSame('d1af4b23d0cc9313e5b2d3ef2fb9696c94afaa81', $firstFile->getNewIndex());
236+
}
237+
238+
public function testThrowErrorOnBlobGetWithoutIndex()
239+
{
240+
$repository = $this->getMockBuilder(Repository::class)->disableOriginalConstructor()->getMock();
241+
$file = new File('oldName', 'newName', '100755', '100644', '', '', false);
242+
$file->setRepository($repository);
243+
244+
try {
245+
$file->getOldBlob();
246+
} catch(\RuntimeException $exception) {
247+
$this->assertSame('Index is missing to return Blob object.', $exception->getMessage());
248+
}
249+
250+
try {
251+
$file->getNewBlob();
252+
} catch(\RuntimeException $exception) {
253+
$this->assertSame('Index is missing to return Blob object.', $exception->getMessage());
254+
}
255+
256+
$this->assertFalse($file->isCreation());
257+
$this->assertFalse($file->isDeletion());
258+
$this->assertTrue($file->isChangeMode());
259+
$this->assertSame('', $file->getOldIndex());
260+
$this->assertSame('', $file->getNewIndex());
261+
}
262+
154263
public function testEmptyNewFile()
155264
{
156265
$diff = Diff::parse("diff --git a/test b/test\nnew file mode 100644\nindex 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391\n");

0 commit comments

Comments
 (0)