Skip to content

Commit

Permalink
Changing AttributedSpans.collapseSpans to treat end positions as excl…
Browse files Browse the repository at this point in the history 
…usive
  • Loading branch information
@jmatth
jmatth committed Jun 23, 2022
1 parent f7b0b59 commit 531a889
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 40 deletions.
29 changes: 8 additions & 21 deletions attributed_text/lib/src/attributed_spans.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -852,13 +852,13 @@ class AttributedSpans {
return [];
}

if (markers.isEmpty || markers.first.offset > contentLength - 1) {
if (markers.isEmpty || markers.first.offset > contentLength) {
// There is content but no attributions that apply to it.
return [MultiAttributionSpan(attributions: {}, start: 0, end: contentLength - 1)];
return [MultiAttributionSpan(attributions: {}, start: 0, end: contentLength)];
}

final collapsedSpans = <MultiAttributionSpan>[];
var currentSpan = MultiAttributionSpan(attributions: {}, start: 0, end: contentLength - 1);
var currentSpan = MultiAttributionSpan(attributions: {}, start: 0, end: contentLength);

_log.fine('walking list of markers to determine collapsed spans.');
for (final marker in markers) {
Expand All @@ -869,31 +869,18 @@ class AttributedSpans {
break;
}

if ((marker.isStart && marker.offset > currentSpan.start) ||
(marker.isEnd && marker.offset >= currentSpan.start)) {
if (marker.offset > currentSpan.start) {
// We reached the boundary between the current span and the next. Finalize the current span, commit it, and
// prepare the next one.
_log.fine(
'encountered a span boundary with ${marker.isStart ? "a start" : "an end"} marker at offset ${marker.offset}.');

// Calculate the end of the current span.
//
// If the current marker is an end marker, then the current span at that marker. Otherwise, if the
// marker is an start marker, the current span ends 1 unit before the marker.
final currentEnd = marker.isEnd ? marker.offset : marker.offset - 1;

// Commit the completed span.
collapsedSpans.add(currentSpan.copyWith(end: currentEnd));
collapsedSpans.add(currentSpan.copyWith(end: marker.offset));
_log.fine('committed span ${collapsedSpans.last}');

// Calculate the start of the next span.
//
// If the current marker is a start marker, then the next span begins at that marker. Otherwise, if the
// marker is an end marker, the next span begins 1 unit after the marker.
final nextStart = marker.isStart ? marker.offset : marker.offset + 1;

// Create the next span and continue consumeing markers
currentSpan = currentSpan.copyWith(start: nextStart);
// Create the next span and continue consuming markers.
currentSpan = currentSpan.copyWith(start: marker.offset);
_log.fine('new current span is $currentSpan');
}

Expand All @@ -910,7 +897,7 @@ class AttributedSpans {
}
}

if (collapsedSpans.last.end < contentLength - 1) {
if (collapsedSpans.last.end < contentLength) {
// The last span committed during the loop does not reach the end of the requested content range. We either ran
// out of markers or the remaining markers are outside the content range. In both cases the value in currentSpan
// should already have the correct start, end, and attributions values to cover the remaining content.
Expand Down
37 changes: 19 additions & 18 deletions attributed_text/test/attributed_spans_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -474,14 +474,15 @@ void main() {
test('empty spans', () {
// Make sure no exceptions are thrown when collapsing
// spans on an empty AttributedSpans.
AttributedSpans().collapseSpans(contentLength: 0);
final spans = AttributedSpans().collapseSpans(contentLength: 0);
expect(spans, isEmpty);
});

test('non-empty span with no attributions', () {
final collapsedSpans = AttributedSpans().collapseSpans(contentLength: 10);
expect(collapsedSpans, hasLength(1));
expect(collapsedSpans.first.start, 0);
expect(collapsedSpans.first.end, 9);
expect(collapsedSpans.first.end, 10);
expect(collapsedSpans.first.attributions, isEmpty);
});

Expand All @@ -491,7 +492,7 @@ void main() {
const SpanMarker(attribution: ExpectedSpans.bold, offset: 0, markerType: SpanMarkerType.start),
const SpanMarker(attribution: ExpectedSpans.bold, offset: 16, markerType: SpanMarkerType.end),
],
).collapseSpans(contentLength: 17);
).collapseSpans(contentLength: 16);

expect(collapsedSpans.length, 1);
expect(collapsedSpans.first.start, 0);
Expand All @@ -508,21 +509,21 @@ void main() {
const SpanMarker(attribution: ExpectedSpans.bold, offset: 7, markerType: SpanMarkerType.start),
const SpanMarker(attribution: ExpectedSpans.bold, offset: 10, markerType: SpanMarkerType.end),
],
).collapseSpans(contentLength: 17);
).collapseSpans(contentLength: 16);

expect(collapsedSpans.length, 4);
expect(collapsedSpans[0].start, 0);
expect(collapsedSpans[0].end, 3);
expect(collapsedSpans[0].attributions.length, 1);
expect(collapsedSpans[0].attributions.first, ExpectedSpans.bold);
expect(collapsedSpans[1].start, 4);
expect(collapsedSpans[1].end, 6);
expect(collapsedSpans[1].start, 3);
expect(collapsedSpans[1].end, 7);
expect(collapsedSpans[1].attributions.length, 0);
expect(collapsedSpans[2].start, 7);
expect(collapsedSpans[2].end, 10);
expect(collapsedSpans[2].attributions.length, 1);
expect(collapsedSpans[2].attributions.first, ExpectedSpans.bold);
expect(collapsedSpans[3].start, 11);
expect(collapsedSpans[3].start, 10);
expect(collapsedSpans[3].end, 16);
expect(collapsedSpans[3].attributions.length, 0);
});
Expand All @@ -531,17 +532,17 @@ void main() {
final collapsedSpans = AttributedSpans(
attributions: [
const SpanMarker(attribution: ExpectedSpans.bold, offset: 0, markerType: SpanMarkerType.start),
const SpanMarker(attribution: ExpectedSpans.bold, offset: 4, markerType: SpanMarkerType.end),
const SpanMarker(attribution: ExpectedSpans.bold, offset: 5, markerType: SpanMarkerType.end),
const SpanMarker(attribution: ExpectedSpans.italics, offset: 5, markerType: SpanMarkerType.start),
const SpanMarker(attribution: ExpectedSpans.italics, offset: 9, markerType: SpanMarkerType.end),
const SpanMarker(attribution: ExpectedSpans.italics, offset: 10, markerType: SpanMarkerType.end),
],
).collapseSpans(contentLength: 10);

expect(collapsedSpans, hasLength(2));
expect(collapsedSpans.first.start, 0);
expect(collapsedSpans.first.end, 4);
expect(collapsedSpans.first.end, 5);
expect(collapsedSpans.last.start, 5);
expect(collapsedSpans.last.end, 9);
expect(collapsedSpans.last.end, 10);
});

