diff --git a/lib/prosereflect/hard_break.rb b/lib/prosereflect/hard_break.rb index 96c4d0e..baf5794 100644 --- a/lib/prosereflect/hard_break.rb +++ b/lib/prosereflect/hard_break.rb @@ -17,6 +17,14 @@ def self.create(marks = nil) new(marks: marks) end + def leaf? + true + end + + def inline? + true + end + def text_content "\n" end diff --git a/lib/prosereflect/horizontal_rule.rb b/lib/prosereflect/horizontal_rule.rb index 95d71e8..97daa0b 100644 --- a/lib/prosereflect/horizontal_rule.rb +++ b/lib/prosereflect/horizontal_rule.rb @@ -28,6 +28,10 @@ def self.create(attrs = nil) new(attrs: attrs) end + def leaf? + true + end + def style=(value) @style = value self.attrs ||= {} diff --git a/lib/prosereflect/image.rb b/lib/prosereflect/image.rb index 844bc1a..325c233 100644 --- a/lib/prosereflect/image.rb +++ b/lib/prosereflect/image.rb @@ -41,6 +41,14 @@ def self.create(attrs = nil) new(attrs: attrs) end + def leaf? + true + end + + def inline? + true + end + # Update the image source URL def src=(src_url) @src = src_url diff --git a/lib/prosereflect/node.rb b/lib/prosereflect/node.rb index fe477a5..39e21c6 100644 --- a/lib/prosereflect/node.rb +++ b/lib/prosereflect/node.rb @@ -221,6 +221,21 @@ def text? false end + # Whether this node can never hold content. A leaf occupies a position but + # has no interior, so a range may never resolve inside one. + # Overridden to true in HardBreak, Image, HorizontalRule and User. + def leaf? + false + end + + # Whether this node lives inside a block rather than beside one. Note this + # is not the inverse of leaf?: HardBreak and Image are inline leaves, while + # HorizontalRule is a block leaf. + # Overridden to true in Text, HardBreak, Image and User. + def inline? + false + end + # Return a copy of this node with content restricted to the given range. # Positions are relative to the start of this node's content. def cut(from = 0, to = nil) @@ -235,6 +250,26 @@ def cut(from = 0, to = nil) end end + # Replace the range [from, to) with the given nodes, returning a new node. + # + # Positions are local to this node: its own token sits at 0 and its children + # begin at offset 1. An element child's token sits at its start. + # + # A text child needs care, because its characters and its span differ. Its + # characters occupy [start, start + text.length), but its node_size is + # text.length + 1, so it spans [start, start + text.length + 1). The extra + # position is the caret after the final character, at start + text.length; + # the next sibling begins one past that. This gap between "last character" + # and "end of span" is the source of most off-by-one errors in here. + # For "Hello" at start 2: characters 2..6, carets 2..7, next sibling at 8. + def replace(from, to, nodes = []) + index, child_start = child_to_descend_into(from, to, nodes) + return splice(from, to, nodes) unless index + + child = content[index].replace(from - child_start, to - child_start, nodes) + copy(content.each_with_index.map { |node, i| i == index ? child : node }) + end + # Iterate over all nodes between two positions in this node. # Accepts a block or a callable as the third positional argument. def nodes_between(from, to, callback = nil, node_start = 0, &block) @@ -275,8 +310,8 @@ def eq?(other) end # Create a copy of this node with different content. - def copy(new_content = nil) - new_node = self.class.new(type: type, attrs: attrs, marks: raw_marks) + def copy(new_content = nil, new_attrs = attrs) + new_node = self.class.new(type: type, attrs: new_attrs, marks: raw_marks) case new_content when nil # no content @@ -310,6 +345,104 @@ def node(depth) private + # Yields each child with its local start and end offsets. + def each_child_span + pos = 1 + (content || []).each_with_index do |child, index| + child_end = pos + child.node_size + yield child, index, pos, child_end + pos = child_end + end + end + + # [index, start] of the child the range resolves inside, or nil to splice at + # this level. Text and leaf children have no interior to descend into: a + # position just past a leaf's token belongs after it, not inside it. + def child_to_descend_into(from, to, nodes) + each_child_span do |child, index, child_start, child_end| + next if child.text? || child.leaf? + next unless from >= child_start + 1 && to <= child_end + next if from == child_end && !inline_insertion?(from, to, nodes) + + return [index, child_start] + end + nil + end + + # This model gives a node no closing token, so a position at child_end is + # both the end of that child's content and the start of its next sibling. + # Inline content has nowhere valid to live out here among block siblings, + # so it resolves inward; block content resolves outward as a sibling. + def inline_insertion?(from, to, nodes) + from == to && nodes.any? && nodes.all?(&:inline?) + end + + # Rebuild this node's children around the replaced range. Children wholly + # outside the range are carried across untouched; only the trimmed edges and + # the inserted nodes meet, so only they are merged. Coalescing untouched + # siblings would silently change this node's size, and every position after + # it, for an edit that never reached them. + def splice(from, to, nodes) + kept_before = [] + kept_after = [] + heads = [] + tails = [] + + each_child_span do |child, _index, child_start, child_end| + if child_end <= from + kept_before << child + elsif child_start >= to + kept_after << child + else + heads << head_of(child, from, child_start) if from > child_start + tails << tail_of(child, to, child_start) if to < child_end + end + end + + spliced = merge_adjacent_text(heads + nodes.to_a + tails) + copy(kept_before + spliced + kept_after) + end + + # The part of a straddling child that survives before `from`. + def head_of(child, from, child_start) + offset = from - child_start + return child.cut(0, offset) if child.text? + + child.replace(offset, child.node_size, []) + end + + # The part of a straddling child that survives from `to` onward. + def tail_of(child, to, child_start) + offset = to - child_start + return child.cut(offset) if child.text? + + child.replace(1, offset, []) + end + + # Drops empty text (a trimmed edge cut to "") and joins same-mark text into + # one node. Applied only to the spliced run -- see splice for why untouched + # children are deliberately left unmerged. + def merge_adjacent_text(nodes) + nodes.reject { |node| node.text? && node.text_content.empty? } + .each_with_object([]) do |node, result| + previous = result.last + if mergeable_text?(previous, node) + result[-1] = previous.class.new(text: previous.text + node.text, + marks: previous.raw_marks) + else + result << node + end + end + end + + # Compares serialized marks, since unmarked text is represented as both nil + # and [] and the two must not be treated as different marks. + def mergeable_text?(previous, node) + return false unless previous&.text? && node.text? + + (previous.marks || []) == (node.marks || []) + end + def cut_content(from, to) return [] unless content diff --git a/lib/prosereflect/text.rb b/lib/prosereflect/text.rb index d3463fa..c591520 100644 --- a/lib/prosereflect/text.rb +++ b/lib/prosereflect/text.rb @@ -33,6 +33,10 @@ def text? true end + def inline? + true + end + # Return a copy of this text node with content restricted to range def cut(from = 0, to = nil) txt = text || "" diff --git a/lib/prosereflect/transform/attr_step.rb b/lib/prosereflect/transform/attr_step.rb index 440f4f8..59c92a0 100644 --- a/lib/prosereflect/transform/attr_step.rb +++ b/lib/prosereflect/transform/attr_step.rb @@ -69,7 +69,7 @@ def compute_new_attrs(target_node) def replace_node_with_new_attrs(doc, target_node, new_attrs) new_content = doc.content.to_a.map { |node| replace_node(node, target_node, new_attrs) } - doc.class.new(content: Fragment.new(new_content), attrs: doc.attrs.dup) + doc.copy(new_content, doc.attrs.dup) end def replace_node(node, target_node, new_attrs) diff --git a/lib/prosereflect/transform/mark_step.rb b/lib/prosereflect/transform/mark_step.rb index 09257b2..9a37622 100644 --- a/lib/prosereflect/transform/mark_step.rb +++ b/lib/prosereflect/transform/mark_step.rb @@ -68,7 +68,7 @@ def self.from_json(_schema, json) def add_mark_to_range(doc) new_content = doc.content.map { |node| apply_mark_to_node(node) } - doc.class.new(content: Fragment.new(new_content), attrs: doc.attrs.dup) + doc.copy(new_content, doc.attrs.dup) end def apply_mark_to_node(node) @@ -83,7 +83,7 @@ def apply_mark_to_node(node) def remove_mark_from_range(doc) new_content = doc.content.map { |node| remove_mark_from_node_single(node) } - doc.class.new(content: Fragment.new(new_content), attrs: doc.attrs.dup) + doc.copy(new_content, doc.attrs.dup) end def remove_mark_from_node_single(node) @@ -191,7 +191,7 @@ def self.from_json(_schema, json) def add_mark_to_node(doc) new_content = doc.content.map { |node| add_mark_to_single_node(node) } - doc.class.new(content: Fragment.new(new_content), attrs: doc.attrs.dup) + doc.copy(new_content, doc.attrs.dup) end def add_mark_to_single_node(node) @@ -253,7 +253,7 @@ def self.from_json(_schema, json) def remove_mark_from_node(doc) new_content = doc.content.map { |node| remove_mark_from_single_node(node) } - doc.class.new(content: Fragment.new(new_content), attrs: doc.attrs.dup) + doc.copy(new_content, doc.attrs.dup) end def remove_mark_from_single_node(node) diff --git a/lib/prosereflect/transform/replace_around_step.rb b/lib/prosereflect/transform/replace_around_step.rb index a3373b0..c2d1fe9 100644 --- a/lib/prosereflect/transform/replace_around_step.rb +++ b/lib/prosereflect/transform/replace_around_step.rb @@ -173,8 +173,7 @@ def content_after(doc, pos) end def rebuild_doc(doc, new_content) - attrs = doc.attrs.dup - doc.class.new(content: Fragment.new(new_content), attrs: attrs) + doc.copy(new_content, doc.attrs.dup) end end end diff --git a/lib/prosereflect/transform/replace_step.rb b/lib/prosereflect/transform/replace_step.rb index 6ac4e79..1d2148a 100644 --- a/lib/prosereflect/transform/replace_step.rb +++ b/lib/prosereflect/transform/replace_step.rb @@ -20,10 +20,9 @@ def apply(doc) return Result.fail("Invalid positions") if @from > @to return Result.fail("from < 0") if @from.negative? return Result.fail("to > doc size") if @to > doc.node_size + return Result.fail("Slice open boundaries are not supported") if open_boundaries? - # Build the new document - new_doc = apply_replace(doc) - Result.ok(new_doc) + Result.ok(doc.replace(@from, @to, @slice.content.to_a)) rescue StandardError => e Result.fail(e.message) end @@ -34,8 +33,9 @@ def get_map end def invert(doc) - # Find what was removed - removed = content_between(doc, @from, @to) + # Find what was removed. Wrapped in a Slice because a step's slice is a + # Slice, not a Fragment: apply reads open_start/open_end off it. + removed = Slice.new(content_between(doc, @from, @to)) ReplaceStep.new(@from, @from + @slice.size, removed) end @@ -107,30 +107,12 @@ def self.from_json(_schema, json) private - def apply_replace(doc) - # Get content before, during, and after the replaced range - before = content_before(doc, @from) - after = content_after(doc, @to) + # Open depths describe how the slice's content joins at its boundaries, so + # they are inert when there is no content to join. + def open_boundaries? + return false if @slice.content.empty? - # Build new document - new_content = [] - new_content.concat(before) unless before.empty? - new_content.concat(@slice.content.to_a) unless @slice.empty? - new_content.concat(after) unless after.empty? - - rebuild_doc(doc, new_content) - end - - def content_before(doc, pos) - result = [] - doc.nodes_between(0, pos) { |node| result << node } - result - end - - def content_after(doc, pos) - result = [] - doc.nodes_between(pos, doc.node_size) { |node| result << node } - result + !@slice.open_start.zero? || !@slice.open_end.zero? end def content_between(doc, from, to) @@ -143,15 +125,6 @@ def join_slices(left, right) new_content = Fragment.new(left.content.to_a + right.content.to_a) Slice.new(new_content, left.open_start, right.open_end) end - - def rebuild_doc(doc, new_content) - # Create a new document with the same structure but new content - attrs = doc.attrs.dup - Fragment.new(new_content) - # For simplicity, return a new Document with the new content - # In reality this would preserve the doc type - doc.class.new(content: Fragment.new(new_content), attrs: attrs) - end end end end diff --git a/lib/prosereflect/user.rb b/lib/prosereflect/user.rb index ae34039..7d8dca5 100644 --- a/lib/prosereflect/user.rb +++ b/lib/prosereflect/user.rb @@ -47,6 +47,14 @@ def add_child(*) raise NotImplementedError, "User mention nodes cannot have children" end + def leaf? + true + end + + def inline? + true + end + def content [] end diff --git a/spec/prosereflect/transform/replace_spec.rb b/spec/prosereflect/transform/replace_spec.rb index 0ce1668..207481b 100644 --- a/spec/prosereflect/transform/replace_spec.rb +++ b/spec/prosereflect/transform/replace_spec.rb @@ -36,6 +36,17 @@ def build_doc_with_paragraphs(*texts) expect(result).to be_ok expect(result.doc).to be_a(Prosereflect::Document) + expect(result.doc.text_content).to eq("Ho") + end + + it "keeps the deleted text in a single text node" do + doc = build_doc_with_text("Hello") + step = Prosereflect::Transform::ReplaceStep.new(3, 6, Prosereflect::Transform::Slice.empty) + result = step.apply(doc) + + paragraph = result.doc.content.first + expect(paragraph).to be_a(Prosereflect::Paragraph) + expect(paragraph.content.map(&:text)).to eq(["Ho"]) end it "deletes at the start of the document" do @@ -45,6 +56,7 @@ def build_doc_with_paragraphs(*texts) result = step.apply(doc) expect(result).to be_ok + expect(result.doc.text_content).to eq("c") end it "deletes at the end of the document" do @@ -54,6 +66,7 @@ def build_doc_with_paragraphs(*texts) result = step.apply(doc) expect(result).to be_ok + expect(result.doc.text_content).to eq("a") end it "deletes an entire paragraph" do @@ -86,6 +99,7 @@ def build_doc_with_paragraphs(*texts) result = step.apply(doc) expect(result).to be_ok + expect(result.doc.text_content).to eq("HXYo") end it "replaces a single character" do @@ -110,6 +124,7 @@ def build_doc_with_paragraphs(*texts) result = step.apply(doc) expect(result).to be_ok + expect(result.doc.text_content).to eq("XYZb") # New doc should be larger expect(result.doc.node_size).to be > doc.node_size end @@ -124,6 +139,8 @@ def build_doc_with_paragraphs(*texts) result = step.apply(doc) expect(result).to be_ok + expect(result.doc.text_content).to eq("aZf") + expect(result.doc.node_size).to be < doc.node_size end end @@ -138,6 +155,7 @@ def build_doc_with_paragraphs(*texts) result = step.apply(doc) expect(result).to be_ok + expect(result.doc.text_content).to eq("abc") expect(result.doc.node_size).to be > doc.node_size end @@ -151,6 +169,7 @@ def build_doc_with_paragraphs(*texts) result = step.apply(doc) expect(result).to be_ok + expect(result.doc.text_content).to eq("abcd") end it "inserts at the end of a paragraph" do @@ -164,6 +183,7 @@ def build_doc_with_paragraphs(*texts) result = step.apply(doc) expect(result).to be_ok + expect(result.doc.text_content).to eq("abcd") end end @@ -182,6 +202,22 @@ def build_doc_with_paragraphs(*texts) result = step.apply(doc) expect(result).to be_ok + expect(result.doc.text_content).to eq("XY") + end + + it "merges adjacent replacement text nodes into one node" do + doc = build_doc_with_text("Hello") + replacement = Prosereflect::Transform::Slice.new( + Prosereflect::Fragment.new( + [Prosereflect::Text.new(text: "X"), Prosereflect::Text.new(text: "Y")], + ), + ) + step = Prosereflect::Transform::ReplaceStep.new(2, 7, replacement) + result = step.apply(doc) + + # Each text node adds +1 to node_size, so leaving these unmerged would + # corrupt every downstream position. + expect(result.doc.content.first.content.map(&:text)).to eq(["XY"]) end it "replaces with an empty slice that has open boundaries" do @@ -194,7 +230,24 @@ def build_doc_with_paragraphs(*texts) step = Prosereflect::Transform::ReplaceStep.new(2, 5, slice) result = step.apply(doc) + # Open depths describe how slice *content* joins; with no content they + # are inert, so this is a plain deletion. expect(result).to be_ok + expect(result.doc.text_content).to eq("") + end + + it "rejects a slice whose open boundaries would affect the result" do + doc = build_doc_with_text("abc") + slice = Prosereflect::Transform::Slice.new( + Prosereflect::Fragment.new([Prosereflect::Text.new(text: "z")]), + 1, + 0, + ) + step = Prosereflect::Transform::ReplaceStep.new(2, 5, slice) + result = step.apply(doc) + + expect(result).not_to be_ok + expect(result.failed).to match(/open/i) end it "replaces a paragraph with a slice containing a paragraph" do @@ -212,6 +265,244 @@ def build_doc_with_paragraphs(*texts) end end + context "with leaf nodes" do + # A leaf occupies a position but has no interior, so a caret just past its + # token belongs after it. Descending into one produces e.g. a hard_break + # containing text, and the inserted text disappears from text_content. + it "inserts after a hard_break rather than inside it" do + doc = Prosereflect::Parser.parse_document( + "type" => "doc", + "content" => [ + { "type" => "paragraph", + "content" => [{ "type" => "hard_break" }, { "type" => "text", "text" => "a" }] }, + ], + ) + insert = Prosereflect::Transform::Slice.new( + Prosereflect::Fragment.new([Prosereflect::Text.new(text: "X")]), + ) + result = Prosereflect::Transform::ReplaceStep.new(3, 3, insert).apply(doc) + + expect(result).to be_ok + # Text#content is nil too, so asserting only text_content and a nil first + # child would pass for [text("X"), text("a")] -- assert the break itself + # and the text order. "a" lies wholly after the insertion point, so it is + # untouched and "X" abuts it rather than coalescing. + children = result.doc.content.first.content + expect(children.map(&:type)).to eq(%w[hard_break text text]) + expect(children.drop(1).map(&:text)).to eq(%w[X a]) + expect(children.first.content).to be_nil + end + + it "inserts after a user mention rather than inside it" do + doc = Prosereflect::Parser.parse_document( + "type" => "doc", + "content" => [ + { "type" => "paragraph", + "content" => [{ "type" => "user", "attrs" => { "id" => "u1" } }, + { "type" => "text", "text" => "a" }] }, + ], + ) + insert = Prosereflect::Transform::Slice.new( + Prosereflect::Fragment.new([Prosereflect::Text.new(text: "X")]), + ) + result = Prosereflect::Transform::ReplaceStep.new(3, 3, insert).apply(doc) + + expect(result).to be_ok + # User#text_content is empty and both inserts are type "text", so neither + # text_content nor the type list can tell where "X" landed. Assert the + # actual text order, and that the mention took no children. + children = result.doc.content.first.content + expect(children.map(&:type)).to eq(%w[user text text]) + expect(children.drop(1).map(&:text)).to eq(%w[X a]) + expect(children.first.content).to eq([]) + end + end + + context "when inserting an empty text node" do + # An empty text node is not representable content -- ProseMirror's + # schema.text("") throws -- and with no characters there is nothing for its + # marks to apply to. Dropping it is canonical normalisation, so the step + # succeeds and the document is untouched. + it "is a no-op that leaves the document unchanged" do + doc = build_doc_with_text("ab") + slice = Prosereflect::Transform::Slice.new( + Prosereflect::Fragment.new( + [Prosereflect::Text.new(text: "", marks: [{ "type" => "bold" }])], + ), + ) + result = Prosereflect::Transform::ReplaceStep.new(3, 3, slice).apply(doc) + + expect(result).to be_ok + expect(result.doc.to_h).to eq(doc.to_h) + end + end + + # KNOWN LIMITATION, asserted so it cannot pass unnoticed: replace is not + # schema-aware. A plain Node carries no NodeType, so replace cannot know what + # a parent may contain and places content wherever the position resolves. The + # inward/outward tie-break is right for Document, where blocks are valid + # siblings, and cannot know it is wrong inside a list. Fixing this means + # consulting lib/prosereflect/schema; see docs/plans/fix-replace-step.md. + # Asserts today's behaviour, NOT the desired behaviour. + context "when inserting a block into a list" do + it "places the block as an invalid sibling of the list items" do + doc = Prosereflect::Document.create + list = doc.add_bullet_list + list.add_item("a") + list.add_item("b") + + para = Prosereflect::Paragraph.create + para.add_text("X") + slice = Prosereflect::Transform::Slice.new(Prosereflect::Fragment.new([para])) + result = Prosereflect::Transform::ReplaceStep.new(6, 6, slice).apply(doc) + + expect(result).to be_ok + # bullet_list accepts only list_item; a paragraph here is schema-invalid. + expect(result.doc.content.first.content.map(&:type)) + .to eq(%w[list_item paragraph list_item]) + end + end + + context "when children lie outside the replaced range" do + it "leaves untouched sibling text nodes alone" do + doc = Prosereflect::Parser.parse_document( + "type" => "doc", + "content" => [ + { "type" => "paragraph", + "content" => [{ "type" => "text", "text" => "a" }, + { "type" => "text", "text" => "b" }, + { "type" => "text", "text" => "Z" }] }, + ], + ) + replacement = Prosereflect::Transform::Slice.new( + Prosereflect::Fragment.new([Prosereflect::Text.new(text: "Q")]), + ) + # Replace "b" only. Coalescing "a"/"Z" into it would shift node_size by + # -2 for an edit whose delta is 0, moving every position after it. + result = Prosereflect::Transform::ReplaceStep.new(4, 5, replacement).apply(doc) + + expect(result.doc.content.first.content.map(&:text)).to eq(%w[a Q Z]) + expect(result.doc.node_size).to eq(doc.node_size) + end + end + + context "when inserting at a child boundary" do + # This model gives a node no closing token, so a position at a child's end + # is ambiguous: both the end of that child's content and the start of its + # next sibling. Inline content resolves inward (it has nowhere valid to + # live among block siblings); block content resolves outward. + def inline_slice(text) + Prosereflect::Transform::Slice.new( + Prosereflect::Fragment.new([Prosereflect::Text.new(text: text)]), + ) + end + + it "inserts text into an empty paragraph rather than beside it" do + doc = Prosereflect::Parser.parse_document( + "type" => "doc", "content" => [{ "type" => "paragraph" }], + ) + result = Prosereflect::Transform::ReplaceStep.new(2, 2, inline_slice("X")).apply(doc) + + expect(result).to be_ok + expect(result.doc.content.length).to eq(1) + expect(result.doc.content.first.content.map(&:text)).to eq(["X"]) + end + + it "inserts text after a trailing hard_break rather than beside the paragraph" do + doc = Prosereflect::Parser.parse_document( + "type" => "doc", + "content" => [{ "type" => "paragraph", "content" => [{ "type" => "hard_break" }] }], + ) + result = Prosereflect::Transform::ReplaceStep.new(3, 3, inline_slice("X")).apply(doc) + + expect(result).to be_ok + expect(result.doc.content.length).to eq(1) + expect(result.doc.content.first.content.map(&:type)).to eq(%w[hard_break text]) + end + + # Inline atoms are leaves but still inline, so they resolve inward like + # text. HorizontalRule is a leaf too, but a block one, so it stays outward. + it "inserts an inline atom into the paragraph rather than beside it" do + doc = build_doc_with_text("abc") + # para spans [1,6), so 6 is the boundary + insert = Prosereflect::Transform::Slice.new( + Prosereflect::Fragment.new([Prosereflect::HardBreak.new]), + ) + result = Prosereflect::Transform::ReplaceStep.new(6, 6, insert).apply(doc) + + expect(result).to be_ok + expect(result.doc.content.map(&:type)).to eq(["paragraph"]) + expect(result.doc.content.first.content.map(&:type)).to eq(%w[text hard_break]) + end + + it "keeps a horizontal_rule beside the paragraph as a block leaf" do + doc = build_doc_with_text("abc") + insert = Prosereflect::Transform::Slice.new( + Prosereflect::Fragment.new([Prosereflect::HorizontalRule.new]), + ) + result = Prosereflect::Transform::ReplaceStep.new(6, 6, insert).apply(doc) + + expect(result.doc.content.map(&:type)).to eq(%w[paragraph horizontal_rule]) + end + + it "appends text to the preceding paragraph at a block boundary" do + doc = build_doc_with_paragraphs("abc", "def") + result = Prosereflect::Transform::ReplaceStep.new(6, 6, inline_slice("X")).apply(doc) + + expect(result.doc.text_content).to eq("abcX\ndef") + # "abc" is wholly outside the range, so it is carried across untouched + # and the insert abuts it rather than coalescing into it. + expect(result.doc.content.map { |node| node.content.map(&:text) }) + .to eq([%w[abc X], ["def"]]) + end + + it "inserts a paragraph between two paragraphs rather than nesting it" do + doc = build_doc_with_paragraphs("abc", "def") + # para1 spans [1,6), para2 spans [6,11) + para = Prosereflect::Paragraph.new(type: "paragraph") + para.add_text("X") + insert = Prosereflect::Transform::Slice.new(Prosereflect::Fragment.new([para])) + result = Prosereflect::Transform::ReplaceStep.new(6, 6, insert).apply(doc) + + expect(result).to be_ok + expect(result.doc.content.map { |node| node.content.map(&:text) }) + .to eq([["abc"], ["X"], ["def"]]) + end + end + + context "when text carries marks" do + it "merges replacement text into unmarked text regardless of nil/empty marks" do + doc = Prosereflect::Parser.parse_document( + "type" => "doc", + "content" => [ + { "type" => "paragraph", + "content" => [{ "type" => "text", "text" => "ab", "marks" => [] }] }, + ], + ) + insert = Prosereflect::Transform::Slice.new( + Prosereflect::Fragment.new([Prosereflect::Text.new(text: "X")]), + ) + result = Prosereflect::Transform::ReplaceStep.new(2, 3, insert).apply(doc) + + expect(result.doc.content.first.content.map(&:text)).to eq(["Xb"]) + end + + it "does not merge text carrying different marks" do + doc = Prosereflect::Parser.parse_document( + "type" => "doc", + "content" => [ + { "type" => "paragraph", + "content" => [{ "type" => "text", "text" => "ab" }] }, + ], + ) + bold = Prosereflect::Text.new(text: "X", marks: [{ "type" => "bold" }]) + insert = Prosereflect::Transform::Slice.new(Prosereflect::Fragment.new([bold])) + result = Prosereflect::Transform::ReplaceStep.new(2, 3, insert).apply(doc) + + expect(result.doc.content.first.content.map(&:text)).to eq(%w[X b]) + end + end + context "with validation" do it "fails when from > to" do doc = build_doc_with_text("abc") @@ -292,11 +583,39 @@ def build_doc_with_paragraphs(*texts) expect(inverted.from).to eq(3) expect(inverted.to).to eq(5) # The inverted step's slice is the content that was at positions 3-3 (empty) - # Note: invert returns a Fragment, not a Slice, due to content_between - expect(inverted.slice).to be_a(Prosereflect::Fragment) + expect(inverted.slice).to be_a(Prosereflect::Transform::Slice) expect(inverted.slice.empty?).to be true end + it "produces an inverse whose slice apply can read" do + doc = build_doc_with_text("Hello") + step = Prosereflect::Transform::ReplaceStep.new(3, 6, Prosereflect::Transform::Slice.empty) + deleted = step.apply(doc) + expect(deleted.doc.text_content).to eq("Ho") + + # An inverse whose slice is a bare Fragment fails in apply, which reads + # open_start/open_end off the slice. + expect(step.invert(doc).apply(deleted.doc)).to be_ok + end + + # KNOWN LIMITATION, asserted so it cannot pass unnoticed: an inverse does NOT + # round-trip. content_between collects whole visited nodes via nodes_between + # (which yields at every depth) rather than cutting the removed range, so the + # inverse of deleting "ell" carries the entire Paragraph("Hello") and + # re-applying it nests a paragraph inside a paragraph. Capturing the removed + # range properly needs the same slice/open-depth machinery that is out of + # scope here (see docs/plans/fix-replace-step.md). Asserting today's + # behaviour, NOT the desired behaviour. + it "does not round-trip: the inverse re-inserts whole visited nodes" do + doc = build_doc_with_text("Hello") + step = Prosereflect::Transform::ReplaceStep.new(3, 6, Prosereflect::Transform::Slice.empty) + deleted = step.apply(doc) + restored = step.invert(doc).apply(deleted.doc) + + expect(restored.doc.text_content).to eq("HHelloo") + expect(restored.doc.text_content).not_to eq(doc.text_content) + end + it "produces a step that reverses a replacement" do doc = build_doc_with_text("Hello") replacement = Prosereflect::Transform::Slice.new( @@ -705,6 +1024,10 @@ def build_doc_with_paragraphs(*texts) end end + # A cross-block range trims each block but does NOT merge the surviving + # blocks into one. Joining is what a slice's open_start/open_end depths + # drive, and those are not implemented (see docs/plans/fix-replace-step.md). + # ProseMirror would collapse these into a single paragraph. context "when replacing across node boundaries" do it "deletes content spanning two paragraphs" do doc = build_doc_with_paragraphs("ab", "cd") @@ -716,6 +1039,17 @@ def build_doc_with_paragraphs(*texts) result = step.apply(doc) expect(result).to be_ok + expect(result.doc.content.map { |para| para.content.map(&:text) }).to eq([["a"], ["d"]]) + end + + it "leaves the two paragraphs unjoined rather than merging them" do + doc = build_doc_with_paragraphs("ab", "cd") + step = Prosereflect::Transform::ReplaceStep.new(3, 7, Prosereflect::Transform::Slice.empty) + result = step.apply(doc) + + # Documents current behaviour, not desired ProseMirror parity: a join + # would yield a single paragraph "ad". + expect(result.doc.content.length).to eq(2) end it "replaces content spanning two paragraphs with new content" do @@ -728,6 +1062,30 @@ def build_doc_with_paragraphs(*texts) expect(result).to be_ok end + + # KNOWN LIMITATION, asserted so it cannot pass unnoticed: an inline slice + # replacing a cross-block range is spliced at the doc level, producing a + # text node as a sibling of paragraphs -- which is not a valid ProseMirror + # document (text may only live inside a block). Placing it correctly needs + # the open-depth fitting that is out of scope here; see the plan. This + # asserts today's behaviour, NOT the desired behaviour. + it "splices an inline slice at doc level across a block boundary (invalid doc)" do + doc = build_doc_with_paragraphs("ab", "cd") + replacement = Prosereflect::Transform::Slice.new( + Prosereflect::Fragment.new([Prosereflect::Text.new(text: "XY")]), + ) + step = Prosereflect::Transform::ReplaceStep.new(3, 7, replacement) + result = step.apply(doc) + + expect(result.doc.to_h).to eq( + { "type" => "doc", + "content" => [ + { "type" => "paragraph", "content" => [{ "type" => "text", "text" => "a" }] }, + { "type" => "text", "text" => "XY" }, + { "type" => "paragraph", "content" => [{ "type" => "text", "text" => "d" }] }, + ] }, + ) + end end context "with table structures" do diff --git a/spec/prosereflect/transform/step_content_spec.rb b/spec/prosereflect/transform/step_content_spec.rb new file mode 100644 index 0000000..43f5e59 --- /dev/null +++ b/spec/prosereflect/transform/step_content_spec.rb @@ -0,0 +1,98 @@ +# frozen_string_literal: true + +require "spec_helper" + +# Node#content is declared `collection: true`, so lutaml-model wraps a non-array +# value into an array. Handing it a Fragment therefore yields `[Fragment]` rather +# than the nodes, and node_size then blows up on the Fragment. Steps must rebuild +# documents through Node#copy, which unwraps a Fragment correctly. +RSpec.describe "Transform steps rebuild documents with node content" do # rubocop:disable RSpec/DescribeClass + def build_doc(text: "hi", attrs: nil) + paragraph = { "type" => "paragraph", "content" => [{ "type" => "text", "text" => text }] } + paragraph["attrs"] = attrs if attrs + Prosereflect::Parser.parse_document("type" => "doc", "content" => [paragraph]) + end + + shared_examples "a step yielding node content" do + it "puts nodes in content rather than a Fragment" do + expect(result.doc.content).to all(be_a(Prosereflect::Node)) + end + + it "leaves the document traversable" do + expect { result.doc.node_size }.not_to raise_error + end + end + + describe Prosereflect::Transform::AddMarkStep do + let(:doc) { build_doc } + let(:result) do + described_class.new(0, doc.node_size, Prosereflect::Mark::Bold.new).apply(doc) + end + + it_behaves_like "a step yielding node content" + end + + describe Prosereflect::Transform::AddNodeMarkStep do + let(:doc) { build_doc } + let(:result) { described_class.new(0, Prosereflect::Mark::Bold.new).apply(doc) } + + it_behaves_like "a step yielding node content" + end + + describe Prosereflect::Transform::RemoveNodeMarkStep do + let(:doc) { build_doc } + let(:result) { described_class.new(0, Prosereflect::Mark::Bold.new).apply(doc) } + + it_behaves_like "a step yielding node content" + end + + describe Prosereflect::Transform::AttrStep do + let(:doc) { build_doc(attrs: { "align" => "left" }) } + let(:result) { described_class.new(0, { "align" => "center" }).apply(doc) } + + it_behaves_like "a step yielding node content" + + it "applies the new attributes" do + expect(result.doc.content.first.attrs).to eq({ "align" => "center" }) + end + end + + describe Prosereflect::Transform::ReplaceStep do + let(:doc) { build_doc(text: "ab") } + let(:result) do + slice = Prosereflect::Transform::Slice.new( + Prosereflect::Fragment.new([Prosereflect::Text.new(text: "X")]), + ) + described_class.new(2, 3, slice).apply(doc) + end + + it_behaves_like "a step yielding node content" + end + + # Node#replace rebuilds ancestors through copy, whose new_attrs defaults to the + # original's attrs. That object is passed to the constructor, but lutaml-model + # copies it on assignment, so the rebuilt tree does not share mutable attrs + # state with the source document. + describe "attrs isolation between the source and rebuilt trees" do + let(:doc) { build_doc(text: "ab", attrs: { "align" => "left" }) } + let(:rebuilt) { doc.replace(2, 3, [Prosereflect::Text.new(text: "X")]) } + + it "does not share the attrs hash" do + expect(rebuilt.content.first.attrs).not_to be(doc.content.first.attrs) + end + + it "does not leak a mutation of the rebuilt tree back to the source" do + rebuilt.content.first.attrs["align"] = "center" + + expect(doc.content.first.attrs["align"]).to eq("left") + end + + it "does not share nested attrs values" do + nested = build_doc(text: "ab", attrs: { "style" => { "color" => "red" } }) + copy = nested.replace(2, 3, [Prosereflect::Text.new(text: "X")]) + copy.content.first.attrs["style"]["color"] = "blue" + + expect(nested.content.first.attrs["style"]["color"]).to eq("red") + end + end +end