Implement merge elements in Tree.Edit

.github/workflows/ci.yml

@@ -7,7 +7,7 @@ on:
     branches: [ main ]
 
 env:
-  GO_VERSION: '1.19.2'
+  GO_VERSION: '1.21'
 
 jobs:
   build:

Makefile

@@ -22,7 +22,7 @@ default: help
 tools: ## install tools for developing yorkie
 	go install github.com/gogo/protobuf/protoc-gen-gogo@v1.3.2
 	go install github.com/gogo/protobuf/protoc-gen-gofast@v1.3.2
-	go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.50.0
+	go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.55.1
 
 proto: ## generate proto files
 	protoc \

cmd/yorkie/config/config.go

@@ -152,7 +152,7 @@ func Delete() error {
 }
 
 // Preload read configuration file for viper before running command
-func Preload(cmd *cobra.Command, args []string) error {
+func Preload(_ *cobra.Command, _ []string) error {
 	configPathValue, err := configPath()
 	if err != nil {
 		fmt.Fprintln(os.Stderr, "get config path: %w", err)

pkg/document/crdt/text.go

@@ -135,9 +135,7 @@ func (t *Text) String() string {
 
 	node := t.rgaTreeSplit.initialHead.next
 	for node != nil {
-		if node.createdAt().Compare(t.createdAt) == 0 {
-			// last line
-		} else if node.removedAt == nil {
+		if node.createdAt().Compare(t.createdAt) != 0 && node.removedAt == nil {
 			values = append(values, node.String())
 		}
 		node = node.next
@@ -152,9 +150,7 @@ func (t *Text) Marshal() string {
 
 	node := t.rgaTreeSplit.initialHead.next
 	for node != nil {
-		if node.createdAt().Compare(t.createdAt) == 0 {
-			// last line
-		} else if node.removedAt == nil {
+		if node.createdAt().Compare(t.createdAt) != 0 && node.removedAt == nil {
 			values = append(values, node.Marshal())
 		}
 		node = node.next

pkg/document/crdt/tree.go

@@ -19,6 +19,7 @@ package crdt
 import (
 	"errors"
 	"fmt"
+	"slices"
 	"strconv"
 	"strings"
 	"unicode/utf16"
@@ -587,17 +588,39 @@ func (t *Tree) Edit(from, to *TreePos,
 	}
 
 	// 02. remove the nodes and update index tree.
+	var toBeRemoveds []*TreeNode
+	var toBeMovedToFromParents []*TreeNode
 	createdAtMapByActor := make(map[string]*time.Ticket)
-	var toBeRemoved []*TreeNode
 
 	err = t.traverseInPosRange(fromParent.Value, fromLeft.Value, toParent.Value, toLeft.Value,
 		func(node *TreeNode, contain index.TagContained) {
-			// If node is a element node and half-contained in the range,
-			// it should not be removed.
-			if !node.IsText() && contain != index.AllContained {
+			// NOTE(hackerwins): If the node overlaps as a closing tag with the
+			// range, then we need to keep it.
+			if !node.IsText() && contain == index.ClosingContained {
 				return
 			}
 
+			// NOTE(hackerwins): If the node overlaps as an opening tag with the
+			// range then we need to move the remaining children to fromParent.
+			if !node.IsText() && contain == index.OpeningContained {
+				// TODO(hackerwins): Define more clearly merge-able rules
+				// between two parents. For now, we only merge two parents are
+				// both element nodes having text children.
+				// e.g. <p>a|b</p><p>c|d</p> -> <p>a|d</p>
+				if !fromParent.Value.IndexTreeNode.HasTextChild() ||
+					!toParent.Value.IndexTreeNode.HasTextChild() {
+					return
+				}
+
+				for _, child := range node.IndexTreeNode.Children() {
+					if slices.Contains(toBeRemoveds, child.Value) {
+						continue
+					}
+
+					toBeMovedToFromParents = append(toBeMovedToFromParents, child.Value)
+				}
+			}
+
 			actorIDHex := node.ID.CreatedAt.ActorIDHex()
 
 			var latestCreatedAt *time.Ticket
@@ -618,20 +641,26 @@ func (t *Tree) Edit(from, to *TreePos,
 				if latestCreatedAt == nil || createdAt.After(latestCreatedAt) {
 					createdAtMapByActor[actorIDHex] = createdAt
 				}
-				toBeRemoved = append(toBeRemoved, node)
+				toBeRemoveds = append(toBeRemoveds, node)
 			}
 
 		})
 	if err != nil {
 		return nil, err
 	}
 
-	for _, node := range toBeRemoved {
+	for _, node := range toBeRemoveds {
 		if node.remove(editedAt) {
 			t.removedNodeMap[node.ID.toIDString()] = node
 		}
 	}
 
+	for _, node := range toBeMovedToFromParents {
+		if err := fromParent.Append(node.IndexTreeNode); err != nil {
+			return nil, err
+		}
+	}
+
 	// 03. insert the given node at the given position.
 	if len(contents) != 0 {
 		leftInChildren := fromLeft

pkg/document/crdt/tree_test.go

@@ -210,7 +210,7 @@ func TestTree(t *testing.T) {
 		assert.Equal(t, 2, node.Children[0].Size)
 		assert.Equal(t, 2, node.Children[0].Children[0].Size)
 
-		// 02. Delete b from the first paragraph.
+		// 02. Delete b from the second paragraph.
 		// 	     0   1 2    3   4 5 6    7
 		// <root> <p> a </p> <p> c d </p> </root>
 		_, err = tree.EditByIndex(2, 3, nil, nil, helper.IssueTime(ctx))
@@ -244,27 +244,61 @@ func TestTree(t *testing.T) {
 		assert.NoError(t, err)
 		assert.Equal(t, "<root><p>ab</p><p>cd</p></root>", tree.ToXML())
 
-		// 02. delete b, c and first paragraph.
+		// 02. delete b, c and the second paragraph.
 		//       0   1 2 3    4
 		// <root> <p> a d </p> </root>
 		_, err = tree.EditByIndex(2, 6, nil, nil, helper.IssueTime(ctx))
 		assert.NoError(t, err)
-		assert.Equal(t, "<root><p>a</p><p>d</p></root>", tree.ToXML())
+		assert.Equal(t, "<root><p>ad</p></root>", tree.ToXML())
 
-		// TODO(sejongk): Use the below assertions after implementing Tree.Move.
-		// assert.Equal(t, "<root><p>ad</p></root>", tree.ToXML())
+		node := tree.ToTreeNodeForTest()
+		assert.Equal(t, 4, node.Size)
+		assert.Equal(t, 2, node.Children[0].Size)
+		assert.Equal(t, 1, node.Children[0].Children[0].Size)
+		assert.Equal(t, 1, node.Children[0].Children[1].Size)
+
+		// 03. insert a new text node at the start of the first paragraph.
+		_, err = tree.EditByIndex(1, 1, nil, []*crdt.TreeNode{crdt.NewTreeNode(helper.IssuePos(ctx),
+			"text", nil, "@")}, helper.IssueTime(ctx))
+		assert.NoError(t, err)
+		assert.Equal(t, "<root><p>@ad</p></root>", tree.ToXML())
+	})
+
+	t.Run("delete nodes between element nodes in different levels test", func(t *testing.T) {
+		// 01. Create a tree with 2 paragraphs.
+		//       0   1   2 3 4    5    6   7 8 9    10
+		// <root> <p> <b> a b </b> </p> <p> c d </p>  </root>
 
-		// node := tree.ToTreeNodeForTest()
-		// assert.Equal(t, 4, node.Size)
-		// assert.Equal(t, 2, node.Children[0].Size)
-		// assert.Equal(t, 1, node.Children[0].Children[0].Size)
-		// assert.Equal(t, 1, node.Children[0].Children[1].Size)
+		ctx := helper.TextChangeContext(helper.TestRoot())
+		tree := crdt.NewTree(crdt.NewTreeNode(helper.IssuePos(ctx), "root", nil), helper.IssueTime(ctx))
+		_, err := tree.EditByIndex(0, 0, nil, []*crdt.TreeNode{
+			crdt.NewTreeNode(helper.IssuePos(ctx), "p", nil),
+		}, helper.IssueTime(ctx))
+		assert.NoError(t, err)
+		_, err = tree.EditByIndex(1, 1, nil, []*crdt.TreeNode{
+			crdt.NewTreeNode(helper.IssuePos(ctx), "b", nil),
+		}, helper.IssueTime(ctx))
+		assert.NoError(t, err)
+		_, err = tree.EditByIndex(2, 2, nil, []*crdt.TreeNode{
+			crdt.NewTreeNode(helper.IssuePos(ctx), "text", nil, "ab"),
+		}, helper.IssueTime(ctx))
+		assert.NoError(t, err)
+		_, err = tree.EditByIndex(6, 6, nil, []*crdt.TreeNode{
+			crdt.NewTreeNode(helper.IssuePos(ctx), "p", nil),
+		}, helper.IssueTime(ctx))
+		assert.NoError(t, err)
+		_, err = tree.EditByIndex(7, 7, nil, []*crdt.TreeNode{
+			crdt.NewTreeNode(helper.IssuePos(ctx), "text", nil, "cd"),
+		}, helper.IssueTime(ctx))
+		assert.NoError(t, err)
+		assert.Equal(t, "<root><p><b>ab</b></p><p>cd</p></root>", tree.ToXML())
 
-		// // 03. insert a new text node at the start of the first paragraph.
-		// _, err = tree.EditByIndex(1, 1, nil, []*crdt.TreeNode{crdt.NewTreeNode(helper.IssuePos(ctx),
-		// 	"text", nil, "@")}, helper.IssueTime(ctx))
-		// assert.NoError(t, err)
-		// assert.Equal(t, "<root><p>@ad</p></root>", tree.ToXML())
+		// 02. delete b, c and the second paragraph.
+		//       0   1   2 3 4    5
+		// <root> <p> <b> a d </b> </root>
+		_, err = tree.EditByIndex(3, 8, nil, nil, helper.IssueTime(ctx))
+		assert.NoError(t, err)
+		assert.Equal(t, "<root><p><b>ad</b></p></root>", tree.ToXML())
 	})
 
 	t.Run("style node with element attributes test", func(t *testing.T) {
@@ -403,9 +437,6 @@ func TestTree(t *testing.T) {
 
 		_, err = tree.EditByIndex(2, 18, nil, nil, helper.IssueTime(ctx))
 		assert.NoError(t, err)
-		assert.Equal(t, "<root><p>a</p><p>f</p></root>", tree.ToXML())
-
-		// TODO(sejongk): Use the below assertion after implementing Tree.Move.
-		// assert.Equal(t, "<root><p>af</p></root>", tree.ToXML())
+		assert.Equal(t, "<root><p>af</p></root>", tree.ToXML())
 	})
 }

pkg/index/tree.go

@@ -569,8 +569,8 @@ func (n *Node[V]) InsertAfter(newNode, referenceNode *Node[V]) error {
 	return nil
 }
 
-// hasTextChild returns true if the node has a text child.
-func (n *Node[V]) hasTextChild() bool {
+// HasTextChild returns true if the node has a text child.
+func (n *Node[V]) HasTextChild() bool {
 	for _, child := range n.Children() {
 		if child.IsText() {
 			return true
@@ -765,7 +765,7 @@ func (t *Tree[V]) PathToTreePos(path []int) (*TreePos[V], error) {
 		}
 	}
 
-	if node.hasTextChild() {
+	if node.HasTextChild() {
 		return findTextPos(node, path[len(path)-1])
 	}
 	if len(node.Children()) < path[len(path)-1] {

pkg/llrb/llrb_test.go

@@ -41,9 +41,9 @@ func (k *intKey) Compare(other llrb.Key) int {
 		return 1
 	} else if k.key < o.key {
 		return -1
-	} else {
-		return 0
 	}
+
+	return 0
 }
 
 type intValue struct {