From fa6088f2be4ea4159b6a5c6d368fa45dc9049d0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Wed, 6 Feb 2019 13:33:07 +0100 Subject: [PATCH 1/7] The `fastdiff()` function now also accepts arrays as input. --- src/fastdiff.js | 205 ++++++++++++----- tests/fastdiff.js | 569 +++++++++++++++++++++++++++++++++------------- 2 files changed, 560 insertions(+), 214 deletions(-) diff --git a/src/fastdiff.js b/src/fastdiff.js index 1f5b9d9..d9fb43b 100644 --- a/src/fastdiff.js +++ b/src/fastdiff.js @@ -8,8 +8,7 @@ */ /** - * Finds position of the first and last change in the given strings and generates set of changes. Set of changes - * can be applied to the input text in order to transform it into the output text, for example: + * Finds position of the first and last change in the given string/array and generates a set of changes: * * fastDiff( '12a', '12xyza' ); * // [ { index: 2, type: 'insert', values: [ 'x', 'y', 'z' ] } ] @@ -20,13 +19,27 @@ * fastDiff( '12xyza', '12a' ); * // [ { index: 2, type: 'delete', howMany: 3 } ] * - * fastDiff( '12aa', '12a' ); + * fastDiff( [ '1', '2', 'a', 'a' ], [ '1', '2', 'a' ] ); * // [ { index: 3, type: 'delete', howMany: 1 } ] * - * fastDiff( '12abc3', '2ab' ); + * fastDiff( [ '1', '2', 'a', 'b', 'c', '3' ], [ '2', 'a', 'b' ] ); * // [ { index: 0, type: 'insert', values: [ '2', 'a', 'b' ] }, { index: 3, type: 'delete', howMany: 6 } ] * - * Using returned results you can modify `oldText` to transform it into `newText`: + * Passed arrays can contain any type of data, however to compare them correctly custom comparator function + * should be passed as a third parameter: + * + * fastDiff( [ { value: 1 }, { value: 2 } ], [ { value: 1 }, { value: 3 } ], ( a, b ) => { + * return a.value === b.value; + * } ); + * // [ { index: 1, type: 'insert', values: [ { value: 3 } ] }, { index: 2, type: 'delete', howMany: 1 } ] + * + * By passing `true` as a fourth parameter (`linearChanges`) the output compatible with + * {@link module:utils/diff~diff diff()} function will be returned: + * + * fastDiff( '12a', '12xyza' ); + * // [ 'equal', 'equal', 'insert', 'insert', 'insert', 'equal' ] + * + * The resulted set of changes can be applied to the input in order to transform it into the output, for example: * * let input = '12abc3'; * const output = '2ab'; @@ -40,101 +53,136 @@ * } * } ); * - * input === output; // -> true + * // input equals output now + * + * or in case of arrays: + * + * let input = [ '1', '2', 'a', 'b', 'c', '3' ]; + * const output = [ '2', 'a', 'b' ]; + * const changes = fastDiff( input, output ); + * + * changes.forEach( change => { + * if ( change.type == 'insert' ) { + * input = input.slice( 0, change.index ).concat( change.values, input.slice( change.index ) ); + * } else if ( change.type == 'delete' ) { + * input = input.slice( 0, change.index ).concat( input.slice( change.index + change.howMany ) ); + * } + * } ); + * + * // input equals output now * - * The output format of this function is compatible with {@link module:utils/difftochanges~diffToChanges} output format. + * The output format of this function is compatible with {@link module:utils/difftochanges~diffToChanges diffToChanges()} + * function output format. * - * @param {String} oldText Input string. - * @param {String} newText Input string. + * @param {Array|String} a Input array or string. + * @param {Array|String} b Input array or string. + * @param {Function} [cmp] Optional function used to compare array values, by default === is used. + * @param {Boolean} [linearChanges=false] Whether array of `inset|delete|equal` operations should be returned or changes set. * @returns {Array} Array of changes. */ -export default function fastDiff( oldText, newText ) { - // Check if both texts are equal. - if ( oldText === newText ) { - return []; +export default function fastDiff( a, b, cmp, linearChanges = false ) { + // Set the comparator function. + cmp = cmp || function( a, b ) { + return a === b; + }; + + // Transform text into arrays for easier, consistent processing. + if ( typeof a === 'string' ) { + a = a.split( '' ); } - const changeIndexes = findChangeBoundaryIndexes( oldText, newText ); + if ( typeof b === 'string' ) { + b = b.split( '' ); + } - return changeIndexesToChanges( newText, changeIndexes ); + // Find first and last change. + const changeIndexes = findChangeBoundaryIndexes( a, b, cmp ); + + // Transform into changes array. + return linearChanges ? changeIndexesToLinearChanges( changeIndexes, b.length ) : changeIndexesToChanges( b, changeIndexes ); } -// Finds position of the first and last change in the given strings. For example: +// Finds position of the first and last change in the given arrays. For example: // -// const indexes = findChangeBoundaryIndexes( '1234', '13424' ); +// const indexes = findChangeBoundaryIndexes( [ '1', '2', '3', '4' ], [ '1', '3', '4', '2', '4' ] ); // console.log( indexes ); // { firstIndex: 1, lastIndexOld: 3, lastIndexNew: 4 } // -// The above indexes means that in `oldText` modified part is `1[23]4` and in the `newText` it is `1[342]4`. -// Based on such indexes, array with `insert`/`delete` operations which allows transforming -// old text to the new one can be generated. -// -// It is expected that `oldText` and `newText` are different. +// The above indexes means that in the first array the modified part is `1[23]4` and in the second array it is `1[342]4`. +// Based on such indexes, array with `insert`/`delete` operations which allows transforming first value into the second one +// can be generated. // -// @param {String} oldText -// @param {String} newText +// @param {Array} arr1 +// @param {Array} arr2 +// @param {Function} cmp Comparator function. // @returns {Object} -// @returns {Number} return.firstIndex Index of the first change in both strings (always the same for both). -// @returns {Number} result.lastIndexOld Index of the last common character in `oldText` string. -// @returns {Number} result.lastIndexNew Index of the last common character in `newText` string. -function findChangeBoundaryIndexes( oldText, newText ) { - // Find the first difference between texts. - const firstIndex = findFirstDifferenceIndex( oldText, newText ); - - // Remove the common part of texts and reverse them to make it simpler to find the last difference between texts. - const oldTextReversed = cutAndReverse( oldText, firstIndex ); - const newTextReversed = cutAndReverse( newText, firstIndex ); - - // Find the first difference between reversed texts. - // It should be treated as "how many characters from the end the last difference occurred". +// @returns {Number} return.firstIndex Index of the first change in both values (always the same for both). +// @returns {Number} result.lastIndexOld Index of the last common value in `arr1`. +// @returns {Number} result.lastIndexNew Index of the last common value in `arr2`. +function findChangeBoundaryIndexes( arr1, arr2, cmp ) { + // Find the first difference between passed values. + const firstIndex = findFirstDifferenceIndex( arr1, arr2, cmp ); + + // If arrays are equal return -1 indexes object. + if ( firstIndex === -1 ) { + return { firstIndex: -1, lastIndexOld: -1, lastIndexNew: -1 }; + } + + // Remove the common part of each value and reverse them to make it simpler to find the last difference between them. + const oldTextReversed = cutAndReverse( arr1, firstIndex ); + const newTextReversed = cutAndReverse( arr2, firstIndex ); + + // Find the first difference between reversed values. + // It should be treated as "how many elements from the end the last difference occurred". // // For example: // - // initial -> after cut -> reversed: - // oldText: '321ba' -> '21ba' -> 'ab12' - // newText: '31xba' -> '1xba' -> 'abx1' - // lastIndex: -> 2 + // initial -> after cut -> reversed: + // oldValue: '321ba' -> '21ba' -> 'ab12' + // newValue: '31xba' -> '1xba' -> 'abx1' + // lastIndex: -> 2 // - // So the last change occurred two characters from the end of the texts. - const lastIndex = findFirstDifferenceIndex( oldTextReversed, newTextReversed ); + // So the last change occurred two characters from the end of the arrays. + const lastIndex = findFirstDifferenceIndex( oldTextReversed, newTextReversed, cmp ); // Use `lastIndex` to calculate proper offset, starting from the beginning (`lastIndex` kind of starts from the end). - const lastIndexOld = oldText.length - lastIndex; - const lastIndexNew = newText.length - lastIndex; + const lastIndexOld = arr1.length - lastIndex; + const lastIndexNew = arr2.length - lastIndex; return { firstIndex, lastIndexOld, lastIndexNew }; } -// Returns a first index on which `oldText` and `newText` differ. +// Returns a first index on which given arrays differ. If both arrays are the same, -1 is returned. // -// @param {String} oldText -// @param {String} newText +// @param {Array} arr1 +// @param {Array} arr2 +// @param {Function} cmp Comparator function. // @returns {Number} -function findFirstDifferenceIndex( oldText, newText ) { - for ( let i = 0; i < Math.max( oldText.length, newText.length ); i++ ) { - if ( oldText[ i ] !== newText[ i ] ) { +function findFirstDifferenceIndex( arr1, arr2, cmp ) { + for ( let i = 0; i < Math.max( arr1.length, arr2.length ); i++ ) { + if ( arr1[ i ] === undefined || arr2[ i ] === undefined || !cmp( arr1[ i ], arr2[ i ] ) ) { return i; } } - // No "backup" return cause we assume that `oldText` and `newText` differ. This means that they either have a - // difference or they have a different lengths. This means that the `if` condition will always be met eventually. + + return -1; // Return -1 if arrays are equal. } -// Removes `howMany` characters from the given `text` string starting from the beginning, then reverses and returns it. +// Returns a copy of the given array with `howMany` elements removed starting from the beginning and in reversed order. // -// @param {String} text Text to be processed. -// @param {Number} howMany How many characters from text beginning to cut. -// @returns {String} Shortened and reversed text. -function cutAndReverse( text, howMany ) { - return text.substring( howMany ).split( '' ).reverse().join( '' ); +// @param {Array} arr Array to be processed. +// @param {Number} howMany How many elements from text beginning to remove. +// @returns {Array} Shortened and reversed array. +function cutAndReverse( arr, howMany ) { + return arr.slice( howMany ).reverse(); } // Generates changes array based on change indexes from `findChangeBoundaryIndexes` function. This function will // generate array with 0 (no changes), 1 (deletion or insertion) or 2 records (insertion and deletion). // -// @param {String} newText New text for which change indexes were calculated. +// @param {Array} newArray New array for which change indexes were calculated. // @param {Object} changeIndexes Change indexes object from `findChangeBoundaryIndexes` function. // @returns {Array.} Array of changes compatible with {@link module:utils/difftochanges~diffToChanges} format. -function changeIndexesToChanges( newText, changeIndexes ) { +function changeIndexesToChanges( newArray, changeIndexes ) { const result = []; const { firstIndex, lastIndexOld, lastIndexNew } = changeIndexes; @@ -145,7 +193,7 @@ function changeIndexesToChanges( newText, changeIndexes ) { result.push( { index: firstIndex, type: 'insert', - values: newText.substring( firstIndex, lastIndexNew ).split( '' ) + values: newArray.slice( firstIndex, lastIndexNew ) } ); } @@ -159,3 +207,36 @@ function changeIndexesToChanges( newText, changeIndexes ) { return result; } + +// Generates array with set `equal|insert|delete` operations based on change indexes from `findChangeBoundaryIndexes` function. +// +// @param {Object} changeIndexes Change indexes object from `findChangeBoundaryIndexes` function. +// @param {Number} newLength Length of the new array/string on which `findChangeBoundaryIndexes` calculated change indexes. +// @returns {Array.} Array of changes compatible with {@link module:utils/diff~diff} format. +function changeIndexesToLinearChanges( changeIndexes, newLength ) { + const { firstIndex, lastIndexOld, lastIndexNew } = changeIndexes; + + // No changes. + if ( firstIndex === -1 ) { + return Array( newLength ).fill( 'equal' ); + } + + let result = []; + if ( firstIndex > 0 ) { + result = result.concat( Array( firstIndex ).fill( 'equal' ) ); + } + + if ( lastIndexNew - firstIndex > 0 ) { + result = result.concat( Array( lastIndexNew - firstIndex ).fill( 'insert' ) ); + } + + if ( lastIndexOld - firstIndex > 0 ) { + result = result.concat( Array( lastIndexOld - firstIndex ).fill( 'delete' ) ); + } + + if ( lastIndexNew < newLength ) { + result = result.concat( Array( newLength - lastIndexNew ).fill( 'equal' ) ); + } + + return result; +} diff --git a/tests/fastdiff.js b/tests/fastdiff.js index f3ba7f7..37cfc88 100644 --- a/tests/fastdiff.js +++ b/tests/fastdiff.js @@ -8,205 +8,470 @@ import diff from '../src/diff'; import diffToChanges from '../src/difftochanges'; describe( 'fastDiff', () => { - it( 'should diff identical texts', () => { - expectDiff( '123', '123', [] ); - } ); - - describe( 'insertion', () => { - it( 'should diff if old text is empty', () => { - expectDiff( '', '123', [ { index: 0, type: 'insert', values: [ '1', '2', '3' ] } ] ); + describe( 'changes object', () => { + it( 'should diff identical texts', () => { + expectDiff( '123', '123', [] ); } ); - it( 'should diff insertion on the beginning', () => { - expectDiff( '123', 'abc123', [ { index: 0, type: 'insert', values: [ 'a', 'b', 'c' ] } ] ); + it( 'should diff identical arrays', () => { + expectDiff( [ '1', '2', '3' ], [ '1', '2', '3' ], [] ); } ); - it( 'should diff insertion on the beginning (repetitive substring)', () => { - // Do not check compatibility with 'diffToChanges' as it generates: - // [ { index: 0, type: 'insert', values: [ 'a', 'b' ] }, { index: 5, type: 'insert', values: [ 'c', '1', '2', '3' ] } ] - expectDiff( '123', 'ab123c123', [ { index: 0, type: 'insert', values: [ 'a', 'b', '1', '2', '3', 'c' ] } ], false ); + it( 'should diff arrays with custom comparator', () => { + expectDiff( [ 'a', 'b', 'c' ], [ 'A', 'B', 'C' ], [], true, ( a, b ) => a.toLowerCase() === b.toLowerCase() ); } ); - it( 'should diff insertion on the end', () => { - expectDiff( '123', '123abc', [ { index: 3, type: 'insert', values: [ 'a', 'b', 'c' ] } ] ); + describe( 'insertion', () => { + it( 'should diff if old text is empty', () => { + expectDiff( '', '123', [ { index: 0, type: 'insert', values: [ '1', '2', '3' ] } ] ); + } ); + + it( 'should diff if old array is empty', () => { + expectDiff( [], [ '1', '2', '3' ], [ { index: 0, type: 'insert', values: [ '1', '2', '3' ] } ] ); + } ); + + it( 'should diff insertion on the beginning', () => { + expectDiff( '123', 'abc123', [ { index: 0, type: 'insert', values: [ 'a', 'b', 'c' ] } ] ); + } ); + + it( 'should diff insertion on the beginning (repetitive substring)', () => { + // Do not check compatibility with 'diffToChanges' as it generates: + // [ { index: 0, type: 'insert', values: [ 'a', 'b' ] }, { index: 5, type: 'insert', values: [ 'c', '1', '2', '3' ] } ] + expectDiff( '123', 'ab123c123', [ { index: 0, type: 'insert', values: [ 'a', 'b', '1', '2', '3', 'c' ] } ], false ); + } ); + + it( 'should diff insertion on the end', () => { + expectDiff( '123', '123abc', [ { index: 3, type: 'insert', values: [ 'a', 'b', 'c' ] } ] ); + } ); + + it( 'should diff insertion on the end (repetitive substring)', () => { + expectDiff( '123', '123ab123c', [ { index: 3, type: 'insert', values: [ 'a', 'b', '1', '2', '3', 'c' ] } ] ); + } ); + + it( 'should diff insertion in the middle', () => { + expectDiff( '123', '12abc3', [ { index: 2, type: 'insert', values: [ 'a', 'b', 'c' ] } ] ); + } ); + + it( 'should diff insertion in the middle (repetitive substring)', () => { + // Do not check compatibility with 'diffToChanges' as it generates: + // [ { index: 2, type: 'insert', values: [ 'a', 'b', '1', '2' ] }, { index: 7, type: 'insert', values: [ 'c', '3' ] } ] + expectDiff( '123', '12ab123c3', [ { index: 2, type: 'insert', values: [ 'a', 'b', '1', '2', '3', 'c' ] } ], false ); + } ); + + it( 'should diff insertion of duplicated content', () => { + expectDiff( '123', '123123', [ { index: 3, type: 'insert', values: [ '1', '2', '3' ] } ] ); + } ); + + it( 'should diff insertion of partially duplicated content', () => { + expectDiff( '123', '12323', [ { index: 3, type: 'insert', values: [ '2', '3' ] } ] ); + } ); + + it( 'should diff insertion on both boundaries', () => { + // Do not check compatibility with 'diffToChanges' as it generates: + // [ { index: 2, type: 'insert', values: [ 'a', 'b' ] }, { index: 5, type: 'insert', values: [ 'c' ] } ] + expectDiff( '123', 'ab123c', [ + { index: 0, type: 'insert', values: [ 'a', 'b', '1', '2', '3', 'c' ] }, + { index: 6, type: 'delete', howMany: 3 } + ], false ); + } ); + + it( 'should diff insertion in array of objects', () => { + const o1 = { foo: 1 }; + const o2 = { bar: 2 }; + + expectDiff( [ o1, o2 ], [ o1, o2, { baz: 3 } ], [ + { index: 2, type: 'insert', values: [ { baz: 3 } ] } + ] ); + } ); + + it( 'should diff insertion in array of objects with comparator', () => { + expectDiff( [ { text: 'foo' }, { text: 'bar' } ], [ { text: 'foo' }, { text: 'bar' }, { text: 'baz' } ], [ + { index: 2, type: 'insert', values: [ { text: 'baz' } ] } + ], true, ( a, b ) => a.text === b.text ); + } ); } ); - it( 'should diff insertion on the end (repetitive substring)', () => { - expectDiff( '123', '123ab123c', [ { index: 3, type: 'insert', values: [ 'a', 'b', '1', '2', '3', 'c' ] } ] ); + describe( 'deletion', () => { + it( 'should diff if new text is empty', () => { + expectDiff( '123', '', [ { index: 0, type: 'delete', howMany: 3 } ] ); + } ); + + it( 'should diff if new array is empty', () => { + expectDiff( [ '1', '2', '3' ], [], [ { index: 0, type: 'delete', howMany: 3 } ] ); + } ); + + it( 'should diff deletion on the beginning', () => { + expectDiff( 'abc123', '123', [ { index: 0, type: 'delete', howMany: 3 } ] ); + } ); + + it( 'should diff deletion on the beginning (repetitive substring)', () => { + // Do not check compatibility with 'diffToChanges' as it generates: + // [ { index: 0, type: 'delete', howMany: 2 }, { index: 3, type: 'delete', howMany: 4 } ] + expectDiff( 'ab123c123', '123', [ { index: 0, type: 'delete', howMany: 6 } ], false ); + } ); + + it( 'should diff deletion on the end', () => { + expectDiff( '123abc', '123', [ { index: 3, type: 'delete', howMany: 3 } ] ); + } ); + + it( 'should diff deletion on the end (repetitive substring)', () => { + expectDiff( '123ab123c', '123', [ { index: 3, type: 'delete', howMany: 6 } ] ); + } ); + + it( 'should diff deletion in the middle', () => { + expectDiff( '12abc3', '123', [ { index: 2, type: 'delete', howMany: 3 } ] ); + } ); + + it( 'should diff deletion in the middle (repetitive substring)', () => { + // Do not check compatibility with 'diffToChanges' as it generates: + // [ { index: 2, type: 'delete', howMany: 4 }, { index: 3, type: 'delete', howMany: 2 } ] + expectDiff( '12ab123c3', '123', [ { index: 2, type: 'delete', howMany: 6 } ], false ); + } ); + + it( 'should diff deletion on both boundaries', () => { + // Do not check compatibility with 'diffToChanges' as it generates: + // [ { index: 0, type: 'delete', howMany: 1 }, { index: 3, type: 'delete', howMany: 2 } ] + expectDiff( '12abc3', '2ab', [ + { index: 0, type: 'insert', values: [ '2', 'a', 'b' ] }, + { index: 3, type: 'delete', howMany: 6 } + ], false ); + } ); + + it( 'should diff deletion of duplicated content', () => { + expectDiff( '123123', '123', [ { index: 3, type: 'delete', howMany: 3 } ] ); + } ); + + it( 'should diff deletion of partially duplicated content', () => { + expectDiff( '12323', '123', [ { index: 3, type: 'delete', howMany: 2 } ] ); + } ); + + it( 'should diff deletion of partially duplicated content 2', () => { + // Do not check compatibility with 'diffToChanges' as it generates: + // [ { index: 1, type: 'delete', howMany: 2 }, { index: 2, type: 'delete', howMany: 1 } ] + expectDiff( '11233', '13', [ { index: 1, type: 'delete', howMany: 3 } ], false ); + } ); + + it( 'should diff deletion in array of objects', () => { + const o1 = { foo: 1 }; + const o2 = { bar: 2 }; + + expectDiff( [ o1, o2 ], [ o2 ], [ + { index: 0, type: 'delete', howMany: 1 } + ] ); + } ); + + it( 'should diff insertion in array of objects with comparator', () => { + expectDiff( [ { text: 'foo' }, { text: 'bar' } ], [ { text: 'bar' } ], [ + { index: 0, type: 'delete', howMany: 1 } + ], true, ( a, b ) => a.text === b.text ); + } ); } ); - it( 'should diff insertion in the middle', () => { - expectDiff( '123', '12abc3', [ { index: 2, type: 'insert', values: [ 'a', 'b', 'c' ] } ] ); + describe( 'replacement', () => { + it( 'should diff replacement of entire text', () => { + // Do not check compatibility with 'diffToChanges' as it has changes in reveres order ('delete', 'insert') here. + expectDiff( '12345', 'abcd', [ + { index: 0, type: 'insert', values: [ 'a', 'b', 'c', 'd' ] }, + { index: 4, type: 'delete', howMany: 5 } + ], false ); + } ); + + it( 'should diff replacement on the beginning', () => { + expectDiff( '12345', 'abcd345', [ + { index: 0, type: 'insert', values: [ 'a', 'b', 'c', 'd' ] }, + { index: 4, type: 'delete', howMany: 2 } + ] ); + } ); + + it( 'should diff replacement on the beginning (repetitive substring)', () => { + // Do not check compatibility with 'diffToChanges' as it has changes in reveres order ('delete', 'insert') here. + expectDiff( '12345', '345345', [ + { index: 0, type: 'insert', values: [ '3', '4', '5' ] }, + { index: 3, type: 'delete', howMany: 2 } + ], false ); + } ); + + it( 'should diff replacement on the end', () => { + // Do not check compatibility with 'diffToChanges' as it has changes in reveres order ('delete', 'insert') here. + expectDiff( '12345', '12ab', [ + { index: 2, type: 'insert', values: [ 'a', 'b' ] }, + { index: 4, type: 'delete', howMany: 3 } + ], false ); + } ); + + it( 'should diff replacement on the end (repetitive substring)', () => { + // Do not check compatibility with 'diffToChanges' as it generates: + // [ { index: 3, type: 'insert', values: [ '1', '2', '3' ] }, { index: 7, type: 'delete', howMany: 1 } ] + expectDiff( '12345', '1231234', [ + { index: 3, type: 'insert', values: [ '1', '2', '3', '4' ] }, + { index: 7, type: 'delete', howMany: 2 } + ], false ); + } ); + + it( 'should diff insertion of duplicated content', () => { + expectDiff( '1234', '123123', [ + { index: 3, type: 'insert', values: [ '1', '2', '3' ] }, + { index: 6, type: 'delete', howMany: 1 } + ], false ); + } ); + + it( 'should diff insertion of duplicated content', () => { + expectDiff( '1234', '13424', [ + { index: 1, type: 'insert', values: [ '3', '4', '2' ] }, + { index: 4, type: 'delete', howMany: 2 } + ], false ); + } ); + + it( 'should diff replacement in the middle', () => { + expectDiff( '12345', '12ab5', [ + { index: 2, type: 'insert', values: [ 'a', 'b' ] }, + { index: 4, type: 'delete', howMany: 2 } + ] ); + } ); + + it( 'should diff replacement in the middle (repetitive substring)', () => { + // Do not check compatibility with 'diffToChanges' as it generates: + // [ { index: 2, type: 'insert', values: [ '1', '2' ] }, { index: 7, type: 'insert', values: [ '5' ] } ] + expectDiff( '12345', '12123455', [ + { index: 2, type: 'insert', values: [ '1', '2', '3', '4', '5' ] }, + { index: 7, type: 'delete', howMany: 2 } + ], false ); + } ); + + it( 'should diff replacement of duplicated content', () => { + // Do not check compatibility with 'diffToChanges' as it has changes in reveres order ('delete', 'insert') here. + expectDiff( '123123', '123333', [ + { index: 3, type: 'insert', values: '33'.split( '' ) }, + { index: 5, type: 'delete', howMany: 2 } + ], false ); + } ); + + it( 'should diff replacement in array of objects', () => { + const o1 = { foo: 1 }; + const o2 = { bar: 2 }; + + expectDiff( [ o1, o2 ], [ o1, { baz: 3 } ], [ + { index: 1, type: 'insert', values: [ { baz: 3 } ] }, + { index: 2, type: 'delete', howMany: 1 } + ] ); + } ); + + it( 'should diff insertion in array of objects with comparator', () => { + expectDiff( [ { text: 'foo' }, { text: 'bar' } ], [ { text: 'foo' }, { text: 'baz' } ], [ + { index: 1, type: 'insert', values: [ { text: 'baz' } ] }, + { index: 2, type: 'delete', howMany: 1 } + ], true, ( a, b ) => a.text === b.text ); + } ); } ); + } ); - it( 'should diff insertion in the middle (repetitive substring)', () => { - // Do not check compatibility with 'diffToChanges' as it generates: - // [ { index: 2, type: 'insert', values: [ 'a', 'b', '1', '2' ] }, { index: 7, type: 'insert', values: [ 'c', '3' ] } ] - expectDiff( '123', '12ab123c3', [ { index: 2, type: 'insert', values: [ 'a', 'b', '1', '2', '3', 'c' ] } ], false ); + describe( 'changes linear', () => { + it( 'should diff identical texts', () => { + expectDiffLinear( '123', '123', 'eee' ); } ); - it( 'should diff insertion of duplicated content', () => { - expectDiff( '123', '123123', [ { index: 3, type: 'insert', values: [ '1', '2', '3' ] } ] ); + it( 'should diff identical arrays', () => { + expectDiffLinear( [ '1', '2', '3' ], [ '1', '2', '3' ], 'eee' ); } ); - it( 'should diff insertion of partially duplicated content', () => { - expectDiff( '123', '12323', [ { index: 3, type: 'insert', values: [ '2', '3' ] } ] ); + it( 'should diff arrays with custom comparator', () => { + expectDiffLinear( [ 'a', 'b', 'c' ], [ 'A', 'B', 'C' ], 'eee', true, ( a, b ) => a.toLowerCase() === b.toLowerCase() ); } ); - it( 'should diff insertion on both boundaries', () => { - // Do not check compatibility with 'diffToChanges' as it generates: - // [ { index: 2, type: 'insert', values: [ 'a', 'b' ] }, { index: 5, type: 'insert', values: [ 'c' ] } ] - expectDiff( '123', 'ab123c', [ - { index: 0, type: 'insert', values: [ 'a', 'b', '1', '2', '3', 'c' ] }, - { index: 6, type: 'delete', howMany: 3 } - ], false ); - } ); - } ); + describe( 'insertion', () => { + it( 'should diff if old text is empty', () => { + expectDiffLinear( '', '123', 'iii' ); + } ); - describe( 'deletion', () => { - it( 'should diff if new text is empty', () => { - expectDiff( '123', '', [ { index: 0, type: 'delete', howMany: 3 } ] ); - } ); + it( 'should diff if old array is empty', () => { + expectDiffLinear( [], [ '1', '2', '3' ], 'iii' ); + } ); - it( 'should diff deletion on the beginning', () => { - expectDiff( 'abc123', '123', [ { index: 0, type: 'delete', howMany: 3 } ] ); - } ); + it( 'should diff insertion on the beginning', () => { + expectDiffLinear( '123', 'abc123', 'iiieee' ); + } ); - it( 'should diff deletion on the beginning (repetitive substring)', () => { - // Do not check compatibility with 'diffToChanges' as it generates: - // [ { index: 0, type: 'delete', howMany: 2 }, { index: 3, type: 'delete', howMany: 4 } ] - expectDiff( 'ab123c123', '123', [ { index: 0, type: 'delete', howMany: 6 } ], false ); - } ); + it( 'should diff insertion on the beginning (repetitive substring)', () => { + expectDiffLinear( '123', 'ab123c123', 'iiiiiieee', false ); + } ); - it( 'should diff deletion on the end', () => { - expectDiff( '123abc', '123', [ { index: 3, type: 'delete', howMany: 3 } ] ); - } ); + it( 'should diff insertion on the end', () => { + expectDiffLinear( '123', '123abc', 'eeeiii' ); + } ); - it( 'should diff deletion on the end (repetitive substring)', () => { - expectDiff( '123ab123c', '123', [ { index: 3, type: 'delete', howMany: 6 } ] ); - } ); + it( 'should diff insertion on the end (repetitive substring)', () => { + expectDiffLinear( '123', '123ab123c', 'eeeiiiiii' ); + } ); - it( 'should diff deletion in the middle', () => { - expectDiff( '12abc3', '123', [ { index: 2, type: 'delete', howMany: 3 } ] ); - } ); + it( 'should diff insertion in the middle', () => { + expectDiffLinear( '123', '12abc3', 'eeiiie' ); + } ); - it( 'should diff deletion in the middle (repetitive substring)', () => { - // Do not check compatibility with 'diffToChanges' as it generates: - // [ { index: 2, type: 'delete', howMany: 4 }, { index: 3, type: 'delete', howMany: 2 } ] - expectDiff( '12ab123c3', '123', [ { index: 2, type: 'delete', howMany: 6 } ], false ); - } ); + it( 'should diff insertion in the middle (repetitive substring)', () => { + expectDiffLinear( '123', '12ab123c3', 'eeiiiiiie', false ); + } ); - it( 'should diff deletion on both boundaries', () => { - // Do not check compatibility with 'diffToChanges' as it generates: - // [ { index: 0, type: 'delete', howMany: 1 }, { index: 3, type: 'delete', howMany: 2 } ] - expectDiff( '12abc3', '2ab', [ - { index: 0, type: 'insert', values: [ '2', 'a', 'b' ] }, - { index: 3, type: 'delete', howMany: 6 } - ], false ); - } ); + it( 'should diff insertion of duplicated content', () => { + expectDiffLinear( '123', '123123', 'eeeiii' ); + } ); - it( 'should diff deletion of duplicated content', () => { - expectDiff( '123123', '123', [ { index: 3, type: 'delete', howMany: 3 } ] ); - } ); + it( 'should diff insertion of partially duplicated content', () => { + expectDiffLinear( '123', '12323', 'eeeii' ); + } ); - it( 'should diff deletion of partially duplicated content', () => { - expectDiff( '12323', '123', [ { index: 3, type: 'delete', howMany: 2 } ] ); - } ); + it( 'should diff insertion on both boundaries', () => { + expectDiffLinear( '123', 'ab123c', 'iiiiiiddd', false ); + } ); - it( 'should diff deletion of partially duplicated content 2', () => { - // Do not check compatibility with 'diffToChanges' as it generates: - // [ { index: 1, type: 'delete', howMany: 2 }, { index: 2, type: 'delete', howMany: 1 } ] - expectDiff( '11233', '13', [ { index: 1, type: 'delete', howMany: 3 } ], false ); - } ); - } ); + it( 'should diff insertion in array of objects', () => { + const o1 = { foo: 1 }; + const o2 = { bar: 2 }; - describe( 'replacement', () => { - it( 'should diff replacement of entire text', () => { - // Do not check compatibility with 'diffToChanges' as it has changes in reveres order ('delete', 'insert') here. - expectDiff( '12345', 'abcd', [ - { index: 0, type: 'insert', values: [ 'a', 'b', 'c', 'd' ] }, - { index: 4, type: 'delete', howMany: 5 } - ], false ); - } ); + expectDiffLinear( [ o1, o2 ], [ o1, o2, { baz: 3 } ], 'eei' ); + } ); - it( 'should diff replacement on the beginning', () => { - expectDiff( '12345', 'abcd345', [ - { index: 0, type: 'insert', values: [ 'a', 'b', 'c', 'd' ] }, - { index: 4, type: 'delete', howMany: 2 } - ] ); + it( 'should diff insertion in array of objects with comparator', () => { + expectDiffLinear( [ { text: 'foo' }, { text: 'bar' } ], [ { text: 'foo' }, { text: 'bar' }, { text: 'baz' } ], + 'eei', true, ( a, b ) => a.text === b.text ); + } ); } ); - it( 'should diff replacement on the beginning (repetitive substring)', () => { - // Do not check compatibility with 'diffToChanges' as it has changes in reveres order ('delete', 'insert') here. - expectDiff( '12345', '345345', [ - { index: 0, type: 'insert', values: [ '3', '4', '5' ] }, - { index: 3, type: 'delete', howMany: 2 } - ], false ); - } ); + describe( 'deletion', () => { + it( 'should diff if new text is empty', () => { + expectDiffLinear( '123', '', 'ddd' ); + } ); - it( 'should diff replacement on the end', () => { - // Do not check compatibility with 'diffToChanges' as it has changes in reveres order ('delete', 'insert') here. - expectDiff( '12345', '12ab', [ - { index: 2, type: 'insert', values: [ 'a', 'b' ] }, - { index: 4, type: 'delete', howMany: 3 } - ], false ); - } ); + it( 'should diff if new array is empty', () => { + expectDiffLinear( [ '1', '2', '3' ], [], 'ddd' ); + } ); - it( 'should diff replacement on the end (repetitive substring)', () => { - // Do not check compatibility with 'diffToChanges' as it generates: - // [ { index: 3, type: 'insert', values: [ '1', '2', '3' ] }, { index: 7, type: 'delete', howMany: 1 } ] - expectDiff( '12345', '1231234', [ - { index: 3, type: 'insert', values: [ '1', '2', '3', '4' ] }, - { index: 7, type: 'delete', howMany: 2 } - ], false ); - } ); + it( 'should diff deletion on the beginning', () => { + expectDiffLinear( 'abc123', '123', 'dddeee' ); + } ); - it( 'should diff insertion of duplicated content', () => { - expectDiff( '1234', '123123', [ - { index: 3, type: 'insert', values: [ '1', '2', '3' ] }, - { index: 6, type: 'delete', howMany: 1 } - ], false ); - } ); + it( 'should diff deletion on the beginning (repetitive substring)', () => { + expectDiffLinear( 'ab123c123', '123', 'ddddddeee', false ); + } ); - it( 'should diff insertion of duplicated content', () => { - expectDiff( '1234', '13424', [ - { index: 1, type: 'insert', values: [ '3', '4', '2' ] }, - { index: 4, type: 'delete', howMany: 2 } - ], false ); - } ); + it( 'should diff deletion on the end', () => { + expectDiffLinear( '123abc', '123', 'eeeddd' ); + } ); - it( 'should diff replacement in the middle', () => { - expectDiff( '12345', '12ab5', [ - { index: 2, type: 'insert', values: [ 'a', 'b' ] }, - { index: 4, type: 'delete', howMany: 2 } - ] ); - } ); + it( 'should diff deletion on the end (repetitive substring)', () => { + expectDiffLinear( '123ab123c', '123', 'eeedddddd' ); + } ); + + it( 'should diff deletion in the middle', () => { + expectDiffLinear( '12abc3', '123', 'eeddde' ); + } ); - it( 'should diff replacement in the middle (repetitive substring)', () => { - // Do not check compatibility with 'diffToChanges' as it generates: - // [ { index: 2, type: 'insert', values: [ '1', '2' ] }, { index: 7, type: 'insert', values: [ '5' ] } ] - expectDiff( '12345', '12123455', [ - { index: 2, type: 'insert', values: [ '1', '2', '3', '4', '5' ] }, - { index: 7, type: 'delete', howMany: 2 } - ], false ); + it( 'should diff deletion in the middle (repetitive substring)', () => { + expectDiffLinear( '12ab123c3', '123', 'eedddddde', false ); + } ); + + it( 'should diff deletion on both boundaries', () => { + expectDiffLinear( '12abc3', '2ab', 'iiidddddd', false ); + } ); + + it( 'should diff deletion of duplicated content', () => { + expectDiffLinear( '123123', '123', 'eeeddd' ); + } ); + + it( 'should diff deletion of partially duplicated content', () => { + expectDiffLinear( '12323', '123', 'eeedd' ); + } ); + + it( 'should diff deletion of partially duplicated content 2', () => { + expectDiffLinear( '11233', '13', 'eddde', false ); + } ); + + it( 'should diff deletion in array of objects', () => { + const o1 = { foo: 1 }; + const o2 = { bar: 2 }; + + expectDiffLinear( [ o1, o2 ], [ o2 ], 'de' ); + } ); + + it( 'should diff insertion in array of objects with comparator', () => { + expectDiffLinear( [ { text: 'foo' }, { text: 'bar' } ], [ { text: 'bar' } ], 'de', true, ( a, b ) => a.text === b.text ); + } ); } ); - it( 'should diff replacement of duplicated content', () => { - // Do not check compatibility with 'diffToChanges' as it has changes in reveres order ('delete', 'insert') here. - expectDiff( '123123', '123333', [ - { index: 3, type: 'insert', values: '33'.split( '' ) }, - { index: 5, type: 'delete', howMany: 2 } - ], false ); + describe( 'replacement', () => { + it( 'should diff replacement of entire text', () => { + expectDiffLinear( '12345', 'abcd', 'iiiiddddd', false ); + } ); + + it( 'should diff replacement on the beginning', () => { + expectDiffLinear( '12345', 'abcd345', 'iiiiddeee' ); + } ); + + it( 'should diff replacement on the beginning (repetitive substring)', () => { + expectDiffLinear( '12345', '345345', 'iiiddeee', false ); + } ); + + it( 'should diff replacement on the end', () => { + expectDiffLinear( '12345', '12ab', 'eeiiddd', false ); + } ); + + it( 'should diff replacement on the end (repetitive substring)', () => { + expectDiffLinear( '12345', '1231234', 'eeeiiiidd', false ); + } ); + + it( 'should diff insertion of duplicated content', () => { + expectDiffLinear( '1234', '123123', 'eeeiiid' ); + } ); + + it( 'should diff insertion of duplicated content', () => { + expectDiffLinear( '1234', '13424', 'eiiidde', false ); + } ); + + it( 'should diff replacement in the middle', () => { + expectDiffLinear( '12345', '12ab5', 'eeiidde' ); + } ); + + it( 'should diff replacement in the middle (repetitive substring)', () => { + expectDiffLinear( '12345', '12123455', 'eeiiiiidde', false ); + } ); + + it( 'should diff replacement of duplicated content', () => { + expectDiffLinear( '123123', '123333', 'eeeiidde', false ); + } ); + + it( 'should diff replacement in array of objects', () => { + const o1 = { foo: 1 }; + const o2 = { bar: 2 }; + + expectDiffLinear( [ o1, o2 ], [ o1, { baz: 3 } ], 'eid' ); + } ); + + it( 'should diff insertion in array of objects with comparator', () => { + expectDiffLinear( [ { text: 'foo' }, { text: 'bar' } ], [ { text: 'foo' }, { text: 'baz' } ], 'eid', + true, ( a, b ) => a.text === b.text ); + } ); } ); } ); } ); -function expectDiff( oldText, newText, expected, checkDiffToChangesCompatibility = true ) { - const result = fastDiff( oldText, newText ); +function expectDiff( oldText, newText, expected, checkDiffToChangesCompatibility = true, comparator = null ) { + const result = fastDiff( oldText, newText, comparator ); - expect( result ).to.deep.equals( expected ); + expect( result ).to.deep.equals( expected, 'fastDiff changes failed' ); if ( checkDiffToChangesCompatibility ) { - expect( result ).to.deep.equals( diffToChanges( diff( oldText, newText ), newText ), 'diffToChanges compatibility' ); + expect( result ).to.deep.equals( + diffToChanges( diff( oldText, newText, comparator ), newText ), 'diffToChanges compatibility failed' ); + } +} + +function expectDiffLinear( oldText, newText, expected, checkDiffCompatibility = true, comparator = null ) { + const actions = { d: 'delete', e: 'equal', i: 'insert' }; + const expectedArray = expected.split( '' ).map( item => actions[ item ] ); + const result = fastDiff( oldText, newText, comparator, true ); + + expect( result ).to.deep.equals( expectedArray, 'fastDiff linear result failed' ); + + if ( checkDiffCompatibility ) { + expect( result ).to.deep.equals( diff( oldText, newText, comparator ), 'diff compatibility failed' ); } } From bebc61fecf61fe8dec384bd34049038da37edc92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Mon, 11 Feb 2019 13:42:57 +0100 Subject: [PATCH 2/7] Now `diff()` uses `fastDiff()` internally for longer inputs. --- src/diff.js | 16 ++++++++- tests/_utils-tests/longtext.js | 46 ++++++++++++++++++++++++++ tests/_utils/longtext.js | 20 ++++++++++++ tests/_utils/longtext.txt | 1 + tests/diff.js | 37 +++++++++++++++++++++ tests/manual/diff/diff.html | 1 + tests/manual/diff/diff.js | 60 ++++++++++++++++++++++++++++++++++ tests/manual/diff/diff.md | 3 ++ 8 files changed, 183 insertions(+), 1 deletion(-) create mode 100644 tests/_utils-tests/longtext.js create mode 100644 tests/_utils/longtext.js create mode 100644 tests/_utils/longtext.txt create mode 100644 tests/manual/diff/diff.html create mode 100644 tests/manual/diff/diff.js create mode 100644 tests/manual/diff/diff.md diff --git a/src/diff.js b/src/diff.js index 700afcc..b9b85d2 100644 --- a/src/diff.js +++ b/src/diff.js @@ -7,6 +7,8 @@ * @module utils/diff */ +import fastDiff from '../src/fastdiff'; + // The following code is based on the "O(NP) Sequence Comparison Algorithm" // by Sun Wu, Udi Manber, Gene Myers, Webb Miller. @@ -27,11 +29,19 @@ export default function diff( a, b, cmp ) { return a === b; }; + const aLength = a.length; + const bLength = b.length; + + // Perform `fastDiff` for longer strings/arrays. + if ( Math.min( aLength, bLength ) >= 400 && aLength + bLength >= 1300 || aLength + bLength >= 2000 ) { + return diff.fastDiff( a, b, cmp, true ); + } + // Temporary action type statics. let _insert, _delete; // Swapped the arrays to use the shorter one as the first one. - if ( b.length < a.length ) { + if ( bLength < aLength ) { const tmp = a; a = b; @@ -117,3 +127,7 @@ export default function diff( a, b, cmp ) { // We remove the first item that represents the action for the injected nulls. return es[ delta ].slice( 1 ); } + +// Store the API in static property to easily overwrite it in tests. +// Too bad dependency injection does not work in Webpack + ES 6 (const) + Babel. +diff.fastDiff = fastDiff; diff --git a/tests/_utils-tests/longtext.js b/tests/_utils-tests/longtext.js new file mode 100644 index 0000000..be6e957 --- /dev/null +++ b/tests/_utils-tests/longtext.js @@ -0,0 +1,46 @@ +/** + * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +import getLongText from '../../tests/_utils/longtext'; + +describe( 'utils', () => { + describe( 'getLongText', () => { + it( 'should return text with 0 length', () => { + expect( getLongText( 0 ).length ).to.equal( 0 ); + } ); + + it( 'should return text with 553 length', () => { + expect( getLongText( 553 ).length ).to.equal( 553 ); + } ); + + it( 'should return text with 1500 length', () => { + expect( getLongText( 1500 ).length ).to.equal( 1500 ); + } ); + + it( 'should return text with 4000 length', () => { + expect( getLongText( 4000 ).length ).to.equal( 4000 ); + } ); + + it( 'should return different text with fromStart=false', () => { + expect( getLongText( 100 ) ).to.not.equal( getLongText( 100, false ) ); + } ); + + it( 'should return reversed text', () => { + const text1 = getLongText( 100 ); + const text2 = getLongText( 100, true, true ); + + expect( text1 ).to.not.equal( text2 ); + expect( text1 ).to.equal( text2.split( '' ).reverse().join( '' ) ); + } ); + + it( 'should return reversed text (with fromStart=false)', () => { + const text1 = getLongText( 150, false ); + const text2 = getLongText( 150, false, true ); + + expect( text1 ).to.not.equal( text2 ); + expect( text1 ).to.equal( text2.split( '' ).reverse().join( '' ) ); + } ); + } ); +} ); diff --git a/tests/_utils/longtext.js b/tests/_utils/longtext.js new file mode 100644 index 0000000..113e709 --- /dev/null +++ b/tests/_utils/longtext.js @@ -0,0 +1,20 @@ +/** + * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +import longtext from './longtext.txt'; + +/** + * Returns text of a given length. + * + * @param {Number} length Length of the resulting text. + * @param {Boolean} fromStart Whether text should be extracted from the start (or end) of the template string. + * @param {Boolean} reversed Whether given text should be reversed. + * @returns {String} Text of a given length. + */ +export default function getLongText( length, fromStart = true, reversed = false ) { + const baseText = longtext.repeat( Math.ceil( length / longtext.length ) ); + const text = fromStart ? baseText.substring( 0, length ) : baseText.substring( longtext.length - length ); + return reversed ? text.split( '' ).reverse().join( '' ) : text; +} diff --git a/tests/_utils/longtext.txt b/tests/_utils/longtext.txt new file mode 100644 index 0000000..370aea4 --- /dev/null +++ b/tests/_utils/longtext.txt @@ -0,0 +1 @@ +Ut at iaculis turpis, ut interdum erat. Proin quis erat eu odio posuere feugiat. Proin nec dui ac mauris facilisis condimentum et at mi. Vestibulum ante ipsum primis in faucibus orci luctus et ultrices posuere cubilia Curae; Cras mi leo, tempor at ante eget, venenatis porttitor felis. Curabitur et velit mauris. Morbi blandit vestibulum leo. Donec sit amet nisi enim. Morbi viverra tortor arcu, et gravida orci venenatis et. Maecenas nec nulla ullamcorper, viverra ligula pulvinar, maximus dui. Nullam placerat vel tellus ac porttitor. Cras a mi risus. Nullam dictum sem leo, vel blandit magna pharetra et. Vestibulum scelerisque, ex vel vehicula pellentesque, lacus arcu mollis erat, a suscipit odio nulla et leo. Sed feugiat suscipit quam at rhoncus. Nulla luctus mi vitae bibendum venenatis. Suspendisse accumsan dui et consectetur maximus. Vivamus finibus tempus aliquam. Mauris enim est, tincidunt a eros nec, rhoncus mattis ipsum. Ut at eros pretium, aliquam lectus a, commodo augue. Aliquam ac diam id tellus dapibus egestas at at lorem. Nam elementum nisi non nisl condimentum, eu interdum nibh lobortis. Sed laoreet porttitor enim ut scelerisque. Fusce laoreet, massa at lobortis dictum, dolor felis bibendum ante, nec pulvinar est est iaculis nibh. Aenean a massa eros. Apulvinar, maximus dui. Nullam placerat vel tellus ac porttitor. Cras a mi risus. Nullam dictum sem leo, vel blandit magna pharetra et. Vestibulum scelerisque, ex vel vehicula pellentesque, lacus arcu mollis erat, a suscipit odio nulla et leo. Sed feugiat suscipit quam at rhoncus. Nulla luctus mi vitae bibendum venenati. diff --git a/tests/diff.js b/tests/diff.js index 7cf2250..6f7c719 100644 --- a/tests/diff.js +++ b/tests/diff.js @@ -5,29 +5,66 @@ import diff from '../src/diff'; +import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; +import getLongText from './_utils/longtext'; + describe( 'diff', () => { + let fastDiffSpy; + + testUtils.createSinonSandbox(); + + beforeEach( () => { + fastDiffSpy = testUtils.sinon.spy( diff, 'fastDiff' ); + } ); + it( 'should diff strings', () => { expect( diff( 'aba', 'acca' ) ).to.deep.equals( [ 'equal', 'insert', 'insert', 'delete', 'equal' ] ); + testUtils.sinon.assert.notCalled( fastDiffSpy ); } ); it( 'should diff arrays', () => { expect( diff( Array.from( 'aba' ), Array.from( 'acca' ) ) ).to.deep.equals( [ 'equal', 'insert', 'insert', 'delete', 'equal' ] ); + testUtils.sinon.assert.notCalled( fastDiffSpy ); } ); it( 'should reverse result if the second string is shorter', () => { expect( diff( 'acca', 'aba' ) ).to.deep.equals( [ 'equal', 'delete', 'delete', 'insert', 'equal' ] ); + testUtils.sinon.assert.notCalled( fastDiffSpy ); } ); it( 'should diff if strings are same', () => { expect( diff( 'abc', 'abc' ) ).to.deep.equals( [ 'equal', 'equal', 'equal' ] ); + testUtils.sinon.assert.notCalled( fastDiffSpy ); } ); it( 'should diff if one string is empty', () => { expect( diff( '', 'abc' ) ).to.deep.equals( [ 'insert', 'insert', 'insert' ] ); + testUtils.sinon.assert.notCalled( fastDiffSpy ); } ); it( 'should use custom comparator', () => { expect( diff( 'aBc', 'abc' ) ).to.deep.equals( [ 'equal', 'insert', 'delete', 'equal' ] ); expect( diff( 'aBc', 'abc', ( a, b ) => a.toLowerCase() == b.toLowerCase() ) ).to.deep.equals( [ 'equal', 'equal', 'equal' ] ); + testUtils.sinon.assert.notCalled( fastDiffSpy ); + } ); + + it( 'should use fastDiff() internally for strings with 400+ length and length sum of 1400+', () => { + diff( getLongText( 400 ), getLongText( 1000 ) ); + testUtils.sinon.assert.called( fastDiffSpy ); + } ); + + it( 'should use fastDiff() internally for arrays with 400+ length and length sum of 1400+', () => { + diff( getLongText( 500 ).split( '' ), getLongText( 950 ).split( '' ) ); + testUtils.sinon.assert.called( fastDiffSpy ); + } ); + + it( 'should use fastDiff() internally for strings with length sum of 2000+', () => { + diff( getLongText( 100 ), getLongText( 2000 ) ); + testUtils.sinon.assert.called( fastDiffSpy ); + } ); + + it( 'should use fastDiff() internally for strings with length sum of 2000+', () => { + diff( getLongText( 10 ), getLongText( 1990 ) ); + testUtils.sinon.assert.called( fastDiffSpy ); } ); } ); diff --git a/tests/manual/diff/diff.html b/tests/manual/diff/diff.html new file mode 100644 index 0000000..59d9d62 --- /dev/null +++ b/tests/manual/diff/diff.html @@ -0,0 +1 @@ +See browser dev console output. diff --git a/tests/manual/diff/diff.js b/tests/manual/diff/diff.js new file mode 100644 index 0000000..c534922 --- /dev/null +++ b/tests/manual/diff/diff.js @@ -0,0 +1,60 @@ +/** + * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/* global console, setTimeout */ + +import diff from '../../../src/diff'; +import getLongText from '../../_utils/longtext'; + +// Tests +setTimeout( () => { + console.log( 'Testing... (t1 length - t2 length - avg time - times)' ); + + execTime( getLongText( 300 ), getLongText( 700, false, true ) ); + execTime( getLongText( 350 ), getLongText( 700, false, true ) ); + execTime( getLongText( 400 ), getLongText( 700, false, true ) ); + execTime( getLongText( 450 ), getLongText( 700, false, true ) ); + + execTime( getLongText( 300 ), getLongText( 800, false, true ) ); + execTime( getLongText( 350 ), getLongText( 800, false, true ) ); + execTime( getLongText( 400 ), getLongText( 800, false, true ) ); + execTime( getLongText( 450 ), getLongText( 800, false, true ) ); + + execTime( getLongText( 300 ), getLongText( 900, false, true ) ); + execTime( getLongText( 350 ), getLongText( 900, false, true ) ); + execTime( getLongText( 400 ), getLongText( 900, false, true ) ); + execTime( getLongText( 450 ), getLongText( 900, false, true ) ); + + execTime( getLongText( 300 ), getLongText( 1000, false, true ) ); + execTime( getLongText( 350 ), getLongText( 1000, false, true ) ); + execTime( getLongText( 400 ), getLongText( 1000, false, true ) ); + execTime( getLongText( 450 ), getLongText( 1000, false, true ) ); + + execTime( getLongText( 300 ), getLongText( 1200, false, true ) ); + execTime( getLongText( 350 ), getLongText( 1200, false, true ) ); + execTime( getLongText( 400 ), getLongText( 1200, false, true ) ); + execTime( getLongText( 450 ), getLongText( 1200, false, true ) ); + + execTime( getLongText( 300 ), getLongText( 1500, false, true ) ); + execTime( getLongText( 350 ), getLongText( 1500, false, true ) ); + execTime( getLongText( 400 ), getLongText( 1500, false, true ) ); + execTime( getLongText( 450 ), getLongText( 1500, false, true ) ); +}, 500 ); + +// Helpers +function execTime( text1, text2 ) { + const times = []; + + for ( let i = 0; i < 15; i++ ) { + const start = Number( new Date() ); + + diff( text1, text2 ); + + times.push( Number( new Date() ) - start ); + } + + console.log( 'l1: ' + text1.length, 'l2: ' + text2.length, + 'avg: ' + Math.round( times.reduce( ( a, b ) => a + b, 0 ) / times.length ) + 'ms', times ); +} diff --git a/tests/manual/diff/diff.md b/tests/manual/diff/diff.md new file mode 100644 index 0000000..82c1e98 --- /dev/null +++ b/tests/manual/diff/diff.md @@ -0,0 +1,3 @@ +## `diff()` + +Checks `diff()` performance. From 2263658374b3fb47c33f263581bb44929ab07b39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Mon, 11 Feb 2019 14:08:12 +0100 Subject: [PATCH 3/7] `fastDiff()` function improvements. --- src/fastdiff.js | 10 +++++----- tests/fastdiff.js | 31 +++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/src/fastdiff.js b/src/fastdiff.js index d9fb43b..e8aff96 100644 --- a/src/fastdiff.js +++ b/src/fastdiff.js @@ -86,13 +86,13 @@ export default function fastDiff( a, b, cmp, linearChanges = false ) { return a === b; }; - // Transform text into arrays for easier, consistent processing. - if ( typeof a === 'string' ) { - a = a.split( '' ); + // Transform text or any iterable into arrays for easier, consistent processing. + if ( !Array.isArray( a ) ) { + a = Array.from( a ); } - if ( typeof b === 'string' ) { - b = b.split( '' ); + if ( !Array.isArray( b ) ) { + b = Array.from( b ); } // Find first and last change. diff --git a/tests/fastdiff.js b/tests/fastdiff.js index 37cfc88..a7aeef1 100644 --- a/tests/fastdiff.js +++ b/tests/fastdiff.js @@ -3,11 +3,42 @@ * For licensing, see LICENSE.md. */ +/* global document */ + import fastDiff from '../src/fastdiff'; import diff from '../src/diff'; import diffToChanges from '../src/difftochanges'; describe( 'fastDiff', () => { + describe( 'input types', () => { + it( 'should correctly handle strings', () => { + const changes = fastDiff( '123', 'abc123' ); + expect( changes ).to.deep.equal( [ { index: 0, type: 'insert', values: [ 'a', 'b', 'c' ] } ] ); + } ); + + it( 'should correctly handle arrays', () => { + const changes = fastDiff( [ '1', '2', '3' ], [ 'a', 'b', 'c', '1', '2', '3' ] ); + expect( changes ).to.deep.equal( [ { index: 0, type: 'insert', values: [ 'a', 'b', 'c' ] } ] ); + } ); + + it( 'should correctly handle node lists', () => { + const el1 = document.createElement( 'p' ); + const el2 = document.createElement( 'h1' ); + + el1.appendChild( document.createElement( 'span' ) ); + el1.appendChild( document.createElement( 'strong' ) ); + + el2.appendChild( document.createElement( 'div' ) ); + el2.appendChild( document.createElement( 'strong' ) ); + + const changes = fastDiff( el1.childNodes, el2.childNodes ); + expect( changes ).to.deep.equal( [ + { index: 0, type: 'insert', values: [ el2.childNodes[ 0 ], el2.childNodes[ 1 ] ] }, + { index: 2, type: 'delete', howMany: 2 } + ] ); + } ); + } ); + describe( 'changes object', () => { it( 'should diff identical texts', () => { expectDiff( '123', '123', [] ); From b857007341c8527f777c4ce82fcb7d4786fb9321 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Mon, 11 Feb 2019 14:14:26 +0100 Subject: [PATCH 4/7] Docs: Rewording. [skip ci] --- src/diff.js | 2 +- src/fastdiff.js | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/diff.js b/src/diff.js index b9b85d2..98ac5fb 100644 --- a/src/diff.js +++ b/src/diff.js @@ -32,7 +32,7 @@ export default function diff( a, b, cmp ) { const aLength = a.length; const bLength = b.length; - // Perform `fastDiff` for longer strings/arrays. + // Perform `fastDiff` for longer strings/arrays (see #269). if ( Math.min( aLength, bLength ) >= 400 && aLength + bLength >= 1300 || aLength + bLength >= 2000 ) { return diff.fastDiff( a, b, cmp, true ); } diff --git a/src/fastdiff.js b/src/fastdiff.js index e8aff96..7d8381c 100644 --- a/src/fastdiff.js +++ b/src/fastdiff.js @@ -77,7 +77,8 @@ * @param {Array|String} a Input array or string. * @param {Array|String} b Input array or string. * @param {Function} [cmp] Optional function used to compare array values, by default === is used. - * @param {Boolean} [linearChanges=false] Whether array of `inset|delete|equal` operations should be returned or changes set. + * @param {Boolean} [linearChanges=false] Whether array of `inset|delete|equal` operations should + * be returned instead of changes set. * @returns {Array} Array of changes. */ export default function fastDiff( a, b, cmp, linearChanges = false ) { From 81bfe2da6cbe4146fdd7b881e8c6c11c2cbf0ee2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Tue, 12 Feb 2019 13:51:06 +0100 Subject: [PATCH 5/7] Smaller threshold for `fastDiff()`. --- src/diff.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/diff.js b/src/diff.js index 98ac5fb..da412e8 100644 --- a/src/diff.js +++ b/src/diff.js @@ -33,7 +33,7 @@ export default function diff( a, b, cmp ) { const bLength = b.length; // Perform `fastDiff` for longer strings/arrays (see #269). - if ( Math.min( aLength, bLength ) >= 400 && aLength + bLength >= 1300 || aLength + bLength >= 2000 ) { + if ( aLength > 200 || bLength > 200 || aLength + bLength > 300 ) { return diff.fastDiff( a, b, cmp, true ); } From 484c07bac383906b2d4c48a5c3ef19fa76cdbe49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Tue, 12 Feb 2019 13:53:30 +0100 Subject: [PATCH 6/7] Naming and docs improvements in `fastDiff()`. --- src/fastdiff.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/fastdiff.js b/src/fastdiff.js index 7d8381c..4d7b390 100644 --- a/src/fastdiff.js +++ b/src/fastdiff.js @@ -129,8 +129,8 @@ function findChangeBoundaryIndexes( arr1, arr2, cmp ) { } // Remove the common part of each value and reverse them to make it simpler to find the last difference between them. - const oldTextReversed = cutAndReverse( arr1, firstIndex ); - const newTextReversed = cutAndReverse( arr2, firstIndex ); + const oldArrayReversed = cutAndReverse( arr1, firstIndex ); + const newArrayReversed = cutAndReverse( arr2, firstIndex ); // Find the first difference between reversed values. // It should be treated as "how many elements from the end the last difference occurred". @@ -143,7 +143,7 @@ function findChangeBoundaryIndexes( arr1, arr2, cmp ) { // lastIndex: -> 2 // // So the last change occurred two characters from the end of the arrays. - const lastIndex = findFirstDifferenceIndex( oldTextReversed, newTextReversed, cmp ); + const lastIndex = findFirstDifferenceIndex( oldArrayReversed, newArrayReversed, cmp ); // Use `lastIndex` to calculate proper offset, starting from the beginning (`lastIndex` kind of starts from the end). const lastIndexOld = arr1.length - lastIndex; @@ -171,7 +171,7 @@ function findFirstDifferenceIndex( arr1, arr2, cmp ) { // Returns a copy of the given array with `howMany` elements removed starting from the beginning and in reversed order. // // @param {Array} arr Array to be processed. -// @param {Number} howMany How many elements from text beginning to remove. +// @param {Number} howMany How many elements from array beginning to remove. // @returns {Array} Shortened and reversed array. function cutAndReverse( arr, howMany ) { return arr.slice( howMany ).reverse(); @@ -212,7 +212,7 @@ function changeIndexesToChanges( newArray, changeIndexes ) { // Generates array with set `equal|insert|delete` operations based on change indexes from `findChangeBoundaryIndexes` function. // // @param {Object} changeIndexes Change indexes object from `findChangeBoundaryIndexes` function. -// @param {Number} newLength Length of the new array/string on which `findChangeBoundaryIndexes` calculated change indexes. +// @param {Number} newLength Length of the new array on which `findChangeBoundaryIndexes` calculated change indexes. // @returns {Array.} Array of changes compatible with {@link module:utils/diff~diff} format. function changeIndexesToLinearChanges( changeIndexes, newLength ) { const { firstIndex, lastIndexOld, lastIndexNew } = changeIndexes; From a99c73486638596c4d089e62292f6030fe584446 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Wed, 13 Feb 2019 16:19:30 +0100 Subject: [PATCH 7/7] Docs. --- src/diff.js | 5 ++++ src/fastdiff.js | 61 ++++++++++++++++++++++++++++++------------------- 2 files changed, 42 insertions(+), 24 deletions(-) diff --git a/src/diff.js b/src/diff.js index da412e8..7421fb6 100644 --- a/src/diff.js +++ b/src/diff.js @@ -18,6 +18,11 @@ import fastDiff from '../src/fastdiff'; * * diff( 'aba', 'acca' ); // [ 'equal', 'insert', 'insert', 'delete', 'equal' ] * + * This function is based on the "O(NP) Sequence Comparison Algorithm" by Sun Wu, Udi Manber, Gene Myers, Webb Miller. + * Unfortunately, while it gives the most precise results, its to complex for longer strings/arrow (above 200 items). + * Therefore, `diff()` automatically switches to {@link module:utils/fastdiff~fastDiff `fastDiff()`} when detecting + * such a scenario. The return formats of both functions are identical. + * * @param {Array|String} a Input array or string. * @param {Array|String} b Output array or string. * @param {Function} [cmp] Optional function used to compare array values, by default === is used. diff --git a/src/fastdiff.js b/src/fastdiff.js index 4d7b390..73e42c7 100644 --- a/src/fastdiff.js +++ b/src/fastdiff.js @@ -8,7 +8,7 @@ */ /** - * Finds position of the first and last change in the given string/array and generates a set of changes: + * Finds positions of the first and last change in the given string/array and generates a set of changes: * * fastDiff( '12a', '12xyza' ); * // [ { index: 2, type: 'insert', values: [ 'x', 'y', 'z' ] } ] @@ -28,22 +28,16 @@ * Passed arrays can contain any type of data, however to compare them correctly custom comparator function * should be passed as a third parameter: * - * fastDiff( [ { value: 1 }, { value: 2 } ], [ { value: 1 }, { value: 3 } ], ( a, b ) => { - * return a.value === b.value; - * } ); - * // [ { index: 1, type: 'insert', values: [ { value: 3 } ] }, { index: 2, type: 'delete', howMany: 1 } ] - * - * By passing `true` as a fourth parameter (`linearChanges`) the output compatible with - * {@link module:utils/diff~diff diff()} function will be returned: - * - * fastDiff( '12a', '12xyza' ); - * // [ 'equal', 'equal', 'insert', 'insert', 'insert', 'equal' ] + * fastDiff( [ { value: 1 }, { value: 2 } ], [ { value: 1 }, { value: 3 } ], ( a, b ) => { + * return a.value === b.value; + * } ); + * // [ { index: 1, type: 'insert', values: [ { value: 3 } ] }, { index: 2, type: 'delete', howMany: 1 } ] * * The resulted set of changes can be applied to the input in order to transform it into the output, for example: * - * let input = '12abc3'; - * const output = '2ab'; - * const changes = fastDiff( input, output ); + * let input = '12abc3'; + * const output = '2ab'; + * const changes = fastDiff( input, output ); * * changes.forEach( change => { * if ( change.type == 'insert' ) { @@ -58,8 +52,8 @@ * or in case of arrays: * * let input = [ '1', '2', 'a', 'b', 'c', '3' ]; - * const output = [ '2', 'a', 'b' ]; - * const changes = fastDiff( input, output ); + * const output = [ '2', 'a', 'b' ]; + * const changes = fastDiff( input, output ); * * changes.forEach( change => { * if ( change.type == 'insert' ) { @@ -71,17 +65,36 @@ * * // input equals output now * - * The output format of this function is compatible with {@link module:utils/difftochanges~diffToChanges diffToChanges()} - * function output format. + * By passing `true` as the fourth parameter (`atomicChanges`) the output of this function will become compatible with + * the {@link module:utils/diff~diff `diff()`} function: + * + * fastDiff( '12a', '12xyza' ); + * // [ 'equal', 'equal', 'insert', 'insert', 'insert', 'equal' ] + * + * The default output format of this function is compatible with the output format of + * {@link module:utils/difftochanges~diffToChanges `diffToChanges()`}. The `diffToChanges()` input format is, in turn, + * compatible with the output of {@link module:utils/diff~diff `diff()`}: + * + * const a = '1234'; + * const b = '12xyz34'; + * + * // Both calls will return the same results (grouped changes format). + * fastDiff( a, b ); + * diffToChanges( diff( a, b ) ); + * + * // Again, both calls will return the same results (atomic changes format). + * fastDiff( a, b, null, true ); + * diff( a, b ); + * * * @param {Array|String} a Input array or string. * @param {Array|String} b Input array or string. - * @param {Function} [cmp] Optional function used to compare array values, by default === is used. - * @param {Boolean} [linearChanges=false] Whether array of `inset|delete|equal` operations should - * be returned instead of changes set. + * @param {Function} [cmp] Optional function used to compare array values, by default `===` (strict equal operator) is used. + * @param {Boolean} [atomicChanges=false] Whether an array of `inset|delete|equal` operations should + * be returned instead of changes set. This makes this function compatible with {@link module:utils/diff~diff `diff()`}. * @returns {Array} Array of changes. */ -export default function fastDiff( a, b, cmp, linearChanges = false ) { +export default function fastDiff( a, b, cmp, atomicChanges = false ) { // Set the comparator function. cmp = cmp || function( a, b ) { return a === b; @@ -100,7 +113,7 @@ export default function fastDiff( a, b, cmp, linearChanges = false ) { const changeIndexes = findChangeBoundaryIndexes( a, b, cmp ); // Transform into changes array. - return linearChanges ? changeIndexesToLinearChanges( changeIndexes, b.length ) : changeIndexesToChanges( b, changeIndexes ); + return atomicChanges ? changeIndexesToAtomicChanges( changeIndexes, b.length ) : changeIndexesToChanges( b, changeIndexes ); } // Finds position of the first and last change in the given arrays. For example: @@ -214,7 +227,7 @@ function changeIndexesToChanges( newArray, changeIndexes ) { // @param {Object} changeIndexes Change indexes object from `findChangeBoundaryIndexes` function. // @param {Number} newLength Length of the new array on which `findChangeBoundaryIndexes` calculated change indexes. // @returns {Array.} Array of changes compatible with {@link module:utils/diff~diff} format. -function changeIndexesToLinearChanges( changeIndexes, newLength ) { +function changeIndexesToAtomicChanges( changeIndexes, newLength ) { const { firstIndex, lastIndexOld, lastIndexNew } = changeIndexes; // No changes.