test('multiple non-overlapping attributions', () {
Expand All @@ -552,21 +553,21 @@ void main() {
const SpanMarker(attribution: ExpectedSpans.italics, offset: 7, markerType: SpanMarkerType.start),
const SpanMarker(attribution: ExpectedSpans.italics, offset: 10, markerType: SpanMarkerType.end),
],
).collapseSpans(contentLength: 17);
).collapseSpans(contentLength: 16);

expect(collapsedSpans.length, 4);
expect(collapsedSpans[0].start, 0);
expect(collapsedSpans[0].end, 3);
expect(collapsedSpans[0].attributions.length, 1);
expect(collapsedSpans[0].attributions.first, ExpectedSpans.bold);
expect(collapsedSpans[1].start, 4);
expect(collapsedSpans[1].end, 6);
expect(collapsedSpans[1].start, 3);
expect(collapsedSpans[1].end, 7);
expect(collapsedSpans[1].attributions.length, 0);
expect(collapsedSpans[2].start, 7);
expect(collapsedSpans[2].end, 10);
expect(collapsedSpans[2].attributions.length, 1);
expect(collapsedSpans[2].attributions.first, ExpectedSpans.italics);
expect(collapsedSpans[3].start, 11);
expect(collapsedSpans[3].start, 10);
expect(collapsedSpans[3].end, 16);
expect(collapsedSpans[3].attributions.length, 0);
});
Expand All @@ -579,18 +580,18 @@ void main() {
const SpanMarker(attribution: ExpectedSpans.italics, offset: 6, markerType: SpanMarkerType.start),
const SpanMarker(attribution: ExpectedSpans.italics, offset: 16, markerType: SpanMarkerType.end),
],
).collapseSpans(contentLength: 17);
).collapseSpans(contentLength: 16);

expect(collapsedSpans.length, 3);
expect(collapsedSpans[0].start, 0);
expect(collapsedSpans[0].end, 5);
expect(collapsedSpans[0].end, 6);
expect(collapsedSpans[0].attributions.length, 1);
expect(collapsedSpans[0].attributions.first, ExpectedSpans.bold);
expect(collapsedSpans[1].start, 6);
expect(collapsedSpans[1].end, 8);
expect(collapsedSpans[1].attributions.length, 2);
expect(collapsedSpans[1].attributions, equals({ExpectedSpans.bold, ExpectedSpans.italics}));
expect(collapsedSpans[2].start, 9);
expect(collapsedSpans[2].start, 8);
expect(collapsedSpans[2].end, 16);
expect(collapsedSpans[2].attributions.length, 1);
expect(collapsedSpans[2].attributions.first, ExpectedSpans.italics);
Expand Down
2 changes: 1 addition & 1 deletion super_editor/lib/src/infrastructure/attributed_text_styles.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ extension ComputerTextSpan on AttributedText {
final collapsedSpans = spans.collapseSpans(contentLength: text.length);
final textSpans = collapsedSpans
.map((attributedSpan) => TextSpan(
text: text.substring(attributedSpan.start, attributedSpan.end + 1),
text: text.substring(attributedSpan.start, attributedSpan.end),
style: styleBuilder(attributedSpan.attributions),
))
.toList();
Expand Down

0 comments on commit 531a889

Please sign in to comment.