From 5c35de28eb5efd7ce0fb4c0f4f4946cc83838291 Mon Sep 17 00:00:00 2001 From: Rodrigo Fernandes Date: Tue, 26 Nov 2019 19:02:25 +0000 Subject: [PATCH] refactor: Separate matching in side-by-side algorithm --- src/__tests__/diff2html-tests.ts | 12 +- src/__tests__/side-by-side-printer-tests.ts | 104 +++-- src/side-by-side-renderer.ts | 426 ++++++++++---------- src/types.ts | 4 +- 4 files changed, 293 insertions(+), 253 deletions(-) diff --git a/src/__tests__/diff2html-tests.ts b/src/__tests__/diff2html-tests.ts index 1466537..b097148 100644 --- a/src/__tests__/diff2html-tests.ts +++ b/src/__tests__/diff2html-tests.ts @@ -141,11 +141,11 @@ const htmlSideExample1 = '
@@ -1 +1 @@
\n' + " \n" + "\n" + - ' \n' + + ' \n' + " 1\n" + " \n" + - ' \n' + - '
\n' + + ' \n' + + '
\n' + ' -\n' + ' test\n' + "
\n" + @@ -165,11 +165,11 @@ const htmlSideExample1 = '
\n' + " \n" + "\n" + - ' \n' + + ' \n' + " 1\n" + " \n" + - ' \n' + - '
\n' + + ' \n' + + '
\n' + ' +\n' + ' test1\n' + "
\n" + diff --git a/src/__tests__/side-by-side-printer-tests.ts b/src/__tests__/side-by-side-printer-tests.ts index 9aaa1de..427b422 100644 --- a/src/__tests__/side-by-side-printer-tests.ts +++ b/src/__tests__/side-by-side-printer-tests.ts @@ -75,7 +75,7 @@ describe("SideBySideRenderer", () => { isCombined: false }; - const fileHtml = sideBySideRenderer.generateSideBySideFileHtml(file); + const fileHtml = sideBySideRenderer.generateFileHtml(file); const expectedLeft = "\n" + @@ -94,11 +94,11 @@ describe("SideBySideRenderer", () => { "
\n" + " \n" + "\n" + - ' \n' + + ' \n' + " 20\n" + " \n" + - ' \n' + - '
\n' + + ' \n' + + '
\n' + ' -\n' + ' removed\n' + "
\n" + @@ -133,11 +133,11 @@ describe("SideBySideRenderer", () => { "
\n" + " \n" + "\n" + - ' \n' + + ' \n' + " 20\n" + " \n" + - ' \n' + - '
\n' + + ' \n' + + '
\n' + ' +\n' + ' added\n' + "
\n" + @@ -163,38 +163,76 @@ describe("SideBySideRenderer", () => { it("should work for insertions", () => { const hoganUtils = new HoganJsUtils({}); const sideBySideRenderer = new SideBySideRenderer(hoganUtils, {}); - const fileHtml = sideBySideRenderer.generateSingleLineHtml(false, CSSLineClass.INSERTS, "test", 30, "+"); - const expected = - "\n" + - ' \n' + - " 30\n" + - " \n" + - ' \n' + - '
\n' + - ' +\n' + - ' test\n' + - "
\n" + - " \n" + - ""; + const fileHtml = sideBySideRenderer.generateSingleLineHtml(undefined, { + type: CSSLineClass.INSERTS, + prefix: "+", + content: "test", + number: 30 + }); + + const expected = { + left: + "\n" + + ' \n' + + " \n" + + " \n" + + ' \n' + + '
\n' + + "
\n" + + " \n" + + "", + right: + "\n" + + ' \n' + + " 30\n" + + " \n" + + ' \n' + + '
\n' + + ' +\n' + + ' test\n' + + "
\n" + + " \n" + + "" + }; expect(fileHtml).toEqual(expected); }); it("should work for deletions", () => { const hoganUtils = new HoganJsUtils({}); const sideBySideRenderer = new SideBySideRenderer(hoganUtils, {}); - const fileHtml = sideBySideRenderer.generateSingleLineHtml(false, CSSLineClass.DELETES, "test", 30, "-"); - const expected = - "\n" + - ' \n' + - " 30\n" + - " \n" + - ' \n' + - '
\n' + - ' -\n' + - ' test\n' + - "
\n" + - " \n" + - ""; + const fileHtml = sideBySideRenderer.generateSingleLineHtml( + { + type: CSSLineClass.DELETES, + prefix: "-", + content: "test", + number: 30 + }, + undefined + ); + const expected = { + left: + "\n" + + ' \n' + + " 30\n" + + " \n" + + ' \n' + + '
\n' + + ' -\n' + + ' test\n' + + "
\n" + + " \n" + + "", + right: + "\n" + + ' \n' + + " \n" + + " \n" + + ' \n' + + '
\n' + + "
\n" + + " \n" + + "" + }; expect(fileHtml).toEqual(expected); }); diff --git a/src/side-by-side-renderer.ts b/src/side-by-side-renderer.ts index 4f7ee63..e15286a 100644 --- a/src/side-by-side-renderer.ts +++ b/src/side-by-side-renderer.ts @@ -1,7 +1,16 @@ import HoganJsUtils from "./hoganjs-utils"; import * as Rematch from "./rematch"; import * as renderUtils from "./render-utils"; -import { DiffLine, LineType, DiffFile } from "./types"; +import { + DiffLine, + LineType, + DiffFile, + DiffBlock, + DiffLineContext, + DiffLineDeleted, + DiffLineInserted, + DiffLineContent +} from "./types"; export interface SideBySideRendererConfig extends renderUtils.RenderConfig { renderNothingWhenEmpty?: boolean; @@ -35,7 +44,7 @@ export default class SideBySideRenderer { .map(file => { let diffs; if (file.blocks.length) { - diffs = this.generateSideBySideFileHtml(file); + diffs = this.generateFileHtml(file); } else { diffs = this.generateEmptyDiff(); } @@ -82,147 +91,172 @@ export default class SideBySideRenderer { }; } - // TODO: Make this private after improving tests - generateSideBySideFileHtml(file: DiffFile): FileHtml { + generateFileHtml(file: DiffFile): FileHtml { const matcher = Rematch.newMatcherFn( Rematch.newDistanceFn((e: DiffLine) => renderUtils.deconstructLine(e.content, file.isCombined).content) ); - const fileHtml = { - right: "", - left: "" - }; + return file.blocks + .map(block => { + const fileHtml = { + left: this.makeHeaderHtml(block.header), + right: this.makeHeaderHtml("") + }; - file.blocks.forEach(block => { - fileHtml.left += this.makeSideHtml(block.header); - fileHtml.right += this.makeSideHtml(""); - - let oldLines: DiffLine[] = []; - let newLines: DiffLine[] = []; - - const processChangeBlock = (): void => { - let matches; - let insertType: renderUtils.CSSLineClass; - let deleteType: renderUtils.CSSLineClass; - - const comparisons = oldLines.length * newLines.length; - - const maxLineSizeInBlock = Math.max.apply( - null, - oldLines.concat(newLines).map(elem => elem.content.length) - ); - - const doMatching = - comparisons < this.config.matchingMaxComparisons && - maxLineSizeInBlock < this.config.maxLineSizeInBlockForComparison && - (this.config.matching === "lines" || this.config.matching === "words"); - - if (doMatching) { - matches = matcher(oldLines, newLines); - insertType = renderUtils.CSSLineClass.INSERT_CHANGES; - deleteType = renderUtils.CSSLineClass.DELETE_CHANGES; - } else { - matches = [[oldLines, newLines]]; - insertType = renderUtils.CSSLineClass.INSERTS; - deleteType = renderUtils.CSSLineClass.DELETES; - } - - matches.forEach(match => { - oldLines = match[0]; - newLines = match[1]; - - const common = Math.min(oldLines.length, newLines.length); - const max = Math.max(oldLines.length, newLines.length); - - 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); - - fileHtml.left += this.generateSingleLineHtml( - file.isCombined, - deleteType, - diff.oldLine.content, - oldLine.oldNumber, - diff.oldLine.prefix - ); - fileHtml.right += this.generateSingleLineHtml( - file.isCombined, - insertType, - diff.newLine.content, - newLine.newNumber, - diff.newLine.prefix - ); - } - - if (max > common) { - const oldSlice = oldLines.slice(common); - const newSlice = newLines.slice(common); - - const tmpHtml = this.processLines(file.isCombined, oldSlice, newSlice); - fileHtml.left += tmpHtml.left; - fileHtml.right += tmpHtml.right; + 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); + 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( + { + type: renderUtils.CSSLineClass.CONTEXT, + prefix: prefix, + content: content, + number: line.oldNumber + }, + { + type: renderUtils.CSSLineClass.CONTEXT, + prefix: prefix, + content: content, + number: line.newNumber + } + ); + fileHtml.left += left; + fileHtml.right += right; + }); + } else if (oldLines.length || newLines.length) { + const { left, right } = this.processLines(file.isCombined, oldLines, newLines); + fileHtml.left += left; + fileHtml.right += right; + } else { + console.error("Unknown state reached while processing groups of lines", contextLines, oldLines, newLines); } }); + return fileHtml; + }) + .reduce( + (accomulated, html) => { + return { left: accomulated.left + html.left, right: accomulated.right + html.right }; + }, + { left: "", right: "" } + ); + } + + applyLineGroupping(block: DiffBlock): DiffLineGroups { + const blockLinesGroups: DiffLineGroups = []; + + let oldLines: (DiffLineDeleted & DiffLineContent)[] = []; + let newLines: (DiffLineInserted & DiffLineContent)[] = []; + + 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 = []; - }; - - 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)) - ) { - processChangeBlock(); - } - - if (diffLine.type === LineType.CONTEXT) { - fileHtml.left += this.generateSingleLineHtml( - file.isCombined, - renderUtils.toCSSClass(diffLine.type), - content, - diffLine.oldNumber, - prefix - ); - fileHtml.right += this.generateSingleLineHtml( - file.isCombined, - renderUtils.toCSSClass(diffLine.type), - content, - diffLine.newNumber, - prefix - ); - } else if (diffLine.type === LineType.INSERT && !oldLines.length) { - fileHtml.left += this.generateSingleLineHtml(file.isCombined, renderUtils.CSSLineClass.CONTEXT, ""); - fileHtml.right += this.generateSingleLineHtml( - file.isCombined, - renderUtils.toCSSClass(diffLine.type), - content, - diffLine.newNumber, - prefix - ); - } else if (diffLine.type === LineType.DELETE) { - oldLines.push(diffLine); - } else if (diffLine.type === LineType.INSERT && Boolean(oldLines.length)) { - newLines.push(diffLine); - } else { - console.error("unknown state in html side-by-side generator"); - processChangeBlock(); - } } - processChangeBlock(); - }); + 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[][][] { + const comparisons = oldLines.length * newLines.length; + const maxLineSizeInBlock = Math.max.apply( + null, + [0].concat(oldLines.concat(newLines).map(elem => elem.content.length)) + ); + const doMatching = + comparisons < this.config.matchingMaxComparisons && + maxLineSizeInBlock < this.config.maxLineSizeInBlockForComparison && + (this.config.matching === "lines" || this.config.matching === "words"); + + const matches = doMatching ? matcher(oldLines, newLines) : [[oldLines, newLines]]; + + 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 - makeSideHtml(blockHeader: string): string { + makeHeaderHtml(blockHeader: string): string { return this.hoganUtils.render(genericTemplatesPath, "block-header", { CSSLineClass: renderUtils.CSSLineClass, blockHeader: blockHeader, @@ -243,105 +277,71 @@ export default class SideBySideRenderer { const oldLine = oldLines[i]; const newLine = newLines[i]; - let oldContent; - let newContent; - let oldPrefix; - let newPrefix; + const preparedOldLine = + oldLine !== undefined && oldLine.oldNumber !== undefined + ? { + ...renderUtils.deconstructLine(oldLine.content, isCombined), + type: renderUtils.toCSSClass(oldLine.type), + number: oldLine.oldNumber + } + : undefined; - if (oldLine) { - const { prefix, content } = renderUtils.deconstructLine(oldLine.content, isCombined); - oldContent = content; - oldPrefix = prefix; - } else { - oldContent = ""; - oldPrefix = ""; - } + const preparedNewLine = + newLine !== undefined && newLine.newNumber !== undefined + ? { + ...renderUtils.deconstructLine(newLine.content, isCombined), + type: renderUtils.toCSSClass(newLine.type), + number: newLine.newNumber + } + : undefined; - if (newLine) { - const { prefix, content } = renderUtils.deconstructLine(newLine.content, isCombined); - newContent = content; - newPrefix = prefix; - } else { - newContent = ""; - oldPrefix = ""; - } - - if (oldLine && newLine) { - fileHtml.left += this.generateSingleLineHtml( - isCombined, - renderUtils.toCSSClass(oldLine.type), - oldContent, - oldLine.oldNumber, - oldPrefix - ); - fileHtml.right += this.generateSingleLineHtml( - isCombined, - renderUtils.toCSSClass(newLine.type), - newContent, - newLine.newNumber, - newPrefix - ); - } else if (oldLine) { - fileHtml.left += this.generateSingleLineHtml( - isCombined, - renderUtils.toCSSClass(oldLine.type), - oldContent, - oldLine.oldNumber, - oldPrefix - ); - fileHtml.right += this.generateSingleLineHtml(isCombined, renderUtils.CSSLineClass.CONTEXT, ""); - } else if (newLine) { - fileHtml.left += this.generateSingleLineHtml(isCombined, renderUtils.CSSLineClass.CONTEXT, ""); - fileHtml.right += this.generateSingleLineHtml( - isCombined, - renderUtils.toCSSClass(newLine.type), - newContent, - newLine.newNumber, - newPrefix - ); - } + const { left, right } = this.generateSingleLineHtml(preparedOldLine, preparedNewLine); + fileHtml.left += left; + fileHtml.right += right; } return fileHtml; } // TODO: Make this private after improving tests - generateSingleLineHtml( - isCombined: boolean, - type: renderUtils.CSSLineClass, - content: string, - number?: number, - possiblePrefix?: string - ): string { - let lineWithoutPrefix = content; - let prefix = possiblePrefix; - let lineClass = "d2h-code-side-linenumber"; - let contentClass = "d2h-code-side-line"; - let preparedType: string = type; + generateSingleLineHtml(oldLine?: DiffPreparedLine, newLine?: DiffPreparedLine): FileHtml { + const lineClass = "d2h-code-side-linenumber"; + const contentClass = "d2h-code-side-line"; - if (!number && !content) { - lineClass += " d2h-code-side-emptyplaceholder"; - contentClass += " d2h-code-side-emptyplaceholder"; - preparedType += " d2h-emptyplaceholder"; - prefix = " "; - lineWithoutPrefix = " "; - } else if (!prefix) { - const lineWithPrefix = renderUtils.deconstructLine(content, isCombined); - prefix = lineWithPrefix.prefix; - lineWithoutPrefix = lineWithPrefix.content; - } - - return this.hoganUtils.render(genericTemplatesPath, "line", { - type: preparedType, - lineClass: lineClass, - contentClass: contentClass, - prefix: prefix === " " ? " " : prefix, - content: lineWithoutPrefix, - lineNumber: number - }); + 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 + }) + }; } } +type DiffLineGroups = [ + (DiffLineContext & DiffLineContent)[], + (DiffLineDeleted & DiffLineContent)[], + (DiffLineInserted & DiffLineContent)[] +][]; + +type DiffPreparedLine = { + type: renderUtils.CSSLineClass; + prefix: string; + content: string; + number: number; +}; + type FileHtml = { right: string; left: string; diff --git a/src/types.ts b/src/types.ts index 6128f6e..f080f16 100644 --- a/src/types.ts +++ b/src/types.ts @@ -27,10 +27,12 @@ export interface DiffLineContext { newNumber: number; } -export type DiffLine = (DiffLineDeleted | DiffLineInserted | DiffLineContext) & { +export type DiffLineContent = { content: string; }; +export type DiffLine = (DiffLineDeleted | DiffLineInserted | DiffLineContext) & DiffLineContent; + export interface DiffBlock { oldStartLine: number; oldStartLine2?: number;