Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cache offset and length in document nodes #408

Merged
merged 2 commits into from
Sep 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions packages/parchment/lib/src/document/attributes.dart
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import 'dart:math' as math;

import 'package:collection/collection.dart';
import 'package:quiver/core.dart';

/// Scope of a style attribute, defines context in which an attribute can be
/// applied.
Expand Down Expand Up @@ -254,7 +253,7 @@
}

@override
int get hashCode => hash3(key, scope, value);
int get hashCode => Object.hash(key, scope, value);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with this but it would have been nice to have it in another PR :)


@override
String toString() => '$key: $value';
Expand Down Expand Up @@ -397,8 +396,9 @@

@override
int get hashCode {
final hashes = _data.entries.map((entry) => hash2(entry.key, entry.value));
return hashObjects(hashes);
final hashes =
_data.entries.map((entry) => Object.hash(entry.key, entry.value));
return Object.hashAll(hashes);

Check warning on line 401 in packages/parchment/lib/src/document/attributes.dart

View check run for this annotation

Codecov / codecov/patch

packages/parchment/lib/src/document/attributes.dart#L400-L401

Added lines #L400 - L401 were not covered by tests
}

@override
Expand Down
10 changes: 4 additions & 6 deletions packages/parchment/lib/src/document/embeds.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import 'package:collection/collection.dart';
import 'package:quiver/core.dart';

const _dataEquality = DeepCollectionEquality();

Expand Down Expand Up @@ -60,12 +59,11 @@

@override
int get hashCode {
if (_data.isEmpty) return hash2(type, inline);
if (_data.isEmpty) return Object.hash(type, inline);

final dataHash = hashObjects(
_data.entries.map((e) => hash2(e.key, e.value)),
);
return hash3(type, inline, dataHash);
final dataHash =
Object.hashAll(_data.entries.map((e) => Object.hash(e.key, e.value)));
return Object.hash(type, inline, dataHash);

Check warning on line 66 in packages/parchment/lib/src/document/embeds.dart

View check run for this annotation

Codecov / codecov/patch

packages/parchment/lib/src/document/embeds.dart#L65-L66

Added lines #L65 - L66 were not covered by tests
}

