Skip to content

Commit

Permalink
Fix edge case when diff changes were exactly "++ " or "-- "
Browse files Browse the repository at this point in the history
In those cases, sometimes, that was leading to the lines
to be recognised as a new file incorrectly, causing various
troubles later in the process.

Now we look for the two lines in a row, and only when they
have something looking like a path:

- /dev/null
- /path/to/something

Covered with test containing the edge cases.
  • Loading branch information
stronk7 committed Mar 24, 2024
1 parent 31e6abd commit 0596a5f
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 12 deletions.
33 changes: 29 additions & 4 deletions diff_extract_changes/diff_extract_changes.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@
class diff_changes_extractor {

/** version of the extractor */
const VERSION = '20120120';
const VERSION = '20240323';

/** @var string The unified diff file to process */
protected $file;
Expand Down Expand Up @@ -130,9 +130,11 @@ public function process() {
$inchunk = false; // To determine if we are in a chunk or no
$deletefile = false; // To detect delete operation and skip those files
$binaryfile = false; // To detect binary files and handle them specially
$afterminus = false; // To detect if we are after a minus (-) line
$isnewfile = false; // To detect if we are in a new file

// Skip always these lines
$skiplines = array('diff', 'inde', '--- ');
$skiplines = array('diff', 'inde');

// Let's read the diff file, line by line.
$fh = fopen($this->file, 'r');
Expand All @@ -155,16 +157,39 @@ public function process() {
continue;
}

// If it's the start of a new file (--- ), we mark it for next line.
if ($lineheader === '--- ') {
// Only if the line has something looking like a path.
if (preg_match('~^--- a?(/)(.+)$~', $line, $match)) {
if (!empty($match[2]) && !empty($match[1]) && $match[1] === '/') {
$afterminus = true;
}
}
}

// If it's the start of a new file (+++ ), and we are after a minus (--- )
// we raise the new file flag.
if ($lineheader === '+++ ' && $afterminus) {
// Only if the line has something looking like a path.
if (preg_match('~^\+\+\+ b?(/)(.+)$~', $line, $match)) {
if (!empty($match[2]) && !empty($match[1]) && $match[1] === '/') {
$isnewfile = true;
}
}
}

// If it's one Binary file, we mark it
if ($lineheader === 'Bina') {
$binaryfile = true;
}

// Detect if we are changing of file
if ($lineheader === '+++ ' || $binaryfile) {
if ($isnewfile || $binaryfile) {

$clineinfile = 0;
$inchunk = false;
$afterminus = false;
$isnewfile = false;

// Output interval end
if (!empty($clineint)) {
Expand Down Expand Up @@ -300,7 +325,7 @@ private function output_begin() {
*/
private function output_end() {
if ($this->output == 'xml') {
echo '</diffchanges>';
echo '</diffchanges>' . PHP_EOL;
}
}

Expand Down
20 changes: 14 additions & 6 deletions tests/1-diff_extract_changes.bats
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ load libs/shared_setup
# usage: pssert_diff_extract_changes format fixturefilename expectedfilename
assert_diff_extract_changes() {
format=$1
fixture=$BATS_TEST_DIRNAME/fixtures/$2
expected=$BATS_TEST_DIRNAME/fixtures/$3
fixture=$BATS_TEST_DIRNAME/fixtures/diff_extract_changes/$2
expected=$BATS_TEST_DIRNAME/fixtures/diff_extract_changes/$3

out=$BATS_TMPDIR/diff_extract_changes-out

Expand All @@ -17,10 +17,18 @@ assert_diff_extract_changes() {
rm $out
}

@test "diff_extract_changes: xml" {
assert_diff_extract_changes xml diff_extract_changes-input.patch diff_extract_changes-expected.xml
@test "diff_extract_changes (normal): xml" {
assert_diff_extract_changes xml diff_extract_changes-normal-input.patch diff_extract_changes-normal-expected.xml
}

@test "diff_extract_changes: txt" {
assert_diff_extract_changes txt diff_extract_changes-input.patch diff_extract_changes-expected.txt
@test "diff_extract_changes (normal): txt" {
assert_diff_extract_changes txt diff_extract_changes-normal-input.patch diff_extract_changes-normal-expected.txt
}

@test "diff_extract_changes (edge): xml" {
assert_diff_extract_changes xml diff_extract_changes-edge-input.patch diff_extract_changes-edge-expected.xml
}

@test "diff_extract_changes (edge): txt" {
assert_diff_extract_changes txt diff_extract_changes-edge-input.patch diff_extract_changes-edge-expected.txt
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
another_binary.png:
diff_extract_changes_edge_cases.txt:10-10;13-13;
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?xml version="1.0" encoding="UTF-8" ?>
<diffchanges version="20240323">
<file name="another_binary.png">
</file>
<file name="diff_extract_changes_edge_cases.txt">
<lines from="10" to="10"/>
<lines from="13" to="13"/>
</file>
</diffchanges>
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
diff --git a/another_binary.png b/another_binary.png
new file mode 100644
index 00000000000..3cb4b3d9aaf
Binary files /dev/null and b/another_binary.png differ
diff --git a/diff_extract_changes_edge_cases.txt b/diff_extract_changes_edge_cases.txt
index 4e020b9abcd..21a2225922f 100644
--- a/diff_extract_changes_edge_cases.txt
+++ b/diff_extract_changes_edge_cases.txt
@@ -5,10 +5,9 @@ Let's see how out diff extractor behaves with some edge cases.
+
-
++
---
+++
----
++
---
+++
+
-
+--
diff --git a/some_binary1.png b/some_binary1.png
deleted file mode 100644
index f6277460437..00000000000
Binary files a/some_binary1.png and /dev/null differ
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="UTF-8" ?>
<diffchanges version="20120120">
<diffchanges version="20240323">
<file name=".stylelintrc">
<lines from="1" to="93"/>
</file>
Expand Down Expand Up @@ -506,4 +506,4 @@
<lines from="67" to="67"/>
<lines from="241" to="242"/>
</file>
</diffchanges>
</diffchanges>

0 comments on commit 0596a5f

Please sign in to comment.