From d7a4b9e28481fe7b7c58ef6868f5a965f085047d Mon Sep 17 00:00:00 2001 From: Youngteac Hong Date: Thu, 2 Nov 2023 18:26:30 +0900 Subject: [PATCH] Implement merge elements in Tree.Edit --- src/document/crdt/tree.ts | 31 ++++++++++++++++++++--- test/integration/tree_test.ts | 8 +----- test/unit/document/crdt/tree_test.ts | 38 +++++++++++++--------------- 3 files changed, 46 insertions(+), 31 deletions(-) diff --git a/src/document/crdt/tree.ts b/src/document/crdt/tree.ts index 17084947d..a1bee6c56 100644 --- a/src/document/crdt/tree.ts +++ b/src/document/crdt/tree.ts @@ -696,6 +696,7 @@ export class CRDTTree extends CRDTGCElement { }); const toBeRemoveds: Array = []; + const toBeMovedsToFromParent: Array = []; const latestCreatedAtMap = new Map(); this.traverseInPosRange( @@ -704,12 +705,31 @@ export class CRDTTree extends CRDTGCElement { toParent, toLeft, (node, contain) => { - // If node is a element node and half-contained in the range, - // it should not be removed. - if (!node.isText && contain != TagContained.All) { + // NOTE(hackerwins): If the node overlaps as a closing tag with the + // range then we need to keep the node. + if (!node.isText && contain == TagContained.Closing) { return; } + // NOTE(hackerwins): If the node overlaps as an opening tag with the + // range then we need to move the remaing children to fromParent. + // TODO(hackerwins): Define more clearly mergable rules between fromParent + // and toParent. For now, if fromParent and toParent are the same + // type, then we can merge them. + if ( + !node.isText && + contain == TagContained.Opening && + fromParent.type === toParent.type + ) { + for (const child of node.children) { + if (toBeRemoveds.includes(child)) { + continue; + } + + toBeMovedsToFromParent.push(child); + } + } + const actorID = node.getCreatedAt().getActorID()!; const latestCreatedAt = latestCreatedAtMapByActor ? latestCreatedAtMapByActor!.has(actorID!) @@ -732,12 +752,15 @@ export class CRDTTree extends CRDTGCElement { for (const node of toBeRemoveds) { node.remove(editedAt); - if (node.isRemoved) { this.removedNodeMap.set(node.id.toIDString(), node); } } + for (const node of toBeMovedsToFromParent) { + fromParent.insertAt(node, fromParent._children.length); + } + // 03. insert the given node at the given position. if (contents?.length) { let leftInChildren = fromLeft; // tree diff --git a/test/integration/tree_test.ts b/test/integration/tree_test.ts index d73eed329..caa2119be 100644 --- a/test/integration/tree_test.ts +++ b/test/integration/tree_test.ts @@ -963,13 +963,7 @@ describe('Tree.edit', function () { ); doc.update((root) => root.t.edit(2, 18)); - assert.equal( - doc.getRoot().t.toXML(), - /*html*/ `

a

f

`, - ); - - // TODO(sejongk): Use the below assertion after implementing Tree.Move. - // assert.equal(doc.getRoot().t.toXML(), /*html*/ `

af

`); + assert.equal(doc.getRoot().t.toXML(), /*html*/ `

af

`); }); }); diff --git a/test/unit/document/crdt/tree_test.ts b/test/unit/document/crdt/tree_test.ts index 87c771f30..362dddf58 100644 --- a/test/unit/document/crdt/tree_test.ts +++ b/test/unit/document/crdt/tree_test.ts @@ -91,7 +91,7 @@ describe('CRDTTreeNode', function () { }); // NOTE: To see the XML string as highlighted, install es6-string-html plugin in VSCode. -describe('CRDTTree', function () { +describe('CRDTTree.Edit', function () { it('Can inserts nodes with edit', function () { // 0 // @@ -250,7 +250,7 @@ describe('CRDTTree', function () { }); }); -describe.skip('Tree.split', function () { +describe.skip('CRDTTree.Split', function () { it('Can split text nodes', function () { // 00. Create a tree with 2 paragraphs. // 0 1 6 11 @@ -417,7 +417,7 @@ describe.skip('Tree.split', function () { }); }); -describe('Tree.move', function () { +describe('CRDTTree.Merge', function () { it('Can delete nodes between element nodes with edit', function () { // 01. Create a tree with 2 paragraphs. // 0 1 2 3 4 5 6 7 8 @@ -444,23 +444,21 @@ describe('Tree.move', function () { // 0 1 2 3 4 //

a d

tree.editByIndex([2, 6], undefined, issueTime()); - assert.deepEqual(tree.toXML(), /*html*/ `

a

d

`); - // TODO(sejongk): Use the below assertion after implementing Tree.Move. - // assert.deepEqual(tree.toXML(), /*html*/ `

ad

`); - - // const treeNode = tree.toTestTreeNode(); - // assert.equal(treeNode.size, 4); // root - // assert.equal(treeNode.children![0].size, 2); // p - // assert.equal(treeNode.children![0].children![0].size, 1); // a - // assert.equal(treeNode.children![0].children![1].size, 1); // d - - // // 03. insert a new text node at the start of the first paragraph. - // tree.editByIndex( - // [1, 1], - // [new CRDTTreeNode(issuePos(), 'text', '@')], - // issueTime(), - // ); - // assert.deepEqual(tree.toXML(), /*html*/ `

@ad

`); + assert.deepEqual(tree.toXML(), /*html*/ `

ad

`); + + const node = tree.toTestTreeNode(); + assert.equal(node.size, 4); // root + assert.equal(node.children![0].size, 2); // p + assert.equal(node.children![0].children![0].size, 1); // a + assert.equal(node.children![0].children![1].size, 1); // d + + // 03. insert a new text node at the start of the first paragraph. + tree.editByIndex( + [1, 1], + [new CRDTTreeNode(issuePos(), 'text', '@')], + issueTime(), + ); + assert.deepEqual(tree.toXML(), /*html*/ `

@ad

`); }); it.skip('Can merge different levels with edit', function () {