Skip to content

Commit

Permalink
Always set added and removed to true or false, rather than le…
Browse files Browse the repository at this point in the history 
…aving them unset or explicitly using `undefined` (#455)

* Always set `added` and `removed` to `true` or `false`, rather than leaving them unset or explicitly using `undefined`

Resolves #233

* Add release notes
  • Loading branch information
@ExplodingCabbage
ExplodingCabbage authored Jan 2, 2024
1 parent 3351c82 commit 3a99253
Show file tree
Hide file tree
Showing 7 changed files with 79 additions and 80 deletions.
6 changes: 2 additions & 4 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,10 +145,8 @@ All methods above which accept the optional `callback` method will run in sync m
Many of the methods above return change objects. These objects consist of the following fields:
* `value`: Text content
* `added`: True if the value was inserted into the new string
* `removed`: True if the value was removed from the old string
Note that some cases may omit a particular flag field. Comparison on the flag fields should always be done in a truthy or falsy manner.
* `added`: true if the value was inserted into the new string, otherwise false
* `removed`: true if the value was removed from the old string, otherwise false
## Examples
Expand Down
1 change: 1 addition & 0 deletions release-notes.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

- [#435](https://github.com/kpdecker/jsdiff/pull/435) Fix `parsePatch` handling of control characters. `parsePatch` used to interpret various unusual control characters - namely vertical tabs, form feeds, lone carriage returns without a line feed, and EBCDIC NELs - as line breaks when parsing a patch file. This was inconsistent with the behavior of both JsDiff's own `diffLines` method and also the Unix `diff` and `patch` utils, which all simply treat those control characters as ordinary characters. The result of this discrepancy was that some well-formed patches - produced either by `diff` or by JsDiff itself and handled properly by the `patch` util - would be wrongly parsed by `parsePatch`, with the effect that it would disregard the remainder of a hunk after encountering one of these control characters.
- [#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.)

## Development

Expand Down
8 changes: 4 additions & 4 deletions src/diff/base.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ Diff.prototype = {
let newPos = this.extractCommon(bestPath[0], newString, oldString, 0);
if (bestPath[0].oldPos + 1 >= oldLen && newPos + 1 >= newLen) {
// Identity per the equality and tokenizer
return done([{value: this.join(newString), count: newString.length}]);
return done([{value: this.join(newString), count: newString.length, added: false, removed: false}]);
}

// Once we hit the right edge of the edit graph on some diagonal k, we can
Expand Down Expand Up @@ -95,9 +95,9 @@ Diff.prototype = {
// path whose position in the old string is the farthest from the origin
// and does not pass the bounds of the diff graph
if (!canRemove || (canAdd && removePath.oldPos < addPath.oldPos)) {
basePath = self.addToPath(addPath, true, undefined, 0);
basePath = self.addToPath(addPath, true, false, 0);
} else {
basePath = self.addToPath(removePath, undefined, true, 1);
basePath = self.addToPath(removePath, false, true, 1);
}

newPos = self.extractCommon(basePath, newString, oldString, diagonalPath);
Expand Down Expand Up @@ -173,7 +173,7 @@ Diff.prototype = {
}

if (commonCount) {
basePath.lastComponent = {count: commonCount, previousComponent: basePath.lastComponent};
basePath.lastComponent = {count: commonCount, previousComponent: basePath.lastComponent, added: false, removed: false};
}

basePath.oldPos = oldPos;
Expand Down
30 changes: 15 additions & 15 deletions test/diff/array.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ describe('diff/array', function() {
const diffResult = diffArrays([a, b, c], [a, c, b]);
console.log(diffResult);
expect(diffResult).to.deep.equals([
{count: 1, value: [a]},
{count: 1, value: [b], removed: true, added: undefined},
{count: 1, value: [c]},
{count: 1, value: [b], removed: undefined, added: true}
{count: 1, value: [a], removed: false, added: false},
{count: 1, value: [b], removed: true, added: false},
{count: 1, value: [c], removed: false, added: false},
{count: 1, value: [b], removed: false, added: true}
]);
});
it('should diff falsey values', function() {
Expand All @@ -24,20 +24,20 @@ describe('diff/array', function() {
const arrayB = [c, b, a, b, a, c];
const diffResult = diffArrays(arrayA, arrayB);
expect(diffResult).to.deep.equals([
{count: 2, value: [a, b], removed: true, added: undefined},
{count: 1, value: [c]},
{count: 1, value: [b], removed: undefined, added: true},
{count: 2, value: [a, b]},
{count: 1, value: [b], removed: true, added: undefined},
{count: 1, value: [a]},
{count: 1, value: [c], removed: undefined, added: true}
{count: 2, value: [a, b], removed: true, added: false},
{count: 1, value: [c], removed: false, added: false},
{count: 1, value: [b], removed: false, added: true},
{count: 2, value: [a, b], removed: false, added: false},
{count: 1, value: [b], removed: true, added: false},
{count: 1, value: [a], removed: false, added: false},
{count: 1, value: [c], removed: false, added: true}
]);
});
describe('anti-aliasing', function() {
// Test apparent contract that no chunk value is ever an input argument.
const value = [0, 1, 2];
const expected = [
{count: value.length, value: value}
{count: value.length, value: value, removed: false, added: false}
];

const input = value.slice();
Expand Down Expand Up @@ -70,9 +70,9 @@ describe('diff/array', function() {
const diffResult = diffArrays([a, b, c], [a, b, d], { comparator: comparator });
console.log(diffResult);
expect(diffResult).to.deep.equals([
{count: 2, value: [a, b]},
{count: 1, value: [c], removed: true, added: undefined},
{count: 1, value: [d], removed: undefined, added: true}
{count: 2, value: [a, b], removed: false, added: false},
{count: 1, value: [c], removed: true, added: false},
{count: 1, value: [d], removed: false, added: true}
]);
});
});
Expand Down
82 changes: 41 additions & 41 deletions test/diff/json.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ describe('diff/json', function() {
{a: 123, b: 456, c: 789},
{a: 123, b: 456}
)).to.eql([
{ count: 3, value: '{\n "a": 123,\n "b": 456,\n' },
{ count: 1, value: ' "c": 789\n', added: undefined, removed: true },
{ count: 1, value: '}' }
{ count: 3, value: '{\n "a": 123,\n "b": 456,\n', removed: false, added: false },
{ count: 1, value: ' "c": 789\n', added: false, removed: true },
{ count: 1, value: '}', removed: false, added: false }
]);
});

Expand All @@ -21,9 +21,9 @@ describe('diff/json', function() {
{a: 123, b: 456, c: 789},
{b: 456, a: 123}
)).to.eql([
{ count: 3, value: '{\n "a": 123,\n "b": 456,\n' },
{ count: 1, value: ' "c": 789\n', added: undefined, removed: true },
{ count: 1, value: '}' }
{ count: 3, value: '{\n "a": 123,\n "b": 456,\n', removed: false, added: false },
{ count: 1, value: ' "c": 789\n', added: false, removed: true },
{ count: 1, value: '}', removed: false, added: false }
]);
});

Expand All @@ -32,9 +32,9 @@ describe('diff/json', function() {
{a: 123, b: 456, c: [1, 2, {foo: 'bar'}, 4]},
{a: 123, b: 456, c: [1, {foo: 'bar'}, 4]}
)).to.eql([
{ count: 5, value: '{\n "a": 123,\n "b": 456,\n "c": [\n 1,\n' },
{ count: 1, value: ' 2,\n', added: undefined, removed: true },
{ count: 6, value: ' {\n "foo": "bar"\n },\n 4\n ]\n}' }
{ count: 5, value: '{\n "a": 123,\n "b": 456,\n "c": [\n 1,\n', removed: false, added: false },
{ count: 1, value: ' 2,\n', added: false, removed: true },
{ count: 6, value: ' {\n "foo": "bar"\n },\n 4\n ]\n}', removed: false, added: false }
]);
});

Expand All @@ -43,12 +43,12 @@ describe('diff/json', function() {
{a: new Date(123), b: new Date(456), c: new Date(789)},
{a: new Date(124), b: new Date(456)}
)).to.eql([
{ count: 1, value: '{\n' },
{ count: 1, value: ' "a": "1970-01-01T00:00:00.123Z",\n', added: undefined, removed: true },
{ count: 1, value: ' "a": "1970-01-01T00:00:00.124Z",\n', added: true, removed: undefined },
{ count: 1, value: ' "b": "1970-01-01T00:00:00.456Z",\n' },
{ count: 1, value: ' "c": "1970-01-01T00:00:00.789Z"\n', added: undefined, removed: true },
{ count: 1, value: '}' }
{ count: 1, value: '{\n', removed: false, added: false },
{ count: 1, value: ' "a": "1970-01-01T00:00:00.123Z",\n', added: false, removed: true },
{ count: 1, value: ' "a": "1970-01-01T00:00:00.124Z",\n', added: true, removed: false },
{ count: 1, value: ' "b": "1970-01-01T00:00:00.456Z",\n', removed: false, added: false },
{ count: 1, value: ' "c": "1970-01-01T00:00:00.789Z"\n', added: false, removed: true },
{ count: 1, value: '}', removed: false, added: false }
]);
});

