Skip to content

Commit

Permalink
fix: extract nested slices (#1821)
Browse files Browse the repository at this point in the history 
* fix: extract nested slices

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
bachbui authored Nov 13, 2024
1 parent 29ae2b7 commit 4940868
Show file tree
Hide file tree
Showing 2 changed files with 299 additions and 0 deletions.
34 changes: 34 additions & 0 deletions packages/@atjson/util/src/index.ts
View file
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,30 @@ export function extractSlices(value: {
});
}

// Normalize the ranges again, as we may have extended ranges into each other
if (rangesToDelete.length > 1) {
const normalizedRanges = [rangesToDelete[0]];
// First sort the ranges
rangesToDelete.sort(compareRanges);
for (
let j = 1, lastRange = rangesToDelete[0];
j < rangesToDelete.length;
j++
) {
const currentRange = rangesToDelete[j];
// Since they are sorted, check if the current range can be merged into the
// previous one
if (currentRange[0] <= lastRange[1] && lastRange[1] <= currentRange[1]) {
lastRange[1] = currentRange[1];
} else {
// Otherwise we can add the range to the list
normalizedRanges.push(currentRange);
lastRange = currentRange;
}
}
rangesToDelete = normalizedRanges;
}

let firstRange = rangesToDelete[0];
let text = firstRange ? value.text.slice(0, firstRange[0]) : "";
let lastEnd = firstRange ? firstRange[1] : 0;
Expand Down
265 changes: 265 additions & 0 deletions packages/@atjson/util/test/extract-slices.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -422,13 +422,31 @@ describe("extractSlices", () => {
refs: ["B00000000"],
},
},
{
id: "M00000003",
type: "italic",
range: "(6..7]",
attributes: {},
},
],
};
let [doc, slices] = extractSlices(original);
expect(slices.get("M00000000")?.text).toEqual("AAA");
expect(slices.get("M00000001")?.text).toEqual("B");
expect(slices.get("M00000002")?.text).toEqual("C");
expect(doc.text).toEqual("D");
expect(doc.marks).toMatchInlineSnapshot(`
[
{
"attributes": {},
"end": 2,
"id": "M00000003",
"range": "(1..2]",
"start": 1,
"type": "italic",
},
]
`);
});

test("start slice position matches", () => {
Expand Down Expand Up @@ -542,13 +560,242 @@ describe("extractSlices", () => {
refs: ["B00000000"],
},
},
{
id: "M00000003",
type: "italic",
range: "(5..6]",
attributes: {},
},
],
};
let [doc, slices] = extractSlices(original);
expect(slices.get("M00000000")?.text).toEqual("A");
expect(slices.get("M00000001")?.text).toEqual("BB");
expect(slices.get("M00000002")?.text).toEqual("C");
expect(doc.text).toEqual("D");
expect(doc.marks).toMatchInlineSnapshot(`
[
{
"attributes": {},
"end": 2,
"id": "M00000003",
"range": "(1..2]",
"start": 1,
"type": "italic",
},
]
`);
});

test("marks overlapping multiple slices", () => {
const original = {
text: "ABCBD",
blocks: [
{
id: "B00000000",
type: "paragraph",
parents: [],
selfClosing: false,
attributes: {},
},
],
marks: [
{
id: "M00000000",
type: "slice",
range: "(1..5]",
attributes: {
refs: ["B00000000"],
},
},
{
id: "M00000001",
type: "slice",
range: "(2..5]",
attributes: {
refs: ["B00000000"],
},
},
{
id: "M00000002",
type: "slice",
range: "(3..4]",
attributes: {
refs: ["B00000000"],
},
},
{
id: "M00000003",
type: "italic",
range: "(5..6]",
attributes: {},
},
{
id: "M00000004",
type: "bold",
range: "(1..6]",
attributes: {},
},
],
};

let [doc, slices] = extractSlices(original);
expect(slices.get("M00000000")?.text).toEqual("A");
expect(slices.get("M00000000")?.marks).toHaveLength(0);
expect(slices.get("M00000001")?.text).toEqual("BB");
expect(slices.get("M00000001")?.marks).toHaveLength(0);
expect(slices.get("M00000002")?.text).toEqual("C");
expect(slices.get("M00000002")?.marks).toHaveLength(0);
expect(doc.text).toEqual("D");
expect(doc.marks).toMatchInlineSnapshot(`
[
{
"attributes": {},
"end": 2,
"id": "M00000003",
"range": "(1..2]",
"start": 1,
"type": "italic",
},
{
"attributes": {},
"end": 2,
"id": "M00000004",
"range": "(1..2]",
"start": 1,
"type": "bold",
},
]
`);
});

