fix: unexpected Combined output due to HTML formatting

#30

Signed-off-by: Jack Cherng <jfcherng@gmail.com>

Author: jfcherng

src/Renderer/Html/AbstractHtml.php

@@ -34,6 +34,16 @@ abstract class AbstractHtml extends AbstractRenderer
         SequenceMatcher::OP_REP => 'rep',
     ];
 
+    /**
+     * Auto format the content in "changes" to be suitable for HTML output.
+     *
+     * This may not be a wanted behavior for some (custom) renderers
+     * if they want to do this by themselves in a later stage.
+     *
+     * @var bool
+     */
+    const AUTO_FORMAT_CHANGES = true;
+
     /**
      * {@inheritdoc}
      */
@@ -80,37 +90,18 @@ public function getChanges(Differ $differ): array
                     }
                 }
 
-                $oldLines = $this->formatLines(\array_slice($old, $i1, $i2 - $i1));
-                $newLines = $this->formatLines(\array_slice($new, $j1, $j2 - $j1));
-
-                if ($op === SequenceMatcher::OP_EQ) {
-                    $block['old']['lines'] = $oldLines;
-                    $block['new']['lines'] = $newLines;
-
-                    continue;
-                }
-
-                if ($op & (SequenceMatcher::OP_REP | SequenceMatcher::OP_DEL)) {
-                    $block['old']['lines'] = \str_replace(
-                        RendererConstant::HTML_CLOSURES,
-                        RendererConstant::HTML_CLOSURES_DEL,
-                        $oldLines
-                    );
-                }
-
-                if ($op & (SequenceMatcher::OP_REP | SequenceMatcher::OP_INS)) {
-                    $block['new']['lines'] = \str_replace(
-                        RendererConstant::HTML_CLOSURES,
-                        RendererConstant::HTML_CLOSURES_INS,
-                        $newLines
-                    );
-                }
+                $block['old']['lines'] = \array_slice($old, $i1, $i2 - $i1);
+                $block['new']['lines'] = \array_slice($new, $j1, $j2 - $j1);
             }
             unset($block);
 
             $changes[] = $change;
         }
 
+        if (static::AUTO_FORMAT_CHANGES) {
+            $this->formatChanges($changes);
+        }
+
         return $changes;
     }
 
@@ -194,6 +185,37 @@ protected function getDefaultBlock(int $op, int $i1, int $j1): array
         ];
     }
 
+    /**
+     * Make the content in "changes" suitable for HTML output.
+     *
+     * @param array[][] $changes the changes
+     */
+    final protected function formatChanges(array &$changes): void
+    {
+        foreach ($changes as &$hunk) {
+            foreach ($hunk as &$block) {
+                $block['old']['lines'] = $this->formatLines($block['old']['lines']);
+                $block['new']['lines'] = $this->formatLines($block['new']['lines']);
+
+                if ($block['tag'] & (SequenceMatcher::OP_REP | SequenceMatcher::OP_DEL)) {
+                    $block['old']['lines'] = \str_replace(
+                        RendererConstant::HTML_CLOSURES,
+                        RendererConstant::HTML_CLOSURES_DEL,
+                        $block['old']['lines']
+                    );
+                }
+
+                if ($block['tag'] & (SequenceMatcher::OP_REP | SequenceMatcher::OP_INS)) {
+                    $block['new']['lines'] = \str_replace(
+                        RendererConstant::HTML_CLOSURES,
+                        RendererConstant::HTML_CLOSURES_INS,
+                        $block['new']['lines']
+                    );
+                }
+            }
+        }
+    }
+
     /**
      * Make a series of lines suitable for outputting in a HTML rendered diff.
      *

src/Renderer/Html/Combined.php

@@ -25,6 +25,11 @@ final class Combined extends AbstractHtml
         'type' => 'Html',
     ];
 
+    /**
+     * {@inheritdoc}
+     */
+    const AUTO_FORMAT_CHANGES = false;
+
     /**
      * {@inheritdoc}
      */
