diff --git a/api/converter/from_pb.go b/api/converter/from_pb.go index 96ba7ca9c..276f5ffdd 100644 --- a/api/converter/from_pb.go +++ b/api/converter/from_pb.go @@ -511,23 +511,24 @@ func fromTreeStyle(pbTreeStyle *api.Operation_TreeStyle) (*operations.TreeStyle, return nil, err } + createdAtMapByActor, err := fromCreatedAtMapByActor( + pbTreeStyle.CreatedAtMapByActor, + ) + if err != nil { + return nil, err + } + if len(pbTreeStyle.AttributesToRemove) > 0 { return operations.NewTreeStyleRemove( parentCreatedAt, from, to, + createdAtMapByActor, pbTreeStyle.AttributesToRemove, executedAt, ), nil } - createdAtMapByActor, err := fromCreatedAtMapByActor( - pbTreeStyle.CreatedAtMapByActor, - ) - if err != nil { - return nil, err - } - return operations.NewTreeStyle( parentCreatedAt, from, diff --git a/pkg/document/crdt/tree.go b/pkg/document/crdt/tree.go index f4841dbb2..5c63c090b 100644 --- a/pkg/document/crdt/tree.go +++ b/pkg/document/crdt/tree.go @@ -394,6 +394,10 @@ func (n *TreeNode) canDelete(removedAt *time.Ticket, maxCreatedAt *time.Ticket) } func (n *TreeNode) canStyle(editedAt *time.Ticket, maxCreatedAt *time.Ticket) bool { + if n.IsText() { + return false + } + return !n.id.CreatedAt.After(maxCreatedAt) && (n.removedAt == nil || editedAt.After(n.removedAt)) } @@ -982,7 +986,7 @@ func (t *Tree) Style( } } - if node.canStyle(editedAt, maxCreatedAt) && !node.IsText() && len(attrs) > 0 { + if node.canStyle(editedAt, maxCreatedAt) && len(attrs) > 0 { maxCreatedAt = createdAtMapByActor[actorIDHex] createdAt := node.id.CreatedAt if maxCreatedAt == nil || createdAt.After(maxCreatedAt) { @@ -1011,37 +1015,56 @@ func (t *Tree) RemoveStyle( to *TreePos, attrs []string, editedAt *time.Ticket, -) ([]GCPair, error) { + maxCreatedAtMapByActor map[string]*time.Ticket, +) (map[string]*time.Ticket, []GCPair, error) { fromParent, fromLeft, err := t.FindTreeNodesWithSplitText(from, editedAt) if err != nil { - return nil, err + return nil, nil, err } toParent, toLeft, err := t.FindTreeNodesWithSplitText(to, editedAt) if err != nil { - return nil, err + return nil, nil, err } var pairs []GCPair + createdAtMapByActor := make(map[string]*time.Ticket) if err = t.traverseInPosRange(fromParent, fromLeft, toParent, toLeft, func(token index.TreeToken[*TreeNode], _ bool) { node := token.Node - if node.IsRemoved() || node.IsText() { - return + actorIDHex := node.id.CreatedAt.ActorIDHex() + + var maxCreatedAt *time.Ticket + if maxCreatedAtMapByActor == nil { + maxCreatedAt = time.MaxTicket + } else { + if createdAt, ok := maxCreatedAtMapByActor[actorIDHex]; ok { + maxCreatedAt = createdAt + } else { + maxCreatedAt = time.InitialTicket + } } - for _, attr := range attrs { - rhtNodes := node.RemoveAttr(attr, editedAt) - for _, rhtNode := range rhtNodes { - pairs = append(pairs, GCPair{ - Parent: node, - Child: rhtNode, - }) + if node.canStyle(editedAt, maxCreatedAt) && len(attrs) > 0 { + maxCreatedAt = createdAtMapByActor[actorIDHex] + createdAt := node.id.CreatedAt + if maxCreatedAt == nil || createdAt.After(maxCreatedAt) { + createdAtMapByActor[actorIDHex] = createdAt + } + + for _, attr := range attrs { + rhtNodes := node.RemoveAttr(attr, editedAt) + for _, rhtNode := range rhtNodes { + pairs = append(pairs, GCPair{ + Parent: node, + Child: rhtNode, + }) + } } } }); err != nil { - return nil, err + return nil, nil, err } - return pairs, nil + return createdAtMapByActor, pairs, nil } // FindTreeNodesWithSplitText finds TreeNode of the given crdt.TreePos and diff --git a/pkg/document/json/tree.go b/pkg/document/json/tree.go index 4f356d142..faae3f807 100644 --- a/pkg/document/json/tree.go +++ b/pkg/document/json/tree.go @@ -262,7 +262,7 @@ func (t *Tree) RemoveStyle(fromIdx, toIdx int, attributesToRemove []string) bool } ticket := t.context.IssueTimeTicket() - pairs, err := t.Tree.RemoveStyle(fromPos, toPos, attributesToRemove, ticket) + maxCreationMapByActor, pairs, err := t.Tree.RemoveStyle(fromPos, toPos, attributesToRemove, ticket, nil) if err != nil { panic(err) } @@ -275,6 +275,7 @@ func (t *Tree) RemoveStyle(fromIdx, toIdx int, attributesToRemove []string) bool t.CreatedAt(), fromPos, toPos, + maxCreationMapByActor, attributesToRemove, ticket, )) diff --git a/pkg/document/operations/tree_style.go b/pkg/document/operations/tree_style.go index 3510b81b5..d642b2171 100644 --- a/pkg/document/operations/tree_style.go +++ b/pkg/document/operations/tree_style.go @@ -71,16 +71,18 @@ func NewTreeStyleRemove( parentCreatedAt *time.Ticket, from *crdt.TreePos, to *crdt.TreePos, + maxCreatedAtMapByActor map[string]*time.Ticket, attributesToRemove []string, executedAt *time.Ticket, ) *TreeStyle { return &TreeStyle{ - parentCreatedAt: parentCreatedAt, - from: from, - to: to, - attributes: map[string]string{}, - attributesToRemove: attributesToRemove, - executedAt: executedAt, + parentCreatedAt: parentCreatedAt, + from: from, + to: to, + maxCreatedAtMapByActor: maxCreatedAtMapByActor, + attributes: map[string]string{}, + attributesToRemove: attributesToRemove, + executedAt: executedAt, } } @@ -100,7 +102,7 @@ func (e *TreeStyle) Execute(root *crdt.Root) error { return err } } else { - pairs, err = obj.RemoveStyle(e.from, e.to, e.attributesToRemove, e.executedAt) + _, pairs, err = obj.RemoveStyle(e.from, e.to, e.attributesToRemove, e.executedAt, e.maxCreatedAtMapByActor) if err != nil { return err } diff --git a/test/integration/tree_concurrency_test.go b/test/integration/tree_concurrency_test.go index 80a72b3b8..b342bebcb 100644 --- a/test/integration/tree_concurrency_test.go +++ b/test/integration/tree_concurrency_test.go @@ -490,7 +490,10 @@ func TestTreeConcurrencyEditStyle(t *testing.T) { } initialXML := `

a

b

c

` - content := &json.TreeNode{Type: "p", Attributes: map[string]string{"italic": "true"}, Children: []json.TreeNode{{Type: "text", Value: `d`}}} + content := &json.TreeNode{Type: "p", Attributes: map[string]string{ + "italic": "true", + "color": "blue", + }, Children: []json.TreeNode{{Type: "text", Value: `d`}}} ranges := []twoRangesType{ // equal:

b

-

b

@@ -519,7 +522,7 @@ func TestTreeConcurrencyEditStyle(t *testing.T) { } styleOperations := []operationInterface{ - styleOperationType{RangeAll, StyleRemove, "color", "", `remove-bold`}, + styleOperationType{RangeAll, StyleRemove, "color", "", `remove-color`}, styleOperationType{RangeAll, StyleSet, "bold", "aa", `set-bold-aa`}, }