test("marks contained in multiple slices", () => {
const original = {
text: "ABCBD",
blocks: [
{
id: "B00000000",
type: "paragraph",
parents: [],
selfClosing: false,
attributes: {},
},
],
marks: [
{
id: "M00000000",
type: "slice",
range: "(1..5]",
attributes: {
refs: ["B00000000"],
},
},
{
id: "M00000001",
type: "slice",
range: "(2..5]",
attributes: {
refs: ["B00000000"],
},
},
{
id: "M00000002",
type: "slice",
range: "(3..5]",
attributes: {
refs: ["B00000000"],
},
},
{
id: "M00000003",
type: "italic",
range: "(3..5]",
attributes: {},
},
],
};

let [doc, slices] = extractSlices(original);
expect(slices.get("M00000000")?.text).toEqual("A");
expect(slices.get("M00000000")?.marks).toHaveLength(0);
expect(slices.get("M00000001")?.text).toEqual("B");
expect(slices.get("M00000001")?.marks).toHaveLength(0);
expect(slices.get("M00000002")?.text).toEqual("CB");
expect(slices.get("M00000002")?.marks).toMatchInlineSnapshot(`
[
{
"attributes": {},
"end": 3,
"id": "M00000002-M00000003",
"range": "(1..3]",
"start": 1,
"type": "italic",
},
]
`);
expect(doc.text).toEqual("D");
expect(doc.marks).toHaveLength(0);
});

test("marks crossing multiple slices but contained in none are dropped", () => {
const original = {
text: "ABCBD",
blocks: [
{
id: "B00000000",
type: "paragraph",
parents: [],
selfClosing: false,
attributes: {},
},
],
marks: [
{
id: "M00000000",
type: "slice",
range: "(1..5]",
attributes: {
refs: ["B00000000"],
},
},
{
id: "M00000001",
type: "slice",
range: "(2..5]",
attributes: {
refs: ["B00000000"],
},
},
{
id: "M00000002",
type: "slice",
range: "(3..5]",
attributes: {
refs: ["B00000000"],
},
},
{
id: "M00000003",
type: "italic",
// this mark is split by multiple slices so it is not
// fully contained in any of them. Currently this means
// it is not a part of any slice. In the future we may split
// this mark like we do blocks
range: "(1..5]",
attributes: {},
},
],
};

let [doc, slices] = extractSlices(original);
expect(slices.get("M00000000")?.text).toEqual("A");
expect(slices.get("M00000000")?.marks).toHaveLength(0);
expect(slices.get("M00000001")?.text).toEqual("B");
expect(slices.get("M00000001")?.marks).toHaveLength(0);
expect(slices.get("M00000002")?.text).toEqual("CB");
expect(slices.get("M00000002")?.marks).toHaveLength(0);
expect(doc.text).toEqual("D");
expect(doc.marks).toHaveLength(0);
});

test("hanging overlapping slices", () => {
Expand Down Expand Up @@ -580,12 +827,30 @@ describe("extractSlices", () => {
refs: ["B00000000"],
},
},
{
id: "M00000002",
type: "italic",
range: "(4..5]",
attributes: {},
},
],
};
let [doc, slices] = extractSlices(original);
expect(slices.get("M00000000")?.text).toEqual("AB");
expect(slices.get("M00000001")?.text).toEqual("BC");
expect(doc.text).toEqual("D");
expect(doc.marks).toMatchInlineSnapshot(`
[
{
"attributes": {},
"end": 2,
"id": "M00000002",
"range": "(1..2]",
"start": 1,
"type": "italic",
},
]
`);
});

describe("retain", () => {
Expand Down

0 comments on commit 4940868

Please sign in to comment.