Keep the source envelope across a load and a save, and pin where a composite figure stands - #16
Merged
Merged
Conversation
A composite figure with a block attribute above it came back out of this editor
without the attribute:
Input:
```
{#fig-x}
::: figure

^ (a) One
:::
```
Output, before this change:
```
::: figure

^ (a) One
:::
```
TWO CAUSES, and neither alone explains it.
The bridge returns a document carrying `carveSource`, `carveFingerprint` and
`carveSourceLayout` whenever the rich model would be lossy to write back, and
the serializer writes that source verbatim for as long as the fingerprint still
matches - which is how a construct with no exact editor model survives a save.
Tiptap's `setContent` dispatches `tr.replaceWith(0, doc.content.size, document)`,
which replaces the doc's CONTENT and leaves the doc NODE alone, so the three
attributes never reached `getJSON()`. Measured rather than reasoned about: the
import produces all three and the editor hands back all three as null.
Re-attaching them is inert on its own, which is the second cause. The
fingerprint is taken over the bridge's JSON, which carries only the attributes
that were set; Tiptap returns every attribute the schema declares, so
`{"class":"figure"}` comes back as five keys of which four are null and the two
fingerprints never compare equal. So the editor's attributes are pruned of
their schema defaults before the comparison. That is not a reinterpretation:
the serializer reads each of them with optional chaining, so absent and null
already mean the same thing to it, and what changes is only whether the
fingerprint can recognize its own document.
Loading now goes through `setCarveDocument` and serializing through
`editorToCarve`, so both halves are in the seam the app actually uses rather
than at each call site. Staleness needs no check of its own - the fingerprint is
that check, and an edited document falls through to ordinary serialization.
The tests drove `setContent` plus `serializeToCarve(getJSON())` directly, which
is a copy of the app's path rather than the path; a fix in the seam would not
have reached them. They call the app's own two functions now. The one place
that still serializes the raw editor JSON keeps doing so on purpose, and says
why: it is asserting what the CONTENT tree holds, and the envelope is exactly
what it must not read.
The round-trip section is the page a reader checks before filing "the editor ate my figure", so it needs both halves: the block attribute above a fence now survives, and a composite figure is still a generic container because the mapping lives in carve-grammars and the engine the editor parses with predates the construct.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #15.
The ticket's premise is misplaced, and that changes the shape of the work
The ticket reads "model
figure_groupin the editor schema". The editor schema is not in this repository. The Carve to ProseMirror bridge - the CarveKit node set,carveToProseMirror,serializeToCarve,schema-map.json- is@markup-carve/carve-grammars, and the engine that parses the editor's input is the one carve-grammars pins for its own loader, not the@markup-carve/carvethis app installs for the preview pane. Both were measured:main(7f08374) has nofigure_groupanywhere undertiptap/. Its composite-figure work, feat: composite figures are their own construct in all three grammars carve-grammars#223, went into the syntax-highlighting grammars (Prism, TextMate, highlight.js), not into the ProseMirror bridge.mainpins for its loader, carve-js2bfa5cf, predates feat: a bare figure fence is one figure of ordered panels carve-js#1073 and does not produce the node at all.So the ticket's own blocking condition still holds, and the mapping is a carve-grammars change. What this PR lands is the part that is this repository's: a real degradation found while measuring, and the boundary written down as tests instead of as a claim.
The bug the measurement found
A composite figure carrying a block attribute came out of this editor without it.
Input:
Output, before this change:
Two causes, and neither alone explains it.
The bridge returns a document carrying
carveSource/carveFingerprint/carveSourceLayoutwhenever the rich model would be lossy to write back, andserializeToCarvewrites that source verbatim while the fingerprint still matches. Tiptap'ssetContentdispatchestr.replaceWith(0, doc.content.size, document), which replaces the doc's CONTENT and leaves the doc NODE alone, so those attributes never reachedgetJSON()- the import produced all three and the editor handed back all three asnull.Re-attaching them is inert on its own. The fingerprint is taken over the bridge's JSON, which carries only the attributes that were set, while Tiptap returns every attribute the schema declares -
{"class":"figure"}comes back as five keys of which four are null, and the two never compare equal. So the editor's attributes are pruned of their schema defaults before the comparison. That is not a reinterpretation of the document: the serializer reads each of them with optional chaining, so absent and null already mean the same thing to it.Loading goes through
setCarveDocumentand serializing througheditorToCarve, so both halves sit in the seam rather than at each call site. Staleness needs no separate check - the fingerprint is that check, and an edited document falls through to ordinary serialization.What the tests now cover
tests/composite-figure.test.tsholds the two states the boundary can be in:::: figureis a generic container and round-trips. Its premise is checked rather than assumed (carveDiv, not a figure node), because "it happens to work" and "it is modelled" look identical from the outside.figure_groupAST captured from carve-js3f5dd8c- the installed engine cannot produce it, so it is supplied. It reaches the editor as one opaque source atom: lossless as source, not editable as a figure. The test also names the loss that IS there - a block's position excludes the block-attribute line above it, for every block type - so the group's own{#fig-x}is outside the slice.That second test goes red the day carve-grammars gives the group a schema entry, which is the signal to model it here. It says so in place.
On the control: the titled spelling (
::: figure "T") is a different production under section 4c, and the pair is asserted - but the file states plainly that it does not discriminate anything yet, because the engine in use parses both spellings to the same generic container. Reading a passing pair as evidence the two are told apart is exactly the mistake this construct invites. The discrimination that CAN be made is asserted against the captured AST, and mutating that fixture's node type to one the bridge models turns the opaque-atom assertion intocarveDivand fails the test - so it is keyed on the unmodelled type rather than passing for any input.The three existing test files drove
setContentplusserializeToCarve(getJSON())directly, which is a copy of the app's path rather than the path - a fix in the seam would not have reached them. They call the app's own two functions now. The one place that still serializes the raw editor JSON keeps doing so deliberately and says why: it asserts what the CONTENT tree holds, and the envelope is what it must not read.Not done here, and why
No pin was moved. Bumping carve-grammars past its current pin would drag in markup-carve/carve-grammars#220, which withdraws the quote attribution that
tests/blockquote-attribution.test.tscurrently pins - a different subject with its own test to rewrite, and not something to decide inside a figure ticket.