Map<String, dynamic> toJson() {
Expand Down
12 changes: 9 additions & 3 deletions packages/parchment/lib/src/document/leaf.dart
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ abstract base class LeafNode extends Node with StyledNode {
Object get value => _value;
Object _value;

void _setValue(Object newValue) {
_value = newValue;
parent.invalidateLength();
next?.invalidateOffset();
}

/// Splits this leaf node at [index] and returns new node.
///
/// If this is the last node in its list and [index] equals this node's
Expand All @@ -48,7 +54,7 @@ abstract base class LeafNode extends Node with StyledNode {

if (this is TextNode) {
final text = _value as String;
_value = text.substring(0, index);
_setValue(text.substring(0, index));
final split = LeafNode(text.substring(index));
split.applyStyle(style);
insertAfter(split);
Expand Down Expand Up @@ -205,7 +211,7 @@ abstract base class LeafNode extends Node with StyledNode {
var mergeWith = node.previous as TextNode;
if (mergeWith.style == node.style) {
final combinedValue = mergeWith.value + node.value;
mergeWith._value = combinedValue;
mergeWith._setValue(combinedValue);
node.unlink();
node = mergeWith;
}
Expand All @@ -214,7 +220,7 @@ abstract base class LeafNode extends Node with StyledNode {
var mergeWith = node.next as TextNode;
if (mergeWith.style == node.style) {
final combinedValue = node.value + mergeWith.value;
node._value = combinedValue;
node._setValue(combinedValue);
mergeWith.unlink();
}
}
Expand Down
1 change: 0 additions & 1 deletion packages/parchment/lib/src/document/line.dart
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ final class LineNode extends ContainerNode<LeafNode> with StyledNode {
@override
LeafNode get defaultChild => TextNode();

// TODO: should be able to cache length and invalidate on any child-related operation
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:)

@override
int get length => super.length + 1;

Expand Down
65 changes: 60 additions & 5 deletions packages/parchment/lib/src/document/node.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import 'dart:collection';

import 'package:meta/meta.dart';
import 'package:parchment_delta/parchment_delta.dart';

import 'attributes.dart';
Expand Down Expand Up @@ -33,24 +34,32 @@ abstract base class Node extends LinkedListEntry<Node> {
/// `null`.
bool get mounted => _parent != null;

int? _offsetCache;

/// Offset in characters of this node relative to [parent] node.
///
/// To get offset of this node in the document see [documentOffset].
int get offset {
if (isFirst) return 0;
if (_offsetCache != null) return _offsetCache!;

if (isFirst) return _offsetCache = 0;
var offset = 0;
var node = this;
do {
node = node.previous!;
offset += node.length;
} while (!node.isFirst);
return offset;
return _offsetCache = offset;
}

int? _documentOffsetCache;

/// Offset in characters of this node in the document.
int get documentOffset {
if (_documentOffsetCache != null) return _documentOffsetCache!;

final parentOffset = (_parent is! RootNode) ? _parent!.documentOffset : 0;
return parentOffset + offset;
return _documentOffsetCache = parentOffset + offset;
}

/// Returns `true` if this node contains character at specified [offset] in
Expand Down Expand Up @@ -86,20 +95,40 @@ abstract base class Node extends LinkedListEntry<Node> {
assert(entry._parent == null && _parent != null);
entry._parent = _parent;
super.insertBefore(entry);
_parent?.invalidateLength();
invalidateOffset();
}

@override
void insertAfter(Node entry) {
assert(entry._parent == null && _parent != null);
entry._parent = _parent;
super.insertAfter(entry);
parent?.invalidateLength();
entry.invalidateOffset();
}

@override
void unlink() {
assert(_parent != null);
final oldParent = _parent;
final oldNext = next;
_parent = null;
super.unlink();
oldNext?.invalidateOffset();
oldParent?.invalidateLength();
}

@mustCallSuper
void invalidateOffset() {
_offsetCache = null;
invalidateDocumentOffset();
next?.invalidateOffset();
}

@mustCallSuper
void invalidateDocumentOffset() {
_documentOffsetCache = null;
}
}

Expand Down Expand Up @@ -163,20 +192,29 @@ abstract base class ContainerNode<T extends Node> extends Node {
assert(node._parent == null);
node._parent = this;
_children.add(node);
node.invalidateOffset();
invalidateLength();
}

/// Adds [node] to the beginning of this container children list.
void addFirst(T node) {
assert(node._parent == null);
node._parent = this;
_children.addFirst(node);
node.invalidateOffset();
invalidateLength();
}

/// Removes [node] from this container.
void remove(T node) {
assert(node._parent == this);
node._parent = null;
_children.remove(node);
final oldNext = node.next;
final removed = _children.remove(node);
if (removed) {
invalidateLength();
oldNext?.invalidateOffset();
}
}

/// Moves children of this node to [newParent].
Expand Down Expand Up @@ -222,10 +260,13 @@ abstract base class ContainerNode<T extends Node> extends Node {
@override
String toPlainText() => children.map((child) => child.toPlainText()).join();

int? _length;

/// Content length of this node's children. To get number of children in this
/// node use [childCount].
@override
int get length => _children.fold(0, (current, node) => current + node.length);
int get length => _length ??=
_children.fold<int>(0, (current, node) => current + node.length);

@override
void insert(int index, Object data, ParchmentStyle? style) {
Expand Down Expand Up @@ -258,6 +299,20 @@ abstract base class ContainerNode<T extends Node> extends Node {

@override
String toString() => _children.join('\n');

@override
void invalidateDocumentOffset() {
super.invalidateDocumentOffset();
for (var child in children) {
child.invalidateDocumentOffset();
}
}

void invalidateLength() {
_length = null;
next?.invalidateOffset();
parent?.invalidateLength();
}
}

/// Mixin used by nodes that wish to implement [StyledNode] interface.
Expand Down
4 changes: 2 additions & 2 deletions packages/parchment/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ environment:
dependencies:
collection: ^1.18.0
parchment_delta: ^1.0.0
quiver: ^3.2.1
html: ^0.15.4
meta: ^1.12.0

dev_dependencies:
test: ^1.25.5
test: ^1.25.8
lints: ^4.0.0
2 changes: 2 additions & 0 deletions packages/parchment/test/document/leaf_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,10 @@ void main() {
test('insert in formatted node', () {
line.retain(0, 6, boldStyle);
expect(line.childCount, 2);
expect(line.children.last.offset, 6);
line.insert(3, 'don', null);
expect(line.childCount, 4);
expect(line.children.last.offset, 9);
final b = boldStyle.toJson();
expect(
line.children.elementAt(0).toDelta(),
Expand Down
12 changes: 11 additions & 1 deletion packages/parchment/test/document/line_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,10 @@ void main() {
root.retain(0, 4, boldStyle);
root.retain(16, 6, boldStyle);
final line = root.first as LineNode;
final lastTextSegment = line.children.last;
expect(lastTextSegment.offset, 16);
final newLine = line.splitAt(10);
expect(lastTextSegment.offset, 6);
expect(line.toPlainText(), 'This house\n');
expect(newLine.toPlainText(), ' is a circus\n');
});
Expand Down Expand Up @@ -124,10 +127,17 @@ void main() {
});

test('format line', () {
root.insert(0, 'Hello world', null);
root.insert(0, 'Hello world\n', null);
root.insert(12, 'Second headline\n', null);
root.retain(11, 1, h1Style);
root.retain(11, 1, rightStyle);

final secondHeadline = root.first.next!;
expect(secondHeadline.offset, 12);

root.retain(27, 1, ParchmentStyle().merge(ParchmentAttribute.cl));
expect(secondHeadline.offset, 0);

final line = root.first as LineNode;
expect(line, hasLength(12));

Expand Down
19 changes: 14 additions & 5 deletions packages/parchment/test/document/node_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,20 @@ void main() {
});

test('documentOffset', () {
root.insert(0, 'First line\nSecond line', null);
final line = root.children.last as LineNode;
final text = line.first as TextNode;
expect(line.documentOffset, 11);
expect(text.documentOffset, 11);
root.insert(0, 'First line\nSecond line\nThird line', null);
final secondLine = root.children.first.next as LineNode;
final thirdLine = root.children.last as LineNode;
expect(thirdLine.documentOffset, 23);
secondLine.insert(6, ' styled', ParchmentStyle.fromJson({'b': true}));
final styledText = secondLine.first.next as TextNode;
final lastText = secondLine.last as TextNode;
expect(secondLine.documentOffset, 11);
expect(thirdLine.documentOffset, 30);
expect(styledText.documentOffset, 17);
expect(lastText.documentOffset, 24);
secondLine.remove(styledText);
expect(lastText.documentOffset, 17);
expect(thirdLine.documentOffset, 23);
});

test('containsOffset', () {
Expand Down
Loading