refactor: Separate matching in line-by-line algorithm

src/__tests__/diff2html-tests.ts

@@ -89,23 +89,23 @@ const htmlLineExample1 =
   '        <div class="d2h-code-line d2h-info">@@ -1 +1 @@</div>\n' +
   "    </td>\n" +
   "</tr><tr>\n" +
-  '    <td class="d2h-code-linenumber d2h-del">\n' +
+  '    <td class="d2h-code-linenumber d2h-del d2h-change">\n' +
   '      <div class="line-num1">1</div>\n' +
   '<div class="line-num2"></div>\n' +
   "    </td>\n" +
-  '    <td class="d2h-del">\n' +
-  '        <div class="d2h-code-line d2h-del">\n' +
+  '    <td class="d2h-del d2h-change">\n' +
+  '        <div class="d2h-code-line d2h-del d2h-change">\n' +
   '            <span class="d2h-code-line-prefix">-</span>\n' +
   '            <span class="d2h-code-line-ctn"><del>test</del></span>\n' +
   "        </div>\n" +
   "    </td>\n" +
   "</tr><tr>\n" +
-  '    <td class="d2h-code-linenumber d2h-ins">\n' +
+  '    <td class="d2h-code-linenumber d2h-ins d2h-change">\n' +
   '      <div class="line-num1"></div>\n' +
   '<div class="line-num2">1</div>\n' +
   "    </td>\n" +
-  '    <td class="d2h-ins">\n' +
-  '        <div class="d2h-code-line d2h-ins">\n' +
+  '    <td class="d2h-ins d2h-change">\n' +
+  '        <div class="d2h-code-line d2h-ins d2h-change">\n' +
   '            <span class="d2h-code-line-prefix">+</span>\n' +
   '            <span class="d2h-code-line-ctn"><ins>test1</ins></span>\n' +
   "        </div>\n" +
@@ -446,23 +446,23 @@ describe("Diff2Html", () => {
         "        </div>\n" +
         "    </td>\n" +
         "</tr><tr>\n" +
-        '    <td class="d2h-code-linenumber d2h-del">\n' +
+        '    <td class="d2h-code-linenumber d2h-del d2h-change">\n' +
         '      <div class="line-num1">14</div>\n' +
         '<div class="line-num2"></div>\n' +
         "    </td>\n" +
-        '    <td class="d2h-del">\n' +
-        '        <div class="d2h-code-line d2h-del">\n' +
+        '    <td class="d2h-del d2h-change">\n' +
+        '        <div class="d2h-code-line d2h-del d2h-change">\n' +
         '            <span class="d2h-code-line-prefix">-</span>\n' +
         '            <span class="d2h-code-line-ctn">    - Fix HEAD branch order when redraw [#858](https:&#x2F;&#x2F;github.com&#x2F;FredrikNoren&#x2F;ungit&#x2F;issues&#x2F;858)</span>\n' +
         "        </div>\n" +
         "    </td>\n" +
         "</tr><tr>\n" +
-        '    <td class="d2h-code-linenumber d2h-ins">\n' +
+        '    <td class="d2h-code-linenumber d2h-ins d2h-change">\n' +
         '      <div class="line-num1"></div>\n' +
         '<div class="line-num2">13</div>\n' +
         "    </td>\n" +
-        '    <td class="d2h-ins">\n' +
-        '        <div class="d2h-code-line d2h-ins">\n' +
+        '    <td class="d2h-ins d2h-change">\n' +
+        '        <div class="d2h-code-line d2h-ins d2h-change">\n' +
         '            <span class="d2h-code-line-prefix">+</span>\n' +
         '            <span class="d2h-code-line-ctn"><ins>4</ins>    - Fix HEAD branch order when redraw [#858](https:&#x2F;&#x2F;github.com&#x2F;FredrikNoren&#x2F;ungit&#x2F;issues&#x2F;858)</span>\n' +
         "        </div>\n" +
@@ -519,7 +519,7 @@ describe("Diff2Html", () => {
         "</div>";
 
       const result = html(diffExample2, { drawFileList: false });
-      expect(htmlExample2).toEqual(result);
+      expect(result).toEqual(htmlExample2);
     });
   });
 });

