Skip to content

Commit 1e79116

Browse files
Fix order of arguments to .equals and comparator (#467)
* Ensure the left & right arguments to equals() always correspond to old and new values respectively * Add test * Add release notes
1 parent e6c45b0 commit 1e79116

File tree

3 files changed

+25
-6
lines changed

3 files changed

+25
-6
lines changed

release-notes.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88
- [#439](https://github.com/kpdecker/jsdiff/pull/439) Prefer diffs that order deletions before insertions. When faced with a choice between two diffs with an equal total edit distance, the Myers diff algorithm generally prefers one that does deletions before insertions rather than insertions before deletions. For instance, when diffing `abcd` against `acbd`, it will prefer a diff that says to delete the `b` and then insert a new `b` after the `c`, over a diff that says to insert a `c` before the `b` and then delete the existing `c`. JsDiff deviated from the published Myers algorithm in a way that led to it having the opposite preference in many cases, including that example. This is now fixed, meaning diffs output by JsDiff will more accurately reflect what the published Myers diff algorithm would output.
99
- [#455](https://github.com/kpdecker/jsdiff/pull/455) The `added` and `removed` properties of change objects are now guaranteed to be set to a boolean value. (Previously, they would be set to `undefined` or omitted entirely instead of setting them to false.)
1010
- [#464](https://github.com/kpdecker/jsdiff/pull/464) Specifying `{maxEditLength: 0}` now sets a max edit length of 0 instead of no maximum.
11-
- [#460][https://github.com/kpdecker/jsdiff/pull/460] Added `oneChangePerToken` option.
11+
- [#460](https://github.com/kpdecker/jsdiff/pull/460) Added `oneChangePerToken` option.
12+
- [#467](https://github.com/kpdecker/jsdiff/pull/467) When passing a `comparator(left, right)` to `diffArrays`, values from the old array will now consistently be passed as the first argument (`left`) and values from the new array as the second argument (`right`). Previously this was almost (but not quite) always the other way round.
1213

1314
## Development
1415

src/diff/base.js

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ Diff.prototype = {
166166
newPos = oldPos - diagonalPath,
167167

168168
commonCount = 0;
169-
while (newPos + 1 < newLen && oldPos + 1 < oldLen && this.equals(newString[newPos + 1], oldString[oldPos + 1])) {
169+
while (newPos + 1 < newLen && oldPos + 1 < oldLen && this.equals(oldString[oldPos + 1], newString[newPos + 1])) {
170170
newPos++;
171171
oldPos++;
172172
commonCount++;
@@ -259,10 +259,15 @@ function buildValues(diff, lastComponent, newString, oldString, useLongestToken)
259259
// For this case we merge the terminal into the prior string and drop the change.
260260
// This is only available for string mode.
261261
let finalComponent = components[componentLen - 1];
262-
if (componentLen > 1
263-
&& typeof finalComponent.value === 'string'
264-
&& (finalComponent.added || finalComponent.removed)
265-
&& diff.equals('', finalComponent.value)) {
262+
if (
263+
componentLen > 1
264+
&& typeof finalComponent.value === 'string'
265+
&& (
266+
(finalComponent.added && diff.equals('', finalComponent.value))
267+
||
268+
(finalComponent.removed && diff.equals(finalComponent.value, ''))
269+
)
270+
) {
266271
components[componentLen - 2].value += finalComponent.value;
267272
components.pop();
268273
}

test/diff/array.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,5 +75,18 @@ describe('diff/array', function() {
7575
{count: 1, value: [d], removed: false, added: true}
7676
]);
7777
});
78+
it('Should pass old/new tokens as the left/right comparator args respectively', function() {
79+
diffArrays(
80+
['a', 'b', 'c'],
81+
['x', 'y', 'z'],
82+
{
83+
comparator: function(left, right) {
84+
expect(left).to.be.oneOf(['a', 'b', 'c']);
85+
expect(right).to.be.oneOf(['x', 'y', 'z']);
86+
return left === right;
87+
}
88+
}
89+
);
90+
});
7891
});
7992
});

0 commit comments

Comments
 (0)