Skip to content

Commit 5bf53e7

Browse files
SpacePossumsebastianbergmann
authored andcommitted
Fix line diff. detection.
1 parent c249614 commit 5bf53e7

File tree

2 files changed

+92
-40
lines changed

2 files changed

+92
-40
lines changed

src/Differ.php

Lines changed: 67 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -82,20 +82,14 @@ private function validateDiffInput($input): string
8282
public function diffToArray($from, $to, LongestCommonSubsequenceCalculator $lcs = null): array
8383
{
8484
if (\is_string($from)) {
85-
$fromMatches = $this->getNewLineMatches($from);
86-
$from = $this->splitStringByLines($from);
87-
} elseif (\is_array($from)) {
88-
$fromMatches = [];
89-
} else {
85+
$from = $this->splitStringByLines($from);
86+
} elseif (!\is_array($from)) {
9087
throw new \InvalidArgumentException('"from" must be an array or string.');
9188
}
9289

9390
if (\is_string($to)) {
94-
$toMatches = $this->getNewLineMatches($to);
95-
$to = $this->splitStringByLines($to);
96-
} elseif (\is_array($to)) {
97-
$toMatches = [];
98-
} else {
91+
$to = $this->splitStringByLines($to);
92+
} elseif (!\is_array($to)) {
9993
throw new \InvalidArgumentException('"to" must be an array or string.');
10094
}
10195

@@ -108,13 +102,6 @@ public function diffToArray($from, $to, LongestCommonSubsequenceCalculator $lcs
108102
$common = $lcs->calculate(\array_values($from), \array_values($to));
109103
$diff = [];
110104

111-
if ($this->detectUnmatchedLineEndings($fromMatches, $toMatches)) {
112-
$diff[] = [
113-
"#Warning: Strings contain different line endings!\n",
114-
3
115-
];
116-
}
117-
118105
foreach ($start as $token) {
119106
$diff[] = [$token, 0 /* OLD */];
120107
}
@@ -149,21 +136,11 @@ public function diffToArray($from, $to, LongestCommonSubsequenceCalculator $lcs
149136
$diff[] = [$token, 0 /* OLD */];
150137
}
151138

152-
return $diff;
153-
}
154-
155-
/**
156-
* Get new strings denoting new lines from a given string.
157-
*
158-
* @param string $string
159-
*
160-
* @return array
161-
*/
162-
private function getNewLineMatches(string $string): array
163-
{
164-
\preg_match_all('(\r\n|\r|\n)', $string, $stringMatches);
139+
if ($this->detectUnmatchedLineEndings($diff)) {
140+
\array_unshift($diff, ["#Warning: Strings contain different line endings!\n", 3]);
141+
}
165142

166-
return $stringMatches;
143+
return $diff;
167144
}
168145

169146
/**
@@ -215,18 +192,70 @@ private function calculateEstimatedFootprint(array $from, array $to)
215192
}
216193

217194
/**
218-
* Returns true if line ends don't match on fromMatches and toMatches.
195+
* Returns true if line ends don't match in a diff.
219196
*
220-
* @param array $fromMatches
221-
* @param array $toMatches
197+
* @param array $diff
222198
*
223199
* @return bool
224200
*/
225-
private function detectUnmatchedLineEndings(array $fromMatches, array $toMatches): bool
201+
private function detectUnmatchedLineEndings(array $diff): bool
226202
{
227-
return isset($fromMatches[0], $toMatches[0]) &&
228-
\count($fromMatches[0]) === \count($toMatches[0]) &&
229-
$fromMatches[0] !== $toMatches[0];
203+
$newLineBreaks = ['' => true];
204+
$oldLineBreaks = ['' => true];
205+
206+
foreach ($diff as $entry) {
207+
if (0 === $entry[1]) { /* OLD */
208+
$ln = $this->getLinebreak($entry[0]);
209+
$oldLineBreaks[$ln] = true;
210+
$newLineBreaks[$ln] = true;
211+
} elseif (1 === $entry[1]) { /* ADDED */
212+
$newLineBreaks[$this->getLinebreak($entry[0])] = true;
213+
} elseif (2 === $entry[1]) { /* REMOVED */
214+
$oldLineBreaks[$this->getLinebreak($entry[0])] = true;
215+
}
216+
}
217+
218+
// if either input or output is a single line without breaks than no warning should be raised
219+
if (['' => true] === $newLineBreaks || ['' => true] === $oldLineBreaks) {
220+
return false;
221+
}
222+
223+
// two way compare
224+
foreach ($newLineBreaks as $break => $set) {
225+
if (!isset($oldLineBreaks[$break])) {
226+
return true;
227+
}
228+
}
229+
230+
foreach ($oldLineBreaks as $break => $set) {
231+
if (!isset($newLineBreaks[$break])) {
232+
return true;
233+
}
234+
}
235+
236+
return false;
237+
}
238+
239+
private function getLinebreak($line): string
240+
{
241+
if (!\is_string($line)) {
242+
return '';
243+
}
244+
245+
$lc = \substr($line, -1);
246+
if ("\r" === $lc) {
247+
return "\r";
248+
}
249+
250+
if ("\n" !== $lc) {
251+
return '';
252+
}
253+
254+
if ("\r\n" === \substr($line, -2)) {
255+
return "\r\n";
256+
}
257+
258+
return "\n";
230259
}
231260

232261
private static function getArrayDiffParted(array &$from, array &$to): array

tests/DifferTest.php

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,25 @@ public function arrayProvider(): array
296296
],
297297
"<?php\r\n",
298298
"<?php\n",
299-
]
299+
],
300+
'test line diff detection in array input' => [
301+
[
302+
[
303+
"#Warning: Strings contain different line endings!\n",
304+
self::WARNING,
305+
],
306+
[
307+
"<?php\r\n",
308+
self::REMOVED,
309+
],
310+
[
311+
"<?php\n",
312+
self::ADDED,
313+
],
314+
],
315+
["<?php\r\n"],
316+
["<?php\n"],
317+
],
300318
];
301319
}
302320

@@ -385,6 +403,11 @@ public function textProvider(): array
385403
"<?php\r\nA\n",
386404
"<?php\nA\n",
387405
],
406+
[
407+
"--- Original\n+++ New\n@@ @@\n #Warning: Strings contain different line endings!\n-a\r\n+\n+c\r",
408+
"a\r\n",
409+
"\nc\r",
410+
],
388411
];
389412
}
390413

@@ -611,7 +634,7 @@ public function testSplitStringByLines(array $expected, string $input)
611634
$this->assertSame($expected, $method->invoke($this->differ, $input));
612635
}
613636

614-
public function provideSplitStringByLinesCases()
637+
public function provideSplitStringByLinesCases(): array
615638
{
616639
return [
617640
[

0 commit comments

Comments
 (0)