Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix order of arguments to .equals and comparator #467

Merged
merged 3 commits into from
Jan 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion release-notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
- [#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.
- [#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.)
- [#464](https://github.com/kpdecker/jsdiff/pull/464) Specifying `{maxEditLength: 0}` now sets a max edit length of 0 instead of no maximum.
- [#460][https://github.com/kpdecker/jsdiff/pull/460] Added `oneChangePerToken` option.
- [#460](https://github.com/kpdecker/jsdiff/pull/460) Added `oneChangePerToken` option.
- [#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.

## Development

Expand Down
15 changes: 10 additions & 5 deletions src/diff/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ Diff.prototype = {
newPos = oldPos - diagonalPath,

commonCount = 0;
while (newPos + 1 < newLen && oldPos + 1 < oldLen && this.equals(newString[newPos + 1], oldString[oldPos + 1])) {
while (newPos + 1 < newLen && oldPos + 1 < oldLen && this.equals(oldString[oldPos + 1], newString[newPos + 1])) {
newPos++;
oldPos++;
commonCount++;
Expand Down Expand Up @@ -259,10 +259,15 @@ function buildValues(diff, lastComponent, newString, oldString, useLongestToken)
// For this case we merge the terminal into the prior string and drop the change.
// This is only available for string mode.
let finalComponent = components[componentLen - 1];
if (componentLen > 1
&& typeof finalComponent.value === 'string'
&& (finalComponent.added || finalComponent.removed)
&& diff.equals('', finalComponent.value)) {
if (
componentLen > 1
&& typeof finalComponent.value === 'string'
&& (
(finalComponent.added && diff.equals('', finalComponent.value))
||
(finalComponent.removed && diff.equals(finalComponent.value, ''))
)
) {
components[componentLen - 2].value += finalComponent.value;
components.pop();
}
Expand Down
13 changes: 13 additions & 0 deletions test/diff/array.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,5 +75,18 @@ describe('diff/array', function() {
{count: 1, value: [d], removed: false, added: true}
]);
});
it('Should pass old/new tokens as the left/right comparator args respectively', function() {
diffArrays(
['a', 'b', 'c'],
['x', 'y', 'z'],
{
comparator: function(left, right) {
expect(left).to.be.oneOf(['a', 'b', 'c']);
expect(right).to.be.oneOf(['x', 'y', 'z']);
return left === right;
}
}
);
});
});
});