From 8973169809055f4330ef92b6eeb62cbed941c44f Mon Sep 17 00:00:00 2001 From: raararaara Date: Tue, 20 Feb 2024 16:44:24 +0900 Subject: [PATCH 1/5] Handle cases with text children only in `treePosToPath` --- src/util/index_tree.ts | 14 +++- test/integration/tree_test.ts | 146 ++++++++++++++++++++++++++++++++++ 2 files changed, 157 insertions(+), 3 deletions(-) diff --git a/src/util/index_tree.ts b/src/util/index_tree.ts index 6d1646ce5..0eb463f73 100644 --- a/src/util/index_tree.ts +++ b/src/util/index_tree.ts @@ -252,10 +252,12 @@ export abstract class IndexTreeNode> { } /** - * `hasTextChild` returns true if the node has an text child. + * `hasTextChild` returns true if the node's children consist of only text children. */ hasTextChild(): boolean { - return this.children.some((child) => child.isText); + return ( + this.children.length > 0 && this.children.every((child) => child.isText) + ); } /** @@ -804,8 +806,14 @@ export class IndexTree> { node.parent! as T, offset, ); - node = node.parent!; path.push(sizeOfLeftSiblings + treePos.offset); + node = node.parent!; + } else if (node.hasTextChild()) { + const sizeOfLeftSiblings = addSizeOfLeftSiblings( + node! as T, + treePos.offset, + ); + path.push(sizeOfLeftSiblings); } else { path.push(treePos.offset); } diff --git a/test/integration/tree_test.ts b/test/integration/tree_test.ts index 704f89d5b..2f8c231a1 100644 --- a/test/integration/tree_test.ts +++ b/test/integration/tree_test.ts @@ -1333,6 +1333,152 @@ describe('Tree.style', function () { ); }); }); + + it('Should return correct range path within doc.subscribe', async function ({ + task, + }) { + await withTwoClientsAndDocuments<{ t: Tree }>(async (c1, d1, c2, d2) => { + d1.update((root) => { + root.t = new Tree({ + type: 'r', + children: [ + { + type: 'c', + children: [ + { + type: 'u', + children: [ + { + type: 'p', + children: [ + { + type: 'n', + children: [], + }, + ], + }, + ], + }, + ], + }, + { + type: 'c', + children: [ + { + type: 'p', + children: [ + { + type: 'n', + children: [], + }, + ], + }, + ], + }, + ], + }); + }); + await c1.sync(); + await c2.sync(); + assert.equal( + d1.getRoot().t.toXML(), + /*html*/ `

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

`, + ); + + d2.update((r) => + r.t.editByPath([1, 0, 0, 0], [1, 0, 0, 0], { + type: 'text', + value: '1', + }), + ); + d2.update((r) => + r.t.editByPath([1, 0, 0, 1], [1, 0, 0, 1], { + type: 'text', + value: '2', + }), + ); + d2.update((r) => + r.t.editByPath([1, 0, 0, 2], [1, 0, 0, 2], { + type: 'text', + value: '3', + }), + ); + await c2.sync(); + await c1.sync(); + assert.equal( + d1.getRoot().t.toXML(), + /*html*/ `

123

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

123

`, + ); + + d1.update((r) => + r.t.editByPath([1, 0, 0, 1], [1, 0, 0, 1], { + type: 'text', + value: 'abcdefgh', + }), + ); + await c1.sync(); + await c2.sync(); + assert.equal( + d1.getRoot().t.toXML(), + /*html*/ `

1abcdefgh23

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

1abcdefgh23

`, + ); + + d2.update((r) => + r.t.editByPath([1, 0, 0, 5], [1, 0, 0, 5], { + type: 'text', + value: '4', + }), + ); + d2.update((r) => r.t.editByPath([1, 0, 0, 6], [1, 0, 0, 7])); + d2.update((r) => + r.t.editByPath([1, 0, 0, 6], [1, 0, 0, 6], { + type: 'text', + value: '5', + }), + ); + await c2.sync(); + await c1.sync(); + + const eventCollector = new EventCollector<{ type: DocEventType }>(); + const unsub = d2.subscribe((event) => { + const { fromPath, toPath } = event.value.operations[0]; + console.log(event.value.operations[0]); + assert.deepEqual(fromPath, [1, 0, 0, 7]); // fromPath should be [1,0,0,7] (same as in the remote change) + assert.deepEqual(toPath, [1, 0, 0, 8]); + eventCollector.add({ type: event.type }); + }); + + d2.update((r) => r.t.editByPath([1, 0, 0, 7], [1, 0, 0, 8])); + + await c2.sync(); + await c1.sync(); + assert.equal( + d1.getRoot().t.toXML(), + /*html*/ `

1abcd45gh23

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

1abcd45gh23

`, + ); + + await eventCollector.waitAndVerifyNthEvent(1, { + type: DocEventType.LocalChange, + }); + unsub(); + }, task.name); + }); }); describe('Tree.edit(concurrent overlapping range)', () => { From 79ee99ec44a23a04942e3edabd6b447f3c0eb1c2 Mon Sep 17 00:00:00 2001 From: raararaara Date: Tue, 20 Feb 2024 17:18:56 +0900 Subject: [PATCH 2/5] Fix test code --- test/integration/tree_test.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/test/integration/tree_test.ts b/test/integration/tree_test.ts index 2f8c231a1..862b7bb06 100644 --- a/test/integration/tree_test.ts +++ b/test/integration/tree_test.ts @@ -26,6 +26,7 @@ import { TreeStyleOpInfo, } from '@yorkie-js-sdk/src/document/operation/operation'; import { Document, DocEventType } from '@yorkie-js-sdk/src/document/document'; +import { Indexable, OperationInfo } from 'yorkie-js-sdk'; describe('Tree', () => { it('Can be created', function ({ task }) { @@ -1453,11 +1454,14 @@ describe('Tree.style', function () { const eventCollector = new EventCollector<{ type: DocEventType }>(); const unsub = d2.subscribe((event) => { - const { fromPath, toPath } = event.value.operations[0]; - console.log(event.value.operations[0]); - assert.deepEqual(fromPath, [1, 0, 0, 7]); // fromPath should be [1,0,0,7] (same as in the remote change) - assert.deepEqual(toPath, [1, 0, 0, 8]); - eventCollector.add({ type: event.type }); + if (event.type === 'local-change' || event.type === 'remote-change') { + const operation = event.value.operations[0] as TreeEditOpInfo; + const { fromPath, toPath } = operation; + console.log(event.value.operations[0]); + assert.deepEqual(fromPath, [1, 0, 0, 7]); // fromPath should be [1,0,0,7] (same as in the remote change) + assert.deepEqual(toPath, [1, 0, 0, 8]); + eventCollector.add({ type: event.type }); + } }); d2.update((r) => r.t.editByPath([1, 0, 0, 7], [1, 0, 0, 8])); From 3a543ee7b0f5a1883fd657032571d45427c642c6 Mon Sep 17 00:00:00 2001 From: raararaara Date: Tue, 20 Feb 2024 17:24:01 +0900 Subject: [PATCH 3/5] Lint --- test/integration/tree_test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/integration/tree_test.ts b/test/integration/tree_test.ts index 862b7bb06..4ba952ce8 100644 --- a/test/integration/tree_test.ts +++ b/test/integration/tree_test.ts @@ -26,7 +26,6 @@ import { TreeStyleOpInfo, } from '@yorkie-js-sdk/src/document/operation/operation'; import { Document, DocEventType } from '@yorkie-js-sdk/src/document/document'; -import { Indexable, OperationInfo } from 'yorkie-js-sdk'; describe('Tree', () => { it('Can be created', function ({ task }) { From b11609b0774cc1c55d6531a011e8ecaf56d0ae21 Mon Sep 17 00:00:00 2001 From: Youngteac Hong Date: Wed, 21 Feb 2024 15:45:19 +0900 Subject: [PATCH 4/5] Update test/integration/tree_test.ts --- test/integration/tree_test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/integration/tree_test.ts b/test/integration/tree_test.ts index 4ba952ce8..b04ac3cd1 100644 --- a/test/integration/tree_test.ts +++ b/test/integration/tree_test.ts @@ -1456,8 +1456,7 @@ describe('Tree.style', function () { if (event.type === 'local-change' || event.type === 'remote-change') { const operation = event.value.operations[0] as TreeEditOpInfo; const { fromPath, toPath } = operation; - console.log(event.value.operations[0]); - assert.deepEqual(fromPath, [1, 0, 0, 7]); // fromPath should be [1,0,0,7] (same as in the remote change) + assert.deepEqual(fromPath, [1, 0, 0, 7]); assert.deepEqual(toPath, [1, 0, 0, 8]); eventCollector.add({ type: event.type }); } From a600dda3aa0a2a5351f5ff79ff442d73452e9345 Mon Sep 17 00:00:00 2001 From: Youngteac Hong Date: Wed, 21 Feb 2024 15:47:34 +0900 Subject: [PATCH 5/5] Update src/util/index_tree.ts --- src/util/index_tree.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/index_tree.ts b/src/util/index_tree.ts index 0eb463f73..07c9bbd9c 100644 --- a/src/util/index_tree.ts +++ b/src/util/index_tree.ts @@ -809,6 +809,8 @@ export class IndexTree> { path.push(sizeOfLeftSiblings + treePos.offset); node = node.parent!; } else if (node.hasTextChild()) { + // TODO(hackerwins): The function does not consider the situation + // where Element and Text nodes are mixed in the Element's Children. const sizeOfLeftSiblings = addSizeOfLeftSiblings( node! as T, treePos.offset,