Bug: diffArrays gives wrong result on array of booleans

[tashburn]

const jsdiff = require('diff')
const a1 = [true,true,true]
const a2 = [true,false,true]
const diff = jsdiff.diffArrays(a1, a2)

outputs

[ { count: 2, value: [ true, true ] },
  { count: 1, added: undefined, removed: true, value: [ true ] } ]

which is wrong! i'd expect something like

[ { count: 1, value: [ true ] },
  { count: 1, added: undefined, removed: true, value: [ true ] }
  { count: 1, added: true, removed: undefined, value: [ false ] }
  { count: 1, value: [ true ] } ]

[pedrottimark]

In addition to false any falsey primitive value including number and string:

const a1 = [1,2,3]
const a2 = [1,0,3]

[ { count: 1, value: [ 1 ] },
  { count: 1, added: undefined, removed: true, value: [ 2 ] },
  { count: 1, value: [ 3 ] } ]

const a1 = ['a','b','c']
const a2 = ['a','','c']

[ { count: 1, value: [ 'a' ] },
  { count: 1, added: undefined, removed: true, value: [ 'b' ] },
  { count: 1, value: [ 'c' ] } ]

It looks like a result of removeEmpty in https://github.com/kpdecker/jsdiff/blob/master/src/diff/base.js#L27-L28

Removing falsey values seems to contradict the description: diffs two arrays, comparing each item for strict equality (===)

Does it make sense to override another method in array.js like:

arrayDiff.removeEmpty = function(value) {
  return value;
};

@wvanderdeijl Are you interested to follow up on this or prefer that I go for it?

[wvanderdeijl]

I won't have time for this over the weekend or beginning next. So feel free to take a crack at it. Otherwise I could have a look late next week

[pedrottimark]

Thank you for quick reply. I submitted a pull request. Take is as appreciation, not impatience.

When you do have time, am happy to get your thought about change to clone arrays in d5003de

I wrote a test to confirm that if the input is arrays for which every item is equal, then the output value property refers to an input array.

Is the goal to avoid aliasing input to output, that is, provide value semantics like strings?

If yes, then what do you think about

• tokenize can make the copy in one place at the beginning
• join can return the value to avoid unneeded copying, especially after slice EDITED

If no, then it seems like the very original code for both methods to return the value is okay.