From: Ben Pfaff <blp@cs.stanford.edu>
Date: Wed, 31 Dec 2025 22:02:44 +0000 (-0800)
Subject: spv outline works
X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1a68f70b3d48e738cb3a04b5fa30f5bff5740e78;p=pspp

spv outline works
---

diff --git a/rust/pspp/src/cli/show_spv.rs b/rust/pspp/src/cli/show_spv.rs
index 4bf125bf85..3a4c7f05f5 100644
--- a/rust/pspp/src/cli/show_spv.rs
+++ b/rust/pspp/src/cli/show_spv.rs
@@ -17,7 +17,7 @@
 use anyhow::Result;
 use clap::{Args, ValueEnum};
 use pspp::{
-    output::{Criteria, Item, ItemInfo},
+    output::{Criteria, Item, Itemlike},
     spv::SpvArchive,
 };
 use std::{fmt::Display, path::PathBuf, sync::Arc};
diff --git a/rust/pspp/src/output.rs b/rust/pspp/src/output.rs
index a915914a75..c1f5987450 100644
--- a/rust/pspp/src/output.rs
+++ b/rust/pspp/src/output.rs
@@ -71,6 +71,9 @@ pub struct Item {
     /// outline view.
     pub show: bool,
 
+    /// Whether the item should start at the top of a page.
+    pub page_break_before: bool,
+
     /// Item details.
     pub details: Details,
 
@@ -80,16 +83,25 @@ pub struct Item {
     pub spv_info: Option<Box<SpvInfo>>,
 }
 
-pub trait ItemInfo {
+pub trait Itemlike {
     fn label(&self) -> Cow<'static, str>;
     fn command_name(&self) -> Option<&str>;
     fn subtype(&self) -> Option<String>;
     fn is_shown(&self) -> bool;
+    fn page_break_before(&self) -> bool;
     fn spv_info(&self) -> Option<&SpvInfo>;
     fn iter_in_order(&self) -> ItemRefIterator<'_, Self>
     where
         Self: Sized;
+    fn children(&self) -> &[Arc<Self>];
+    fn children_mut(&mut self) -> Option<&mut Vec<Arc<Self>>>;
     fn kind(&self) -> ItemKind;
+    fn is_heading(&self) -> bool {
+        self.kind() == ItemKind::Heading
+    }
+    fn is_table(&self) -> bool {
+        self.kind() == ItemKind::Table
+    }
     fn class(&self) -> Class {
         match self.kind() {
             ItemKind::Graph => Class::Graphs,
@@ -99,7 +111,6 @@ pub trait ItemInfo {
                 Severity::Note => Class::Notes,
                 Severity::Error | Severity::Warning => Class::Warnings,
             },
-            ItemKind::PageBreak => Class::Other,
             ItemKind::Table => match self.label().as_ref() {
                 "Warnings" => Class::Warnings,
                 "Notes" => Class::Notes,
@@ -113,12 +124,9 @@ pub trait ItemInfo {
             },
         }
     }
-
-    type Child: AsRef<Self>;
-    fn children(&self) -> &[Self::Child];
 }
 
-impl ItemInfo for Item {
+impl Itemlike for Item {
     fn label(&self) -> Cow<'static, str> {
         match &self.label {
             Some(label) => Cow::from(label.clone()),
@@ -144,6 +152,10 @@ impl ItemInfo for Item {
         self.details.is_heading() || self.show
     }
 
+    fn page_break_before(&self) -> bool {
+        self.page_break_before
+    }
+
     fn spv_info(&self) -> Option<&SpvInfo> {
         self.spv_info.as_deref()
     }
@@ -152,11 +164,14 @@ impl ItemInfo for Item {
         ItemRefIterator::new(self)
     }
 
-    type Child = Arc<Item>;
-    fn children(&self) -> &[Self::Child] {
+    fn children(&self) -> &[Arc<Self>] {
         self.details.children()
     }
 
+    fn children_mut(&mut self) -> Option<&mut Vec<Arc<Item>>> {
+        self.details.children_mut()
+    }
+
     fn kind(&self) -> ItemKind {
         self.details.kind()
     }
@@ -166,6 +181,7 @@ impl Item {
     pub fn new(details: impl Into<Details>) -> Self {
         let details = details.into();
         Self {
+            page_break_before: false,
             label: None,
             command_name: details.command_name().cloned(),
             show: true,
@@ -209,6 +225,13 @@ impl Item {
         }
     }
 
+    pub fn with_page_break_before(self, page_break_before: bool) -> Self {
+        Self {
+            page_break_before,
+            ..self
+        }
+    }
+
     pub fn cursor(self: Arc<Self>) -> ItemCursor<Self> {
         ItemCursor::new(self)
     }
@@ -271,7 +294,6 @@ pub enum ItemKind {
     Image,
     Heading,
     Message(Severity),
-    PageBreak,
     Table,
     Text,
 }
@@ -283,7 +305,6 @@ impl ItemKind {
             ItemKind::Image => "image",
             ItemKind::Heading => "heading",
             ItemKind::Message(_) => "message",
-            ItemKind::PageBreak => "page break",
             ItemKind::Table => "table",
             ItemKind::Text => "text",
         }
@@ -302,7 +323,6 @@ pub enum Details {
     Image(#[serde(skip_serializing)] ImageSurface),
     Heading(Heading),
     Message(Box<Diagnostic>),
-    PageBreak,
     Table(Box<PivotTable>),
     Text(Box<Text>),
 }
@@ -319,7 +339,7 @@ impl Details {
         }
     }
 
-    pub fn mut_children(&mut self) -> Option<&mut Vec<Arc<Item>>> {
+    pub fn children_mut(&mut self) -> Option<&mut Vec<Arc<Item>>> {
         match self {
             Self::Heading(heading) => Some(&mut heading.0),
             _ => None,
@@ -360,7 +380,6 @@ impl Details {
             | Details::Image(_)
             | Details::Heading(_)
             | Details::Message(_)
-            | Details::PageBreak
             | Details::Text(_) => None,
             Details::Table(pivot_table) => pivot_table.metadata.command_c.as_ref(),
         }
@@ -372,7 +391,6 @@ impl Details {
             Details::Image(_) => Cow::from("Image"),
             Details::Heading(_) => Cow::from("Group"),
             Details::Message(diagnostic) => Cow::from(diagnostic.severity.as_title_str()),
-            Details::PageBreak => Cow::from("Page Break"),
             Details::Table(pivot_table) => Cow::from(pivot_table.label()),
             Details::Text(text) => Cow::from(text.type_.as_str()),
         }
@@ -386,10 +404,6 @@ impl Details {
         matches!(self, Self::Message(_))
     }
 
-    pub fn is_page_break(&self) -> bool {
-        matches!(self, Self::PageBreak)
-    }
-
     pub fn is_table(&self) -> bool {
         matches!(self, Self::Table(_))
     }
@@ -404,7 +418,6 @@ impl Details {
             Details::Image(_) => ItemKind::Image,
             Details::Heading(_) => ItemKind::Heading,
             Details::Message(diagnostic) => ItemKind::Message(diagnostic.severity),
-            Details::PageBreak => ItemKind::PageBreak,
             Details::Table(_) => ItemKind::Table,
             Details::Text(_) => ItemKind::Text,
         }
@@ -568,7 +581,7 @@ pub struct ItemRefIterator<'a, T> {
 impl<'a, T> ItemRefIterator<'a, T> {
     pub fn without_hidden(self) -> impl Iterator<Item = &'a T>
     where
-        T: ItemInfo,
+        T: Itemlike,
     {
         self.filter(|item| item.is_shown())
     }
@@ -583,7 +596,7 @@ impl<'a, T> ItemRefIterator<'a, T> {
 
 impl<'a, T> Iterator for ItemRefIterator<'a, T>
 where
-    T: ItemInfo,
+    T: Itemlike,
 {
     type Item = &'a T;
 
@@ -605,22 +618,17 @@ where
     }
 }
 
-pub struct ItemCursor<T>
-where
-    T: ItemInfo,
-    T::Child: Clone,
-{
-    cur: Option<T::Child>,
-    stack: Vec<(T::Child, usize)>,
+pub struct ItemCursor<T> {
+    cur: Option<Arc<T>>,
+    stack: Vec<(Arc<T>, usize)>,
     include_hidden: bool,
 }
 
 impl<T> ItemCursor<T>
 where
-    T: ItemInfo,
-    T::Child: Clone,
+    T: Itemlike,
 {
-    pub fn new(start: T::Child) -> Self {
+    pub fn new(start: Arc<T>) -> Self {
         Self {
             cur: start.as_ref().is_shown().then_some(start),
             stack: Vec::new(),
@@ -628,7 +636,7 @@ where
         }
     }
 
-    pub fn with_hidden(start: T::Child) -> Self {
+    pub fn with_hidden(start: Arc<T>) -> Self {
         Self {
             cur: Some(start),
             stack: Vec::new(),
@@ -636,15 +644,14 @@ where
         }
     }
 
-    pub fn cur(&self) -> Option<&T::Child> {
+    pub fn cur(&self) -> Option<&Arc<T>> {
         self.cur.as_ref()
     }
 
     pub fn next(&mut self) {
         fn inner<T>(this: &mut ItemCursor<T>)
         where
-            T: ItemInfo,
-            T::Child: Clone,
+            T: Itemlike,
         {
             let Some(cur) = this.cur.take() else {
                 return;
@@ -700,7 +707,7 @@ pub struct SpvInfo {
 }
 
 impl SpvInfo {
-    pub fn new(structure_member: &str) -> Self {
+    pub fn new(structure_member: impl Into<String>) -> Self {
         Self {
             error: false,
             structure_member: structure_member.into(),
@@ -985,26 +992,33 @@ pub struct Criteria(pub Vec<Selection>);
 impl Criteria {
     /// Returns output items that are a subset of `input` that match the
     /// criteria.
-    pub fn apply(&self, input: Vec<Item>) -> Vec<Arc<Item>> {
-        fn take_children(item: &Item) -> Vec<&Item> {
-            item.details.children().iter().map(|item| &**item).collect()
+    pub fn apply<T>(&self, input: Vec<T>) -> Vec<Arc<T>>
+    where
+        T: Itemlike + Clone,
+    {
+        fn take_children<T: Itemlike>(item: &T) -> Vec<&T> {
+            item.children().iter().map(|item| item.as_ref()).collect()
         }
-        fn flatten_children<'a>(
-            children: Vec<&'a Item>,
+        fn flatten_children<'a, T: Itemlike>(
+            children: Vec<&'a T>,
             depth: usize,
-            items: &mut Vec<(&'a Item, usize)>,
+            items: &mut Vec<(&'a T, usize)>,
         ) {
             for child in children {
                 flatten(child, depth, items);
             }
         }
-        fn flatten<'a>(item: &'a Item, depth: usize, items: &mut Vec<(&'a Item, usize)>) {
+        fn flatten<'a, T: Itemlike>(item: &'a T, depth: usize, items: &mut Vec<(&'a T, usize)>) {
             let children = take_children(item);
             items.push((item, depth));
             flatten_children(children, depth + 1, items);
         }
 
-        fn select_matches(items: &[(&Item, usize)], selection: &Selection, include: &mut BitVec) {
+        fn select_matches<T: Itemlike>(
+            items: &[(&T, usize)],
+            selection: &Selection,
+            include: &mut BitVec,
+        ) {
             let mut instance_within_command = 0;
             let mut last_instance = None;
             let mut command_item = None;
@@ -1022,23 +1036,19 @@ impl Criteria {
                     continue;
                 }
                 if let Some(visible) = selection.visible
-                    && !item.details.is_heading()
-                    && visible != item.show
+                    && !item.is_heading()
+                    && visible != item.is_shown()
                 {
                     continue;
                 }
                 if let Some(error) = selection.error
-                    && error
-                        != item
-                            .spv_info
-                            .as_ref()
-                            .map_or(false, |spv_info| spv_info.error)
+                    && error != item.spv_info().map_or(false, |spv_info| spv_info.error)
                 {
                     continue;
                 }
                 if !selection
                     .commands
-                    .matches(item.command_name.as_ref().map_or("", |name| name.as_str()))
+                    .matches(item.command_name().unwrap_or(""))
                 {
                     continue;
                 }
@@ -1052,11 +1062,11 @@ impl Criteria {
                     }
                 }
                 if !selection.subtypes.is_default() {
-                    let Some(table) = item.details.as_table() else {
+                    if !item.is_table() {
                         continue;
                     };
-                    let subtype = table.subtype().display(table).to_string();
-                    if !selection.subtypes.matches(&subtype) {
+                    let subtype = item.subtype();
+                    if !selection.subtypes.matches(subtype.as_deref().unwrap_or("")) {
                         continue;
                     }
                 }
@@ -1064,7 +1074,7 @@ impl Criteria {
                     continue;
                 }
                 if !selection.members.is_empty() {
-                    let Some(spv_info) = item.spv_info.as_ref() else {
+                    let Some(spv_info) = item.spv_info() else {
                         continue;
                     };
                     let member_names = spv_info.member_names();
@@ -1092,18 +1102,22 @@ impl Criteria {
                 include.set(index, true);
             }
         }
-        fn unflatten_items(
-            items: Vec<Arc<Item>>,
+        fn unflatten_items<T: Itemlike + Clone>(
+            items: Vec<Arc<T>>,
             include: &mut bit_vec::Iter,
-            out: &mut Vec<Arc<Item>>,
+            out: &mut Vec<Arc<T>>,
         ) {
             for item in items {
                 unflatten_item(Arc::unwrap_or_clone(item), include, out);
             }
         }
-        fn unflatten_item(mut item: Item, include: &mut bit_vec::Iter, out: &mut Vec<Arc<Item>>) {
+        fn unflatten_item<T: Itemlike + Clone>(
+            mut item: T,
+            include: &mut bit_vec::Iter,
+            out: &mut Vec<Arc<T>>,
+        ) {
             let include_item = include.next().unwrap();
-            if let Some(children) = item.details.mut_children() {
+            if let Some(children) = item.children_mut() {
                 if !include_item {
                     unflatten_items(take(children), include, out);
                     return;
diff --git a/rust/pspp/src/output/drivers/csv.rs b/rust/pspp/src/output/drivers/csv.rs
index 1ed5e1b163..801ebfd9e7 100644
--- a/rust/pspp/src/output/drivers/csv.rs
+++ b/rust/pspp/src/output/drivers/csv.rs
@@ -371,10 +371,6 @@ impl Driver for CsvDriver {
                     self.output_table_layer(pivot_table, &layer).unwrap();
                 }
             }
-            Details::PageBreak => {
-                self.start_item();
-                writeln!(&mut self.file).unwrap();
-            }
             Details::Text(text) => match text.type_ {
                 TextType::Syntax | TextType::PageTitle => (),
                 TextType::Title | TextType::Log => {
diff --git a/rust/pspp/src/output/drivers/html.rs b/rust/pspp/src/output/drivers/html.rs
index ab50c15778..eaa623f6b0 100644
--- a/rust/pspp/src/output/drivers/html.rs
+++ b/rust/pspp/src/output/drivers/html.rs
@@ -428,7 +428,6 @@ where
         match &item.details {
             Details::Graph | Details::Image(_) | Details::Heading(_) => todo!(),
             Details::Message(_diagnostic) => todo!(),
-            Details::PageBreak => (),
             Details::Table(pivot_table) => {
                 self.render(pivot_table).unwrap(); // XXX
             }
diff --git a/rust/pspp/src/output/drivers/text.rs b/rust/pspp/src/output/drivers/text.rs
index f43796b058..a9fc41917d 100644
--- a/rust/pspp/src/output/drivers/text.rs
+++ b/rust/pspp/src/output/drivers/text.rs
@@ -29,7 +29,7 @@ use serde::{Deserialize, Serialize};
 use unicode_linebreak::{BreakOpportunity, linebreaks};
 use unicode_width::UnicodeWidthStr;
 
-use crate::output::{ItemInfo, render::Extreme, table::DrawCell};
+use crate::output::{Itemlike, render::Extreme, table::DrawCell};
 
 use crate::output::{
     Details, Item,
@@ -416,7 +416,6 @@ impl TextRenderer {
                 }
                 Details::Heading(_) => unreachable!(),
                 Details::Message(_diagnostic) => todo!(),
-                Details::PageBreak => (),
                 Details::Table(pivot_table) => self.render_table(pivot_table, writer)?,
                 Details::Text(text) => {
                     self.render_table(&PivotTable::from((**text).clone()), writer)?
diff --git a/rust/pspp/src/spv/read.rs b/rust/pspp/src/spv/read.rs
index 804a286e20..2c0fd94432 100644
--- a/rust/pspp/src/spv/read.rs
+++ b/rust/pspp/src/spv/read.rs
@@ -18,7 +18,6 @@ use std::{
     fmt::Display,
     fs::File,
     io::{BufReader, Read, Seek},
-    mem::take,
     path::Path,
 };
 
@@ -29,7 +28,10 @@ use zip::{ZipArchive, result::ZipError};
 use crate::{
     crypto::EncryptedReader,
     output::{Item, page::PageSetup},
-    spv::read::{light::LightWarning, structure::StructureMember},
+    spv::read::{
+        light::LightWarning,
+        structure::{OutlineItem, StructureMember},
+    },
 };
 
 mod css;
@@ -128,17 +130,19 @@ where
     where
         F: FnMut(Warning),
     {
-        todo!() /*
         // Read all the items.
-        let mut members = Vec::new();
+        let mut items = Vec::new();
+        let mut page_setup = None;
         for i in 0..self.0.len() {
-        let name = String::from(self.0.name_for_index(i).unwrap());
-        if name.starts_with("outputViewer") && name.ends_with(".xml") {
-        let member = BufReader::new(self.0.by_index(i)?);
-        members.push(StructureMember::read(member, &name, &mut warn)?);
-        }
+            let name = String::from(self.0.name_for_index(i).unwrap());
+            if name.starts_with("outputViewer") && name.ends_with(".xml") {
+                let member = BufReader::new(self.0.by_index(i)?);
+                let mut member = StructureMember::read(member, &name, &mut warn)?;
+                page_setup = page_setup.or(member.page_setup);
+                items.append(&mut member.items);
+            }
         }
-        Ok(SpvOutline { members })*/
+        Ok(SpvOutline { page_setup, items })
     }
 
     /// Reads and returns the whole SPV file contents.
@@ -157,34 +161,27 @@ where
 /// and graphs.
 #[derive(Clone, Debug)]
 pub struct SpvOutline {
+    /// Optional page setup, from the first structure member.
+    pub page_setup: Option<PageSetup>,
+
     /// The table of contents.
-    pub members: Vec<StructureMember>,
+    pub items: Vec<OutlineItem>,
 }
 
 impl SpvOutline {
-    fn read_items<F, R>(
-        mut self,
-        archive: &mut SpvArchive<R>,
-        warn: &mut F,
-    ) -> Result<SpvFile, Error>
+    fn read_items<F, R>(self, archive: &mut SpvArchive<R>, warn: &mut F) -> Result<SpvFile, Error>
     where
         R: Read + Seek,
         F: FnMut(Warning),
     {
-        let page_setup = self
-            .members
-            .get_mut(0)
-            .and_then(|member| take(&mut member.page_setup));
-        let items = self
-            .members
-            .into_iter()
-            .flat_map(|member| {
-                member
-                    .root
-                    .read_items(&mut archive.0, &member.member_name, warn)
-            })
-            .collect();
-        Ok(SpvFile { items, page_setup })
+        Ok(SpvFile {
+            page_setup: self.page_setup,
+            items: self
+                .items
+                .into_iter()
+                .map(|member| member.read_item(&mut archive.0, warn))
+                .collect(),
+        })
     }
 }
 
diff --git a/rust/pspp/src/spv/read/structure.rs b/rust/pspp/src/spv/read/structure.rs
index 4f5952493e..14d9ab6c44 100644
--- a/rust/pspp/src/spv/read/structure.rs
+++ b/rust/pspp/src/spv/read/structure.rs
@@ -27,132 +27,113 @@ use crate::{
 /// Part of the outline of an SPV file.
 #[derive(Clone, Debug)]
 pub struct StructureMember {
-    /// The name of the file inside the Zip archive that contains this
-    /// [StructureMember].
-    ///
-    /// This is useful for user messages.
-    pub member_name: String,
-
     /// The [PageSetup] in the file, if any.
     pub page_setup: Option<PageSetup>,
 
     /// The contents.
-    ///
-    /// `root` itself is not interesting (it is normally just named `Output`)
-    /// and should be ignored.  The contents are its children, which are
-    /// siblings of the children of the roots in all the other
-    /// [StructureMember]s in the SPV file.
-    pub root: Heading,
+    pub items: Vec<OutlineItem>,
 }
 
 impl StructureMember {
+    pub fn into_parts(self) -> (Option<PageSetup>, Vec<OutlineItem>) {
+        (self.page_setup, self.items)
+    }
+
     pub fn read<R>(
         reader: R,
         member_name: &str,
         warn: &mut dyn FnMut(Warning),
-    ) -> Result<Heading, Error>
+    ) -> Result<Self, Error>
     where
         R: BufRead,
     {
-        let heading: raw::Heading =
+        let mut heading: raw::Heading =
             serde_path_to_error::deserialize(&mut quick_xml::de::Deserializer::from_reader(reader))
                 .map_err(|error| Error::DeserializeError {
                     member_name: member_name.into(),
                     error,
                 })?;
-        Ok(heading.decode(member_name, warn))
+        Ok(Self {
+            page_setup: heading
+                .page_setup
+                .take()
+                .map(|page_setup| page_setup.decode(warn, member_name)),
+            items: heading.decode(member_name, warn),
+        })
     }
 }
 
 #[derive(Clone, Debug)]
-pub struct Heading {
+pub struct OutlineHeading {
     structure_member: String,
-    page_setup: Option<PageSetup>,
     expand: bool,
     label: String,
-    children: Vec<HeadingChild>,
+    children: Vec<OutlineItem>,
     command_name: Option<String>,
 }
 
-impl Heading {
-    pub fn read_items<R, F>(
-        self,
-        archive: &mut ZipArchive<R>,
-        structure_member: &str,
-        warn: &mut F,
-    ) -> Vec<Item>
-    where
-        R: Read + Seek,
-        F: FnMut(Warning),
-    {
-        let mut items = Vec::new();
-        for child in self.children {
-            let mut spv_info = SpvInfo::new(structure_member).with_members(child.members());
-            let item = match child {
-                HeadingChild::Container(container) => {
-                    if container.page_break_before {
-                        items.push(
-                            Details::PageBreak
-                                .into_item()
-                                .with_spv_info(SpvInfo::new(structure_member)),
-                        );
-                    }
-                    let result = match container.content {
-                        Content::Table(table) => table.decode(archive, &mut *warn),
-                        Content::Graph(_) => Err(Error::GraphTodo),
-                        Content::Text(container_text) => Ok(container_text.into_item()),
-                        Content::Image(image) => image.decode(archive),
-                        Content::Model => Err(Error::ModelTodo),
-                        Content::Tree => Err(Error::TreeTodo),
-                    };
-                    spv_info.error = result.is_err();
-                    result
-                        .unwrap_or_else(|error| {
-                            Text::new_log(error.to_string())
-                                .into_item()
-                                .with_label("Error")
-                        })
-                        .with_show(container.show)
-                        .with_command_name(Some(container.command_name))
-                        .with_label(container.label)
-                }
-                HeadingChild::Heading(mut heading) => {
-                    let expand = heading.expand;
-                    let label = take(&mut heading.label);
-                    let command_name = take(&mut heading.command_name);
-                    heading
-                        .read_items(archive, structure_member, &mut *warn)
-                        .into_iter()
-                        .collect::<Item>()
-                        .with_show(expand)
-                        .with_label(label)
-                        .with_command_name(command_name)
-                        .with_spv_info(SpvInfo::new(structure_member))
-                }
-            };
-            items.push(item.with_spv_info(spv_info));
-        }
-        items
+impl OutlineHeading {
+    pub fn spv_info(&self) -> SpvInfo {
+        SpvInfo::new(&self.structure_member)
     }
 }
 
 #[derive(Clone, Debug)]
-pub enum HeadingChild {
-    Heading(Heading),
+pub enum OutlineItem {
+    Heading(OutlineHeading),
     Container(Container),
 }
 
-impl HeadingChild {
-    fn members(&self) -> Option<SpvMembers> {
+impl OutlineItem {
+    pub fn read_item<R, F>(self, archive: &mut ZipArchive<R>, warn: &mut F) -> Item
+    where
+        R: Read + Seek,
+        F: FnMut(Warning),
+    {
         match self {
-            HeadingChild::Heading(_) => None,
-            HeadingChild::Container(container) => container.content.members(),
+            OutlineItem::Container(container) => {
+                let mut spv_info = container.spv_info();
+                let result = match container.content {
+                    Content::Table(table) => table.decode(archive, &mut *warn),
+                    Content::Graph(_) => Err(Error::GraphTodo),
+                    Content::Text(container_text) => Ok(container_text.into_item()),
+                    Content::Image(image) => image.decode(archive),
+                    Content::Model => Err(Error::ModelTodo),
+                    Content::Tree => Err(Error::TreeTodo),
+                };
+                spv_info.error = result.is_err();
+                result
+                    .unwrap_or_else(|error| {
+                        Text::new_log(error.to_string())
+                            .into_item()
+                            .with_label("Error")
+                    })
+                    .with_show(container.show)
+                    .with_command_name(Some(container.command_name))
+                    .with_label(container.label)
+            }
+            OutlineItem::Heading(mut heading) => {
+                let expand = heading.expand;
+                let label = take(&mut heading.label);
+                let command_name = take(&mut heading.command_name);
+                let spv_info = heading.spv_info();
+                heading
+                    .children
+                    .into_iter()
+                    .map(|child| child.read_item(archive, &mut *warn))
+                    .collect::<Item>()
+                    .with_show(expand)
+                    .with_label(label)
+                    .with_command_name(command_name)
+                    .with_spv_info(spv_info)
+            }
         }
     }
 }
 
 #[derive(Clone, Debug)]
 pub struct Container {
+    structure_member: String,
     show: bool,
     page_break_before: bool,
     text_align: Option<HorzAlign>,
@@ -162,6 +143,12 @@ pub struct Container {
     content: Content,
 }
 
+impl Container {
+    pub fn spv_info(&self) -> SpvInfo {
+        SpvInfo::new(&self.structure_member).with_members(self.content.members())
+    }
+}
+
 #[derive(Clone, Debug)]
 pub enum Content {
     Text(Text),
@@ -297,6 +284,8 @@ impl Graph {
 }
 
 mod raw {
+    use std::mem::take;
+
     use paper_sizes::PaperSize;
     use serde::Deserialize;
 
@@ -314,11 +303,13 @@ mod raw {
             html::{self, Document},
             read::{
                 TableType, Warning, WarningDetails,
-                structure::{Content, HeadingChild},
+                structure::{Content, OutlineItem},
             },
         },
     };
 
+    use super::OutlineHeading;
+
     #[derive(Deserialize, Debug)]
     #[serde(rename_all = "camelCase")]
     pub struct Heading {
@@ -339,8 +330,8 @@ mod raw {
             self,
             structure_member: &str,
             warn: &mut dyn FnMut(Warning),
-        ) -> super::Heading {
-            let mut children = Vec::new();
+        ) -> Vec<OutlineItem> {
+            let mut items = Vec::new();
             for child in self.children {
                 match child {
                     HeadingContent::Container(container) => {
@@ -391,7 +382,8 @@ mod raw {
                             ContainerContent::Model(model) => (Content::Model, model.command_name),
                             ContainerContent::Tree(tree) => (Content::Tree, tree.command_name),
                         };
-                        children.push(HeadingChild::Container(super::Container {
+                        items.push(OutlineItem::Container(super::Container {
+                            structure_member: structure_member.into(),
                             show: container.visibility != Visibility::Hidden,
                             page_break_before: container.page_break_before
                                 == PageBreakBefore::Always,
@@ -402,23 +394,18 @@ mod raw {
                             content,
                         }));
                     }
-                    HeadingContent::Heading(heading) => {
-                        children.push(HeadingChild::Heading(
-                            heading.decode(structure_member, warn),
-                        ));
+                    HeadingContent::Heading(mut heading) => {
+                        items.push(OutlineItem::Heading(OutlineHeading {
+                            structure_member: structure_member.into(),
+                            expand: !heading.visibility.is_some(),
+                            label: take(&mut heading.label.text),
+                            command_name: heading.command_name.take(),
+                            children: heading.decode(structure_member, warn),
+                        }));
                     }
                 }
             }
-            super::Heading {
-                structure_member: structure_member.into(),
-                page_setup: self
-                    .page_setup
-                    .map(|page_setup| page_setup.decode(warn, structure_member)),
-                expand: !self.visibility.is_some(),
-                label: self.label.text,
-                command_name: self.command_name,
-                children,
-            }
+            items
         }
     }
 
diff --git a/rust/pspp/src/spv/write.rs b/rust/pspp/src/spv/write.rs
index 1776bd96d9..92fd02f9fd 100644
--- a/rust/pspp/src/spv/write.rs
+++ b/rust/pspp/src/spv/write.rs
@@ -33,7 +33,7 @@ use crate::{
     data::{Datum, EncodedString},
     format::{Format, Type},
     output::{
-        Details, Item, ItemInfo, Text,
+        Details, Item, Itemlike, Text,
         page::{ChartSize, PageSetup},
         pivot::{
             Axis2, Axis3, Category, Dimension, Footnote, FootnoteMarkerPosition,
@@ -57,7 +57,6 @@ where
     W: Write + Seek,
 {
     writer: ZipWriter<W>,
-    needs_page_break: bool,
     next_table_id: u64,
     next_heading_id: u64,
     page_setup: Option<PageSetup>,
@@ -75,7 +74,6 @@ where
         writer.write_all("allowPivoting=true".as_bytes())?;
         Ok(Self {
             writer,
-            needs_page_break: false,
             next_table_id: 1,
             next_heading_id: 1,
             page_setup: None,
@@ -110,12 +108,6 @@ where
         Ok(self.writer.finish()?)
     }
 
-    fn page_break_before(&mut self) -> bool {
-        let page_break_before = self.needs_page_break;
-        self.needs_page_break = false;
-        page_break_before
-    }
-
     fn write_table<X>(
         &mut self,
         item: &Item,
@@ -221,10 +213,6 @@ where
             Details::Message(diagnostic) => {
                 self.write_text(item, &Text::from(diagnostic.as_ref()), structure)
             }
-            Details::PageBreak => {
-                self.needs_page_break = true;
-                Ok(())
-            }
             Details::Table(pivot_table) => self.write_table(item, pivot_table, structure),
             Details::Text(text) => self.write_text(item, text, structure),
         }
@@ -244,7 +232,7 @@ where
         writer
             .create_element("container")
             .with_attributes(
-                self.page_break_before()
+                item.page_break_before()
                     .then_some(("page-break-before", "always")),
             )
             .with_attribute(("visibility", if item.show { "visible" } else { "hidden" }))
@@ -560,11 +548,6 @@ where
 {
     /// Writes `item` to the SPV file.
     pub fn write(&mut self, item: &Item) -> Result<(), Error> {
-        if item.details.is_page_break() {
-            self.needs_page_break = true;
-            return Ok(());
-        }
-
         let mut headings = XmlWriter::new(Cursor::new(Vec::new()));
         let element = headings
             .create_element("heading")