From cdce4e061dc6198f9d84a1a7b79dfe488dddf76a Mon Sep 17 00:00:00 2001 From: Nate Weller Date: Mon, 7 Oct 2024 14:50:51 -0600 Subject: [PATCH] Migrate DiffViewer component from Protect plugin into Components package --- pnpm-lock.yaml | 3 - .../changelog/add-diff-viewer-component | 4 + .../components/diff-viewer/README.md | 8 +- .../components/diff-viewer/index.tsx | 97 ++++++++ .../components/diff-viewer/parse-filename.ts | 75 +++++++ .../components/diff-viewer/parse-patch.ts | 212 ++++++++++++++++++ .../diff-viewer/stories/index.stories.tsx | 39 ++++ .../components/diff-viewer/styles.module.scss | 0 projects/js-packages/components/index.ts | 1 + .../changelog/add-diff-viewer-component | 4 + projects/plugins/protect/package.json | 1 - .../src/js/components/diff-viewer/index.jsx | 156 ------------- .../diff-viewer/stories/index.stories.jsx | 25 --- .../js/components/threats-list/paid-list.jsx | 3 +- 14 files changed, 437 insertions(+), 191 deletions(-) create mode 100644 projects/js-packages/components/changelog/add-diff-viewer-component rename projects/{plugins/protect/src/js => js-packages/components}/components/diff-viewer/README.md (75%) create mode 100644 projects/js-packages/components/components/diff-viewer/index.tsx create mode 100644 projects/js-packages/components/components/diff-viewer/parse-filename.ts create mode 100644 projects/js-packages/components/components/diff-viewer/parse-patch.ts create mode 100644 projects/js-packages/components/components/diff-viewer/stories/index.stories.tsx rename projects/{plugins/protect/src/js => js-packages/components}/components/diff-viewer/styles.module.scss (100%) create mode 100644 projects/plugins/protect/changelog/add-diff-viewer-component delete mode 100644 projects/plugins/protect/src/js/components/diff-viewer/index.jsx delete mode 100644 projects/plugins/protect/src/js/components/diff-viewer/stories/index.stories.jsx diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index be4fce55d9bca..c4e74a4371136 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -4243,9 +4243,6 @@ importers: clsx: specifier: 2.1.1 version: 2.1.1 - diff: - specifier: ^4.0.2 - version: 4.0.2 moment: specifier: 2.29.4 version: 2.29.4 diff --git a/projects/js-packages/components/changelog/add-diff-viewer-component b/projects/js-packages/components/changelog/add-diff-viewer-component new file mode 100644 index 0000000000000..2a305290fe23c --- /dev/null +++ b/projects/js-packages/components/changelog/add-diff-viewer-component @@ -0,0 +1,4 @@ +Significance: minor +Type: added + +Add DiffViewer component diff --git a/projects/plugins/protect/src/js/components/diff-viewer/README.md b/projects/js-packages/components/components/diff-viewer/README.md similarity index 75% rename from projects/plugins/protect/src/js/components/diff-viewer/README.md rename to projects/js-packages/components/components/diff-viewer/README.md index 0753deb9a3a53..8cf4a6783431f 100644 --- a/projects/plugins/protect/src/js/components/diff-viewer/README.md +++ b/projects/js-packages/components/components/diff-viewer/README.md @@ -1,6 +1,6 @@ # Unified Diff Viewer -Forked over from [Calypso](https://github.com/Automattic/wp-calypso/tree/b7a4a07/client/components/diff-viewer). +Originally forked over from [Calypso](https://github.com/Automattic/wp-calypso/tree/b7a4a07/client/components/diff-viewer). This component renders the output of a unified diff (`git diff` or `diff -u`) in a visual format recognizable by someone who works with `diff` and comparing files. @@ -29,9 +29,9 @@ export const CommitView = ( { commitHash, description, diff } ) => ( ### Additional usage information The diff output should be the full text produced by the diff command (including newlines). -Internally this component relies on `jsdiff` to parse the output (the patch) and produce -the data structure used to display files, hunks (sections of change in the files), and -the actual lines of change and context. +Internally this component parses the output (the patch) and produces the data structure +used to display files, hunks (sections of change in the files), and the actual lines of +change and context. ``` diff --git a/circle.yml b/circle.yml diff --git a/projects/js-packages/components/components/diff-viewer/index.tsx b/projects/js-packages/components/components/diff-viewer/index.tsx new file mode 100644 index 0000000000000..8e9bd3e76e47c --- /dev/null +++ b/projects/js-packages/components/components/diff-viewer/index.tsx @@ -0,0 +1,97 @@ +import { Fragment } from 'react'; +import parseFilename from './parse-filename'; +import parsePatch from './parse-patch'; +import styles from './styles.module.scss'; + +const filename = ( { + oldFileName, + newFileName, +}: { + oldFileName: string; + newFileName: string; +} ): JSX.Element => { + const { prev, next } = parseFilename( oldFileName, newFileName ); + + if ( prev.prefix + prev.path === next.prefix + next.path ) { + return ( + + { prev.prefix && ( + { prev.prefix } + ) } + { prev.path } + + ); + } + + return ( + + { !! prev.prefix && ( + { prev.prefix } + ) } + { prev.path } + { ' → ' } + { !! next.prefix && ( + { next.prefix } + ) } + { next.path } + + ); +}; + +export const DiffViewer = ( { diff } ) => ( +
+ { parsePatch( diff ).map( ( file, fileIndex ) => ( + +
+ { filename( file ) } +
+
+
+ { file.hunks.map( ( hunk, hunkIndex ) => { + let lineOffset = 0; + return hunk.lines.map( ( line, index ) => ( +
+ { line[ 0 ] === '+' ? '\u00a0' : hunk.oldStart + lineOffset++ } +
+ ) ); + } ) } +
+
+ { file.hunks.map( ( hunk, hunkIndex ) => { + let lineOffset = 0; + return hunk.lines.map( ( line, index ) => ( +
+ { line[ 0 ] === '-' ? '\u00a0' : hunk.newStart + lineOffset++ } +
+ ) ); + } ) } +
+
+ { file.hunks.map( ( hunk, hunkIndex ) => + hunk.lines.map( ( line, index ) => { + const output = line.slice( 1 ).replace( /^\s*$/, '\u00a0' ); + const key = `${ hunkIndex }-${ index }`; + + switch ( line[ 0 ] ) { + case ' ': + return
{ output }
; + + case '-': + return { output }; + + case '+': + return { output }; + + default: + return undefined; + } + } ) + ) } +
+
+ + ) ) } +
+); + +export default DiffViewer; diff --git a/projects/js-packages/components/components/diff-viewer/parse-filename.ts b/projects/js-packages/components/components/diff-viewer/parse-filename.ts new file mode 100644 index 0000000000000..5e51f8569c06a --- /dev/null +++ b/projects/js-packages/components/components/diff-viewer/parse-filename.ts @@ -0,0 +1,75 @@ +type ParsedFilename = { + prefix: string; + path: string; +}; + +const decompose = ( path: string ): ParsedFilename => { + const lastSlash = path.lastIndexOf( '/' ); + + return lastSlash > -1 + ? { prefix: path.slice( 0, lastSlash ), path: path.slice( lastSlash ) } + : { prefix: '', path }; +}; + +/** + * Parse the filename from a diff + * + * Uses a heuristic to return proper file name indicators + * + * It searches for the longest shared prefix and returns + * whatever remains after that. If the paths are identical + * it only returns a single filename as we have detected + * that the diff compares changes to only one file. + * + * An exception is made for `a/` and `b/` prefixes often + * added by `git` and other utilities to separate the left + * from the right when looking at the contents of a single + * file over time. + * + * @param {string} prev - filename of left contents + * @param {string} next - filename of right contents + * + * @return {object} - parsed filename + */ +export default function ( + prev: string, + next: string +): { prev: ParsedFilename; next: ParsedFilename } { + // Remove 'a/' and 'b/' prefixes if present + const isLikelyPrefixed = prev.startsWith( 'a/' ) && next.startsWith( 'b/' ); + prev = isLikelyPrefixed ? prev.slice( 2 ) : prev; + next = isLikelyPrefixed ? next.slice( 2 ) : next; + + if ( prev === next ) { + // Paths are identical + const { prefix, path } = decompose( prev ); + return { prev: { prefix, path }, next: { prefix, path } }; + } + + // Find longest shared base path ending with a slash + const length = Math.max( prev.length, next.length ); + for ( let i = 0, slash = 0; i < length; i++ ) { + if ( prev[ i ] === '/' && next[ i ] === '/' ) { + slash = i; + } + + if ( prev[ i ] !== next[ i ] ) { + return { + prev: { + prefix: prev.slice( 0, slash ), + path: prev.slice( slash ), + }, + next: { + prefix: next.slice( 0, slash ), + path: next.slice( slash ), + }, + }; + } + } + + // No shared base path + return { + prev: decompose( prev ), + next: decompose( next ), + }; +} diff --git a/projects/js-packages/components/components/diff-viewer/parse-patch.ts b/projects/js-packages/components/components/diff-viewer/parse-patch.ts new file mode 100644 index 0000000000000..69281bbdc63a3 --- /dev/null +++ b/projects/js-packages/components/components/diff-viewer/parse-patch.ts @@ -0,0 +1,212 @@ +type Hunk = { + oldStart: number; + oldLines: number; + newStart: number; + newLines: number; + lines: string[]; +}; + +type Index = { + index?: string; + hunks?: Hunk[]; + oldFileName?: string; + newFileName?: string; + oldHeader?: string; + newHeader?: string; +}; + +/** + * Parse Patch + * + * Adapted from https://github.com/kpdecker/jsdiff/blob/master/src/patch/parse.js + * + * @param {string} uniDiff - diff string + * @return {Array} - array of parsed files + */ +export default function parsePatch( uniDiff: string ) { + const diffstr = uniDiff.split( /\n/ ); + const list = []; + let i = 0; + + /** + * Parse Index + */ + function parseIndex() { + const index: Index = {}; + + list.push( index ); + + // Parse diff metadata + while ( i < diffstr.length ) { + const line = diffstr[ i ]; + + // File header found, end parsing diff metadata + if ( /^(---|\+\+\+|@@)\s/.test( line ) ) { + break; + } + + // Diff index + const header = /^(?:Index:|diff(?: -r \w+)+)\s+(.+?)\s*$/.exec( line ); + + if ( header ) { + index.index = header[ 1 ]; + } + + i++; + } + + // Parse file headers if they are defined. Unified diff requires them, but + // there's no technical issues to have an isolated hunk without file header + parseFileHeader( index ); + parseFileHeader( index ); + + // Parse hunks + index.hunks = []; + + while ( i < diffstr.length ) { + const _line = diffstr[ i ]; + + if ( + /^(Index:\s|diff\s|---\s|\+\+\+\s|===================================================================)/.test( + _line + ) + ) { + break; + } else if ( /^@@/.test( _line ) ) { + index.hunks.push( parseHunk() ); + } else if ( _line ) { + throw new Error( 'Unknown line ' + ( i + 1 ) + ' ' + JSON.stringify( _line ) ); + } else { + i++; + } + } + } + + /** + * Parse File Header + * + * Parses the --- and +++ headers, if none are found, no lines + * are consumed. + * + * @param {Array} index - array of parsed files + * @param {unknown} index.index - index + * @param {object[]} index.hunks - hunks + */ + function parseFileHeader( index: Index ) { + const fileHeader = /^(---|\+\+\+)\s+(.*)\r?$/.exec( diffstr[ i ] ); + + if ( fileHeader ) { + const keyPrefix = fileHeader[ 1 ] === '---' ? 'old' : 'new'; + + const data = fileHeader[ 2 ].split( '\t', 2 ); + + let fileName = data[ 0 ].replace( /\\\\/g, '\\' ); + + if ( /^".*"$/.test( fileName ) ) { + fileName = fileName.substr( 1, fileName.length - 2 ); + } + + index[ keyPrefix + 'FileName' ] = fileName; + + index[ keyPrefix + 'Header' ] = ( data[ 1 ] || '' ).trim(); + + i++; + } + } + + /** + * Parse Hunk + * This assumes that we are at the start of a hunk. + * + * @return {object} - The parsed hunk. + */ + function parseHunk(): Hunk { + const chunkHeaderIndex = i, + chunkHeaderLine = diffstr[ i++ ], + chunkHeader = chunkHeaderLine.split( /@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@/ ); + + const hunk = { + oldStart: +chunkHeader[ 1 ], + oldLines: typeof chunkHeader[ 2 ] === 'undefined' ? 1 : +chunkHeader[ 2 ], + newStart: +chunkHeader[ 3 ], + newLines: typeof chunkHeader[ 4 ] === 'undefined' ? 1 : +chunkHeader[ 4 ], + lines: [], + }; + + // Unified Diff Format quirk: If the chunk size is 0, + // the first number is one lower than one would expect. + // https://www.artima.com/weblogs/viewpost.jsp?thread=164293 + + if ( hunk.oldLines === 0 ) { + hunk.oldStart += 1; + } + + if ( hunk.newLines === 0 ) { + hunk.newStart += 1; + } + + let addCount = 0, + removeCount = 0, + _diffstr$i; + for ( + ; + i < diffstr.length && + ( removeCount < hunk.oldLines || + addCount < hunk.newLines || + ( ( _diffstr$i = diffstr[ i ] ) !== null && + _diffstr$i !== void 0 && + _diffstr$i.startsWith( '\\' ) ) ); + i++ + ) { + const operation = + diffstr[ i ].length === 0 && i !== diffstr.length - 1 ? ' ' : diffstr[ i ][ 0 ]; + + if ( operation === '+' || operation === '-' || operation === ' ' || operation === '\\' ) { + hunk.lines.push( diffstr[ i ] ); + + if ( operation === '+' ) { + addCount++; + } else if ( operation === '-' ) { + removeCount++; + } else if ( operation === ' ' ) { + addCount++; + removeCount++; + } + } else { + throw new Error( + `Hunk at line ${ chunkHeaderIndex + 1 } contained invalid line ${ diffstr[ i ] }` + ); + } + } + + // Handle the empty block count case + if ( ! addCount && hunk.newLines === 1 ) { + hunk.newLines = 0; + } + + if ( ! removeCount && hunk.oldLines === 1 ) { + hunk.oldLines = 0; + } + + // Perform sanity checking + if ( addCount !== hunk.newLines ) { + throw new Error( + 'Added line count did not match for hunk at line ' + ( chunkHeaderIndex + 1 ) + ); + } + + if ( removeCount !== hunk.oldLines ) { + throw new Error( + 'Removed line count did not match for hunk at line ' + ( chunkHeaderIndex + 1 ) + ); + } + + return hunk; + } + + while ( i < diffstr.length ) { + parseIndex(); + } + + return list; +} diff --git a/projects/js-packages/components/components/diff-viewer/stories/index.stories.tsx b/projects/js-packages/components/components/diff-viewer/stories/index.stories.tsx new file mode 100644 index 0000000000000..4f986981f9761 --- /dev/null +++ b/projects/js-packages/components/components/diff-viewer/stories/index.stories.tsx @@ -0,0 +1,39 @@ +/* eslint-disable react/react-in-jsx-scope */ +import React from 'react'; +import DiffViewer from '..'; + +export default { + title: 'JS Packages/Components/Diff Viewer', + component: DiffViewer, +}; + +const diff = `diff --git a/package.json b/package.json +Index: a31e51f..c3b21a1 100644 +--- a/package.json ++++ b/package.json +@@ -1,7 +1,7 @@ + { + "name": "hello-world", +- "version": "1.0.0", ++ "version": "1.0.1", + "description": "Hello, World!", +- "main": "index.js", ++ "main": "index.ts", + "scripts": { +- "start": "node index.js" ++ "start": "node index.ts" + +diff --git a/src/index.js b/src/index.ts +Index: 17c882a..d3f041b 100644 +--- a/src/index.js ++++ b/src/index.ts +@@ -0,0 +1,1 @@ ++console.log( 'Hello, world!' );`; + +const Template = args => ; + +export const Default = Template.bind( {} ); + +Default.args = { + diff, +}; diff --git a/projects/plugins/protect/src/js/components/diff-viewer/styles.module.scss b/projects/js-packages/components/components/diff-viewer/styles.module.scss similarity index 100% rename from projects/plugins/protect/src/js/components/diff-viewer/styles.module.scss rename to projects/js-packages/components/components/diff-viewer/styles.module.scss diff --git a/projects/js-packages/components/index.ts b/projects/js-packages/components/index.ts index b4740fcff7404..ea8814edf8dbd 100644 --- a/projects/js-packages/components/index.ts +++ b/projects/js-packages/components/index.ts @@ -78,4 +78,5 @@ export { default as UpsellBanner } from './components/upsell-banner'; export { getUserLocale, cleanLocale } from './lib/locale'; export { default as RadioControl } from './components/radio-control'; export { default as StatCard } from './components/stat-card'; +export { default as DiffViewer } from './components/diff-viewer'; export * from './components/global-notices'; diff --git a/projects/plugins/protect/changelog/add-diff-viewer-component b/projects/plugins/protect/changelog/add-diff-viewer-component new file mode 100644 index 0000000000000..2a305290fe23c --- /dev/null +++ b/projects/plugins/protect/changelog/add-diff-viewer-component @@ -0,0 +1,4 @@ +Significance: minor +Type: added + +Add DiffViewer component diff --git a/projects/plugins/protect/package.json b/projects/plugins/protect/package.json index 1a95d9b80cede..fc36ae5e2ce05 100644 --- a/projects/plugins/protect/package.json +++ b/projects/plugins/protect/package.json @@ -41,7 +41,6 @@ "@wordpress/url": "4.8.1", "camelize": "1.0.1", "clsx": "2.1.1", - "diff": "^4.0.2", "moment": "2.29.4", "prop-types": "15.8.1", "react": "18.3.1", diff --git a/projects/plugins/protect/src/js/components/diff-viewer/index.jsx b/projects/plugins/protect/src/js/components/diff-viewer/index.jsx deleted file mode 100644 index 2f26a4ae9c19b..0000000000000 --- a/projects/plugins/protect/src/js/components/diff-viewer/index.jsx +++ /dev/null @@ -1,156 +0,0 @@ -import { parsePatch } from 'diff/lib/patch/parse'; -import { Fragment } from 'react'; -import styles from './styles.module.scss'; - -const decompose = path => { - const lastSlash = path.lastIndexOf( '/' ); - - return lastSlash > -1 ? [ path.slice( 0, lastSlash ), path.slice( lastSlash ) ] : [ '', path ]; -}; - -/** - * Uses a heuristic to return proper file name indicators - * - * This function lists the filenames for the left and right - * side of the diff in a single string. - * - * It searches for the longest shared prefix and returns - * whatever remains after that. If the paths are identical - * it only returns a single filename as we have detected - * that the diff compares changes to only one file. - * - * An exception is made for `a/` and `b/` prefixes often - * added by `git` and other utilities to separate the left - * from the right when looking at the contents of a single - * file over time. - * - * @param {object} options - deconstructed argument - * @param {string} options.oldFileName - filename of left contents - * @param {string} options.newFileName - filename of right contents - * @return {window.Element} description of the file or files in the diff - */ -const filename = ( { oldFileName, newFileName } ) => { - // if we think the diff utility added a bogus - // prefix then cut it off - const isLikelyPrefixed = - 'a' === oldFileName[ 0 ] && - '/' === oldFileName[ 1 ] && - 'b' === newFileName[ 0 ] && - '/' === newFileName[ 1 ]; - - const [ prev, next ] = isLikelyPrefixed - ? [ oldFileName.slice( 2 ), newFileName.slice( 2 ) ] - : [ oldFileName, newFileName ]; - - if ( prev === next ) { - const [ base, name ] = decompose( prev ); - - // it's the same file, return the single name - return ( - - { base && { base } } - { name } - - ); - } - - // find the longest shared path prefix - const length = Math.max( prev.length, next.length ); - for ( let i = 0, slash = 0; i < length; i++ ) { - if ( prev[ i ] === '/' && next[ i ] === '/' ) { - slash = i; - } - - if ( prev[ i ] !== next[ i ] ) { - return ( - - { slash !== 0 && ( - - { prev.slice( 0, slash ) } - - ) } - { prev.slice( slash ) } - { ' → ' } - { slash !== 0 && ( - - { next.slice( 0, slash ) } - - ) } - { next.slice( slash ) } - - ); - } - } - - // otherwise we have no shared prefix - const [ prevBase, prevName ] = decompose( prev ); - const [ nextBase, nextName ] = decompose( next ); - - return ( - - { prevBase && { prevBase } } - { prevName } - { ' → ' } - { nextBase && { nextBase } } - { nextName } - - ); -}; - -export const DiffViewer = ( { diff } ) => ( -
- { parsePatch( diff ).map( ( file, fileIndex ) => ( - -
- { filename( file ) } -
-
-
- { file.hunks.map( ( hunk, hunkIndex ) => { - let lineOffset = 0; - return hunk.lines.map( ( line, index ) => ( -
- { line[ 0 ] === '+' ? '\u00a0' : hunk.oldStart + lineOffset++ } -
- ) ); - } ) } -
-
- { file.hunks.map( ( hunk, hunkIndex ) => { - let lineOffset = 0; - return hunk.lines.map( ( line, index ) => ( -
- { line[ 0 ] === '-' ? '\u00a0' : hunk.newStart + lineOffset++ } -
- ) ); - } ) } -
-
- { file.hunks.map( ( hunk, hunkIndex ) => - hunk.lines.map( ( line, index ) => { - const output = line.slice( 1 ).replace( /^\s*$/, '\u00a0' ); - const key = `${ hunkIndex }-${ index }`; - - switch ( line[ 0 ] ) { - case ' ': - return
{ output }
; - - case '-': - return { output }; - - case '+': - return { output }; - - default: - return undefined; - } - } ) - ) } -
-
- - ) ) } -
-); - -export default DiffViewer; diff --git a/projects/plugins/protect/src/js/components/diff-viewer/stories/index.stories.jsx b/projects/plugins/protect/src/js/components/diff-viewer/stories/index.stories.jsx deleted file mode 100644 index 22968cb1e33f8..0000000000000 --- a/projects/plugins/protect/src/js/components/diff-viewer/stories/index.stories.jsx +++ /dev/null @@ -1,25 +0,0 @@ -/* eslint-disable react/react-in-jsx-scope */ -import React from 'react'; -import DiffViewer from '../index.jsx'; - -export default { - title: 'Plugins/Protect/Diff Viewer', - component: DiffViewer, -}; - -const diff = `index 51455bdb14..bc0622d001 100644 ---- a/circle.yml -+++ b/circle.yml -@@ -1,6 +1,6 @@ - machine: - node: -- version: 8.9.4 -+ version: 8.11.0 - test: - pre: - - ? |`; - -export const Default = args => ; -Default.args = { - diff, -}; diff --git a/projects/plugins/protect/src/js/components/threats-list/paid-list.jsx b/projects/plugins/protect/src/js/components/threats-list/paid-list.jsx index e40178266cd08..d17e16a179e16 100644 --- a/projects/plugins/protect/src/js/components/threats-list/paid-list.jsx +++ b/projects/plugins/protect/src/js/components/threats-list/paid-list.jsx @@ -1,10 +1,9 @@ -import { Text, Button, useBreakpointMatch } from '@automattic/jetpack-components'; +import { Text, Button, useBreakpointMatch, DiffViewer } from '@automattic/jetpack-components'; import { __, sprintf } from '@wordpress/i18n'; import React, { useCallback } from 'react'; import useAnalyticsTracks from '../../hooks/use-analytics-tracks'; import useFixers from '../../hooks/use-fixers'; import useModal from '../../hooks/use-modal'; -import DiffViewer from '../diff-viewer'; import MarkedLines from '../marked-lines'; import PaidAccordion, { PaidAccordionItem } from '../paid-accordion'; import Pagination from './pagination';