From 8f1208eb01f9156abae695e83b587d9d7289ed89 Mon Sep 17 00:00:00 2001 From: Rodrigo Fernandes Date: Tue, 26 Nov 2019 23:57:47 +0000 Subject: [PATCH] refactor: Unify line-by-line and side-by-side --- src/__tests__/line-by-line-tests.ts | 40 ++++- src/__tests__/side-by-side-printer-tests.ts | 64 +++---- src/line-by-line-renderer.ts | 188 +++++++++++++------- src/side-by-side-renderer.ts | 124 +++++-------- 4 files changed, 237 insertions(+), 179 deletions(-) diff --git a/src/__tests__/line-by-line-tests.ts b/src/__tests__/line-by-line-tests.ts index c641eb4..8d0851f 100644 --- a/src/__tests__/line-by-line-tests.ts +++ b/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 = "\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 = "\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 = "\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 = "\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 = "\n" + diff --git a/src/__tests__/side-by-side-printer-tests.ts b/src/__tests__/side-by-side-printer-tests.ts index 427b422..3237fa9 100644 --- a/src/__tests__/side-by-side-printer-tests.ts +++ b/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", () => { " \n" + ' \n' + '
\n' + + '  \n' + + '  \n' + "
\n" + " \n" + "", @@ -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", () => { " \n" + ' \n' + '
\n' + + '  \n' + + '  \n' + "
\n" + " \n" + "" @@ -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 expectedLeft = - "\n" + - ' \n' + - " 1\n" + - " \n" + - ' \n' + - '
\n' + - ' -\n' + - ' test\n' + - "
\n" + - " \n" + - ""; + const html = sideBySideRenderer.processChangedLines(false, oldLines, newLines); + const expected = { + left: + "\n" + + ' \n' + + " 1\n" + + " \n" + + ' \n' + + '
\n' + + ' -\n' + + ' test\n' + + "
\n" + + " \n" + + "", + right: + "\n" + + ' \n' + + " 1\n" + + " \n" + + ' \n' + + '
\n' + + ' +\n' + + ' test1r\n' + + "
\n" + + " \n" + + "" + }; - const expectedRight = - "\n" + - ' \n' + - " 1\n" + - " \n" + - ' \n' + - '
\n' + - ' +\n' + - ' test1r\n' + - "
\n" + - " \n" + - ""; - - expect(html.left).toEqual(expectedLeft); - expect(html.right).toEqual(expectedRight); + expect(html).toEqual(expected); }); }); }); diff --git a/src/line-by-line-renderer.ts b/src/line-by-line-renderer.ts index 8a011b4..8e609de 100644 --- a/src/line-by-line-renderer.ts +++ b/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) - .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, - line.oldNumber, - line.newNumber - ); - }) - .join(""); + this.applyRematchMatching(oldLines, newLines, matcher).map(([oldLines, newLines]) => { + const { left, right } = this.processChangedLines(file.isCombined, oldLines, newLines); + lines += left; + lines += right; + }); + } else if (contextLines.length) { + contextLines.forEach(line => { + const { prefix, content } = renderUtils.deconstructLine(line.content, file.isCombined); + lines += this.generateSingleLineHtml({ + type: renderUtils.CSSLineClass.CONTEXT, + prefix: prefix, + content: content, + oldNumber: line.oldNumber, + newNumber: line.newNumber + }); + }); + } 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[][][] { - const blockLinesGroups: DiffLine[][][] = []; + applyLineGroupping(block: DiffBlock): DiffLineGroups { + const blockLinesGroups: DiffLineGroups = []; - let oldLines: DiffLine[] = []; - let newLines: DiffLine[] = []; + let oldLines: (DiffLineDeleted & DiffLineContent)[] = []; + 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 { - let oldLinesHtml = ""; - let newLinesHtml = ""; + // TODO: Make this private after improving tests + processChangedLines(isCombined: boolean, oldLines: DiffLine[], newLines: DiffLine[]): FileHtml { + 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; - for (let j = 0; j < common; j++) { - oldLine = oldLines[j]; - newLine = newLines[j]; + const diff = + oldLine !== undefined && newLine !== undefined + ? renderUtils.diffHighlight(oldLine.content, newLine.content, isCombined, this.config) + : 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( - renderUtils.CSSLineClass.DELETE_CHANGES, - diff.oldLine.prefix, - diff.oldLine.content, - oldLine.oldNumber, - oldLine.newNumber - ); - newLinesHtml += this.makeLineHtml( - renderUtils.CSSLineClass.INSERT_CHANGES, - diff.newLine.prefix, - diff.newLine.content, - newLine.oldNumber, - newLine.newNumber - ); + 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) + }), + oldNumber: newLine.oldNumber, + newNumber: newLine.newNumber + } + : undefined; + + const { left, right } = this.generateLineHtml(preparedOldLine, preparedNewLine); + fileHtml.left += left; + fileHtml.right += right; } - 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; + return fileHtml; } // TODO: Make this private after improving tests - makeLineHtml( - type: renderUtils.CSSLineClass, - prefix: string, - content: string, - oldNumber?: number, - newNumber?: number - ): string { + generateLineHtml(oldLine?: DiffPreparedLine, newLine?: DiffPreparedLine): FileHtml { + return { + left: this.generateSingleLineHtml(oldLine), + right: this.generateSingleLineHtml(newLine) + }; + } + + generateSingleLineHtml(line?: DiffPreparedLine): string { + if (line === undefined) return ""; + const lineNumberHtml = this.hoganUtils.render(baseTemplatesPath, "numbers", { - oldNumber: oldNumber || "", - newNumber: newNumber || "" + oldNumber: line.oldNumber || "", + newNumber: line.newNumber || "" }); return this.hoganUtils.render(genericTemplatesPath, "line", { - type: type, + type: line.type, lineClass: "d2h-code-linenumber", contentClass: "d2h-code-line", - prefix: prefix === " " ? " " : prefix, - content: content, + prefix: line.prefix === " " ? " " : line.prefix, + 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; +}; diff --git a/src/side-by-side-renderer.ts b/src/side-by-side-renderer.ts index e15286a..d7f714b 100644 --- a/src/side-by-side-renderer.ts +++ b/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 ? { - ...renderUtils.deconstructLine(oldLine.content, isCombined), - type: renderUtils.toCSSClass(oldLine.type), + ...(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) + }), number: oldLine.oldNumber } : undefined; @@ -289,13 +254,21 @@ export default class SideBySideRenderer { const preparedNewLine = newLine !== undefined && newLine.newNumber !== undefined ? { - ...renderUtils.deconstructLine(newLine.content, isCombined), - type: renderUtils.toCSSClass(newLine.type), + ...(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) + }), 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 { - left: this.hoganUtils.render(genericTemplatesPath, "line", { - type: oldLine?.type || `${renderUtils.CSSLineClass.CONTEXT} d2h-emptyplaceholder`, - lineClass: oldLine !== undefined ? lineClass : `${lineClass} d2h-code-side-emptyplaceholder`, - contentClass: oldLine !== undefined ? contentClass : `${contentClass} d2h-code-side-emptyplaceholder`, - prefix: oldLine?.prefix === " " ? " " : oldLine?.prefix || " ", - content: oldLine?.content || " ", - 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 === " " ? " " : newLine?.prefix || " ", - content: newLine?.content || " ", - lineNumber: newLine?.number - }) - }; + return this.hoganUtils.render(genericTemplatesPath, "line", { + type: line?.type || `${renderUtils.CSSLineClass.CONTEXT} d2h-emptyplaceholder`, + lineClass: line !== undefined ? lineClass : `${lineClass} d2h-code-side-emptyplaceholder`, + contentClass: line !== undefined ? contentClass : `${contentClass} d2h-code-side-emptyplaceholder`, + prefix: line?.prefix === " " ? " " : line?.prefix || " ", + content: line?.content || " ", + lineNumber: line?.number + }); } } @@ -343,6 +313,6 @@ type DiffPreparedLine = { }; type FileHtml = { - right: string; left: string; + right: string; };