oficina / docs Public

Insert: a horizontal line, and a page break

The first two greyed-out menu items, built. They are two nodes rather
than one with a flag, and keeping them apart is most of the work.

A horizontal line is prose. It is a thematic break — a change of
subject — so it is announced to a screen reader as a separator, it is
`---` in Markdown, and it is a drawn line in the PDF. A page break is a
layout instruction with no meaning in the text at all: it is
aria-hidden, it has no Markdown spelling, and the PDF obeys it rather
than drawing it. Folded into one node with an attribute, every consumer
would have to read that attribute and branch, and any that forgot would
silently turn somebody's page break into a decorative line.

horizontal_rule already existed in prosemirror-schema-basic and had
simply never been rendered by anything downstream — fragment.rs would
have dropped it into the "unknown node" arm and emitted its children
with no wrapper. page_break is new, and is an atom so it cannot be
merged into a neighbouring block.

Five surfaces had to learn both: the schema, the shared command table,
fragment.rs's three renderers, the Markdown writer and the PDF layout.

In fragment.rs neither is a block for the text renderer's purposes.
They end the block being accumulated, so the words either side do not
run together, and contribute no line of their own — deliberately not
added to the is_block set, which pushes an empty entry for a childless
block. That is right for `<p></p>`, which renders as a blank paragraph,
and wrong for a rule, which is not an empty paragraph.

Markdown gets `---` for the rule and `<!-- page break -->` for the
break. The alternatives are both worse: a second `---` would spell a
page break as a thematic break permanently, and emitting nothing would
be silent data loss. A test holds that `---` never lands directly under
a line of text, where CommonMark reads it as a setext H2 underline and
swallows both the rule and the paragraph above it.

In the PDF the rule calls advance BEFORE drawing, so a rule at a page
boundary lands on the page it belongs on rather than below the bottom
margin of a full one — off the paper, with no clipping to make it
obvious. The page break is guarded on atPageTop so a break with nothing
in front of it emits no blank page, which is what every word processor
does and what a person otherwise blames the exporter for.

fixtures/pages.mjs now honours an explicit break: it wins over the
measured arithmetic unconditionally, the gap goes after the marker
rather than before it, and measurement restarts from the new page's
top. pages.test.mjs is new and is the first test breakPositions has
ever had — it also pins the margin bug that was fixed there earlier,
where summing block heights ran about 15% short.

Everything here was mutation-tested. Removing the forced-break branch
turns three pages tests red; removing 'insert-rule' from the command
table turns TestMenuCommandsExistInTheBundle red; changing the rule's
margin in the stylesheet turns TestPageGeometryMatchesTheStylesheet
red. Three of my own expected offsets in pages.test.mjs were wrong on
the first run and the code was right — corrected by hand rather than by
pasting in what came out.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DoVTH3jZqj9svgt2UD9t1p

Paul Campbell pushed by paul@keymail.dev 1 month ago 4d3420ebaaf23284dd6ea7372a5cfe473663fdb3 parent b8f6629

15 files changed, +765 −4

.amadan/ci.d/10-gate +3 −0
core/src/fragment.rs +46 −0
core/tests/fragment.rs +78 −0
fixtures/editor-entry.js +35 −1
fixtures/pages.mjs +34 −1
fixtures/pages.test.mjs +151 −0
fixtures/toolbar.mjs +48 −0
internal/docs/export_test.go +13 −0
internal/docs/menu.go +11 −2
internal/docs/views/show.html +71 −0
internal/export/document.go +13 −0
internal/export/markdown.go +38 −0
internal/export/markdown_test.go +58 −0
internal/export/pdf.go +57 −0
internal/export/pdf_test.go +109 −0

Intent

full ledger: docs-finish

diff --git a/.amadan/ci.d/10-gate b/.amadan/ci.d/10-gate

index 629b58a..43a0844 100755

--- a/.amadan/ci.d/10-gate

+++ b/.amadan/ci.d/10-gate

@@ -63,6 +63,9 @@ echo "=== fixtures: the document list's live search"

echo "=== fixtures: renaming a document from its title bar"

(cd fixtures && node --test title.test.mjs)

+echo "=== fixtures: where a page ends, measured and explicit"

+(cd fixtures && node --test pages.test.mjs)

# fixtures/build-bundles.mjs is byte-reproducible: no sourcemaps, no

# banners, no timestamps, no absolute paths baked into the output, and

# esbuild's actual work happens in a pinned native binary (0.28.2),

diff --git a/core/src/fragment.rs b/core/src/fragment.rs

index 3e18e97..33764d5 100644

--- a/core/src/fragment.rs

+++ b/core/src/fragment.rs