@@ -142,6 +147,11 @@ protected function renderTableBlockEqual(array $block): string
      */
     protected function renderTableBlockInsert(array $block): string
     {
+        $block['new']['lines'] = $this->customFormatLines(
+            $block['new']['lines'],
+            SequenceMatcher::OP_INS
+        );
+
         $ret = '';
 
         foreach ($block['new']['lines'] as $newLine) {
@@ -158,6 +168,11 @@ protected function renderTableBlockInsert(array $block): string
      */
     protected function renderTableBlockDelete(array $block): string
     {
+        $block['old']['lines'] = $this->customFormatLines(
+            $block['old']['lines'],
+            SequenceMatcher::OP_DEL
+        );
+
         $ret = '';
 
         foreach ($block['old']['lines'] as $oldLine) {
@@ -195,6 +210,9 @@ protected function renderTableBlockReplace(array $block): string
             $oldLinesCount = $newLinesCount = 1;
         }
 
+        $oldLines = $this->customFormatLines($oldLines, SequenceMatcher::OP_DEL);
+        $newLines = $this->customFormatLines($newLines, SequenceMatcher::OP_INS);
+
         // now $oldLines must has the same line counts with $newlines
         for ($no = 0; $no < $newLinesCount; ++$no) {
             $mergedLine = $this->mergeReplaceLines($oldLines[$no], $newLines[$no]);
@@ -376,21 +394,9 @@ protected function markReplaceBlockDiff(array $oldBlock, array $newBlock): array
 
         $lineRenderer->render($mbOld, $mbNew);
 
-        $oldLine = \str_replace(
-            RendererConstant::HTML_CLOSURES,
-            RendererConstant::HTML_CLOSURES_DEL,
-            $mbOld->get()
-        );
-
-        $newLine = \str_replace(
-            RendererConstant::HTML_CLOSURES,
-            RendererConstant::HTML_CLOSURES_INS,
-            $mbNew->get()
-        );
-
         return [
-            [$oldLine], // one-line block for the old
-            [$newLine], // one-line block for the new
+            [$mbOld->get()], // one-line block for the old
+            [$mbNew->get()], // one-line block for the new
         ];
     }
 
@@ -461,4 +467,25 @@ function (array $matches) use (&$parts, $part, $ld, $rd): string {
             );
         }
     }
+
+    /**
+     * Make lines suitable for HTML output.
+     *
+     * @param string[] $lines the lines
+     * @param int      $op    the operation
+     */
+    protected function customFormatLines(array $lines, int $op): array
+    {
+        $htmlClosures = $op === SequenceMatcher::OP_DEL
+            ? RendererConstant::HTML_CLOSURES_DEL
+            : RendererConstant::HTML_CLOSURES_INS;
+
+        $lines = $this->formatLines($lines);
+
+        foreach ($lines as &$line) {
+            $line = \str_replace(RendererConstant::HTML_CLOSURES, $htmlClosures, $line);
+        }
+
+        return $lines;
+    }
 }

tests/Renderer/Html/CombinedTest.php

@@ -0,0 +1,33 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Jfcherng\Diff\Test\Renderer\Html;
+
+use Jfcherng\Diff\DiffHelper;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Test Combined.
+ *
+ * @coversNothing
+ *
+ * @internal
+ */
+final class CombinedTest extends TestCase
+{
+    /**
+     * Test the internal HTML output formatting.
+     *
+     * @covers \Jfcherng\Diff\Renderer\Html\Combined
+     *
+     * @see https://github.com/jfcherng/php-diff/issues/30
+     */
+    public function testHtmlFormatting(): void
+    {
+        $result = DiffHelper::calculate('<', " \nA<B", 'Combined', ['detailLevel' => 'word']);
+        $result = \htmlspecialchars_decode($result);
+
+        static::assertThat($result, static::logicalNot(static::stringContains(';')));
+    }
+}