Combinations of numbers and letters no longer considered a single token in v6

[greysteil]

First up, thanks for all your work on jsdiff, and for the v6 release.

We have a bunch of tests around diffs, and I noticed an unexpected change:

On v5:

// Diff.diffWordsWithSpace("at least $2m", "at least $4m")
Array [
  Object {
    "count": 5,
    "value": "at least $",
  },
  Object {
    "added": undefined,
    "count": 1,
    "removed": true,
    "value": "2m",
  },
  Object {
    "added": true,
    "count": 1,
    "removed": undefined,
    "value": "4m",
  },
]

On v6:

// Diff.diffWordsWithSpace("at least $2m", "at least $4m")
Array [
  Object {
    "added": false,
    "count": 5,
    "removed": false,
    "value": "at least $",
  },
  Object {
    "added": false,
    "count": 1,
    "removed": true,
    "value": "2",
  },
  Object {
    "added": true,
    "count": 1,
    "removed": false,
    "value": "4",
  },
  Object {
    "added": false,
    "count": 1,
    "removed": false,
    "value": "m",
  },
]

Not sure if this is a deliberate change or not? I think where words and numbers are combined it's probably best to treat them as a single Word.

[efstathiosntonas]

I believe this also breaks emojis in react-native TextInput (or in general).

const changes = diffChars(originalText, changedText) as CharactersDiffChange[];

will result in "��" instead of "✌️😒"

downgrading to 5.2.0 everything is fine.

[ExplodingCabbage]

Not sure if this is a deliberate change or not? I think where words and numbers are combined it's probably best to treat them as a single Word.

It wasn't deliberate, and I agree with you. Damn, I was really hoping I could pull off that 6.0.0 release without introducing any new bugs. :(

[ExplodingCabbage]

@efstathiosntonas what you describe (with diffChars) sounds like an entirely separate bug that I've introduced in the same release. Can you provide minimal instructions on how to reproduce it, either here or in a new issue?

[efstathiosntonas]

@ExplodingCabbage will create an expo repro!

[ExplodingCabbage]

Looks like the first bug here - the one reported by @greysteil - was introduced by #494. Looks like I carelessly assumed that the pre-existing extendedWordChars regex would match any characters that we should consider part of a word, when in fact (whether by design or bug - hard to say because the old logic was also broken!) it doesn't do so. On the bright side, I think it's probably a super-easy one-line fix.

I'll try to get a new release out today with both these bugs fixed.

[greysteil]

You're the best @ExplodingCabbage ❤️

[efstathiosntonas]

I'll try to get a new release out today with both these bugs fixed.

@ExplodingCabbage do you still need a repro for diffChars?

[ExplodingCabbage]

Would be appreciated - even just posting what two strings to diff against each other to see a bug! Otherwise will try to figure it out myself.

[efstathiosntonas]

I think the fastest would be to read or implement it on an expo project: https://gist.github.com/efstathiosntonas/01142fd9243573d649caddd952f81829

Just try to enter emojis on the textinput from the native keyboard of the device/simulator, they will turn out as ?? instead of the emojis.

[ExplodingCabbage]

Original issue fixed by #554.

I'll have a play around with your Gist and try to figure out the diffChars thing, @efstathiosntonas. I do note though that if I just try diffing some text involving the ✌ and 😒 emojis with diffChars it appears to behave correctly on v6.0.0:

> diff.diffChars('', '✌😒')
[ { count: 2, added: true, removed: false, value: '✌😒' } ]
> diff.diffChars('✌', '😒')
[
  { count: 1, added: false, removed: true, value: '✌' },
  { count: 1, added: true, removed: false, value: '😒' }
]

By contrast, v5.2.0 gets the counts wrong, treating 😒 as two characters (because it is outside the Basic Multilingual Plane):

> diff.diffChars('', '✌😒')
[ { count: 3, added: true, removed: undefined, value: '✌😒' } ]
> diff.diffChars('✌', '😒')
[
  { count: 1, added: undefined, removed: true, value: '✌' },
  { count: 2, added: true, removed: undefined, value: '😒' }
]

So it doesn't appear to me at first glance like I have broken anything emoji-related and I am rather hoping I am going to find that whatever the problem is is not in jsdiff but in your code... but I will see if I can repro now and then get to work on establishing whether that's true.

[ExplodingCabbage]

Okay, I just got round to glancing at the Gist, @efstathiosntonas. Didn't try to get it running since I haven't used React Native for eons and don't remember how, but did take a quick look at the utils.ts code. I would bet that the problem is to do with the change in concept of string length/count between 5.2.0 and 6.0.0 - that before, 😒 had length 2 (the number of UTF-16 code units required to encode it in UTF-16, as returned by '😒'.length) and now it has length 1 (the number of unicode code points, as returned by Array.from('😒').length). I bet some of your stuff to do with incrementing cursor positions assumes it is dealing with string indexes when conceiving of the string as a sequence of UTF-16 code units, and is therefore now incompatible with jsdiff v6.0.0's count attribute returning lengths as counts of Unicode code points.

Replacing all use of the .count property of change objects with .value.length will probably restore the old behaviour.

Note the change in Unicode handling in diffChars was intentional and documented - the release-notes say

#500 diffChars now diffs Unicode code points instead of UTF-16 code units.

and the README says:

Diff.diffChars(oldStr, newStr[, options]) - diffs two blocks of text, treating each character as a token.

("Characters" here means Unicode code points ...

If after doing some more debugging you can give me a snippet of code that just invokes jsdiff and gets a result you think is wrong, let me know, but I'm gonna assume diffChars is working as intended. (It is a breaking change of course, and I'm sorry it broke something for you, but that's why it was in a major release!)