From 0596a5f7ade1f7a4e36f09eb98a302f084142275 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Sun, 24 Mar 2024 23:40:23 +0100 Subject: [PATCH] Fix edge case when diff changes were exactly "++ " or "-- " 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. --- diff_extract_changes/diff_extract_changes.php | 33 ++++++++++++++++--- tests/1-diff_extract_changes.bats | 20 +++++++---- .../diff_extract_changes-edge-expected.txt | 2 ++ .../diff_extract_changes-edge-expected.xml | 9 +++++ .../diff_extract_changes-edge-input.patch | 25 ++++++++++++++ .../diff_extract_changes-normal-expected.txt} | 0 .../diff_extract_changes-normal-expected.xml} | 4 +-- .../diff_extract_changes-normal-input.patch} | 0 8 files changed, 81 insertions(+), 12 deletions(-) create mode 100644 tests/fixtures/diff_extract_changes/diff_extract_changes-edge-expected.txt create mode 100644 tests/fixtures/diff_extract_changes/diff_extract_changes-edge-expected.xml create mode 100644 tests/fixtures/diff_extract_changes/diff_extract_changes-edge-input.patch rename tests/fixtures/{diff_extract_changes-expected.txt => diff_extract_changes/diff_extract_changes-normal-expected.txt} (100%) rename tests/fixtures/{diff_extract_changes-expected.xml => diff_extract_changes/diff_extract_changes-normal-expected.xml} (99%) rename tests/fixtures/{diff_extract_changes-input.patch => diff_extract_changes/diff_extract_changes-normal-input.patch} (100%) diff --git a/diff_extract_changes/diff_extract_changes.php b/diff_extract_changes/diff_extract_changes.php index 10acb126..d881effc 100644 --- a/diff_extract_changes/diff_extract_changes.php +++ b/diff_extract_changes/diff_extract_changes.php @@ -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; @@ -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'); @@ -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)) { @@ -300,7 +325,7 @@ private function output_begin() { */ private function output_end() { if ($this->output == 'xml') { - echo ''; + echo '' . PHP_EOL; } } diff --git a/tests/1-diff_extract_changes.bats b/tests/1-diff_extract_changes.bats index 36a71701..af388de4 100755 --- a/tests/1-diff_extract_changes.bats +++ b/tests/1-diff_extract_changes.bats @@ -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 @@ -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 } diff --git a/tests/fixtures/diff_extract_changes/diff_extract_changes-edge-expected.txt b/tests/fixtures/diff_extract_changes/diff_extract_changes-edge-expected.txt new file mode 100644 index 00000000..89316253 --- /dev/null +++ b/tests/fixtures/diff_extract_changes/diff_extract_changes-edge-expected.txt @@ -0,0 +1,2 @@ +another_binary.png: +diff_extract_changes_edge_cases.txt:10-10;13-13; diff --git a/tests/fixtures/diff_extract_changes/diff_extract_changes-edge-expected.xml b/tests/fixtures/diff_extract_changes/diff_extract_changes-edge-expected.xml new file mode 100644 index 00000000..8b717696 --- /dev/null +++ b/tests/fixtures/diff_extract_changes/diff_extract_changes-edge-expected.xml @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/tests/fixtures/diff_extract_changes/diff_extract_changes-edge-input.patch b/tests/fixtures/diff_extract_changes/diff_extract_changes-edge-input.patch new file mode 100644 index 00000000..e7701a95 --- /dev/null +++ b/tests/fixtures/diff_extract_changes/diff_extract_changes-edge-input.patch @@ -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 diff --git a/tests/fixtures/diff_extract_changes-expected.txt b/tests/fixtures/diff_extract_changes/diff_extract_changes-normal-expected.txt similarity index 100% rename from tests/fixtures/diff_extract_changes-expected.txt rename to tests/fixtures/diff_extract_changes/diff_extract_changes-normal-expected.txt diff --git a/tests/fixtures/diff_extract_changes-expected.xml b/tests/fixtures/diff_extract_changes/diff_extract_changes-normal-expected.xml similarity index 99% rename from tests/fixtures/diff_extract_changes-expected.xml rename to tests/fixtures/diff_extract_changes/diff_extract_changes-normal-expected.xml index 57b9785c..c4837991 100644 --- a/tests/fixtures/diff_extract_changes-expected.xml +++ b/tests/fixtures/diff_extract_changes/diff_extract_changes-normal-expected.xml @@ -1,5 +1,5 @@ - + @@ -506,4 +506,4 @@ - \ No newline at end of file + diff --git a/tests/fixtures/diff_extract_changes-input.patch b/tests/fixtures/diff_extract_changes/diff_extract_changes-normal-input.patch similarity index 100% rename from tests/fixtures/diff_extract_changes-input.patch rename to tests/fixtures/diff_extract_changes/diff_extract_changes-normal-input.patch