Expand All @@ -57,24 +57,24 @@ describe('diff/json', function() {
{a: 123, b: 456, c: null},
{a: 123, b: 456}
)).to.eql([
{ count: 3, value: '{\n "a": 123,\n "b": 456,\n' },
{ count: 1, value: ' "c": null\n', added: undefined, removed: true },
{ count: 1, value: '}' }
{ count: 3, value: '{\n "a": 123,\n "b": 456,\n', removed: false, added: false },
{ count: 1, value: ' "c": null\n', added: false, removed: true },
{ count: 1, value: '}', removed: false, added: false }
]);
expect(diffJson(
{a: 123, b: 456, c: undefined},
{a: 123, b: 456}
)).to.eql([
{ count: 4, value: '{\n "a": 123,\n "b": 456\n}' }
{ count: 4, value: '{\n "a": 123,\n "b": 456\n}', removed: false, added: false }
]);
expect(diffJson(
{a: 123, b: 456, c: undefined},
{a: 123, b: 456},
{undefinedReplacement: null}
)).to.eql([
{ count: 3, value: '{\n "a": 123,\n "b": 456,\n' },
{ count: 1, value: ' "c": null\n', added: undefined, removed: true },
{ count: 1, value: '}' }
{ count: 3, value: '{\n "a": 123,\n "b": 456,\n', removed: false, added: false },
{ count: 1, value: ' "c": null\n', added: false, removed: true },
{ count: 1, value: '}', removed: false, added: false }
]);
});

