From d531e6926a0aa274e339eaefb9d135690a800d4e Mon Sep 17 00:00:00 2001 From: Raymond Ng Date: Fri, 13 Nov 2020 14:47:04 -0800 Subject: [PATCH 1/4] remove cyclical dependency between printDiffs, diffLines and joinAlignedDiffs --- packages/jest-diff/src/diffLines.ts | 88 +++++++++- packages/jest-diff/src/joinAlignedDiffs.ts | 88 +++++++++- packages/jest-diff/src/printDiffs.ts | 186 +-------------------- 3 files changed, 171 insertions(+), 191 deletions(-) diff --git a/packages/jest-diff/src/diffLines.ts b/packages/jest-diff/src/diffLines.ts index 89e71f866b49..6997250adcdd 100644 --- a/packages/jest-diff/src/diffLines.ts +++ b/packages/jest-diff/src/diffLines.ts @@ -7,13 +7,97 @@ import diff from 'diff-sequences'; import {DIFF_DELETE, DIFF_EQUAL, DIFF_INSERT, Diff} from './cleanupSemantic'; +import { + joinAlignedDiffsExpand, + joinAlignedDiffsNoExpand, +} from './joinAlignedDiffs'; import {normalizeDiffOptions} from './normalizeDiffOptions'; -import {printDiffLines} from './printDiffs'; -import type {DiffOptions} from './types'; +import type {DiffOptions, DiffOptionsNormalized} from './types'; const isEmptyString = (lines: Array) => lines.length === 1 && lines[0].length === 0; +type ChangeCounts = { + a: number; + b: number; +}; + +const countChanges = (diffs: Array): ChangeCounts => { + let a = 0; + let b = 0; + + diffs.forEach(diff => { + switch (diff[0]) { + case DIFF_DELETE: + a += 1; + break; + + case DIFF_INSERT: + b += 1; + break; + } + }); + + return {a, b}; +}; + +const printAnnotation = ( + { + aAnnotation, + aColor, + aIndicator, + bAnnotation, + bColor, + bIndicator, + includeChangeCounts, + omitAnnotationLines, + }: DiffOptionsNormalized, + changeCounts: ChangeCounts, +): string => { + if (omitAnnotationLines) { + return ''; + } + + let aRest = ''; + let bRest = ''; + + if (includeChangeCounts) { + const aCount = String(changeCounts.a); + const bCount = String(changeCounts.b); + + // Padding right aligns the ends of the annotations. + const baAnnotationLengthDiff = bAnnotation.length - aAnnotation.length; + const aAnnotationPadding = ' '.repeat(Math.max(0, baAnnotationLengthDiff)); + const bAnnotationPadding = ' '.repeat(Math.max(0, -baAnnotationLengthDiff)); + + // Padding left aligns the ends of the counts. + const baCountLengthDiff = bCount.length - aCount.length; + const aCountPadding = ' '.repeat(Math.max(0, baCountLengthDiff)); + const bCountPadding = ' '.repeat(Math.max(0, -baCountLengthDiff)); + + aRest = + aAnnotationPadding + ' ' + aIndicator + ' ' + aCountPadding + aCount; + bRest = + bAnnotationPadding + ' ' + bIndicator + ' ' + bCountPadding + bCount; + } + + return ( + aColor(aIndicator + ' ' + aAnnotation + aRest) + + '\n' + + bColor(bIndicator + ' ' + bAnnotation + bRest) + + '\n\n' + ); +}; + +export const printDiffLines = ( + diffs: Array, + options: DiffOptionsNormalized, +): string => + printAnnotation(options, countChanges(diffs)) + + (options.expand + ? joinAlignedDiffsExpand(diffs, options) + : joinAlignedDiffsNoExpand(diffs, options)); + // Compare two arrays of strings line-by-line. Format as comparison lines. export const diffLinesUnified = ( aLines: Array, diff --git a/packages/jest-diff/src/joinAlignedDiffs.ts b/packages/jest-diff/src/joinAlignedDiffs.ts index 3b3a079eaa6a..450241bc1adf 100644 --- a/packages/jest-diff/src/joinAlignedDiffs.ts +++ b/packages/jest-diff/src/joinAlignedDiffs.ts @@ -6,13 +6,87 @@ */ import {DIFF_DELETE, DIFF_EQUAL, DIFF_INSERT, Diff} from './cleanupSemantic'; -import { - createPatchMark, - printCommonLine, - printDeleteLine, - printInsertLine, -} from './printDiffs'; -import type {DiffOptionsNormalized} from './types'; +import type {DiffOptionsColor, DiffOptionsNormalized} from './types'; + +const formatTrailingSpaces = ( + line: string, + trailingSpaceFormatter: DiffOptionsColor, +): string => line.replace(/\s+$/, match => trailingSpaceFormatter(match)); + +const printDiffLine = ( + line: string, + isFirstOrLast: boolean, + color: DiffOptionsColor, + indicator: string, + trailingSpaceFormatter: DiffOptionsColor, + emptyFirstOrLastLinePlaceholder: string, +): string => + line.length !== 0 + ? color( + indicator + ' ' + formatTrailingSpaces(line, trailingSpaceFormatter), + ) + : indicator !== ' ' + ? color(indicator) + : isFirstOrLast && emptyFirstOrLastLinePlaceholder.length !== 0 + ? color(indicator + ' ' + emptyFirstOrLastLinePlaceholder) + : ''; + +const printDeleteLine = ( + line: string, + isFirstOrLast: boolean, + { + aColor, + aIndicator, + changeLineTrailingSpaceColor, + emptyFirstOrLastLinePlaceholder, + }: DiffOptionsNormalized, +): string => + printDiffLine( + line, + isFirstOrLast, + aColor, + aIndicator, + changeLineTrailingSpaceColor, + emptyFirstOrLastLinePlaceholder, + ); + +const printInsertLine = ( + line: string, + isFirstOrLast: boolean, + { + bColor, + bIndicator, + changeLineTrailingSpaceColor, + emptyFirstOrLastLinePlaceholder, + }: DiffOptionsNormalized, +): string => + printDiffLine( + line, + isFirstOrLast, + bColor, + bIndicator, + changeLineTrailingSpaceColor, + emptyFirstOrLastLinePlaceholder, + ); + +const printCommonLine = ( + line: string, + isFirstOrLast: boolean, + { + commonColor, + commonIndicator, + commonLineTrailingSpaceColor, + emptyFirstOrLastLinePlaceholder, + }: DiffOptionsNormalized, +): string => + printDiffLine( + line, + isFirstOrLast, + commonColor, + commonIndicator, + commonLineTrailingSpaceColor, + emptyFirstOrLastLinePlaceholder, + ); // jest --no-expand // diff --git a/packages/jest-diff/src/printDiffs.ts b/packages/jest-diff/src/printDiffs.ts index 05f8619e2b93..aa6e48cf5fb8 100644 --- a/packages/jest-diff/src/printDiffs.ts +++ b/packages/jest-diff/src/printDiffs.ts @@ -5,111 +5,14 @@ * LICENSE file in the root directory of this source tree. */ -import { - DIFF_DELETE, - DIFF_EQUAL, - DIFF_INSERT, - Diff, - cleanupSemantic, -} from './cleanupSemantic'; -import {diffLinesUnified} from './diffLines'; +import {DIFF_EQUAL, Diff, cleanupSemantic} from './cleanupSemantic'; +import {diffLinesUnified, printDiffLines} from './diffLines'; import diffStrings from './diffStrings'; import getAlignedDiffs from './getAlignedDiffs'; -import { - joinAlignedDiffsExpand, - joinAlignedDiffsNoExpand, -} from './joinAlignedDiffs'; import {normalizeDiffOptions} from './normalizeDiffOptions'; -import type { - DiffOptions, - DiffOptionsColor, - DiffOptionsNormalized, -} from './types'; +import type {DiffOptions, DiffOptionsNormalized} from './types'; -const formatTrailingSpaces = ( - line: string, - trailingSpaceFormatter: DiffOptionsColor, -): string => line.replace(/\s+$/, match => trailingSpaceFormatter(match)); - -const printDiffLine = ( - line: string, - isFirstOrLast: boolean, - color: DiffOptionsColor, - indicator: string, - trailingSpaceFormatter: DiffOptionsColor, - emptyFirstOrLastLinePlaceholder: string, -): string => - line.length !== 0 - ? color( - indicator + ' ' + formatTrailingSpaces(line, trailingSpaceFormatter), - ) - : indicator !== ' ' - ? color(indicator) - : isFirstOrLast && emptyFirstOrLastLinePlaceholder.length !== 0 - ? color(indicator + ' ' + emptyFirstOrLastLinePlaceholder) - : ''; - -export const printDeleteLine = ( - line: string, - isFirstOrLast: boolean, - { - aColor, - aIndicator, - changeLineTrailingSpaceColor, - emptyFirstOrLastLinePlaceholder, - }: DiffOptionsNormalized, -): string => - printDiffLine( - line, - isFirstOrLast, - aColor, - aIndicator, - changeLineTrailingSpaceColor, - emptyFirstOrLastLinePlaceholder, - ); - -export const printInsertLine = ( - line: string, - isFirstOrLast: boolean, - { - bColor, - bIndicator, - changeLineTrailingSpaceColor, - emptyFirstOrLastLinePlaceholder, - }: DiffOptionsNormalized, -): string => - printDiffLine( - line, - isFirstOrLast, - bColor, - bIndicator, - changeLineTrailingSpaceColor, - emptyFirstOrLastLinePlaceholder, - ); - -export const printCommonLine = ( - line: string, - isFirstOrLast: boolean, - { - commonColor, - commonIndicator, - commonLineTrailingSpaceColor, - emptyFirstOrLastLinePlaceholder, - }: DiffOptionsNormalized, -): string => - printDiffLine( - line, - isFirstOrLast, - commonColor, - commonIndicator, - commonLineTrailingSpaceColor, - emptyFirstOrLastLinePlaceholder, - ); - -export const hasCommonDiff = ( - diffs: Array, - isMultiline: boolean, -): boolean => { +const hasCommonDiff = (diffs: Array, isMultiline: boolean): boolean => { if (isMultiline) { // Important: Ignore common newline that was appended to multiline strings! const iLast = diffs.length - 1; @@ -121,87 +24,6 @@ export const hasCommonDiff = ( return diffs.some(diff => diff[0] === DIFF_EQUAL); }; -export type ChangeCounts = { - a: number; - b: number; -}; - -export const countChanges = (diffs: Array): ChangeCounts => { - let a = 0; - let b = 0; - - diffs.forEach(diff => { - switch (diff[0]) { - case DIFF_DELETE: - a += 1; - break; - - case DIFF_INSERT: - b += 1; - break; - } - }); - - return {a, b}; -}; - -export const printAnnotation = ( - { - aAnnotation, - aColor, - aIndicator, - bAnnotation, - bColor, - bIndicator, - includeChangeCounts, - omitAnnotationLines, - }: DiffOptionsNormalized, - changeCounts: ChangeCounts, -): string => { - if (omitAnnotationLines) { - return ''; - } - - let aRest = ''; - let bRest = ''; - - if (includeChangeCounts) { - const aCount = String(changeCounts.a); - const bCount = String(changeCounts.b); - - // Padding right aligns the ends of the annotations. - const baAnnotationLengthDiff = bAnnotation.length - aAnnotation.length; - const aAnnotationPadding = ' '.repeat(Math.max(0, baAnnotationLengthDiff)); - const bAnnotationPadding = ' '.repeat(Math.max(0, -baAnnotationLengthDiff)); - - // Padding left aligns the ends of the counts. - const baCountLengthDiff = bCount.length - aCount.length; - const aCountPadding = ' '.repeat(Math.max(0, baCountLengthDiff)); - const bCountPadding = ' '.repeat(Math.max(0, -baCountLengthDiff)); - - aRest = - aAnnotationPadding + ' ' + aIndicator + ' ' + aCountPadding + aCount; - bRest = - bAnnotationPadding + ' ' + bIndicator + ' ' + bCountPadding + bCount; - } - - return ( - aColor(aIndicator + ' ' + aAnnotation + aRest) + - '\n' + - bColor(bIndicator + ' ' + bAnnotation + bRest) + - '\n\n' - ); -}; - -export const printDiffLines = ( - diffs: Array, - options: DiffOptionsNormalized, -): string => - printAnnotation(options, countChanges(diffs)) + - (options.expand - ? joinAlignedDiffsExpand(diffs, options) - : joinAlignedDiffsNoExpand(diffs, options)); - // In GNU diff format, indexes are one-based instead of zero-based. export const createPatchMark = ( aStart: number, From 54ea3027995cb16115e4bf566aba6eabb6e43930 Mon Sep 17 00:00:00 2001 From: Raymond Ng Date: Fri, 13 Nov 2020 14:50:25 -0800 Subject: [PATCH 2/4] move createPatchMark to joinAlignedDiffs --- packages/jest-diff/src/joinAlignedDiffs.ts | 12 ++++++++++++ packages/jest-diff/src/printDiffs.ts | 14 +------------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/packages/jest-diff/src/joinAlignedDiffs.ts b/packages/jest-diff/src/joinAlignedDiffs.ts index 450241bc1adf..4124853fcf93 100644 --- a/packages/jest-diff/src/joinAlignedDiffs.ts +++ b/packages/jest-diff/src/joinAlignedDiffs.ts @@ -88,6 +88,18 @@ const printCommonLine = ( emptyFirstOrLastLinePlaceholder, ); +// In GNU diff format, indexes are one-based instead of zero-based. +const createPatchMark = ( + aStart: number, + aEnd: number, + bStart: number, + bEnd: number, + {patchColor}: DiffOptionsNormalized, +): string => + patchColor( + `@@ -${aStart + 1},${aEnd - aStart} +${bStart + 1},${bEnd - bStart} @@`, + ); + // jest --no-expand // // Given array of aligned strings with inverse highlight formatting, diff --git a/packages/jest-diff/src/printDiffs.ts b/packages/jest-diff/src/printDiffs.ts index aa6e48cf5fb8..cde7b881fc27 100644 --- a/packages/jest-diff/src/printDiffs.ts +++ b/packages/jest-diff/src/printDiffs.ts @@ -10,7 +10,7 @@ import {diffLinesUnified, printDiffLines} from './diffLines'; import diffStrings from './diffStrings'; import getAlignedDiffs from './getAlignedDiffs'; import {normalizeDiffOptions} from './normalizeDiffOptions'; -import type {DiffOptions, DiffOptionsNormalized} from './types'; +import type {DiffOptions} from './types'; const hasCommonDiff = (diffs: Array, isMultiline: boolean): boolean => { if (isMultiline) { @@ -24,18 +24,6 @@ const hasCommonDiff = (diffs: Array, isMultiline: boolean): boolean => { return diffs.some(diff => diff[0] === DIFF_EQUAL); }; -// In GNU diff format, indexes are one-based instead of zero-based. -export const createPatchMark = ( - aStart: number, - aEnd: number, - bStart: number, - bEnd: number, - {patchColor}: DiffOptionsNormalized, -): string => - patchColor( - `@@ -${aStart + 1},${aEnd - aStart} +${bStart + 1},${bEnd - bStart} @@`, - ); - // Compare two strings character-by-character. // Format as comparison lines in which changed substrings have inverse colors. export const diffStringsUnified = ( From 95ad82fdc07708da88002d337f4ddd52c5ce57cc Mon Sep 17 00:00:00 2001 From: Raymond Ng Date: Mon, 16 Nov 2020 15:27:16 -0800 Subject: [PATCH 3/4] add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cbfb7b613fa..5f6d23abfb9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ - `[jest-runtime]` [**BREAKING**] remove long-deprecated `jest.addMatchers`, `jest.resetModuleRegistry`, and `jest.runTimersToTime` ([#9853](https://github.com/facebook/jest/pull/9853)) - `[jest-transform]` Show enhanced `SyntaxError` message for all `SyntaxError`s ([#10749](https://github.com/facebook/jest/pull/10749)) - `[jest-transform]` [**BREAKING**] Refactor API to pass an options bag around rather than multiple boolean options ([#10753](https://github.com/facebook/jest/pull/10753)) +- `[jest-diff]` Break dependency cycle in `jest-diff` ([#10818](https://github.com/facebook/jest/pull/10818)) ### Chore & Maintenance From ff90ab33398a41381b1a15159a551b7e11396347 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Tue, 17 Nov 2020 12:04:42 +0100 Subject: [PATCH 4/4] Update CHANGELOG.md --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 403b6e5b5407..bd6ceef1d54f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,8 +19,7 @@ - `[jest-runtime]` [**BREAKING**] remove long-deprecated `jest.addMatchers`, `jest.resetModuleRegistry`, and `jest.runTimersToTime` ([#9853](https://github.com/facebook/jest/pull/9853)) - `[jest-transform]` Show enhanced `SyntaxError` message for all `SyntaxError`s ([#10749](https://github.com/facebook/jest/pull/10749)) - `[jest-transform]` [**BREAKING**] Refactor API to pass an options bag around rather than multiple boolean options ([#10753](https://github.com/facebook/jest/pull/10753)) -- `[jest-transform]` [**BREAKING**] Refactor API of transformers to pass an options bag rather than separate `config` and other options -- `[jest-diff]` Break dependency cycle in `jest-diff` ([#10818](https://github.com/facebook/jest/pull/10818)) +- `[jest-transform]` [**BREAKING**] Refactor API of transformers to pass an options bag rather than separate `config` and other options ([#10834](https://github.com/facebook/jest/pull/10834)) ### Chore & Maintenance @@ -29,6 +28,7 @@ - `[*]` [**BREAKING**] Add `exports` field to all `package.json`s ([#9921](https://github.com/facebook/jest/pull/9921)) - `[*]` Make it easier for Jest's packages to use the VM escape hatch ([#10824](https://github.com/facebook/jest/pull/10824)) - `[jest-config]` [**BREAKING**] Remove `enabledTestsMap` config, use `filter` instead ([#10787](https://github.com/facebook/jest/pull/10787)) +- `[jest-diff]` Break dependency cycle ([#10818](https://github.com/facebook/jest/pull/10818)) - `[jest-resolve]` [**BREAKING**] Migrate to ESM ([#10688](https://github.com/facebook/jest/pull/10688)) - `[jest-repl, jest-runtime]` [**BREAKING**] Move the `jest-runtime` CLI into `jest-repl` ([#10016](https://github.com/facebook/jest/pull/10016))