Skip to content

Commit

Permalink
Implement merge elements in Tree.Edit (#659)
Browse files Browse the repository at this point in the history
In a text-based editor, when you have a selection that spans two
paragraphs, if you press the delete key, removes the second paragraph
and merges its children into the first paragraph.

For example:

Before: `<p>a|b</p><p>c|d</p>`
After: `<p>ad</p>`

This commit implements the merge to Tree.Edit.
  • Loading branch information
hackerwins committed Nov 4, 2023
1 parent b5b8598 commit 7892142
Show file tree
Hide file tree
Showing 7 changed files with 176 additions and 108 deletions.
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

# Stage 1: build binary
# Start from the latest golang base image
FROM golang:1.18-buster AS builder
FROM golang:1.21 AS builder

# Add Maintainer Info
LABEL maintainer="hackerwins <susukang98@gmail.com>"
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ protoset: ## generate protoset file
api/yorkie/v1/*.proto

build: ## builds an executable that runs in the current environment
go build -o $(EXECUTABLE) -ldflags "${GO_LDFLAGS}" ./cmd/yorkie
CGO_ENABLED=0 go build -o $(EXECUTABLE) -ldflags "${GO_LDFLAGS}" ./cmd/yorkie

build-binaries: ## builds binaries to attach a new release
rm -rf binaries
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/yorkie-team/yorkie

go 1.19
go 1.21

require (
github.com/go-playground/locales v0.14.0
Expand Down
41 changes: 35 additions & 6 deletions pkg/document/crdt/tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package crdt
import (
"errors"
"fmt"
"slices"
"strconv"
"strings"
"unicode/utf16"
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
69 changes: 50 additions & 19 deletions pkg/document/crdt/tree_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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())
})
}
6 changes: 3 additions & 3 deletions pkg/index/tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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] {
Expand Down
Loading

0 comments on commit 7892142

Please sign in to comment.