Skip to content

Commit

Permalink
Fix issue with mark offsets when using lineBreakChar
Browse files Browse the repository at this point in the history
  • Loading branch information
12joan committed Feb 18, 2024
1 parent b2dfdcd commit 972147f
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 35 deletions.
19 changes: 17 additions & 2 deletions packages/diff/src/computeDiff.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1393,7 +1393,11 @@ const fixtures: Record<string, ComputeDiffFixture> = {
input2: [
{
type: 'paragraph',
children: [{ text: 'Ping\nCode' }],
children: [
{ text: 'Ping\nCo' },
{ text: 'd', bold: true },
{ text: 'e' },
],
},
],
expected: [
Expand All @@ -1402,7 +1406,18 @@ const fixtures: Record<string, ComputeDiffFixture> = {
children: [
{ text: 'Ping' },
{ text: '¶\n', diff: true, diffOperation: { type: 'insert' } },
{ text: 'Code' },
{ text: 'Co' },
{
text: 'd',
bold: true,
diff: true,
diffOperation: {
type: 'update',
properties: { bold: undefined },
newProperties: { bold: true },
},
},
{ text: 'e', bold: undefined },
],
},
],
Expand Down
58 changes: 48 additions & 10 deletions packages/diff/src/internal/transforms/transformDiffTexts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { ComputeDiffOptions } from '../../computeDiff';
import { dmp } from '../utils/dmp';
import { getProperties } from '../utils/get-properties';
import { InlineNodeCharMap } from '../utils/inline-node-char-map';
import { unusedCharGenerator } from '../utils/unused-char-generator';
import { withChangeTracking } from '../utils/with-change-tracking';

// Main function to transform an array of text nodes into another array of text nodes
Expand All @@ -23,15 +24,34 @@ export function transformDiffTexts(
if (nextNodes.length === 0)
throw new Error('must have at least one nextNodes');

const inlineNodeCharMap = new InlineNodeCharMap({
const { lineBreakChar } = options;
const hasLineBreakChar = lineBreakChar !== undefined;

const charGenerator = unusedCharGenerator({
// Do not use any char that is present in the text
unavailableChars: nodes
skipChars: nodes
.concat(nextNodes)
.filter(isText)
.map((n) => n.text)
.join(''),
});

/**
* Chars to represent inserted and deleted line breaks in the diff. These
* must have a length of 1 to keep the offsets consistent. `lineBreakChar`
* itself may have any length.
*/
const insertedLineBreakProxyChar = hasLineBreakChar
? charGenerator.next().value
: undefined;
const deletedLineBreakProxyChar = hasLineBreakChar
? charGenerator.next().value
: undefined;

const inlineNodeCharMap = new InlineNodeCharMap({
charGenerator,
});

// Map inlines nodes to unique text nodes
const texts = nodes.map((n) => inlineNodeCharMap.nodeToText(n));
const nextTexts = nextNodes.map((n) => inlineNodeCharMap.nodeToText(n));
Expand All @@ -58,24 +78,42 @@ export function transformDiffTexts(
}

// After merging, apply split operations based on the target state (`nextTexts`)
for (const op of splitTextNodes(node, nextTexts, options)) {
for (const op of splitTextNodes(node, nextTexts, {
insertedLineBreakChar: insertedLineBreakProxyChar,
deletedLineBreakChar: deletedLineBreakProxyChar,
})) {
nodesEditor.apply(op);
}

nodesEditor.commitChangesToDiffs();
});

const diffTexts: TText[] = (nodesEditor.children[0] as any).children;
let diffTexts: TText[] = (nodesEditor.children[0] as any).children;

// Replace line break proxy chars with the actual line break char
if (hasLineBreakChar) {
diffTexts = diffTexts.map((n) => ({
...n,
text: n.text
.replaceAll(insertedLineBreakProxyChar, lineBreakChar + '\n')
.replaceAll(deletedLineBreakProxyChar, lineBreakChar),
}));
}

// Restore the original inline nodes
return diffTexts.flatMap((t) => inlineNodeCharMap.textToNode(t));
}

interface LineBreakCharsOptions {
insertedLineBreakChar?: string;
deletedLineBreakChar?: string;
}

// Function to compute the text operations needed to transform string `a` into string `b`
function slateTextDiff(
a: string,
b: string,
{ lineBreakChar }: ComputeDiffOptions
{ insertedLineBreakChar, deletedLineBreakChar }: LineBreakCharsOptions
): Op[] {
// Compute the diff between two strings
const diff = dmp.diff_main(a, b);
Expand Down Expand Up @@ -106,9 +144,9 @@ function slateTextDiff(
type: 'remove_text',
offset,
text:
lineBreakChar === undefined
deletedLineBreakChar === undefined
? text
: text.replaceAll('\n', lineBreakChar),
: text.replaceAll('\n', deletedLineBreakChar),
});

break;
Expand All @@ -119,9 +157,9 @@ function slateTextDiff(
type: 'insert_text',
offset,
text:
lineBreakChar === undefined
insertedLineBreakChar === undefined
? text
: text.replaceAll('\n', lineBreakChar + '\n'),
: text.replaceAll('\n', insertedLineBreakChar),
});
// Move the offset forward by the length of the inserted text
offset += text.length;
Expand Down Expand Up @@ -149,7 +187,7 @@ operations.
function splitTextNodes(
node: TText,
split: TText[],
options: ComputeDiffOptions
options: LineBreakCharsOptions
): TOperation[] {
if (split.length === 0) {
// If there are no target nodes, simply remove the original node
Expand Down
8 changes: 6 additions & 2 deletions packages/diff/src/internal/utils/inline-node-char-map.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,15 @@ describe('InlineNodeCharMap', () => {
let map: InlineNodeCharMap;

beforeEach(() => {
map = new InlineNodeCharMap({ unavailableChars: 'ABCDEFG' });
const charGenerator: Generator<string> = (function* () {
yield* 'HI';
})();

map = new InlineNodeCharMap({ charGenerator });
});

describe('nodeToText', () => {
it('should replace inline nodes with unused chars', () => {
it('should replace inline nodes with generated chars', () => {
const inline1 = { type: 'inline1', children: [{ text: '' }] };
const inline2 = { type: 'inline2', children: [{ text: '' }] };
const text1 = map.nodeToText(inline1);
Expand Down
25 changes: 4 additions & 21 deletions packages/diff/src/internal/utils/inline-node-char-map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,17 @@ import {
} from '@udecode/plate-common';

export class InlineNodeCharMap {
private _nextChar: string = 'A';
private _charGenerator: Generator<string>;
private _charToNode: Map<string, TDescendant> = new Map();
private _unavailableChars: string;

constructor({
unavailableChars = '',
}: {
unavailableChars?: string;
} = {}) {
this._unavailableChars = unavailableChars;
constructor({ charGenerator }: { charGenerator: Generator<string> }) {
this._charGenerator = charGenerator;
}

// Replace non-text nodes with a text node containing a unique char
public nodeToText(node: TDescendant): TText {
if (isText(node)) return node;
const c = this.getUnusedChar();
const c = this._charGenerator.next().value;
this._charToNode.set(c, node);
return { text: c };
}
Expand All @@ -37,18 +32,6 @@ export class InlineNodeCharMap {
return outputNodes;
}

private getUnusedChar() {
while (true) {
this._nextChar = String.fromCodePoint(this._nextChar.codePointAt(0)! + 1);

if (!this._unavailableChars.includes(this._nextChar)) {
break;
}
}

return this._nextChar;
}

private replaceCharWithNode(
haystack: TDescendant[],
needle: string,
Expand Down
12 changes: 12 additions & 0 deletions packages/diff/src/internal/utils/unused-char-generator.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { unusedCharGenerator } from './unused-char-generator';

describe('unusedCharGenerator', () => {
it('should generate unique chars that are not skipped', () => {
const generator = unusedCharGenerator({
skipChars: 'DO NOT USE ANY OF THESE CHARS',
});

const chars = Array.from({ length: 10 }, () => generator.next().value);
expect(chars).toEqual(['B', 'G', 'I', 'J', 'K', 'L', 'M', 'P', 'Q', 'V']);
});
});
15 changes: 15 additions & 0 deletions packages/diff/src/internal/utils/unused-char-generator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
export interface UnusedCharGeneratorOptions {
skipChars?: string;
}

export function* unusedCharGenerator({
skipChars = '',
}: UnusedCharGeneratorOptions = {}): Generator<string> {
const skipSet = new Set(skipChars);

for (let code = 'A'.codePointAt(0)!; ; code++) {
const c = String.fromCodePoint(code);
if (skipSet.has(c)) continue;
yield c;
}
}

0 comments on commit 972147f

Please sign in to comment.