gliese1337 / fast-myers-diff

MIT License
47 stars 8 forks source link

Add default export function returning user-friendly patches #16

Closed caub closed 9 months ago

caub commented 10 months ago

Hey @gliese1337 we are using your lib in my company, works well, but we had this wrapper that feels complex and we'd rather move it in your lib

function fastDiff(a, b) {
  const diffs = [];
  let ai = 0;
  const patch = calcPatch(a, b);
  for (const [deleteStart, deleteEnd, insert] of patch) {
    if (deleteStart > ai) {
      diffs.push([0, a.slice(ai, deleteStart)]);
      ai = deleteStart;
    }
    if (deleteStart < deleteEnd) {
      diffs.push([-1, a.slice(deleteStart, deleteEnd)]);
      ai = deleteEnd;
    }
    if (insert.length > 0) {
      diffs.push([1, insert]);
    }
  }
  if (ai < a.length) diffs.push([0, a.slice(ai)]);
  return diffs;
}

that I tried to convert more with your notations in this PR, not 100% tho, can you let us know what you think of it please?

gliese1337 commented 10 months ago

I generally try to avoid default exports, but this does seem like a useful feature to add. Just to make sure I correctly understand what this is supposed to do: a 1 indicates an insertion, a 0 indicates a portion of the common subsequence, and a -1 indicates a deletion?

caub commented 10 months ago

Yes, exactly (like in https://github.com/jhchen/fast-diff), it's practical to use this in UI, typically:

diffs.map(([type, value]) => {
    switch (type) {
      case 1: return <ins>{value.replace(/\n/g, '\n\u202F')}</ins>;
      case -1: return <del>{value.replace(/\n/g, '\n\u202F')}</del>;
      default: return <cite>{value.length > 25 ? `${value.slice(0, 10)}…${value.slice(-10)}` : value}</cite>;
    }
caub commented 10 months ago

I'm not entirely sure the version in this MR (inspired from applyPatch)

function * calcDiff(xs, ys) {
  let i = 0;
  const patch = calcPatch(xs, ys, eq);
  for (const [dels, dele, ins] of patch) {
    if (i < dels) yield [0, xs.slice(i, dels)];
    if (dels < dele) yield [-1, xs.slice(dels, dele)];
    if (ins.length > 0) yield [1, ins];
    i = dele;
  }
  if (i < xs.length) yield [0, xs.slice(i)];
}

behaves exactly like the version we had

function fastDiff(xs, ys) {
  const diffs = [];
  let i = 0;
  const patch = calcPatch(xs, ys);
  for (const [dels, dele, ins] of patch) {
    if (i < dels) {
      diffs.push([0, xs.slice(i, dels)]);
      i = dels;
    }
    if (dels < dele) {
      diffs.push([-1, xs.slice(dels, dele)]);
      i = dele;
    }
    if (ins.length > 0) {
      diffs.push([1, ins]);
    }
  }
  if (i < xs.length) diffs.push([0, xs.slice(i)]);
  return diffs;
}

from quick testing yes, maybe you can confirm

caub commented 10 months ago

we can make it a named export, no issue with that, just struggling to find a good name!

caub commented 9 months ago

Can we proceed with this please? Could we maybe rename the exported diff to something else as it's quite low-level? Let me know

btw adding here similar code wrapper for fast-array-diff module: (in case helpful to someone)

function diff(xs, ys) {
  const str_mode = typeof xs === 'string' && typeof ys === 'string';

  let i = 0;
  const diffs = [];
  const patch = fastArrayDiff.getPatch(typeof xs === 'string' ? [...xs] : xs, typeof ys === 'string' ? [...ys] : ys);

  for (const {type, oldPos, newPos, items} of patch) {
    if (i < oldPos) diffs.push([0, xs.slice(i, oldPos)]);
    if (type === 'remove') diffs.push([-1, items]);
    if (type === 'add') diffs.push([1, items]);
    i = oldPos + (type === 'add' ? 0 : items.length);
  }
  if (i < xs.length) diffs.push([0, xs.slice(i)]);

  return !str_mode ? diffs : diffs.map(([t, vs]) => [t, vs.join('')])
}
gliese1337 commented 9 months ago

I have added an export calcSlices which does what you want along with more extensive tests, and published a new version to NPM.