Expand All @@ -83,9 +83,9 @@ describe('diff/json', function() {
JSON.stringify({a: 123, b: 456, c: 789}, undefined, ' '),
JSON.stringify({a: 123, b: 456}, undefined, ' ')
)).to.eql([
{ count: 3, value: '{\n "a": 123,\n "b": 456,\n' },
{ count: 1, value: ' "c": 789\n', added: undefined, removed: true },
{ count: 1, value: '}' }
{ count: 3, value: '{\n "a": 123,\n "b": 456,\n', removed: false, added: false },
{ count: 1, value: ' "c": 789\n', added: false, removed: true },
{ count: 1, value: '}', removed: false, added: false }
]);
});

Expand Down Expand Up @@ -143,43 +143,43 @@ describe('diff/json', function() {
{a: 123},
{a: /foo/}
)).to.eql([
{ count: 1, value: '{\n' },
{ count: 1, value: ' \"a\": 123\n', added: undefined, removed: true },
{ count: 1, value: ' \"a\": {}\n', added: true, removed: undefined },
{ count: 1, value: '}' }
{ count: 1, value: '{\n', removed: false, added: false },
{ count: 1, value: ' \"a\": 123\n', added: false, removed: true },
{ count: 1, value: ' \"a\": {}\n', added: true, removed: false },
{ count: 1, value: '}', removed: false, added: false }
]);

expect(diffJson(
{a: 123},
{a: /foo/gi},
{stringifyReplacer: (k, v) => v instanceof RegExp ? v.toString() : v}
)).to.eql([
{ count: 1, value: '{\n' },
{ count: 1, value: ' \"a\": 123\n', added: undefined, removed: true },
{ count: 1, value: ' \"a\": "/foo/gi"\n', added: true, removed: undefined },
{ count: 1, value: '}' }
{ count: 1, value: '{\n', removed: false, added: false },
{ count: 1, value: ' \"a\": 123\n', added: false, removed: true },
{ count: 1, value: ' \"a\": "/foo/gi"\n', added: true, removed: false },
{ count: 1, value: '}', removed: false, added: false }
]);

expect(diffJson(
{a: 123},
{a: new Error('ohaider')},
{stringifyReplacer: (k, v) => v instanceof Error ? `${v.name}: ${v.message}` : v}
)).to.eql([
{ count: 1, value: '{\n' },
{ count: 1, value: ' \"a\": 123\n', added: undefined, removed: true },
{ count: 1, value: ' \"a\": "Error: ohaider"\n', added: true, removed: undefined },
{ count: 1, value: '}' }
{ count: 1, value: '{\n', removed: false, added: false },
{ count: 1, value: ' \"a\": 123\n', added: false, removed: true },
{ count: 1, value: ' \"a\": "Error: ohaider"\n', added: true, removed: false },
{ count: 1, value: '}', removed: false, added: false }
]);

