Skip to content

Commit

Permalink
fix: extract nested slices
Browse files Browse the repository at this point in the history
Fix for the util method `extractSlices`. When extracting slices,
we need to adjust the ranges of marks that appear after each slice
in the document. In order to do this, we keep track of the ranges
that are to be deleted and then loop over them, adjusting the mark
ranges as needed.

However, when we have nested slices, each slice may be split by
another slice, causing a slice to have multiple ranges. In this
case, the list of ranges to be deleted would be overlapping,
and thus eventually cause the mark ranges to be adjusted
incorrectly. This fix normalizes the list of ranges to be deleted
before we start updating mark ranges by first sorting those ranges
and then merging overlapping ranges into each other.
  • Loading branch information
bachbui committed Nov 13, 2024
1 parent 29ae2b7 commit 1ab13e8
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 0 deletions.
62 changes: 62 additions & 0 deletions packages/@atjson/renderer-hir/test/renderer-hir.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -468,5 +468,67 @@ describe("@atjson/renderer-hir", () => {
"This document has a spoiler:\nXXXXXXXXXX"
);
});

test("nested slices and subsequent marks", () => {
let atjson = new TestSource({
content: "Pre-text\n\nParent Slice\n\nChild Slice\n\nPost-text",
annotations: [
new Italic({ start: 0, end: 3 }),
new SliceAnnotation({ id: "slice1", start: 10, end: 37 }),
new SliceAnnotation({ id: "slide2", start: 24, end: 30 }),
new Bold({ start: 37, end: 41 }),
],
});

class ConcreteRenderer extends Renderer {
*Bold() {
let text = (yield).join("");
return `__${text}__`;
}
*Italic() {
let text = (yield).join("");
return `*${text}*`;
}
*root() {
let results = (yield).join("");
return results;
}
}

expect(ConcreteRenderer.render(atjson)).toEqual(
"*Pre*-text\n\n__Post__-text"
);
});

test("overlapping slices and subsequent marks", () => {
let atjson = new TestSource({
content: "Pre-text\n\nParent Slice\n\nChild Slice\n\nPost-text",
annotations: [
new Italic({ start: 0, end: 3 }),
new SliceAnnotation({ id: "slice1", start: 10, end: 30 }),
new SliceAnnotation({ id: "slide2", start: 24, end: 37 }),
new Bold({ start: 37, end: 41 }),
],
});

class ConcreteRenderer extends Renderer {
*Bold() {
let text = (yield).join("");
return `__${text}__`;
}
*Italic() {
let text = (yield).join("");
return `*${text}*`;
}
*root() {
let results = (yield).join("");
return results;
}
}

expect(ConcreteRenderer.render(atjson)).toEqual(
"*Pre*-text\n\n__Post__-text"
);
});
});
});
28 changes: 28 additions & 0 deletions packages/@atjson/util/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,16 @@ export function compareSliceTokens(a: SortableSlice, b: SortableSlice) {
return 0;
}

function compareRanges(
[start1, end1]: [number, number],
[start2, end2]: [number, number]
) {
if (start1 === start2) {
return end1 - end2;
}
return start1 - start2;
}

/**
* Extracts slices from a document for the special `slice`
* type provided by atjson. This function removes slices
Expand Down Expand Up @@ -390,6 +400,24 @@ export function extractSlices(value: {
});
}

// Normalize the ranges again, as we may have extended ranges into each other
// First sort the ranges
rangesToDelete = rangesToDelete.sort(compareRanges).reduce((acc, range) => {
// Since they are sorted, check if the current range can be merged into
// the previous one
const last = acc[acc.length - 1];
if (last) {
const [, lastEnd] = last;
if (range[0] <= lastEnd && lastEnd <= range[1]) {
last[1] = range[1];
return acc;
}
}
// Otherwise add the range to the list
acc.push(range);
return acc;
}, [] as [number, number][]);

let firstRange = rangesToDelete[0];
let text = firstRange ? value.text.slice(0, firstRange[0]) : "";
let lastEnd = firstRange ? firstRange[1] : 0;
Expand Down

0 comments on commit 1ab13e8

Please sign in to comment.