refactor: Unify line-by-line and side-by-side

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

@@ -26,7 +26,13 @@ describe("LineByLineRenderer", () => {
     it("should work for insertions", () => {
       const hoganUtils = new HoganJsUtils({});
       const lineByLineRenderer = new LineByLineRenderer(hoganUtils, {});
-      let fileHtml = lineByLineRenderer.makeLineHtml(CSSLineClass.INSERTS, "+", "test", undefined, 30);
+      let fileHtml = lineByLineRenderer.generateSingleLineHtml({
+        type: CSSLineClass.INSERTS,
+        prefix: "+",
+        content: "test",
+        oldNumber: undefined,
+        newNumber: 30
+      });
       fileHtml = fileHtml.replace(/\n\n+/g, "\n");
       const expected =
         "<tr>\n" +
@@ -48,7 +54,13 @@ describe("LineByLineRenderer", () => {
     it("should work for deletions", () => {
       const hoganUtils = new HoganJsUtils({});
       const lineByLineRenderer = new LineByLineRenderer(hoganUtils, {});
-      let fileHtml = lineByLineRenderer.makeLineHtml(CSSLineClass.DELETES, "-", "test", 30, undefined);
+      let fileHtml = lineByLineRenderer.generateSingleLineHtml({
+        type: CSSLineClass.DELETES,
+        prefix: "-",
+        content: "test",
+        oldNumber: 30,
+        newNumber: undefined
+      });
       fileHtml = fileHtml.replace(/\n\n+/g, "\n");
       const expected =
         "<tr>\n" +
@@ -70,7 +82,13 @@ 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(CSSLineClass.INSERTS, "+", "  test", undefined, 30);
+      let fileHtml = lineByLineRenderer.generateSingleLineHtml({
+        type: CSSLineClass.INSERTS,
+        prefix: "+",
+        content: "  test",
+        oldNumber: undefined,
+        newNumber: 30
+      });
       fileHtml = fileHtml.replace(/\n\n+/g, "\n");
       const expected =
         "<tr>\n" +
@@ -92,7 +110,13 @@ 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(CSSLineClass.INSERTS, "+", "    test", undefined, 30);
+      let fileHtml = lineByLineRenderer.generateSingleLineHtml({
+        type: CSSLineClass.INSERTS,
+        prefix: "+",
+        content: "    test",
+        oldNumber: undefined,
+        newNumber: 30
+      });
       fileHtml = fileHtml.replace(/\n\n+/g, "\n");
       const expected =
         "<tr>\n" +
@@ -114,7 +138,13 @@ describe("LineByLineRenderer", () => {
     it("should preserve tabs", () => {
       const hoganUtils = new HoganJsUtils({});
       const lineByLineRenderer = new LineByLineRenderer(hoganUtils, {});
-      let fileHtml = lineByLineRenderer.makeLineHtml(CSSLineClass.INSERTS, "+", "\ttest", undefined, 30);
+      let fileHtml = lineByLineRenderer.generateSingleLineHtml({
+        type: CSSLineClass.INSERTS,
+        prefix: "+",
+        content: "\ttest",
+        oldNumber: undefined,
+        newNumber: 30
+      });
       fileHtml = fileHtml.replace(/\n\n+/g, "\n");
       const expected =
         "<tr>\n" +

src/__tests__/side-by-side-printer-tests.ts

@@ -163,7 +163,7 @@ describe("SideBySideRenderer", () => {
     it("should work for insertions", () => {
       const hoganUtils = new HoganJsUtils({});
       const sideBySideRenderer = new SideBySideRenderer(hoganUtils, {});
-      const fileHtml = sideBySideRenderer.generateSingleLineHtml(undefined, {
+      const fileHtml = sideBySideRenderer.generateLineHtml(undefined, {
         type: CSSLineClass.INSERTS,
         prefix: "+",
         content: "test",
@@ -178,6 +178,8 @@ describe("SideBySideRenderer", () => {
           "    </td>\n" +
           '    <td class="d2h-cntx d2h-emptyplaceholder">\n' +
           '        <div class="d2h-code-side-line d2h-code-side-emptyplaceholder d2h-cntx d2h-emptyplaceholder">\n' +
+          '            <span class="d2h-code-line-prefix">&nbsp;</span>\n' +
+          '            <span class="d2h-code-line-ctn">&nbsp;</span>\n' +
           "        </div>\n" +
           "    </td>\n" +
           "</tr>",
@@ -200,7 +202,7 @@ describe("SideBySideRenderer", () => {
     it("should work for deletions", () => {
       const hoganUtils = new HoganJsUtils({});
       const sideBySideRenderer = new SideBySideRenderer(hoganUtils, {});
-      const fileHtml = sideBySideRenderer.generateSingleLineHtml(
+      const fileHtml = sideBySideRenderer.generateLineHtml(
         {
           type: CSSLineClass.DELETES,
           prefix: "-",
@@ -229,6 +231,8 @@ describe("SideBySideRenderer", () => {
           "    </td>\n" +
           '    <td class="d2h-cntx d2h-emptyplaceholder">\n' +
           '        <div class="d2h-code-side-line d2h-code-side-emptyplaceholder d2h-cntx d2h-emptyplaceholder">\n' +
+          '            <span class="d2h-code-line-prefix">&nbsp;</span>\n' +
+          '            <span class="d2h-code-line-ctn">&nbsp;</span>\n' +
           "        </div>\n" +
           "    </td>\n" +
           "</tr>"
@@ -426,35 +430,35 @@ describe("SideBySideRenderer", () => {
 
       const hoganUtils = new HoganJsUtils({});
       const sideBySideRenderer = new SideBySideRenderer(hoganUtils, { matching: LineMatchingType.LINES });
-      const html = sideBySideRenderer.processLines(false, oldLines, newLines);
+      const html = sideBySideRenderer.processChangedLines(false, oldLines, newLines);
-      const expectedLeft =
+      const expected = {
+        left:
           "<tr>\n" +
-        '    <td class="d2h-code-side-linenumber d2h-del">\n' +
+          '    <td class="d2h-code-side-linenumber d2h-del d2h-change">\n' +
           "      1\n" +
           "    </td>\n" +
-        '    <td class="d2h-del">\n' +
+          '    <td class="d2h-del d2h-change">\n' +
-        '        <div class="d2h-code-side-line d2h-del">\n' +
+          '        <div class="d2h-code-side-line d2h-del d2h-change">\n' +
           '            <span class="d2h-code-line-prefix">-</span>\n' +
-        '            <span class="d2h-code-line-ctn">test</span>\n' +
+          '            <span class="d2h-code-line-ctn"><del>test</del></span>\n' +
           "        </div>\n" +
           "    </td>\n" +
-        "</tr>";
+          "</tr>",
-
+        right:
-      const expectedRight =
           "<tr>\n" +
-        '    <td class="d2h-code-side-linenumber d2h-ins">\n' +
+          '    <td class="d2h-code-side-linenumber d2h-ins d2h-change">\n' +
           "      1\n" +
           "    </td>\n" +
-        '    <td class="d2h-ins">\n' +
+          '    <td class="d2h-ins d2h-change">\n' +
-        '        <div class="d2h-code-side-line d2h-ins">\n' +
+          '        <div class="d2h-code-side-line d2h-ins d2h-change">\n' +
           '            <span class="d2h-code-line-prefix">+</span>\n' +
-        '            <span class="d2h-code-line-ctn">test1r</span>\n' +
+          '            <span class="d2h-code-line-ctn"><ins>test1r</ins></span>\n' +
           "        </div>\n" +
           "    </td>\n" +
-        "</tr>";
+          "</tr>"
+      };
 
-      expect(html.left).toEqual(expectedLeft);
+      expect(html).toEqual(expected);
-      expect(html.right).toEqual(expectedRight);
     });
   });
 });

src/line-by-line-renderer.ts

@@ -1,7 +1,16 @@
 import HoganJsUtils from "./hoganjs-utils";
 import * as Rematch from "./rematch";
 import * as renderUtils from "./render-utils";
-import { DiffFile, DiffLine, LineType, DiffBlock } from "./types";
+import {
+  DiffFile,
+  DiffLine,
+  LineType,
+  DiffBlock,
+  DiffLineDeleted,
+  DiffLineContent,
+  DiffLineContext,
+  DiffLineInserted
+} from "./types";
 
 export interface LineByLineRendererConfig extends renderUtils.RenderConfig {
   renderNothingWhenEmpty?: boolean;
@@ -95,23 +104,26 @@ export default class LineByLineRenderer {
 
         this.applyLineGroupping(block).forEach(([contextLines, oldLines, newLines]) => {
           if (oldLines.length && newLines.length && !contextLines.length) {
-            lines += this.applyRematchMatching(oldLines, newLines, matcher)
+            this.applyRematchMatching(oldLines, newLines, matcher).map(([oldLines, newLines]) => {
-              .map(([oldLines, newLines]) => this.applyLineDiff(file, oldLines, newLines))
+              const { left, right } = this.processChangedLines(file.isCombined, oldLines, newLines);
-              .join("");
+              lines += left;
-          } else if (oldLines.length || newLines.length || contextLines.length) {
+              lines += right;
-            lines += (contextLines || [])
+            });
-              .concat((oldLines || []).concat(newLines || []))
+          } else if (contextLines.length) {
-              .map(line => {
+            contextLines.forEach(line => {
               const { prefix, content } = renderUtils.deconstructLine(line.content, file.isCombined);
-                return this.makeLineHtml(
+              lines += this.generateSingleLineHtml({
-                  renderUtils.toCSSClass(line.type),
+                type: renderUtils.CSSLineClass.CONTEXT,
-                  prefix,
+                prefix: prefix,
-                  content,
+                content: content,
-                  line.oldNumber,
+                oldNumber: line.oldNumber,
-                  line.newNumber
+                newNumber: line.newNumber
-                );
+              });
-              })
+            });
-              .join("");
+          } else if (oldLines.length || newLines.length) {
+            const { left, right } = this.processChangedLines(file.isCombined, oldLines, newLines);
+            lines += left;
+            lines += right;
           } else {
             console.error("Unknown state reached while processing groups of lines", contextLines, oldLines, newLines);
           }
@@ -122,11 +134,11 @@ export default class LineByLineRenderer {
       .join("\n");
   }
 
-  applyLineGroupping(block: DiffBlock): DiffLine[][][] {
+  applyLineGroupping(block: DiffBlock): DiffLineGroups {
-    const blockLinesGroups: DiffLine[][][] = [];
+    const blockLinesGroups: DiffLineGroups = [];
 
-    let oldLines: DiffLine[] = [];
+    let oldLines: (DiffLineDeleted & DiffLineContent)[] = [];
-    let newLines: DiffLine[] = [];
+    let newLines: (DiffLineInserted & DiffLineContent)[] = [];
 
     for (let i = 0; i < block.lines.length; i++) {
       const diffLine = block.lines[i];
@@ -180,67 +192,109 @@ export default class LineByLineRenderer {
     return matches;
   }
 
-  applyLineDiff(file: DiffFile, oldLines: DiffLine[], newLines: DiffLine[]): string {
+  // TODO: Make this private after improving tests
-    let oldLinesHtml = "";
+  processChangedLines(isCombined: boolean, oldLines: DiffLine[], newLines: DiffLine[]): FileHtml {
-    let newLinesHtml = "";
+    const fileHtml = {
+      right: "",
+      left: ""
+    };
 
-    const common = Math.min(oldLines.length, newLines.length);
+    const maxLinesNumber = Math.max(oldLines.length, newLines.length);
+    for (let i = 0; i < maxLinesNumber; i++) {
+      const oldLine = oldLines[i];
+      const newLine = newLines[i];
 
-    let oldLine, newLine;
+      const diff =
-    for (let j = 0; j < common; j++) {
+        oldLine !== undefined && newLine !== undefined
-      oldLine = oldLines[j];
+          ? renderUtils.diffHighlight(oldLine.content, newLine.content, isCombined, this.config)
-      newLine = newLines[j];
+          : undefined;
 
-      const diff = renderUtils.diffHighlight(oldLine.content, newLine.content, file.isCombined, this.config);
+      const preparedOldLine =
+        oldLine !== undefined && oldLine.oldNumber !== undefined
+          ? {
+              ...(diff !== undefined
+                ? {
+                    prefix: diff.oldLine.prefix,
+                    content: diff.oldLine.content,
+                    type: renderUtils.CSSLineClass.DELETE_CHANGES
+                  }
+                : {
+                    ...renderUtils.deconstructLine(oldLine.content, isCombined),
+                    type: renderUtils.toCSSClass(oldLine.type)
+                  }),
+              oldNumber: oldLine.oldNumber,
+              newNumber: oldLine.newNumber
+            }
+          : undefined;
 
-      oldLinesHtml += this.makeLineHtml(
+      const preparedNewLine =
-        renderUtils.CSSLineClass.DELETE_CHANGES,
+        newLine !== undefined && newLine.newNumber !== undefined
-        diff.oldLine.prefix,
+          ? {
-        diff.oldLine.content,
+              ...(diff !== undefined
-        oldLine.oldNumber,
+                ? {
-        oldLine.newNumber
+                    prefix: diff.newLine.prefix,
-      );
+                    content: diff.newLine.content,
-      newLinesHtml += this.makeLineHtml(
+                    type: renderUtils.CSSLineClass.INSERT_CHANGES
-        renderUtils.CSSLineClass.INSERT_CHANGES,
+                  }
-        diff.newLine.prefix,
+                : {
-        diff.newLine.content,
+                    ...renderUtils.deconstructLine(newLine.content, isCombined),
-        newLine.oldNumber,
+                    type: renderUtils.toCSSClass(newLine.type)
-        newLine.newNumber
+                  }),
-      );
+              oldNumber: newLine.oldNumber,
+              newNumber: newLine.newNumber
+            }
+          : undefined;
+
+      const { left, right } = this.generateLineHtml(preparedOldLine, preparedNewLine);
+      fileHtml.left += left;
+      fileHtml.right += right;
     }
 
-    const remainingLines = oldLines
+    return fileHtml;
-      .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(
+  generateLineHtml(oldLine?: DiffPreparedLine, newLine?: DiffPreparedLine): FileHtml {
-    type: renderUtils.CSSLineClass,
+    return {
-    prefix: string,
+      left: this.generateSingleLineHtml(oldLine),
-    content: string,
+      right: this.generateSingleLineHtml(newLine)
-    oldNumber?: number,
+    };
-    newNumber?: number
+  }
-  ): string {
+
+  generateSingleLineHtml(line?: DiffPreparedLine): string {
+    if (line === undefined) return "";
+
     const lineNumberHtml = this.hoganUtils.render(baseTemplatesPath, "numbers", {
-      oldNumber: oldNumber || "",
+      oldNumber: line.oldNumber || "",
-      newNumber: newNumber || ""
+      newNumber: line.newNumber || ""
     });
 
     return this.hoganUtils.render(genericTemplatesPath, "line", {
-      type: type,
+      type: line.type,
       lineClass: "d2h-code-linenumber",
       contentClass: "d2h-code-line",
-      prefix: prefix === " " ? "&nbsp;" : prefix,
+      prefix: line.prefix === " " ? "&nbsp;" : line.prefix,
-      content: content,
+      content: line.content,
       lineNumber: lineNumberHtml
     });
   }
 }
+
+type DiffLineGroups = [
+  (DiffLineContext & DiffLineContent)[],
+  (DiffLineDeleted & DiffLineContent)[],
+  (DiffLineInserted & DiffLineContent)[]
+][];
+
+type DiffPreparedLine = {
+  type: renderUtils.CSSLineClass;
+  prefix: string;
+  content: string;
+  oldNumber?: number;
+  newNumber?: number;
+};
+
+type FileHtml = {
+  left: string;
+  right: string;
+};

src/side-by-side-renderer.ts

@@ -106,14 +106,14 @@ export default class SideBySideRenderer {
         this.applyLineGroupping(block).forEach(([contextLines, oldLines, newLines]) => {
           if (oldLines.length && newLines.length && !contextLines.length) {
             this.applyRematchMatching(oldLines, newLines, matcher).map(([oldLines, newLines]) => {
-              const { left, right } = this.applyLineDiff(file, oldLines, newLines);
+              const { left, right } = this.processChangedLines(file.isCombined, oldLines, newLines);
               fileHtml.left += left;
               fileHtml.right += right;
             });
           } else if (contextLines.length) {
             contextLines.forEach(line => {
               const { prefix, content } = renderUtils.deconstructLine(line.content, file.isCombined);
-              const { left, right } = this.generateSingleLineHtml(
+              const { left, right } = this.generateLineHtml(
                 {
                   type: renderUtils.CSSLineClass.CONTEXT,
                   prefix: prefix,
@@ -131,7 +131,7 @@ export default class SideBySideRenderer {
               fileHtml.right += right;
             });
           } else if (oldLines.length || newLines.length) {
-            const { left, right } = this.processLines(file.isCombined, oldLines, newLines);
+            const { left, right } = this.processChangedLines(file.isCombined, oldLines, newLines);
             fileHtml.left += left;
             fileHtml.right += right;
           } else {
@@ -207,54 +207,6 @@ export default class SideBySideRenderer {
     return matches;
   }
 
-  applyLineDiff(file: DiffFile, oldLines: DiffLine[], newLines: DiffLine[]): FileHtml {
-    const fileHtml = {
-      left: "",
-      right: ""
-    };
-
-    const common = Math.min(oldLines.length, newLines.length);
-
-    // Matched lines
-    for (let j = 0; j < common; j++) {
-      const oldLine = oldLines[j];
-      const newLine = newLines[j];
-
-      const diff = renderUtils.diffHighlight(oldLine.content, newLine.content, file.isCombined, this.config);
-
-      const preparedOldLine =
-        oldLine.oldNumber !== undefined
-          ? {
-              type: renderUtils.CSSLineClass.DELETE_CHANGES,
-              prefix: diff.oldLine.prefix,
-              content: diff.oldLine.content,
-              number: oldLine.oldNumber
-            }
-          : undefined;
-
-      const preparedNewLine =
-        newLine.newNumber !== undefined
-          ? {
-              type: renderUtils.CSSLineClass.INSERT_CHANGES,
-              prefix: diff.newLine.prefix,
-              content: diff.newLine.content,
-              number: newLine.newNumber
-            }
-          : undefined;
-
-      const { left, right } = this.generateSingleLineHtml(preparedOldLine, preparedNewLine);
-      fileHtml.left += left;
-      fileHtml.right += right;
-    }
-
-    // Remaining lines
-    const { left, right } = this.processLines(file.isCombined, oldLines.slice(common), newLines.slice(common));
-    fileHtml.left += left;
-    fileHtml.right += right;
-
-    return fileHtml;
-  }
-
   // TODO: Make this private after improving tests
   makeHeaderHtml(blockHeader: string): string {
     return this.hoganUtils.render(genericTemplatesPath, "block-header", {
@@ -266,7 +218,7 @@ export default class SideBySideRenderer {
   }
 
   // TODO: Make this private after improving tests
-  processLines(isCombined: boolean, oldLines: DiffLine[], newLines: DiffLine[]): FileHtml {
+  processChangedLines(isCombined: boolean, oldLines: DiffLine[], newLines: DiffLine[]): FileHtml {
     const fileHtml = {
       right: "",
       left: ""
@@ -277,11 +229,24 @@ export default class SideBySideRenderer {
       const oldLine = oldLines[i];
       const newLine = newLines[i];
 
+      const diff =
+        oldLine !== undefined && newLine !== undefined
+          ? renderUtils.diffHighlight(oldLine.content, newLine.content, isCombined, this.config)
+          : undefined;
+
       const preparedOldLine =
         oldLine !== undefined && oldLine.oldNumber !== undefined
           ? {
+              ...(diff !== undefined
+                ? {
+                    prefix: diff.oldLine.prefix,
+                    content: diff.oldLine.content,
+                    type: renderUtils.CSSLineClass.DELETE_CHANGES
+                  }
+                : {
                     ...renderUtils.deconstructLine(oldLine.content, isCombined),
-              type: renderUtils.toCSSClass(oldLine.type),
+                    type: renderUtils.toCSSClass(oldLine.type)
+                  }),
               number: oldLine.oldNumber
             }
           : undefined;
@@ -289,13 +254,21 @@ export default class SideBySideRenderer {
       const preparedNewLine =
         newLine !== undefined && newLine.newNumber !== undefined
           ? {
+              ...(diff !== undefined
+                ? {
+                    prefix: diff.newLine.prefix,
+                    content: diff.newLine.content,
+                    type: renderUtils.CSSLineClass.INSERT_CHANGES
+                  }
+                : {
                     ...renderUtils.deconstructLine(newLine.content, isCombined),
-              type: renderUtils.toCSSClass(newLine.type),
+                    type: renderUtils.toCSSClass(newLine.type)
+                  }),
               number: newLine.newNumber
             }
           : undefined;
 
-      const { left, right } = this.generateSingleLineHtml(preparedOldLine, preparedNewLine);
+      const { left, right } = this.generateLineHtml(preparedOldLine, preparedNewLine);
       fileHtml.left += left;
       fileHtml.right += right;
     }
@@ -304,28 +277,25 @@ export default class SideBySideRenderer {
   }
 
   // TODO: Make this private after improving tests
-  generateSingleLineHtml(oldLine?: DiffPreparedLine, newLine?: DiffPreparedLine): FileHtml {
+  generateLineHtml(oldLine?: DiffPreparedLine, newLine?: DiffPreparedLine): FileHtml {
+    return {
+      left: this.generateSingleHtml(oldLine),
+      right: this.generateSingleHtml(newLine)
+    };
+  }
+
+  generateSingleHtml(line?: DiffPreparedLine): string {
     const lineClass = "d2h-code-side-linenumber";
     const contentClass = "d2h-code-side-line";
 
-    return {
+    return this.hoganUtils.render(genericTemplatesPath, "line", {
-      left: this.hoganUtils.render(genericTemplatesPath, "line", {
+      type: line?.type || `${renderUtils.CSSLineClass.CONTEXT} d2h-emptyplaceholder`,
-        type: oldLine?.type || `${renderUtils.CSSLineClass.CONTEXT} d2h-emptyplaceholder`,
+      lineClass: line !== undefined ? lineClass : `${lineClass} d2h-code-side-emptyplaceholder`,
-        lineClass: oldLine !== undefined ? lineClass : `${lineClass} d2h-code-side-emptyplaceholder`,
+      contentClass: line !== undefined ? contentClass : `${contentClass} d2h-code-side-emptyplaceholder`,
-        contentClass: oldLine !== undefined ? contentClass : `${contentClass} d2h-code-side-emptyplaceholder`,
+      prefix: line?.prefix === " " ? "&nbsp;" : line?.prefix || "&nbsp;",
-        prefix: oldLine?.prefix === " " ? "&nbsp;" : oldLine?.prefix || "&nbsp;",
+      content: line?.content || "&nbsp;",
-        content: oldLine?.content || "&nbsp;",
+      lineNumber: line?.number
-        lineNumber: oldLine?.number
+    });
-      }),
-      right: this.hoganUtils.render(genericTemplatesPath, "line", {
-        type: newLine?.type || `${renderUtils.CSSLineClass.CONTEXT} d2h-emptyplaceholder`,
-        lineClass: newLine !== undefined ? lineClass : `${lineClass} d2h-code-side-emptyplaceholder`,
-        contentClass: newLine !== undefined ? contentClass : `${contentClass} d2h-code-side-emptyplaceholder`,
-        prefix: newLine?.prefix === " " ? "&nbsp;" : newLine?.prefix || "&nbsp;",
-        content: newLine?.content || "&nbsp;",
-        lineNumber: newLine?.number
-      })
-    };
   }
 }
 
@@ -343,6 +313,6 @@ type DiffPreparedLine = {
 };
 
 type FileHtml = {
-  right: string;
   left: string;
+  right: string;
 };