src/__tests__/line-by-line-tests.ts

@@ -1,6 +1,6 @@
 import LineByLineRenderer from "../line-by-line-renderer";
 import HoganJsUtils from "../hoganjs-utils";
-import { LineType, DiffLine, DiffFile, LineMatchingType } from "../types";
+import { LineType, DiffFile, LineMatchingType } from "../types";
 import { CSSLineClass } from "../render-utils";
 
 describe("LineByLineRenderer", () => {
@@ -26,7 +26,7 @@ describe("LineByLineRenderer", () => {
     it("should work for insertions", () => {
       const hoganUtils = new HoganJsUtils({});
       const lineByLineRenderer = new LineByLineRenderer(hoganUtils, {});
-      let fileHtml = lineByLineRenderer.makeLineHtml(false, CSSLineClass.INSERTS, "test", undefined, 30, "+");
+      let fileHtml = lineByLineRenderer.makeLineHtml(CSSLineClass.INSERTS, "+", "test", undefined, 30);
       fileHtml = fileHtml.replace(/\n\n+/g, "\n");
       const expected =
         "<tr>\n" +
@@ -48,7 +48,7 @@ describe("LineByLineRenderer", () => {
     it("should work for deletions", () => {
       const hoganUtils = new HoganJsUtils({});
       const lineByLineRenderer = new LineByLineRenderer(hoganUtils, {});
-      let fileHtml = lineByLineRenderer.makeLineHtml(false, CSSLineClass.DELETES, "test", 30, undefined, "-");
+      let fileHtml = lineByLineRenderer.makeLineHtml(CSSLineClass.DELETES, "-", "test", 30, undefined);
       fileHtml = fileHtml.replace(/\n\n+/g, "\n");
       const expected =
         "<tr>\n" +
@@ -70,7 +70,7 @@ describe("LineByLineRenderer", () => {
     it("should convert indents into non breakin spaces (2 white spaces)", () => {
       const hoganUtils = new HoganJsUtils({});
       const lineByLineRenderer = new LineByLineRenderer(hoganUtils, {});
-      let fileHtml = lineByLineRenderer.makeLineHtml(false, CSSLineClass.INSERTS, "  test", undefined, 30, "+");
+      let fileHtml = lineByLineRenderer.makeLineHtml(CSSLineClass.INSERTS, "+", "  test", undefined, 30);
       fileHtml = fileHtml.replace(/\n\n+/g, "\n");
       const expected =
         "<tr>\n" +
@@ -92,7 +92,7 @@ describe("LineByLineRenderer", () => {
     it("should convert indents into non breakin spaces (4 white spaces)", () => {
       const hoganUtils = new HoganJsUtils({});
       const lineByLineRenderer = new LineByLineRenderer(hoganUtils, {});
-      let fileHtml = lineByLineRenderer.makeLineHtml(false, CSSLineClass.INSERTS, "    test", undefined, 30, "+");
+      let fileHtml = lineByLineRenderer.makeLineHtml(CSSLineClass.INSERTS, "+", "    test", undefined, 30);
       fileHtml = fileHtml.replace(/\n\n+/g, "\n");
       const expected =
         "<tr>\n" +
@@ -114,7 +114,7 @@ describe("LineByLineRenderer", () => {
     it("should preserve tabs", () => {
       const hoganUtils = new HoganJsUtils({});
       const lineByLineRenderer = new LineByLineRenderer(hoganUtils, {});
-      let fileHtml = lineByLineRenderer.makeLineHtml(false, CSSLineClass.INSERTS, "\ttest", undefined, 30, "+");
+      let fileHtml = lineByLineRenderer.makeLineHtml(CSSLineClass.INSERTS, "+", "\ttest", undefined, 30);
       fileHtml = fileHtml.replace(/\n\n+/g, "\n");
       const expected =
         "<tr>\n" +
@@ -473,58 +473,6 @@ describe("LineByLineRenderer", () => {
     });
   });
 
-  describe("_processLines", () => {
-    it("should work for simple block header", () => {
-      const hoganUtils = new HoganJsUtils({});
-      const lineByLineRenderer = new LineByLineRenderer(hoganUtils, {});
-      const oldLines: DiffLine[] = [
-        {
-          content: "-test",
-          type: LineType.DELETE,
-          oldNumber: 1,
-          newNumber: undefined
-        }
-      ];
-      const newLines: DiffLine[] = [
-        {
-          content: "+test1r",
-          type: LineType.INSERT,
-          oldNumber: undefined,
-          newNumber: 1
-        }
-      ];
-
-      const html = lineByLineRenderer.processLines(false, oldLines, newLines);
-
-      const expected =
-        "<tr>\n" +
-        '    <td class="d2h-code-linenumber d2h-del">\n' +
-        '      <div class="line-num1">1</div>\n' +
-        '<div class="line-num2"></div>\n' +
-        "    </td>\n" +
-        '    <td class="d2h-del">\n' +
-        '        <div class="d2h-code-line d2h-del">\n' +
-        '            <span class="d2h-code-line-prefix">-</span>\n' +
-        '            <span class="d2h-code-line-ctn">test</span>\n' +
-        "        </div>\n" +
-        "    </td>\n" +
-        "</tr><tr>\n" +
-        '    <td class="d2h-code-linenumber d2h-ins">\n' +
-        '      <div class="line-num1"></div>\n' +
-        '<div class="line-num2">1</div>\n' +
-        "    </td>\n" +
-        '    <td class="d2h-ins">\n' +
-        '        <div class="d2h-code-line d2h-ins">\n' +
-        '            <span class="d2h-code-line-prefix">+</span>\n' +
-        '            <span class="d2h-code-line-ctn">test1r</span>\n' +
-        "        </div>\n" +
-        "    </td>\n" +
-        "</tr>";
-
-      expect(html).toEqual(expected);
-    });
-  });
-
   describe("_generateFileHtml", () => {
     it("should work for simple file", () => {
       const hoganUtils = new HoganJsUtils({});
@@ -595,23 +543,23 @@ describe("LineByLineRenderer", () => {
         "        </div>\n" +
         "    </td>\n" +
         "</tr><tr>\n" +
-        '    <td class="d2h-code-linenumber d2h-del">\n' +
+        '    <td class="d2h-code-linenumber d2h-del d2h-change">\n' +
         '      <div class="line-num1">2</div>\n' +
         '<div class="line-num2"></div>\n' +
         "    </td>\n" +
-        '    <td class="d2h-del">\n' +
-        '        <div class="d2h-code-line d2h-del">\n' +
+        '    <td class="d2h-del d2h-change">\n' +
+        '        <div class="d2h-code-line d2h-del d2h-change">\n' +
         '            <span class="d2h-code-line-prefix">-</span>\n' +
         '            <span class="d2h-code-line-ctn"><del>test</del></span>\n' +
         "        </div>\n" +
         "    </td>\n" +
         "</tr><tr>\n" +
-        '    <td class="d2h-code-linenumber d2h-ins">\n' +
+        '    <td class="d2h-code-linenumber d2h-ins d2h-change">\n' +
         '      <div class="line-num1"></div>\n' +
         '<div class="line-num2">2</div>\n' +
         "    </td>\n" +
-        '    <td class="d2h-ins">\n' +
-        '        <div class="d2h-code-line d2h-ins">\n' +
+        '    <td class="d2h-ins d2h-change">\n' +
+        '        <div class="d2h-code-line d2h-ins d2h-change">\n' +
         '            <span class="d2h-code-line-prefix">+</span>\n' +
         '            <span class="d2h-code-line-ctn"><ins>test1r</ins></span>\n' +
         "        </div>\n" +

src/line-by-line-renderer.ts

@@ -1,7 +1,7 @@
 import HoganJsUtils from "./hoganjs-utils";
 import * as Rematch from "./rematch";
 import * as renderUtils from "./render-utils";
-import { DiffFile, DiffLine, LineType } from "./types";
+import { DiffFile, DiffLine, LineType, DiffBlock } from "./types";
 
 export interface LineByLineRendererConfig extends renderUtils.RenderConfig {
   renderNothingWhenEmpty?: boolean;
@@ -79,7 +79,6 @@ export default class LineByLineRenderer {
     });
   }
 
-  // TODO: Make this private after improving tests
   generateFileHtml(file: DiffFile): string {
     const matcher = Rematch.newMatcherFn(
       Rematch.newDistanceFn((e: DiffLine) => renderUtils.deconstructLine(e.content, file.isCombined).content)
@@ -93,58 +92,79 @@ export default class LineByLineRenderer {
           lineClass: "d2h-code-linenumber",
           contentClass: "d2h-code-line"
         });
-        let oldLines: DiffLine[] = [];
-        let newLines: DiffLine[] = [];
 
-        for (let i = 0; i < block.lines.length; i++) {
-          const diffLine = block.lines[i];
-          const { prefix, content } = renderUtils.deconstructLine(diffLine.content, file.isCombined);
-
-          if (
-            diffLine.type !== LineType.INSERT &&
-            (newLines.length > 0 || (diffLine.type !== LineType.DELETE && oldLines.length > 0))
-          ) {
-            lines += this.processChangeBlock(file, oldLines, newLines, matcher);
-            oldLines = [];
-            newLines = [];
-          }
-
-          if (diffLine.type === LineType.CONTEXT || (diffLine.type === LineType.INSERT && !oldLines.length)) {
-            lines += this.makeLineHtml(
-              file.isCombined,
-              renderUtils.toCSSClass(diffLine.type),
+        this.applyLineGroupping(block).forEach(([contextLines, oldLines, newLines]) => {
+          if (oldLines.length && newLines.length && !contextLines.length) {
+            lines += this.applyRematchMatching(oldLines, newLines, matcher)
+              .map(([oldLines, newLines]) => this.applyLineDiff(file, oldLines, newLines))
+              .join("");
+          } else if (oldLines.length || newLines.length || contextLines.length) {
+            lines += (contextLines || [])
+              .concat((oldLines || []).concat(newLines || []))
+              .map(line => {
+                const { prefix, content } = renderUtils.deconstructLine(line.content, file.isCombined);
+                return this.makeLineHtml(
+                  renderUtils.toCSSClass(line.type),
+                  prefix,
                   content,
-              diffLine.oldNumber,
-              diffLine.newNumber,
-              prefix
+                  line.oldNumber,
+                  line.newNumber
                 );
-          } else if (diffLine.type === LineType.DELETE) {
-            oldLines.push(diffLine);
-          } else if (diffLine.type === LineType.INSERT && Boolean(oldLines.length)) {
-            newLines.push(diffLine);
+              })
+              .join("");
           } else {
-            console.error("Unknown state in html line-by-line generator");
-            lines += this.processChangeBlock(file, oldLines, newLines, matcher);
-            oldLines = [];
-            newLines = [];
+            console.error("Unknown state reached while processing groups of lines", contextLines, oldLines, newLines);
           }
-        }
-
-        lines += this.processChangeBlock(file, oldLines, newLines, matcher);
-        oldLines = [];
-        newLines = [];
+        });
 
         return lines;
       })
       .join("\n");
   }
 
-  processChangeBlock(
-    file: DiffFile,
+  applyLineGroupping(block: DiffBlock): DiffLine[][][] {
+    const blockLinesGroups: DiffLine[][][] = [];
+
+    let oldLines: DiffLine[] = [];
+    let newLines: DiffLine[] = [];
+
+    for (let i = 0; i < block.lines.length; i++) {
+      const diffLine = block.lines[i];
+
+      if (
+        (diffLine.type !== LineType.INSERT && newLines.length) ||
+        (diffLine.type === LineType.CONTEXT && oldLines.length > 0)
+      ) {
+        blockLinesGroups.push([[], oldLines, newLines]);
+        oldLines = [];
+        newLines = [];
+      }
+
+      if (diffLine.type === LineType.CONTEXT) {
+        blockLinesGroups.push([[diffLine], [], []]);
+      } else if (diffLine.type === LineType.INSERT && oldLines.length === 0) {
+        blockLinesGroups.push([[], [], [diffLine]]);
+      } else if (diffLine.type === LineType.INSERT && oldLines.length > 0) {
+        newLines.push(diffLine);
+      } else if (diffLine.type === LineType.DELETE) {
+        oldLines.push(diffLine);
+      }
+    }
+
+    if (oldLines.length || newLines.length) {
+      blockLinesGroups.push([[], oldLines, newLines]);
+      oldLines = [];
+      newLines = [];
+    }
+
+    return blockLinesGroups;
+  }
+
+  applyRematchMatching(
     oldLines: DiffLine[],
     newLines: DiffLine[],
     matcher: Rematch.MatcherFn<DiffLine>
-  ): string {
+  ): DiffLine[][][] {
     const comparisons = oldLines.length * newLines.length;
     const maxLineSizeInBlock = Math.max.apply(
       null,
@@ -155,17 +175,14 @@ export default class LineByLineRenderer {
       maxLineSizeInBlock < this.config.maxLineSizeInBlockForComparison &&
       (this.config.matching === "lines" || this.config.matching === "words");
 
-    const [matches, insertType, deleteType] = doMatching
-      ? [matcher(oldLines, newLines), renderUtils.CSSLineClass.INSERT_CHANGES, renderUtils.CSSLineClass.DELETE_CHANGES]
-      : [[[oldLines, newLines]], renderUtils.CSSLineClass.INSERTS, renderUtils.CSSLineClass.DELETES];
+    const matches = doMatching ? matcher(oldLines, newLines) : [[oldLines, newLines]];
 
-    let lines = "";
-    matches.forEach(match => {
-      oldLines = match[0];
-      newLines = match[1];
+    return matches;
+  }
 
-      let processedOldLines = "";
-      let processedNewLines = "";
+  applyLineDiff(file: DiffFile, oldLines: DiffLine[], newLines: DiffLine[]): string {
+    let oldLinesHtml = "";
+    let newLinesHtml = "";
 
     const common = Math.min(oldLines.length, newLines.length);
 
@@ -176,94 +193,54 @@ export default class LineByLineRenderer {
 
       const diff = renderUtils.diffHighlight(oldLine.content, newLine.content, file.isCombined, this.config);
 
-        processedOldLines += this.makeLineHtml(
-          file.isCombined,
-          deleteType,
+      oldLinesHtml += this.makeLineHtml(
+        renderUtils.CSSLineClass.DELETE_CHANGES,
+        diff.oldLine.prefix,
         diff.oldLine.content,
         oldLine.oldNumber,
-          oldLine.newNumber,
-          diff.oldLine.prefix
-        );
-        processedNewLines += this.makeLineHtml(
-          file.isCombined,
-          insertType,
-          diff.newLine.content,
-          newLine.oldNumber,
-          newLine.newNumber,
-          diff.newLine.prefix
-        );
-      }
-
-      lines += processedOldLines + processedNewLines;
-      lines += this.processLines(file.isCombined, oldLines.slice(common), newLines.slice(common));
-    });
-
-    return lines;
-  }
-
-  // TODO: Make this private after improving tests
-  makeLineHtml(
-    isCombined: boolean,
-    type: renderUtils.CSSLineClass,
-    content: string,
-    oldNumber?: number,
-    newNumber?: number,
-    possiblePrefix?: string
-  ): string {
-    const lineNumberTemplate = this.hoganUtils.render(baseTemplatesPath, "numbers", {
-      oldNumber: oldNumber || "",
-      newNumber: newNumber || ""
-    });
-
-    let lineWithoutPrefix = content;
-    let prefix = possiblePrefix;
-
-    if (!prefix) {
-      const lineWithPrefix = renderUtils.deconstructLine(content, isCombined);
-      prefix = lineWithPrefix.prefix;
-      lineWithoutPrefix = lineWithPrefix.content;
-    }
-
-    if (prefix === " ") {
-      prefix = "&nbsp;";
-    }
-
-    return this.hoganUtils.render(genericTemplatesPath, "line", {
-      type: type,
-      lineClass: "d2h-code-linenumber",
-      contentClass: "d2h-code-line",
-      prefix: prefix,
-      content: lineWithoutPrefix,
-      lineNumber: lineNumberTemplate
-    });
-  }
-
-  // TODO: Make this private after improving tests
-  processLines(isCombined: boolean, oldLines: DiffLine[], newLines: DiffLine[]): string {
-    let lines = "";
-
-    for (let i = 0; i < oldLines.length; i++) {
-      const oldLine = oldLines[i];
-      lines += this.makeLineHtml(
-        isCombined,
-        renderUtils.toCSSClass(oldLine.type),
-        oldLine.content,
-        oldLine.oldNumber,
         oldLine.newNumber
       );
-    }
-
-    for (let j = 0; j < newLines.length; j++) {
-      const newLine = newLines[j];
-      lines += this.makeLineHtml(
-        isCombined,
-        renderUtils.toCSSClass(newLine.type),
-        newLine.content,
+      newLinesHtml += this.makeLineHtml(
+        renderUtils.CSSLineClass.INSERT_CHANGES,
+        diff.newLine.prefix,
+        diff.newLine.content,
         newLine.oldNumber,
         newLine.newNumber
       );
     }
 
-    return lines;
+    const remainingLines = oldLines
+      .slice(common)
+      .concat(newLines.slice(common))
+      .map(line => {
+        const { prefix, content } = renderUtils.deconstructLine(line.content, file.isCombined);
+        return this.makeLineHtml(renderUtils.toCSSClass(line.type), prefix, content, line.oldNumber, line.newNumber);
+      })
+      .join("");
+
+    return oldLinesHtml + newLinesHtml + remainingLines;
+  }
+
+  // TODO: Make this private after improving tests
+  makeLineHtml(
+    type: renderUtils.CSSLineClass,
+    prefix: string,
+    content: string,
+    oldNumber?: number,
+    newNumber?: number
+  ): string {
+    const lineNumberHtml = this.hoganUtils.render(baseTemplatesPath, "numbers", {
+      oldNumber: oldNumber || "",
+      newNumber: newNumber || ""
+    });
+
+    return this.hoganUtils.render(genericTemplatesPath, "line", {
+      type: type,
+      lineClass: "d2h-code-linenumber",
+      contentClass: "d2h-code-line",
+      prefix: prefix === " " ? "&nbsp;" : prefix,
+      content: content,
+      lineNumber: lineNumberHtml
+    });
   }
 }

src/render-utils.ts

@@ -240,19 +240,13 @@ export function diffHighlight(
 
   const changedWords: jsDiff.Change[] = [];
   if (diffStyle === "word" && matching === "words") {
-    let treshold = 0.25;
-
-    if (typeof matchWordsThreshold !== "undefined") {
-      treshold = matchWordsThreshold;
-    }
-
     const removed = diff.filter(element => element.removed);
     const added = diff.filter(element => element.added);
     const chunks = matcher(added, removed);
     chunks.forEach(chunk => {
       if (chunk[0].length === 1 && chunk[1].length === 1) {
         const dist = distance(chunk[0][0], chunk[1][0]);
-        if (dist < treshold) {
+        if (dist < matchWordsThreshold) {
           changedWords.push(chunk[0][0]);
           changedWords.push(chunk[1][0]);
         }

src/side-by-side-renderer.ts

@@ -331,15 +331,11 @@ export default class SideBySideRenderer {
       lineWithoutPrefix = lineWithPrefix.content;
     }
 
-    if (prefix === " ") {
-      prefix = " ";
-    }
-
     return this.hoganUtils.render(genericTemplatesPath, "line", {
       type: preparedType,
       lineClass: lineClass,
       contentClass: contentClass,
-      prefix: prefix,
+      prefix: prefix === " " ? " " : prefix,
       content: lineWithoutPrefix,
       lineNumber: number
     });