From 4b0edae357470e7a663306adc02d451af8dbbfa5 Mon Sep 17 00:00:00 2001 From: Ready22Race <8jsntcw4wg@privaterelay.appleid.com> Date: Thu, 6 Aug 2026 02:54:57 -0700 Subject: [PATCH] fix(presentations): separate slides with a thematic break MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three presentation parsers concatenate every slide's blocks into one list with nothing between them. A slide's title becomes a heading, so decks that title every slide read correctly by accident — but a slide with no title placeholder contributes no structural block at all, and its content becomes indistinguishable from the previous slide's. Two title-less slides in a row produce two adjacent paragraphs, exactly as if they were two paragraphs of one slide. Bullet lists from different slides merge into one list. This is not the pagination that "Markdown has no pages" rightly refuses. A slide is a container in the source model, not a layout artifact, and losing its boundary loses document structure. Block::Rule is the separator the model already has (emitted for
, rendered as ---), so no new concept is introduced and no page number is implied. Emitted BETWEEN slides only: never leading, and a slide that produces no blocks never yields a doubled or dangling break. Applies to pptx, ppt and odp, which all had the same shape. Core-only: every binding already maps model::Block::Rule (node/src/ document.rs:88, python/src/document.rs:83, wasm/src/document.rs:96) and already declares the `rule` kind (node/index.d.ts:60, python/anydoc/_anydoc.pyi:84, wasm/src/typescript.rs:47), so no binding source or type surface changes. --- src/formats/odf/mod.rs | 14 +- src/formats/ppt/mod.rs | 4 + src/formats/pptx/mod.rs | 129 +++++++++++++++++- tests/snapshots/snapshots__odp__pres.odp.snap | 2 + ...pshots__ppt__handmade-multimaster.ppt.snap | 2 + ...pshots__ppt__handmade-sparsenotes.ppt.snap | 2 + tests/snapshots/snapshots__ppt__pres.ppt.snap | 2 + .../snapshots__pptx__handmade-links.pptx.snap | 2 + .../snapshots/snapshots__pptx__pres.pptx.snap | 2 + 9 files changed, 154 insertions(+), 5 deletions(-) diff --git a/src/formats/odf/mod.rs b/src/formats/odf/mod.rs index 94cac91..81b9eb3 100644 --- a/src/formats/odf/mod.rs +++ b/src/formats/odf/mod.rs @@ -75,11 +75,19 @@ fn parse_presentation(pres: &Element, ctx: &Ctx) -> Result, ConvertEr let mut body = Vec::new(); let mut notes = Vec::new(); walk_shapes(page, ctx, &mut title, &mut body, &mut notes)?; - blocks.append(&mut title); - blocks.append(&mut body); + let mut page_blocks: Vec = Vec::new(); + page_blocks.append(&mut title); + page_blocks.append(&mut body); // Speaker notes are included (fixed policy), set off as a quote. if !notes.is_empty() { - blocks.push(Block::BlockQuote(notes)); + page_blocks.push(Block::BlockQuote(notes)); + } + // Separate pages with a thematic break — see the pptx parser for why. + if !page_blocks.is_empty() { + if !blocks.is_empty() { + blocks.push(Block::Rule); + } + blocks.append(&mut page_blocks); } } Ok(blocks) diff --git a/src/formats/ppt/mod.rs b/src/formats/ppt/mod.rs index d0f38de..c7877b7 100644 --- a/src/formats/ppt/mod.rs +++ b/src/formats/ppt/mod.rs @@ -343,6 +343,10 @@ impl Extractor { let mut used = vec![false; notes.len()]; let mut out = Vec::new(); for (sid, blocks) in slides { + // Separate slides with a thematic break — see the pptx parser. + if !blocks.is_empty() && !out.is_empty() { + out.push(Block::Rule); + } out.extend(blocks); for (i, (nid, nblocks)) in notes.iter_mut().enumerate() { if !used[i] && sid.is_some() && *nid == sid { diff --git a/src/formats/pptx/mod.rs b/src/formats/pptx/mod.rs index 800c26e..9d82bfb 100644 --- a/src/formats/pptx/mod.rs +++ b/src/formats/pptx/mod.rs @@ -163,12 +163,23 @@ pub fn parse(bytes: &[u8]) -> Result { instance_counter: &instance_counter, slide_anchors: &slide_anchors, }; + // A slide is a container, not a page: its content must not run into + // the next slide's. A thematic break is the structural separator the + // model already has, and it is emitted BETWEEN slides only — never + // leading, never doubled by an empty slide. + let mut slide_blocks: Vec = Vec::new(); if targeted.contains(slide_path) && let Some(anchor) = slide_anchors.get(slide_path) { - blocks.push(Block::Paragraph(vec![Inline::Anchor(anchor.clone())])); + slide_blocks.push(Block::Paragraph(vec![Inline::Anchor(anchor.clone())])); + } + parse_shapes(sp_tree, &ctx, &mut slide_blocks)?; + if !slide_blocks.is_empty() { + if !blocks.is_empty() { + blocks.push(Block::Rule); + } + blocks.append(&mut slide_blocks); } - parse_shapes(sp_tree, &ctx, &mut blocks)?; // Speaker notes, set off as a quote (fixed policy: included). The // tree is loaded before the `if let` so the package borrow is not @@ -636,3 +647,117 @@ fn parse_table(tbl: &Element, ctx: &SlideCtx, blocks: &mut Vec) -> Result blocks.push(Block::Table(table)); Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + use std::io::Write; + + const PRES_NS: &str = "http://schemas.openxmlformats.org/presentationml/2006/main"; + const DRAWING_NS: &str = "http://schemas.openxmlformats.org/drawingml/2006/main"; + const REL_NS: &str = "http://schemas.openxmlformats.org/officeDocument/2006/relationships"; + const PKG_REL_NS: &str = "http://schemas.openxmlformats.org/package/2006/relationships"; + + /// A slide holding a single text box with `text`, or no shapes at all when + /// `text` is `None` — the "contributes nothing" case the separator must not + /// leave a dangling break for. + fn slide_xml(text: Option<&str>) -> String { + let body = match text { + Some(t) => format!( + r#"{t}"# + ), + None => String::new(), + }; + format!( + r#"{body}"# + ) + } + + /// Minimal .pptx whose slides carry the given text bodies, in order. No + /// layout or master parts: every slide is title-less, which is exactly the + /// shape whose boundary used to vanish. + fn pptx_with_slides(slides: &[Option<&str>]) -> Vec { + let ids: String = (0..slides.len()) + .map(|i| format!(r#""#, 256 + i, i + 1)) + .collect(); + let rels: String = (0..slides.len()) + .map(|i| { + format!( + r#""#, + i + 1, + i + 1 + ) + }) + .collect(); + + let mut w = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); + let opts = zip::write::SimpleFileOptions::default(); + let mut put = |name: &str, body: &str| { + w.start_file(name, opts).unwrap(); + w.write_all(body.as_bytes()).unwrap(); + }; + put( + "_rels/.rels", + &format!( + r#""# + ), + ); + put( + "ppt/presentation.xml", + &format!( + r#"{ids}"# + ), + ); + put( + "ppt/_rels/presentation.xml.rels", + &format!( + r#"{rels}"# + ), + ); + for (i, text) in slides.iter().enumerate() { + put(&format!("ppt/slides/slide{}.xml", i + 1), &slide_xml(*text)); + } + w.finish().unwrap().into_inner() + } + + fn blocks(slides: &[Option<&str>]) -> Vec { + parse(&pptx_with_slides(slides)).expect("minimal pptx should convert").blocks + } + + #[test] + fn untitled_slides_are_separated() { + // The defect: with no title placeholder neither slide contributes a + // heading, so the two paragraphs used to be indistinguishable from two + // paragraphs of one slide. + let out = blocks(&[Some("first"), Some("second")]); + assert!( + matches!(out.as_slice(), [Block::Paragraph(_), Block::Rule, Block::Paragraph(_)]), + "expected a separator between the two slides, got {out:?}" + ); + } + + #[test] + fn separator_never_leads() { + let out = blocks(&[Some("only")]); + assert!( + !matches!(out.first(), Some(Block::Rule)), + "a deck must not open with a separator, got {out:?}" + ); + assert_eq!(out.len(), 1, "single slide should yield exactly its own content: {out:?}"); + } + + #[test] + fn empty_slides_leave_no_dangling_separator() { + // An empty slide contributes no blocks, so it must not push a break of + // its own — neither doubled between two real slides nor trailing. + let out = blocks(&[Some("first"), None, Some("second")]); + let rules = out.iter().filter(|b| matches!(b, Block::Rule)).count(); + assert_eq!(rules, 1, "empty slide must not add a separator, got {out:?}"); + + let trailing = blocks(&[Some("first"), None]); + assert!( + !matches!(trailing.last(), Some(Block::Rule)), + "a trailing empty slide must not leave a dangling separator, got {trailing:?}" + ); + } +} diff --git a/tests/snapshots/snapshots__odp__pres.odp.snap b/tests/snapshots/snapshots__odp__pres.odp.snap index d46d347..4f3e359 100644 --- a/tests/snapshots/snapshots__odp__pres.odp.snap +++ b/tests/snapshots/snapshots__odp__pres.odp.snap @@ -11,6 +11,8 @@ Deck Title Slide > Speaker note for the intro slide. +--- + Numbers Slide | Region | Total | diff --git a/tests/snapshots/snapshots__ppt__handmade-multimaster.ppt.snap b/tests/snapshots/snapshots__ppt__handmade-multimaster.ppt.snap index e5e99b5..79f2352 100644 --- a/tests/snapshots/snapshots__ppt__handmade-multimaster.ppt.snap +++ b/tests/snapshots/snapshots__ppt__handmade-multimaster.ppt.snap @@ -4,4 +4,6 @@ expression: output --- - **Alpha master body text** +--- + *Beta master body text* diff --git a/tests/snapshots/snapshots__ppt__handmade-sparsenotes.ppt.snap b/tests/snapshots/snapshots__ppt__handmade-sparsenotes.ppt.snap index 812a4e5..4b8b11b 100644 --- a/tests/snapshots/snapshots__ppt__handmade-sparsenotes.ppt.snap +++ b/tests/snapshots/snapshots__ppt__handmade-sparsenotes.ppt.snap @@ -4,6 +4,8 @@ expression: output --- First slide text +--- + Second slide text > Notes for the second slide diff --git a/tests/snapshots/snapshots__ppt__pres.ppt.snap b/tests/snapshots/snapshots__ppt__pres.ppt.snap index 67a4c63..38a868d 100644 --- a/tests/snapshots/snapshots__ppt__pres.ppt.snap +++ b/tests/snapshots/snapshots__ppt__pres.ppt.snap @@ -12,6 +12,8 @@ Deck Title Slide > Speaker note for the intro slide. +--- + Numbers Slide Region diff --git a/tests/snapshots/snapshots__pptx__handmade-links.pptx.snap b/tests/snapshots/snapshots__pptx__handmade-links.pptx.snap index aae0a01..b11df89 100644 --- a/tests/snapshots/snapshots__pptx__handmade-links.pptx.snap +++ b/tests/snapshots/snapshots__pptx__handmade-links.pptx.snap @@ -6,6 +6,8 @@ expression: output [External link](https://example.com/) +--- + Second slide content diff --git a/tests/snapshots/snapshots__pptx__pres.pptx.snap b/tests/snapshots/snapshots__pptx__pres.pptx.snap index 61f7f60..a7b9a90 100644 --- a/tests/snapshots/snapshots__pptx__pres.pptx.snap +++ b/tests/snapshots/snapshots__pptx__pres.pptx.snap @@ -12,6 +12,8 @@ Deck Title Slide > Speaker note for the intro slide. +--- + Numbers Slide | Region | Total |