@@ -348,6 +348,33 @@ fn node_html<T: ReadTxn>(txn: &T, node: &XmlOut, out: &mut String) {

// could, and they render after the break rather than

// vanishing, per the module comment.

"hard_break" => ("<br>".to_string(), String::new()),

+ // A thematic break. Void like `<br>`, so it has an

+ // opening and no closing markup, and the loop below

+ // still walks children it can never have through

+ // ProseMirror's schema and could have through raw Yjs

+ // bytes — they render after the rule rather than

+ // vanishing, per the module comment.

+ "horizontal_rule" => ("<hr>".to_string(), String::new()),

+ // An explicit page break: where a person said a new

+ // page starts, as opposed to where the measured text

+ // happened to run out.

+ //

+ // A <div> with a bare attribute rather than an <hr>

+ // with a class, and the reason is that the two are

+ // different things and must stay distinguishable. An

+ // <hr> is a thematic break — a change of subject, which

+ // a reader hears announced and which survives into

+ // Markdown as `---`. A page break is a layout

+ // instruction with no meaning in the prose at all;

+ // show.html hides it from assistive technology and

+ // draws it as a dashed line only in infinite view,

+ // where there are no pages for it to actually make.

+ //

+ // The attribute is a literal in this binary, like every

+ // other byte this function puts in attribute position

+ // (see the module comment): nothing from the document

+ // reaches it.

+ "page_break" => ("<div docs-page-break></div>".to_string(), String::new()),

// Unknown: render children with no wrapper. See the

// module comment — silence here is data loss.

_ => (String::new(), String::new()),

@@ -392,6 +419,25 @@ fn node_text<T: ReadTxn>(txn: &T, node: &XmlOut, blocks: &mut Vec<String>, cur:

cur.push('\n');

return;

}

+ // A rule and a page break are block-level furniture with no

+ // words in them. They END the block being accumulated, so

+ // the text either side of one does not run together into a

+ // single line of the preview — but they contribute no line

+ // of their own, because there is nothing to read out.

+ //

+ // Deliberately not added to the is_block set below: that

+ // branch pushes an empty entry for a childless block, which

+ // is right for `<p></p>` (fragment_html renders it as a

+ // blank paragraph, so the text form has to agree about how

+ // many paragraphs there are) and wrong here. A rule is not

+ // an empty paragraph, and a blank line in the preview for

+ // every one would be this renderer inventing prose.

+ if name == "horizontal_rule" || name == "page_break" {

+ if !cur.is_empty() {

+ blocks.push(std::mem::take(cur));

+ }

+ return;

+ }

let is_block = matches!(

name.as_str(),

"paragraph" | "heading" | "list_item" | "blockquote"

diff --git a/core/tests/fragment.rs b/core/tests/fragment.rs

index 847d441..2d2de6c 100644

--- a/core/tests/fragment.rs

+++ b/core/tests/fragment.rs

@@ -397,6 +397,84 @@ fn a_hard_break_does_not_split_the_block_it_is_in() {

assert_eq!(fragment_text(&doc), "one\ntwo\n\nthree");

}

+// --- horizontal_rule and page_break ---

+//

+// Two nodes that look alike and mean nothing alike, and the reason

+// they are tested together is that the failure mode is confusing them.

+// A horizontal rule is prose — a thematic break, part of what the

+// document says, `<hr>` in the read view and `---` in Markdown. A page

+// break is a layout instruction with no meaning in the text, drawn as

+// nothing where there are pages and as a dashed line where there are

+// not.

+//

+// horizontal_rule is prosemirror-schema-basic's own name;

+// page_break is this app's, declared in fixtures/editor-entry.js. As

+// with hard_break above, the tag in the CRDT is whatever the editor's

+// schema calls the node, so all three renderers have to know both or

+// one silently disappears.

+#[test]

+fn renders_a_rule_and_a_page_break_in_every_renderer() {

+ let doc = Doc::with_options(Options::with_guid_and_client_id("rule".into(), 1));

+ {

+ let f = doc.get_or_insert_xml_fragment("prosemirror");

+ let mut txn = doc.transact_mut();

+ let p1 = f.push_back(&mut txn, yrs::XmlElementPrelim::empty("paragraph"));

+ p1.push_back(&mut txn, yrs::XmlTextPrelim::new("before"));

+ f.push_back(&mut txn, yrs::XmlElementPrelim::empty("horizontal_rule"));

+ let p2 = f.push_back(&mut txn, yrs::XmlElementPrelim::empty("paragraph"));

+ p2.push_back(&mut txn, yrs::XmlTextPrelim::new("between"));

+ f.push_back(&mut txn, yrs::XmlElementPrelim::empty("page_break"));

+ let p3 = f.push_back(&mut txn, yrs::XmlElementPrelim::empty("paragraph"));

+ p3.push_back(&mut txn, yrs::XmlTextPrelim::new("after"));

+ }

+ // Two different elements, deliberately. An <hr> is announced to a

+ // screen reader as a separator; the page break is a div the

+ // stylesheet hides in paged view and aria-hides everywhere.

+ assert_eq!(

+ fragment_html(&doc),

+ "<p>before</p><hr><p>between</p><div docs-page-break></div><p>after</p>"

+ );

+ // Neither contributes a line to the text rendering, and neither

+ // lets the paragraphs either side run together: one blank line

+ // between each pair, exactly as if the furniture were not there.

+ // A rule is NOT an empty paragraph and must not add a blank line

+ // of its own.

+ assert_eq!(fragment_text(&doc), "before\n\nbetween\n\nafter");

+ // Leaves in the structured renderer, under their own names, which

+ // is what internal/export reads to lay out a PDF.

+ assert_eq!(

+ fragment_json(&doc),

+ concat!(

+ "{\"type\":\"doc\",\"content\":[",

+ "{\"type\":\"paragraph\",\"content\":[{\"type\":\"text\",\"text\":\"before\"}]},",

+ "{\"type\":\"horizontal_rule\",\"content\":[]},",

+ "{\"type\":\"paragraph\",\"content\":[{\"type\":\"text\",\"text\":\"between\"}]},",

+ "{\"type\":\"page_break\",\"content\":[]},",

+ "{\"type\":\"paragraph\",\"content\":[{\"type\":\"text\",\"text\":\"after\"}]}]}",

+ )

+ );

+// A rule between two runs of bare text — unreachable through

+// ProseMirror's schema, reachable the moment a client posts arbitrary

+// Yjs bytes through POST /d/{sqid}/updates. The words either side must

+// not run together into one line of the preview.

+#[test]

+fn a_rule_separates_bare_text_runs() {

+ let doc = Doc::with_options(Options::with_guid_and_client_id("rule2".into(), 1));

+ {

+ let f = doc.get_or_insert_xml_fragment("prosemirror");

+ let mut txn = doc.transact_mut();

+ f.push_back(&mut txn, yrs::XmlTextPrelim::new("one"));

+ f.push_back(&mut txn, yrs::XmlElementPrelim::empty("horizontal_rule"));

+ f.push_back(&mut txn, yrs::XmlTextPrelim::new("two"));

+ }

+ assert_eq!(fragment_text(&doc), "one\n\ntwo");

// --- paragraph alignment ---

// The first block-level formatting. Unlike a mark it belongs to the

diff --git a/fixtures/editor-entry.js b/fixtures/editor-entry.js

index 7b732c6..4eec556 100644

--- a/fixtures/editor-entry.js

+++ b/fixtures/editor-entry.js

@@ -131,6 +131,39 @@ const marks = SIZE_MARKS.reduce(

// follows offers alignment on a heading through its own toolbar, and

// adding it would be a change to fragment.rs and pdf.go as well as to

// this line.

+//

+// page_break is the one node in this schema that

+// prosemirror-schema-basic does not already provide — horizontal_rule

+// and image come from it, and horizontal_rule is what Insert →

+// Horizontal line uses unchanged.

+//

+// It is a separate node from horizontal_rule rather than a variant of

+// it, and that is the decision worth defending. The two look similar

+// and mean nothing alike: an <hr> is a THEMATIC break, part of the

+// prose, announced to a screen reader as a separator and exported to

+// Markdown as `---`; a page break is a LAYOUT instruction with no

+// meaning in the text at all, which fixtures/pages.mjs obeys, the PDF

+// obeys, and Markdown has no way to express. Folding them into one

+// node with an attribute would mean every consumer — the Rust

+// renderer, the Markdown writer, the PDF layout — reading that

+// attribute and branching, and any of them that forgot would silently

+// treat a person's page break as a decorative line.

+//

+// atom + defining: it has no content and must not be merged into or

+// absorbed by a neighbouring block. selectable so it can be clicked

+// and deleted, which is the only way to get rid of one.

+const pageBreak = {

+ group: 'block',

+ atom: true,

+ defining: true,

+ selectable: true,

+ parseDOM: [{ tag: 'div[docs-page-break]' }],

+ // The same markup core/src/fragment.rs emits for this node, so the

+ // read view and the editor render a page break identically and a

+ // copy out of one pastes back into the other.

+ toDOM: () => ['div', { 'docs-page-break': '', 'aria-hidden': 'true' }],

const nodes = addListNodes(

basic.spec.nodes

.update(

@@ -138,7 +171,8 @@ const nodes = addListNodes(

withIndent(alignedParagraph(basic.spec.nodes.get('paragraph')), 'p'),

)

.update('heading', withIndent(basic.spec.nodes.get('heading'), 'h1'))

- .update('hard_break', hardBreakIgnoringInterchange(basic.spec.nodes.get('hard_break'))),

+ .update('hard_break', hardBreakIgnoringInterchange(basic.spec.nodes.get('hard_break')))

+ .addToEnd('page_break', pageBreak),

'paragraph block*',

'block',

)

diff --git a/fixtures/pages.mjs b/fixtures/pages.mjs

index c030095..686806d 100644

--- a/fixtures/pages.mjs

+++ b/fixtures/pages.mjs

@@ -54,6 +54,12 @@ export const PAGE_CONTENT_PX = 864

export const PAGED = 'paged'

export const INFINITE = 'infinite'

+// The schema node name for an explicit page break

+// (fixtures/editor-entry.js). Named here rather than written inline

+// because breakPositions below tests for it and a typo in a string

+// comparison is a feature that silently does nothing.

+export const PAGE_BREAK = 'page_break'

// COOKIE, not localStorage.

// The server reads it (internal/docs/handlers.go) and renders

@@ -86,21 +92,48 @@ export function writeMode(mode) {

// Top-level blocks only. A break inside a list or a quote would put a

// page boundary between a list item and its own siblings, which no word

// processor does and which the exporter would not agree with anyway.

-function breakPositions(view) {

+export function breakPositions(view) {

const out = []

const doc = view.state.doc

// The top of the current page, in viewport coordinates. Everything

// below is measured against it.

let pageTop = null

+ // Set by a page_break node, consumed by the next block that has a

+ // box. An EXPLICIT break is a fact about the document rather than a

+ // measurement, so it wins over the arithmetic below unconditionally:

+ // a person who asked for a new page here gets one whether or not

+ // this one is anywhere near full.

+ let forced = false

doc.forEach((node, offset) => {

+ if (node.type.name === PAGE_BREAK) {

+ // The break marker itself never starts a page — the block AFTER

+ // it does. Pushing this offset would put the gap above the

+ // marker and leave it stranded at the bottom of the previous

+ // page, which is the wrong side of the thing it names.

+ forced = true

+ return

+ }

const dom = view.nodeDOM(offset)

if (!dom || !dom.getBoundingClientRect) return

const rect = dom.getBoundingClientRect()

if (rect.height === 0) return

if (pageTop === null) {

+ // The first block with a box starts page one. A page break

+ // before it is not a break at all — there is no page in front of

+ // it to end — so the flag is cleared rather than carried, which

+ // would otherwise break between the first and second blocks.

+ pageTop = rect.top

+ forced = false

+ return

+ }

+ if (forced) {

+ out.push(offset)

pageTop = rect.top

+ forced = false

return

}

diff --git a/fixtures/pages.test.mjs b/fixtures/pages.test.mjs

new file mode 100644

index 0000000..7f1270d

--- /dev/null

+++ b/fixtures/pages.test.mjs

@@ -0,0 +1,151 @@

+// fixtures/pages.test.mjs — where a page ends.

+//

+// breakPositions is the whole of pagination: everything else in

+// pages.mjs is drawing the gaps it returns. It had no test at all until

+// an explicit page break needed one, and the measurement bug it already

+// carried is the argument for having it — it summed

+// getBoundingClientRect().height for a review cycle, which silently

+// ignores margins, ran about 15% short on ordinary prose and let the

+// breaks drift further down the document with each page.

+//

+// A fake view, not jsdom: the function reads two things, a document it

+// can walk and a box per block. Both are cheap to state exactly, and

+// stating them exactly is what lets a test say "this block is 800px

+// tall and its margin is 24px" without a layout engine.

+import { test } from 'node:test'

+import assert from 'node:assert/strict'

+import { breakPositions, PAGE_CONTENT_PX, PAGE_BREAK } from './pages.mjs'

+// A block: how tall it is, and how much space sits above it. Both are

+// what the browser would have already resolved by the time

+// getBoundingClientRect is asked, which is the point of measuring from

+// the page top rather than summing heights.

+function block(height, { marginTop = 0, type = 'paragraph' } = {}) {

+ return { height, marginTop, type }

+function pageBreak() {

+ return { height: 0, marginTop: 0, type: PAGE_BREAK }

+// viewOf lays the blocks out top to bottom at fixed coordinates and

+// returns something shaped like the two calls breakPositions makes.

+// Offsets are the block's index, which is all the function uses them

+// for — it returns them verbatim.

+function viewOf(blocks) {

+ let y = 0

+ const boxes = []

+ for (const b of blocks) {

+ y += b.marginTop

+ boxes.push({ top: y, bottom: y + b.height, height: b.height })

+ y += b.height

+ }

+ return {

+ state: {

+ doc: {

+ forEach(fn) {

+ blocks.forEach((b, i) => fn({ type: { name: b.type } }, i))

+ },

+ nodeDOM(offset) {

+ const rect = boxes[offset]

+ // A page break has no box, exactly as the stylesheet gives it

+ // none. breakPositions must not need one.

+ if (blocks[offset].type === PAGE_BREAK) return null

+ return { getBoundingClientRect: () => rect }

+ },

+ }

+test('a document that fits on one page has no breaks', () => {

+ const blocks = [block(100), block(100), block(100)]

+ assert.deepEqual(breakPositions(viewOf(blocks)), [])

+})

+test('a break falls at the first block that runs past the page', () => {

+ // 300px blocks stacked from 0: block 2 ends at 900, which is the

+ // first bottom past PAGE_CONTENT_PX (864), so it is the block that

+ // starts page two. Block 1 ends at 600 and fits.

+ const blocks = [block(300), block(300), block(300), block(300)]

+ const out = breakPositions(viewOf(blocks))

+ assert.deepEqual(out, [2])

+})

+// THE MEASUREMENT BUG, pinned. Blocks with margins occupy more of the

+// page than their heights add up to, and a summing implementation

+// cannot see that: these four blocks are 800px of height and 1200px of

+// page, so there is a break in here and a sum finds none at all.

+test('margins count toward the page, not just heights', () => {

+ const blocks = [

+ block(200),

+ block(200, { marginTop: 200 }),

+ block(200),

+ ]

+ // The whole document is 800px of block height, which fits a

+ // 864px page — so an implementation that summed heights would find

+ // no break at all. Measured against the page top, block 2 sits at

+ // 800 and ends at 1000, which does not fit. If this assertion ever

+ // fails, the test has stopped proving anything and the numbers need

+ // redoing rather than the expectation.

+ const summed = 200 * 4

+ assert.ok(summed < PAGE_CONTENT_PX, 'the heights alone must fit, or this test proves nothing')

+ const out = breakPositions(viewOf(blocks))

+ assert.deepEqual(out, [2], 'the space between the blocks was not counted')

+})

+// --- explicit page breaks ---

+test('a page break starts a new page at the block after it', () => {

+ const blocks = [block(100), pageBreak(), block(100), block(100)]

+ const out = breakPositions(viewOf(blocks))

+ // Index 2, the block AFTER the marker — never index 1, which would

+ // put the gap above the marker and strand it at the foot of the

+ // previous page.

+ assert.deepEqual(out, [2])

+})

+test('an explicit break wins over a page that is nowhere near full', () => {

+ const blocks = [block(10), pageBreak(), block(10)]

+ assert.deepEqual(breakPositions(viewOf(blocks)), [1 + 1])

+})

+// A break with nothing in front of it is not a break: there is no page

+// before it to end. The PDF exporter refuses the same case, and the two

+// must agree or a document paginates differently on screen and on

+// paper.

+test('a page break before the first block makes no empty page', () => {

+ const blocks = [pageBreak(), block(100), block(100)]

+ assert.deepEqual(breakPositions(viewOf(blocks)), [])

+})

+test('two page breaks in a row make one new page, not two', () => {

+ const blocks = [block(100), pageBreak(), pageBreak(), block(100)]

+ assert.deepEqual(breakPositions(viewOf(blocks)), [3])

+})

+// After an explicit break the measurement restarts from the new page's

+// top. Carrying the old page's origin forward would make the next

+// measured break fire almost immediately.

+test('measurement restarts from the top of the forced page', () => {

+ const blocks = [

+ block(800),

+ pageBreak(),

+ block(300),

+ ]

+ const out = breakPositions(viewOf(blocks))

+ // 2 is the forced break. Then the new page's top is 800, so block 4

+ // (1400..1700) is the first whose bottom is more than 864 past it.

+ assert.deepEqual(out, [2, 4], 'the forced page then fills normally')

+})

+test('a zero-height block is skipped rather than starting a page', () => {

+ const blocks = [block(0), block(100), block(100)]

+ assert.deepEqual(breakPositions(viewOf(blocks)), [])

+})

diff --git a/fixtures/toolbar.mjs b/fixtures/toolbar.mjs

index fda708e..845e45f 100644

--- a/fixtures/toolbar.mjs

+++ b/fixtures/toolbar.mjs

@@ -422,6 +422,22 @@ function buildCommands(schema) {

// than a move.

select_all: selectAll,

clear_formatting: clearFormatting,

+ // The two Insert commands that need nothing but a node type.

+ //

+ // Hyphenated names, matching menu.go's Command strings, which were

+ // agreed with the session building images and tables so its work

+ // drops into this same table. The underscore/hyphen split across

+ // this file is therefore not sloppiness: an underscore name is a

+ // NODE OR MARK name from the schema (strong, bullet_list,

+ // block_heading-1), and a hyphenated one is a menu verb that

+ // happens to have no schema name of its own.

+ //

+ // insertNode honours the null-dispatch contract the whole table is

+ // held to, so both items grey out where they cannot go — inside a

+ // code block, for instance — rather than being clickable and

+ // silently doing nothing.

+ 'insert-rule': insertNode(nodes.horizontal_rule),

+ 'insert-page-break': insertNode(nodes.page_break),

// The paragraph styles, as commands.

// The Style select already sets these, through setBlock and its own

@@ -453,6 +469,38 @@ function buildCommands(schema) {

}

+// insertNode puts one atomic block node at the selection.

+//

+// The null-dispatch contract (see buildCommands' own comment) is what

+// most of this function is for: asked with no dispatch it must report

+// whether the node COULD go here and change nothing. canInsert walks

+// out from the selection looking for a depth whose parent will accept

+// the type, which is prosemirror-example-setup's own idiom and is the

+// honest answer — "somewhere in my ancestry there is a place for this"

+// is exactly what replaceSelectionWith will find.

+//

+// A missing type returns false rather than throwing. A command table

+// built against a schema that does not declare page_break — an older

+// bundle, a future one that drops it — must grey the item out, not

+// break every other command in the table on the way past.

+function insertNode(type) {

+ return (state, dispatch) => {

+ if (!type) return false

+ if (!canInsert(state, type)) return false

+ if (dispatch) dispatch(state.tr.replaceSelectionWith(type.create()).scrollIntoView())

+ return true

+ }

+function canInsert(state, type) {

+ const $from = state.selection.$from

+ for (let d = $from.depth; d >= 0; d--) {

+ const index = $from.index(d)

+ if ($from.node(d).canReplaceWith(index, index, type)) return true

+ }

+ return false

// clearFormatting strips every mark from the selection and returns its

// blocks to plain paragraphs — the thing a person reaches for after a

// bad paste, and the one command in this table that is defined by what

diff --git a/internal/docs/export_test.go b/internal/docs/export_test.go

index ccf9355..6630ddf 100644

--- a/internal/docs/export_test.go

+++ b/internal/docs/export_test.go

@@ -214,6 +214,19 @@ func TestPageGeometryMatchesTheStylesheet(t *testing.T) {

// exporter in points.

{"indent step", `\.docs-indent-1[^}]*margin-inline-start:\s*([\d.]+)in`, export.IndentStepPt / 72},

{"deepest indent", `\.docs-indent-8[^}]*margin-inline-start:\s*([\d.]+)in`, export.IndentStepPt * export.MaxIndent / 72},

+ // The thematic break, Insert → Horizontal line. Three numbers,

+ // all read out of [docs-text] hr: the space above and below it,

+ // its thickness, and its tone.

+ //

+ // The stylesheet states the thickness in CSS pixels and the

+ // exporter in points, so the conversion is part of the

+ // comparison — 96 CSS pixels to the inch against PostScript's

+ // 72. That is the same relationship PAGE_CONTENT_PX in

+ // fixtures/pages.mjs carries, and getting it backwards is the

+ // obvious way for these two to agree by arithmetic while

+ // disagreeing on paper.

+ {"rule space", `\[docs-text\]\s+hr\s*\{[^}]*margin-block:\s*([\d.]+)em`, export.RuleSpaceAround},

+ {"rule thickness", `\[docs-text\]\s+hr\s*\{[^}]*border-block-start:\s*([\d.]+)px`, export.RulePt * 96 / 72},

}

// Every size mark, in pt on both sides.

diff --git a/internal/docs/menu.go b/internal/docs/menu.go

index ea91fe5..d5d62b7 100644

--- a/internal/docs/menu.go

+++ b/internal/docs/menu.go

@@ -239,8 +239,17 @@ func newMenuView(r *http.Request, sqid string) menuView {

{Kind: menuCommand, Command: "insert-image", Label: rastrillo.T(r, "docs.editor.menu.insert.image"), Disabled: true},

{Kind: menuCommand, Command: "insert-table", Label: rastrillo.T(r, "docs.editor.menu.insert.table"), Disabled: true},

{Kind: menuDivider},

- {Kind: menuCommand, Command: "insert-page-break", Label: rastrillo.T(r, "docs.editor.menu.insert.pagebreak"), Disabled: true},

- {Kind: menuCommand, Command: "insert-rule", Label: rastrillo.T(r, "docs.editor.menu.insert.rule"), Disabled: true},

+ // Both built. They are two nodes rather than one

+ // with a flag, and the difference is the whole

+ // reason they sit next to each other rather than

+ // being one item: a horizontal line is part of what

+ // the document SAYS (a thematic break, announced as

+ // a separator, `---` in Markdown), and a page break

+ // is a layout instruction with no meaning in the

+ // prose that Markdown cannot express at all. See

+ // fixtures/editor-entry.js's pageBreak.

+ {Kind: menuCommand, Command: "insert-page-break", Label: rastrillo.T(r, "docs.editor.menu.insert.pagebreak")},

+ {Kind: menuCommand, Command: "insert-rule", Label: rastrillo.T(r, "docs.editor.menu.insert.rule")},

{

diff --git a/internal/docs/views/show.html b/internal/docs/views/show.html

index 7ed44dd..6fc2dd8 100644

--- a/internal/docs/views/show.html

+++ b/internal/docs/views/show.html

@@ -536,6 +536,77 @@

[docs-text] .docs-size-sm { font-size: 10pt; }

[docs-text] .docs-size-lg { font-size: 14pt; }

[docs-text] .docs-size-xl { font-size: 18pt; }

+ /* A thematic break, Insert → Horizontal line. The numbers are read

+ out of this file by TestPageGeometryMatchesTheStylesheet and

+ compared with internal/export/pdf.go's ruleSpaceAround, rulePt and

+ ruleGray, so the line in a PDF is the line on the screen.

+ 1em above and below rather than the tighter spacing a divider

+ usually gets: an <hr> here is a change of subject, and tight to

+ the paragraph above it it reads as that paragraph's underline.

+ A literal colour, like everything else inside [docs-page]: this is

+ ink on paper and must not repaint under a reader's dark theme. */

+ [docs-text] hr {

+ margin-block: 1em;

+ border: 0;

+ border-block-start: 1px solid #c8c8c8;

+ }

+ /* An explicit page break, Insert → Page break.

+ (fixtures/editor-entry.js's pageBreak node, fragment.rs's

+ "page_break" arm.)

+ What it looks like depends entirely on the view, because in one of

+ them it does something and in the other it cannot.

+ In PAGED view it is invisible — fixtures/pages.mjs reads the node

+ and puts a real [docs-page-gap] after it, so drawing the marker as

+ well would be the break announced twice, once as furniture and

+ once as a line. display: none, not visibility: hidden, so it takes

+ no vertical space either: a page break that pushed the following

+ block down by its own height would move the very boundary it

+ names.

+ In INFINITE view there are no pages, so nothing can happen at it —

+ and an instruction with no visible effect at all is one a person

+ inserts, sees nothing, and inserts again. So it draws as a dashed

+ line: not the solid one an <hr> gets, because it is not a rule and

+ must not be mistaken for one.

+ How you DELETE one, since paged view draws no marker to click:

+ put the caret at the start of the block below the break and press

+ Backspace. The node is an atom, so the first press selects it and

+ the second removes it — ProseMirror's own behaviour, working off

+ the model rather than off a box, which is why display: none does

+ not strand it. Infinite view additionally lets you click the

+ dashed line and press Delete.

+ aria-hidden in the markup, both here and in the editor's toDOM: a

+ screen reader user is not reading pages, and a "page break"

+ announced in the middle of a sentence is noise about a thing that

+ does not affect the text. That is a deliberate asymmetry with

+ <hr>, which IS announced, because a thematic break is part of what

+ the document says.

+ In PRINT it is a real break-after, which is the browser's own

+ pagination doing what the PDF exporter does independently. */

+ [docs-page-break] {

+ display: none;

+ }

+ [docs-canvas][data-view="infinite"] [docs-page-break] {

+ display: block;

+ block-size: 0;

+ margin-block: 1em;

+ border-block-start: 1px dashed #a8a8ae;

+ }

+ @media print {

+ [docs-page-break] {

+ display: block;

+ block-size: 0;

+ border: 0;

+ break-after: page;

+ }

/* Indent (fragment.rs's indent_class, fixtures/indent.mjs's

indentClass, and internal/export/pdf.go's indentOf — one step in

four places). Half an inch per level, which is what Docs, Word and

diff --git a/internal/export/document.go b/internal/export/document.go

index b118a0c..70fa313 100644

--- a/internal/export/document.go

+++ b/internal/export/document.go

@@ -99,6 +99,15 @@ const (

blockOrderedList

blockListItem

blockQuote

+ // A thematic break and a page break: two nodes that look alike and

+ // mean nothing alike. See fixtures/editor-entry.js's pageBreak for

+ // the argument. blockRule is prose — a change of subject, announced

+ // as a separator, spelled `---` in Markdown. blockPageBreak is

+ // layout, has no Markdown spelling at all, and is the only node in

+ // this vocabulary the PDF writer obeys as an instruction rather

+ // than drawing.

+ blockRule

+ blockPageBreak

)

func kindOf(n Node) blockKind {

@@ -115,6 +124,10 @@ func kindOf(n Node) blockKind {

return blockListItem

case "blockquote":

return blockQuote

+ case "horizontal_rule":

+ return blockRule

+ case "page_break":

+ return blockPageBreak

}

return blockUnknown

}

diff --git a/internal/export/markdown.go b/internal/export/markdown.go

index d89e7f8..3767765 100644

--- a/internal/export/markdown.go

+++ b/internal/export/markdown.go

@@ -116,6 +116,44 @@ func writeBlock(b *strings.Builder, n Node, indent string) {

b.WriteString(strings.ReplaceAll(text, "\n", "\n"+indent))

b.WriteString("\n\n")

+ case blockRule:

+ // CommonMark's thematic break. `---` rather than `***` or

+ // `___` because it is the one a person reading the file

+ // recognises on sight.

+ //

+ // The blank line ABOVE it is load-bearing and is not written

+ // here: `---` directly under a line of text is a setext H2

+ // underline, so it would turn the paragraph above into a

+ // heading and swallow the rule. Every block case in this

+ // function ends with "\n\n" — a paragraph, a heading, a list,

+ // a quote — which is what guarantees there is one. That is a

+ // property of this file rather than of this case, and it is

+ // worth knowing before somebody makes a block that ends with a

+ // single newline.

+ b.WriteString(indent)

+ b.WriteString("---\n\n")

+ case blockPageBreak:

+ // Markdown has no page break, because Markdown has no pages.

+ //

+ // So this is a comment, and the alternatives are worse. `---`

+ // would spell it as a thematic break, which is a different

+ // thing that this vocabulary deliberately keeps separate — a

+ // reader would see a horizontal line where the author put a

+ // page boundary, and a round trip would turn one into the

+ // other permanently. Emitting nothing would be silent data

+ // loss, which is the rule this whole package is written

+ // against (see the core's renderers: never lose what the

+ // document holds).

+ //

+ // An HTML comment is inert in every Markdown renderer, visible

+ // to a person reading the file, and unambiguous. It does not

+ // round-trip back into a page_break node — nothing in this app

+ // imports Markdown — and claiming it did would be the

+ // overreach worth avoiding here.

+ b.WriteString(indent)

+ b.WriteString("\n\n")

case blockBulletList:

writeList(b, n, indent, false)

diff --git a/internal/export/markdown_test.go b/internal/export/markdown_test.go

index 3e185d0..ae94c25 100644

--- a/internal/export/markdown_test.go

+++ b/internal/export/markdown_test.go

@@ -461,3 +461,61 @@ func TestMarkdownIndentedRunLinesBecomeCodeBlocks(t *testing.T) {

t.Errorf("the text was lost: %q", got)

}

+// A horizontal line and a page break are two nodes that look alike and

+// must not export alike. `---` is CommonMark's thematic break and is

+// what an <hr> means; a page break has no Markdown spelling at all, so

+// it becomes an inert comment rather than either a second `---` (which

+// would silently turn it into a thematic break, permanently, on any

+// round trip) or nothing (which would lose it).

+func TestMarkdownWritesARuleAndAPageBreakDifferently(t *testing.T) {

+ js := `{"type":"doc","content":[

+ {"type":"paragraph","content":[{"type":"text","text":"Before."}]},

+ {"type":"horizontal_rule","content":[]},

+ {"type":"paragraph","content":[{"type":"text","text":"Between."}]},

+ {"type":"page_break","content":[]},

+ {"type":"paragraph","content":[{"type":"text","text":"After."}]}

+ ]}`

+ got := export.Markdown(mustParse(t, "", js))

+ if !strings.Contains(got, "\n---\n") {

+ t.Errorf("no thematic break in:\n%s", got)

+ }

+ if strings.Count(got, "---") != 1 {

+ t.Errorf("the page break was also written as a thematic break:\n%s", got)

+ }

+ if !strings.Contains(got, "") {

+ t.Errorf("the page break was lost:\n%s", got)

+ }

+ // All three paragraphs survive, in order. Losing the words is the

+ // failure this whole package is written against.

+ for _, want := range []string{"Before.", "Between.", "After."} {

+ if !strings.Contains(got, want) {

+ t.Errorf("%q is missing from:\n%s", want, got)

+ }

+ if i, j := strings.Index(got, "Before."), strings.Index(got, "---"); i > j {

+ t.Errorf("the rule is above the paragraph before it:\n%s", got)

+ }

+// `---` directly under a line of text is a setext H2 underline, not a

+// thematic break — it would turn the paragraph above it into a heading

+// and swallow the rule. Every block writer in markdown.go ends with a

+// blank line, which is what prevents it; this is the test that notices

+// if one stops.

+func TestMarkdownRuleIsNotASetextUnderline(t *testing.T) {

+ js := `{"type":"doc","content":[

+ {"type":"paragraph","content":[{"type":"text","text":"Not a heading"}]},

+ {"type":"horizontal_rule","content":[]}

+ ]}`

+ got := export.Markdown(mustParse(t, "", js))

+ lines := strings.Split(got, "\n")

+ for i, line := range lines {

+ if strings.TrimSpace(line) == "---" {

+ if i == 0 || strings.TrimSpace(lines[i-1]) != "" {

+ t.Fatalf("--- has text directly above it, which reads as a setext heading:\n%s", got)

+ }

diff --git a/internal/export/pdf.go b/internal/export/pdf.go

index 68937c2..e78ea74 100644

--- a/internal/export/pdf.go

+++ b/internal/export/pdf.go

@@ -71,6 +71,19 @@ const (

quoteRuleGray = 0.78 // #c8c8c8

codeScale = 0.85 // [docs-text] code { font-size: 0.85em }

+ // A thematic break: the line itself, the space above and below it,

+ // and its tone. views/show.html's [docs-text] hr states the same

+ // three, and TestPageGeometryMatchesTheStylesheet is where the

+ // screen and the page are held to each other.

+ //

+ // The space is symmetrical and generous because an <hr> is a change

+ // of subject rather than a divider between two paragraphs of one:

+ // tight to the text above it, it reads as that paragraph's

+ // underline.

+ RuleSpaceAround = 1.0 // em of body size, above and below

+ RulePt = 0.75 // 1px at 96dpi

+ ruleGray = 0.78 // #c8c8c8, the same tone as a quote's bar

)

// IndentStepPt is one indent level, in points: half an inch, which is

@@ -499,6 +512,50 @@ func (l *layout) block(n Node, ctx blockContext) {

l.paragraph(n.Content, pctx, BodySizePt*ctx.scale*BodyLeading)

l.space(BodySizePt * paragraphSpaceBelow)

+ case blockRule:

+ // A drawn line, laid out like a block: reserve the space above

+ // it, put the rule in the middle of its own line box, reserve

+ // the space below.

+ //

+ // advance() before drawing, not after, and that is the part

+ // worth reading twice. advance is what moves to a new page when

+ // the cursor has run out of room, so calling it first means the

+ // rule is drawn onto whatever page it actually belongs on. Draw

+ // first and a rule at a page boundary lands below the bottom

+ // margin of the page that was already full — off the paper, in

+ // a PDF, with no clipping to make it obvious.

+ l.space(BodySizePt * RuleSpaceAround)

+ l.applyPending()

+ if l.cursor+RulePt > contentBottom {

+ l.newPage()

+ }

+ l.page.Rule(MarginPt+ctx.indent, PageHeightPt-l.cursor, contentWidthPt-ctx.indent, RulePt, ruleGray)

+ l.cursor += RulePt

+ l.atPageTop = false

+ l.space(BodySizePt * RuleSpaceAround)

+ case blockPageBreak:

+ // The only node in this vocabulary that is an instruction

+ // rather than something to draw.

+ //

+ // Guarded on atPageTop, so a page break at the very start of a

+ // page — the first thing in a document, or one directly after

+ // another — does not emit a blank page. That is what every word

+ // processor does with a break that has nothing in front of it,

+ // and a stray empty page in an exported PDF is the kind of

+ // thing a person blames the export for rather than the

+ // document.

+ //

+ // pending is cleared as well: the margin queued by whatever

+ // block came before belongs to the bottom of the page being

+ // left, and applying it at the top of the next one would open

+ // that page with a band of white. newPage clears it too; this

+ // is for the branch that does not call newPage.

+ if !l.atPageTop {

+ l.newPage()

+ }

+ l.pending = 0

case blockBulletList:

l.list(n, ctx, false)

diff --git a/internal/export/pdf_test.go b/internal/export/pdf_test.go

index f7c92e2..feec50b 100644

--- a/internal/export/pdf_test.go

+++ b/internal/export/pdf_test.go

@@ -703,3 +703,112 @@ func TestPDFDropsAnAlignmentItDoesNotKnow(t *testing.T) {

}

+// A page break starts a new page, and everything after it is on that

+// page rather than on the one before.

+//

+// This reads the /Kids list and the drawn text rather than counting

+// l.newPage() calls, because the question is what a reader opens, not

+// what the layout function intended.

+func TestPDFPageBreakStartsANewPage(t *testing.T) {

+ js := `{"type":"doc","content":[

+ {"type":"paragraph","content":[{"type":"text","text":"FirstPageWord"}]},

+ {"type":"page_break","content":[]},

+ {"type":"paragraph","content":[{"type":"text","text":"SecondPageWord"}]}

+ ]}`

+ data, _ := mustPDF(t, mustParse(t, "", js))

+ f := readPDF(t, data)

+ pages := f.pages(t)

+ if len(pages) != 2 {

+ t.Fatalf("the document has %d page(s), want 2 — the break did not start one", len(pages))

+ }

+ if got := f.text(t, pages[0]); !strings.Contains(got, "FirstPageWord") {

+ t.Errorf("page 1 does not hold the paragraph before the break: %q", got)

+ }

+ if got := f.text(t, pages[0]); strings.Contains(got, "SecondPageWord") {

+ t.Errorf("page 1 holds the paragraph after the break: %q", got)

+ }

+ if got := f.text(t, pages[1]); !strings.Contains(got, "SecondPageWord") {

+ t.Errorf("page 2 does not hold the paragraph after the break: %q", got)

+ }

+// A break with nothing in front of it must not produce a blank page.

+// Every word processor does this, and a stray empty first page in an

+// export is blamed on the exporter rather than on the document.

+func TestPDFPageBreakAtTheTopMakesNoBlankPage(t *testing.T) {

+ js := `{"type":"doc","content":[

+ {"type":"page_break","content":[]},

+ {"type":"paragraph","content":[{"type":"text","text":"Only page"}]}

+ ]}`

+ data, _ := mustPDF(t, mustParse(t, "", js))

+ f := readPDF(t, data)

+ if pages := f.pages(t); len(pages) != 1 {

+ t.Fatalf("%d pages, want 1 — a break with nothing before it emitted a blank page", len(pages))

+ }

+// A horizontal rule is DRAWN, and is drawn on the paper rather than in

+// the margin: it starts at the left margin and spans the content

+// width.

+func TestPDFDrawsAHorizontalRule(t *testing.T) {

+ js := `{"type":"doc","content":[

+ {"type":"paragraph","content":[{"type":"text","text":"Above"}]},

+ {"type":"horizontal_rule","content":[]},

+ {"type":"paragraph","content":[{"type":"text","text":"Below"}]}

+ ]}`

+ data, _ := mustPDF(t, mustParse(t, "", js))

+ f := readPDF(t, data)

+ pages := f.pages(t)

+ if len(pages) != 1 {

+ t.Fatalf("%d pages, want 1", len(pages))

+ }

+ var found *rect

+ for i, r := range f.rules(t, pages[0]) {

+ if r.h > 0 && r.h <= export.RulePt+0.01 {

+ found = &f.rules(t, pages[0])[i]

+ break

+ }

+ if found == nil {

+ t.Fatalf("no rule of the right thickness on the page; rectangles: %v", f.rules(t, pages[0]))

+ }

+ if found.x != export.MarginPt {

+ t.Errorf("the rule starts at x=%v, want the left margin %v", found.x, float64(export.MarginPt))

+ }

+ if want := float64(export.PageWidthPt - 2*export.MarginPt); found.w != want {

+ t.Errorf("the rule is %v wide, want the content width %v", found.w, want)

+ }

+ // Both paragraphs still there: a rule must not eat its neighbours.

+ if got := f.text(t, pages[0]); !strings.Contains(got, "Above") || !strings.Contains(got, "Below") {

+ t.Errorf("text around the rule was lost: %q", got)

+ }

+// A rule with no room left on the page goes onto the next one, rather

+// than being drawn below the bottom margin where a PDF does not clip

+// it and it simply sits off the paper.

+func TestPDFRuleAtAPageBoundaryMovesToTheNextPage(t *testing.T) {

+ var b strings.Builder

+ b.WriteString(`{"type":"doc","content":[`)

+ // Enough paragraphs to fill a page and land the rule at the edge.

+ for i := 0; i < 60; i++ {

+ b.WriteString(`{"type":"paragraph","content":[{"type":"text","text":"Filler line"}]},`)

+ }

+ b.WriteString(`{"type":"horizontal_rule","content":[]}]}`)

+ data, _ := mustPDF(t, mustParse(t, "", b.String()))

+ f := readPDF(t, data)

+ for i, page := range f.pages(t) {

+ for _, r := range f.rules(t, page) {

+ // y is measured from the bottom of the page in PDF space,

+ // so anything at or below the bottom margin is off the

+ // text area.

+ if r.h <= export.RulePt+0.01 && r.y < 0 {

+ t.Errorf("page %d has a rule at y=%v, which is off the paper", i+1, r.y)

+ }