expect(diffJson(
{a: 123},
{a: [new Error('ohaider')]},
{stringifyReplacer: (k, v) => v instanceof Error ? `${v.name}: ${v.message}` : v}
)).to.eql([
{ count: 1, value: '{\n' },
{ count: 1, value: ' \"a\": 123\n', added: undefined, removed: true },
{ count: 3, value: ' \"a\": [\n "Error: ohaider"\n ]\n', added: true, removed: undefined },
{ count: 1, value: '}' }
{ count: 1, value: '{\n', removed: false, added: false },
{ count: 1, value: ' \"a\": 123\n', added: false, removed: true },
{ count: 3, value: ' \"a\": [\n "Error: ohaider"\n ]\n', added: true, removed: false },
{ count: 1, value: '}', removed: false, added: false }
]);
});
});
Expand Down
10 changes: 5 additions & 5 deletions test/diff/line.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,18 +113,18 @@ describe('diff/line', function() {

describe('#diffLinesNL', function() {
expect(diffLines('restaurant', 'restaurant\n', {newlineIsToken: true})).to.eql([
{value: 'restaurant', count: 1},
{value: '\n', count: 1, added: true, removed: undefined}
{value: 'restaurant', count: 1, added: false, removed: false},
{value: '\n', count: 1, added: true, removed: false}
]);
expect(diffLines('restaurant', 'restaurant\nhello', {newlineIsToken: true})).to.eql([
{value: 'restaurant', count: 1},
{value: '\nhello', count: 2, added: true, removed: undefined}
{value: 'restaurant', count: 1, added: false, removed: false},
{value: '\nhello', count: 2, added: true, removed: false}
]);
});

describe('Strip trailing CR', function() {
expect(diffLines('line\nline', 'line\r\nline', {stripTrailingCr: true})).to.eql([
{value: 'line\nline', count: 2}
{value: 'line\nline', count: 2, added: false, removed: false}
]);
});
});
22 changes: 11 additions & 11 deletions test/diff/word.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,23 +67,23 @@ describe('WordDiff', function() {
});

it('should include count with identity cases', function() {
expect(diffWords('foo', 'foo')).to.eql([{value: 'foo', count: 1}]);
expect(diffWords('foo bar', 'foo bar')).to.eql([{value: 'foo bar', count: 3}]);
expect(diffWords('foo', 'foo')).to.eql([{value: 'foo', count: 1, removed: false, added: false}]);
expect(diffWords('foo bar', 'foo bar')).to.eql([{value: 'foo bar', count: 3, removed: false, added: false}]);
});
it('should include count with empty cases', function() {
expect(diffWords('foo', '')).to.eql([{value: 'foo', count: 1, added: undefined, removed: true}]);
expect(diffWords('foo bar', '')).to.eql([{value: 'foo bar', count: 3, added: undefined, removed: true}]);
expect(diffWords('foo', '')).to.eql([{value: 'foo', count: 1, added: false, removed: true}]);
expect(diffWords('foo bar', '')).to.eql([{value: 'foo bar', count: 3, added: false, removed: true}]);

expect(diffWords('', 'foo')).to.eql([{value: 'foo', count: 1, added: true, removed: undefined}]);
expect(diffWords('', 'foo bar')).to.eql([{value: 'foo bar', count: 3, added: true, removed: undefined}]);
expect(diffWords('', 'foo')).to.eql([{value: 'foo', count: 1, added: true, removed: false}]);
expect(diffWords('', 'foo bar')).to.eql([{value: 'foo bar', count: 3, added: true, removed: false}]);
});

it('should ignore whitespace', function() {
expect(diffWords('hase igel fuchs', 'hase igel fuchs')).to.eql([{ count: 5, value: 'hase igel fuchs' }]);
expect(diffWords('hase igel fuchs', 'hase igel fuchs\n')).to.eql([{ count: 5, value: 'hase igel fuchs\n' }]);
expect(diffWords('hase igel fuchs\n', 'hase igel fuchs')).to.eql([{ count: 5, value: 'hase igel fuchs\n' }]);
expect(diffWords('hase igel fuchs', 'hase igel\nfuchs')).to.eql([{ count: 5, value: 'hase igel\nfuchs' }]);
expect(diffWords('hase igel\nfuchs', 'hase igel fuchs')).to.eql([{ count: 5, value: 'hase igel fuchs' }]);
expect(diffWords('hase igel fuchs', 'hase igel fuchs')).to.eql([{ count: 5, value: 'hase igel fuchs', removed: false, added: false }]);
expect(diffWords('hase igel fuchs', 'hase igel fuchs\n')).to.eql([{ count: 5, value: 'hase igel fuchs\n', removed: false, added: false }]);
expect(diffWords('hase igel fuchs\n', 'hase igel fuchs')).to.eql([{ count: 5, value: 'hase igel fuchs\n', removed: false, added: false }]);
expect(diffWords('hase igel fuchs', 'hase igel\nfuchs')).to.eql([{ count: 5, value: 'hase igel\nfuchs', removed: false, added: false }]);
expect(diffWords('hase igel\nfuchs', 'hase igel fuchs')).to.eql([{ count: 5, value: 'hase igel fuchs', removed: false, added: false }]);
});

it('should diff whitespace with flag', function() {
Expand Down

0 comments on commit 3a99253

Please sign in to comment.