From 5e76a8e7849cd6b092e8cfcae509b6353425f5da Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Tue, 30 Dec 2025 17:03:02 -0800 Subject: [PATCH] work --- rust/pspp/src/cli/convert.rs | 9 +- rust/pspp/src/cli/show_spv.rs | 155 +++++++++---------- rust/pspp/src/output.rs | 10 +- rust/pspp/src/output/drivers/text.rs | 7 +- rust/pspp/src/spv.rs | 2 +- rust/pspp/src/spv/read.rs | 166 +++++++++++---------- rust/pspp/src/spv/read/light.rs | 213 ++++++++++++++++----------- rust/pspp/src/spv/read/structure.rs | 78 +++++----- rust/pspp/src/spv/read/tests.rs | 10 +- 9 files changed, 354 insertions(+), 296 deletions(-) diff --git a/rust/pspp/src/cli/convert.rs b/rust/pspp/src/cli/convert.rs index ae414aeb16..64054d9975 100644 --- a/rust/pspp/src/cli/convert.rs +++ b/rust/pspp/src/cli/convert.rs @@ -26,6 +26,7 @@ use pspp::{ output::{Criteria, drivers::Driver}, pc::PcFile, por::PortableFile, + spv::SpvArchive, sys::ReadOptions, }; @@ -143,10 +144,10 @@ impl Convert { self.write_data(dictionary, cases) } Some(FileType::Viewer { .. }) => { - let (items, page_setup) = pspp::spv::ReadOptions::new(|e| eprintln!("{e}")) - .with_password(self.password.clone()) - .open_file(&self.input)? - .into_contents(); + let (items, page_setup) = + SpvArchive::open_file(&self.input, self.password.as_deref())? + .read(|e| eprintln!("{e}"))? + .into_contents(); let mut output = self.open_driver("text")?; if let Some(page_setup) = &page_setup { output.setup(page_setup); diff --git a/rust/pspp/src/cli/show_spv.rs b/rust/pspp/src/cli/show_spv.rs index dd97506da2..1a28c46f12 100644 --- a/rust/pspp/src/cli/show_spv.rs +++ b/rust/pspp/src/cli/show_spv.rs @@ -15,21 +15,12 @@ // this program. If not, see . use anyhow::Result; -use binrw::{BinRead, error::ContextExt}; use clap::{Args, ValueEnum}; use pspp::{ - output::{ - Criteria, Item, ItemRefIterator, SpvMembers, - pivot::{Axis3, Dimension, Group, Leaf, PivotTable, value::Value}, - }, - spv::legacy_bin::LegacyBin, -}; -use std::{ - collections::HashMap, - fmt::Display, - io::{Cursor, Read}, - path::PathBuf, + output::{Criteria, Item}, + spv::SpvArchive, }; +use std::{fmt::Display, path::PathBuf, sync::Arc}; /// Show information about SPSS viewer files (SPV files). #[derive(Args, Clone, Debug)] @@ -113,92 +104,88 @@ impl ShowSpv { } } + fn read(&self) -> Result>> { + Ok(self.criteria.apply( + SpvArchive::open_file(&self.input, self.password.as_deref())? + .read(|e| eprintln!("{e}"))? + .into_items(), + )) + } + fn directory(self) -> Result<()> { - let item = pspp::spv::ReadOptions::new(|e| eprintln!("{e}")) - .with_password(self.password) - .open_file(&self.input)? - .into_items(); - let items = self.criteria.apply(item); - for child in items { + for child in self.read()? { print_item_directory(&child, 0, self.show_member_names); } Ok(()) } fn view(self) -> Result<()> { - let item = pspp::spv::ReadOptions::new(|e| eprintln!("{e}")) - .with_password(self.password) - .open_file(&self.input)? - .into_items(); - let items = self.criteria.apply(item); - for child in items { + for child in self.read()? { println!("{child}"); } Ok(()) } fn legacy_data(self) -> Result<()> { - let mut spv_file = pspp::spv::ReadOptions::new(|e| eprintln!("{e}")) - .with_password(self.password) - .open_file(&self.input)?; - - let items = self.criteria.apply(spv_file.items); - for item in items { - for item in ItemRefIterator::with_hidden(&item) { - if let Some(spv_info) = dbg!(&item.spv_info) - && let Some(members) = &spv_info.members - && let SpvMembers::LegacyTable { xml: _, binary } = &members - { - let mut bin_member = spv_file.archive.by_name(&binary)?; - let mut bin_data = Vec::with_capacity(bin_member.size() as usize); - bin_member.read_to_end(&mut bin_data)?; - let mut cursor = Cursor::new(bin_data); - let legacy_bin = LegacyBin::read(&mut cursor).map_err(|e| { - e.with_message(format!( - "While parsing {binary:?} as legacy binary SPV member" - )) - })?; - let data = legacy_bin.decode(); - let n_values = data - .values() - .flat_map(|map| map.values()) - .map(|values| values.len()) - .max() - .unwrap_or(0); - let index = Dimension::new( - Group::new("Index") - .with_multiple(Leaf::numbers(0..n_values)) - .with_label_shown(), - ); - let variables = Dimension::new(Group::new("Variables").with_multiple( - data.iter().map(|(name, contents)| { - Group::new(name.as_str()).with_multiple(contents.keys().map(|name| { - name.replace("categories", "\ncategories") - .replace("labels", "\nlabels") - .replace("group", "\ngroup") - .replace("Label", "\nLabel") - })) - }), - )); - let mut pivot_table = - PivotTable::new([(Axis3::Y, index), (Axis3::X, variables)]); - let formats = HashMap::new(); - for (variable_index, (variable_name, values)) in - data.values().flat_map(|map| map.iter()).enumerate() - { - for (value_index, data_value) in values.iter().enumerate() { - let value = Value::new_datum(&data_value.value).with_value_label( - (variable_name == "cellFormat") - .then(|| data_value.as_format(&formats).to_string()), - ); - pivot_table.insert([value_index, variable_index], value); - } - } - println!("{pivot_table}"); - } - } + todo!() /* + let spv = SpvArchive::open_file(&self.input, self.password.as_deref())?; + let outline = spv.read_outline(|w| eprintln!("{w}"))?; + for item in self.read()? { + for item in ItemRefIterator::new(&item) { + if let Some(spv_info) = &item.spv_info + && let Some(members) = &spv_info.members + && let SpvMembers::LegacyTable { xml: _, binary } = &members + { + let mut bin_member = spv_file.archive.by_name(&binary)?; + let mut bin_data = Vec::with_capacity(bin_member.size() as usize); + bin_member.read_to_end(&mut bin_data)?; + let mut cursor = Cursor::new(bin_data); + let legacy_bin = LegacyBin::read(&mut cursor).map_err(|e| { + e.with_message(format!( + "While parsing {binary:?} as legacy binary SPV member" + )) + })?; + let data = legacy_bin.decode(); + let n_values = data + .values() + .flat_map(|map| map.values()) + .map(|values| values.len()) + .max() + .unwrap_or(0); + let index = Dimension::new( + Group::new("Index") + .with_multiple(Leaf::numbers(0..n_values)) + .with_label_shown(), + ); + let variables = Dimension::new(Group::new("Variables").with_multiple( + data.iter().map(|(name, contents)| { + Group::new(name.as_str()).with_multiple(contents.keys().map(|name| { + name.replace("categories", "\ncategories") + .replace("labels", "\nlabels") + .replace("group", "\ngroup") + .replace("Label", "\nLabel") + })) + }), + )); + let mut pivot_table = + PivotTable::new([(Axis3::Y, index), (Axis3::X, variables)]); + let formats = HashMap::new(); + for (variable_index, (variable_name, values)) in + data.values().flat_map(|map| map.iter()).enumerate() + { + for (value_index, data_value) in values.iter().enumerate() { + let value = Value::new_datum(&data_value.value).with_value_label( + (variable_name == "cellFormat") + .then(|| data_value.as_format(&formats).to_string()), + ); + pivot_table.insert([value_index, variable_index], value); } - Ok(()) + } + println!("{pivot_table}"); + } + } + } + Ok(())*/ } } diff --git a/rust/pspp/src/output.rs b/rust/pspp/src/output.rs index edcecfd6fb..7ef4d325ad 100644 --- a/rust/pspp/src/output.rs +++ b/rust/pspp/src/output.rs @@ -141,6 +141,10 @@ impl Item { pub fn is_shown(&self) -> bool { self.details.is_heading() || self.show } + + pub fn iter_in_order(&self) -> ItemRefIterator<'_> { + ItemRefIterator::new(self) + } } impl From for Item @@ -495,11 +499,11 @@ pub struct ItemRefIterator<'a> { } impl<'a> ItemRefIterator<'a> { - pub fn without_hidden(start: &'a Item) -> impl Iterator { - Self::with_hidden(start).filter(|item| item.is_shown()) + pub fn without_hidden(self) -> impl Iterator { + self.filter(|item| item.is_shown()) } - pub fn with_hidden(start: &'a Item) -> Self { + pub fn new(start: &'a Item) -> Self { Self { next: Some(start), stack: Vec::new(), diff --git a/rust/pspp/src/output/drivers/text.rs b/rust/pspp/src/output/drivers/text.rs index af6a106cbb..dadcec87bd 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::{ItemRefIterator, render::Extreme, table::DrawCell}; +use crate::output::{render::Extreme, table::DrawCell}; use crate::output::{ Details, Item, @@ -400,7 +400,10 @@ impl TextRenderer { where W: FmtWrite, { - for item in ItemRefIterator::without_hidden(item).filter(|item| !item.details.is_heading()) + for item in item + .iter_in_order() + .without_hidden() + .filter(|item| !item.details.is_heading()) { match &item.details { Details::Graph => { diff --git a/rust/pspp/src/spv.rs b/rust/pspp/src/spv.rs index a9868f7041..eb26aaf501 100644 --- a/rust/pspp/src/spv.rs +++ b/rust/pspp/src/spv.rs @@ -30,5 +30,5 @@ mod read; mod write; -pub use read::{Error, ReadOptions, SpvFile, html, legacy_bin}; +pub use read::{Error, SpvArchive, SpvFile, SpvOutline, html, legacy_bin}; pub use write::Writer; diff --git a/rust/pspp/src/spv/read.rs b/rust/pspp/src/spv/read.rs index 10a7e5a22c..2f1fe041a8 100644 --- a/rust/pspp/src/spv/read.rs +++ b/rust/pspp/src/spv/read.rs @@ -15,12 +15,11 @@ // this program. If not, see . #![allow(dead_code)] use std::{ - cell::RefCell, fmt::Display, fs::File, io::{BufReader, Read, Seek}, + mem::take, path::Path, - rc::Rc, }; use displaydoc::Display; @@ -30,7 +29,7 @@ use zip::{ZipArchive, result::ZipError}; use crate::{ crypto::EncryptedReader, output::{Item, page::PageSetup}, - spv::read::light::LightWarning, + spv::read::{light::LightWarning, structure::StructureMember}, }; mod css; @@ -71,81 +70,38 @@ pub enum WarningDetails { UnknownOrientation(String), } -/// Options for reading an SPV file. -#[derive(Clone, Debug)] -pub struct ReadOptions { - /// Function called to report warnings. - pub warn: F, - - /// Password to use to unlock an encrypted SPV file. - /// - /// For an encrypted SPV file, this must be set to the (encoded or - /// unencoded) password. - /// - /// For a plaintext SPV file, this must be None. - pub password: Option, -} - -impl ReadOptions { - /// Construct a new [ReadOptions] without a password. - pub fn new(warn: F) -> Self { - Self { - warn, - password: None, - } - } - - /// Causes the file to be read by decrypting it with the given `password` or - /// without decrypting if `password` is None. - pub fn with_password(self, password: Option) -> Self { - Self { password, ..self } - } -} - pub trait ReadSeek: Read + Seek {} impl ReadSeek for T where T: Read + Seek {} -impl ReadOptions -where - F: FnMut(Warning) + 'static, -{ +/// A ZIP archive that contains an SPV file. +pub struct SpvArchive(pub ZipArchive); + +impl SpvArchive> { /// Opens the file at `path`. - pub fn open_file

(self, path: P) -> Result + pub fn open_file

(path: P, password: Option<&str>) -> Result where P: AsRef, { - self.open_reader(File::open(path)?) + Self::open_reader(File::open(path)?, password) } - /// Opens the file read from `reader`. - pub fn open_reader(self, reader: R) -> Result + /// Opens the provided Zip `archive`. + /// + /// Any password provided for reading the file is unused, because if one was + /// needed then it must have already been used to open the archive. + pub fn open_reader(reader: R, password: Option<&str>) -> Result where R: Read + Seek + 'static, { - let reader = match &self.password { - None => Box::new(reader) as Box, - Some(password) => Box::new(EncryptedReader::open(reader, password)?), + let reader = if let Some(password) = password { + Box::new(EncryptedReader::open(reader, password)?) + } else { + Box::new(reader) as Box }; - self.open_reader_inner(Box::new(reader)) - } - - fn open_reader_inner(self, reader: Box) -> Result { - let archive = ZipArchive::new(reader).map_err(|error| match error { + let mut archive = ZipArchive::new(reader).map_err(|error| match error { ZipError::InvalidArchive(_) => Error::NotSpv, other => other.into(), })?; - Ok(self.open_archive(archive)?) - } - - /// Opens the provided Zip `archive`. - /// - /// Any password provided for reading the file is unused, because if one was - /// needed then it must have already been used to open the archive. - pub fn open_archive( - self, - mut archive: ZipArchive>, - ) -> Result { - // Check manifest. let mut file = archive .by_name("META-INF/MANIFEST.MF") .map_err(|_| Error::NotSpv)?; @@ -156,25 +112,78 @@ where } drop(file); + Ok(Self(archive)) + } +} + +impl SpvArchive +where + R: Read + Seek, +{ + /// Reads and returns an outline of the contents of the SPV file. + /// + /// Most callers want to use [read](Self::read), instead, to read the full + /// contents of the SPV file. + pub fn read_outline(&mut self, mut warn: F) -> Result + where + F: FnMut(Warning), + { // Read all the items. - let warn = Rc::new(RefCell::new(Box::new(self.warn) as Box)); - let mut items = Vec::new(); - let mut page_setup = None; - for i in 0..archive.len() { - let name = String::from(archive.name_for_index(i).unwrap()); + let mut members = Vec::new(); + 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(archive.by_index(i)?); - let member = structure::StructureMember::read(member, &name, &warn)?; - items.extend(member.root.read_items(&mut archive, &name, &warn)); - page_setup = page_setup.or(member.page_setup); + let member = BufReader::new(self.0.by_index(i)?); + members.push(StructureMember::read(member, &name, &mut warn)?); } } + Ok(SpvOutline { members }) + } - Ok(SpvFile { - items: items.into_iter().collect(), - page_setup, - archive, - }) + /// Reads and returns the whole SPV file contents. + pub fn read(&mut self, mut warn: F) -> Result + where + F: FnMut(Warning), + { + self.read_outline(&mut warn)?.read_items(self, &mut warn) + } +} + +/// An outline of an SPV file. +/// +/// Reading the outline only requires reading some of the SPV file. It provides +/// a "table of contents" view of the SPV, without reading the details of tables +/// and graphs. +#[derive(Clone, Debug)] +pub struct SpvOutline { + /// The table of contents. + pub members: Vec, +} + +impl SpvOutline { + fn read_items( + mut self, + archive: &mut SpvArchive, + warn: &mut F, + ) -> Result + 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 }) } } @@ -185,9 +194,6 @@ pub struct SpvFile { /// The page setup in the SPV file, if any. pub page_setup: Option, - - /// The Zip archive that the file was read from. - pub archive: ZipArchive>, } impl SpvFile { @@ -246,7 +252,7 @@ pub enum Error { TreeTodo, } -#[derive(Deserialize, Debug)] +#[derive(Copy, Clone, Deserialize, Debug, PartialEq, Eq)] #[serde(rename_all = "camelCase")] enum TableType { Table, diff --git a/rust/pspp/src/spv/read/light.rs b/rust/pspp/src/spv/read/light.rs index 578f390224..4046464be3 100644 --- a/rust/pspp/src/spv/read/light.rs +++ b/rust/pspp/src/spv/read/light.rs @@ -1,9 +1,7 @@ use std::{ - cell::RefCell, fmt::Debug, io::{Read, Seek}, ops::Deref, - rc::Rc, str::FromStr, sync::Arc, }; @@ -21,7 +19,7 @@ use itertools::Itertools; use crate::{ data::Datum, format::{ - CC, Decimal, Decimals, Epoch, F40, F40_2, Format, NumberStyle, Settings, Type, + self, CC, Decimal, Decimals, Epoch, F40, F40_2, NumberStyle, Settings, Type, UncheckedFormat, Width, }, output::pivot::{ @@ -34,7 +32,7 @@ use crate::{ parse_bool, value::{self, DatumValue, TemplateValue, ValueStyle, VariableValue}, }, - settings::Show, + settings, }; /// A warning decoding a light detail member. @@ -78,24 +76,20 @@ pub enum LightWarning { struct Context { version: Version, - warn: Rc>>, } impl Context { - fn new(version: Version, warn: Rc>>) -> Self { - Self { version, warn } - } - fn warn(&self, warning: LightWarning) { - (self.warn.borrow_mut())(warning); + fn new(version: Version) -> Self { + Self { version } } } #[binread] -#[br(little, import(warn: Rc>>))] +#[br(little)] #[derive(Debug)] pub struct LightTable { header: Header, - #[br(temp, calc(Context::new(header.version, warn)))] + #[br(temp, calc(Context::new(header.version)))] context: Context, #[br(args(&context))] titles: Titles, @@ -164,7 +158,7 @@ impl LightTable { } } - pub fn decode(&self, warn: &mut dyn FnMut(LightWarning)) -> PivotTable { + pub fn decode(&self, mut warn: &mut dyn FnMut(LightWarning)) -> PivotTable { let encoding = self.formats.encoding(warn); let n1 = self.formats.n1(); @@ -175,7 +169,7 @@ impl LightTable { let footnotes = self .footnotes .iter() - .map(|f| f.decode(encoding, &Footnotes::new())) + .map(|f| f.decode(encoding, &Footnotes::new(), warn)) .collect(); let cells = self .cells @@ -183,7 +177,7 @@ impl LightTable { .map(|cell| { ( PrecomputedIndex(cell.index as usize), - cell.value.decode(encoding, &footnotes), + cell.value.decode(encoding, &footnotes, warn), ) }) .collect::>(); @@ -191,10 +185,10 @@ impl LightTable { .dimensions .iter() .map(|d| { - let mut root = Group::new(d.name.decode(encoding, &footnotes)) + let mut root = Group::new(d.name.decode(encoding, &footnotes, warn)) .with_show_label(!d.hide_dim_label); for category in &d.categories { - category.decode(encoding, &footnotes, &mut root); + category.decode(encoding, &footnotes, &mut root, warn); } pivot::Dimension { presentation_order: (0..root.len()).collect(), /*XXX*/ @@ -221,8 +215,8 @@ impl LightTable { show_grid_lines: self.borders.show_grid_lines, show_title: n1.map_or(true, |x1| x1.show_title != 10), show_caption: n1.map_or(true, |x1| x1.show_caption), - show_values: n1.map_or(None, |x1| x1.show_values), - show_variables: n1.map_or(None, |x1| x1.show_variables), + show_values: n1.map_or(None, |x1| x1.show_values.decode(&mut warn)), + show_variables: n1.map_or(None, |x1| x1.show_variables.decode(&mut warn)), sizing: self.table_settings.sizing.decode( &self.formats.column_widths, n2.map_or(&[], |x2| &x2.row_heights), @@ -254,18 +248,22 @@ impl LightTable { None } }), - title: Some(Box::new(self.titles.title.decode(encoding, &footnotes))), - subtype: Some(Box::new(self.titles.subtype.decode(encoding, &footnotes))), + title: Some(Box::new( + self.titles.title.decode(encoding, &footnotes, warn), + )), + subtype: Some(Box::new( + self.titles.subtype.decode(encoding, &footnotes, warn), + )), corner_text: self .titles .corner_text .as_ref() - .map(|corner| Box::new(corner.decode(encoding, &footnotes))), + .map(|corner| Box::new(corner.decode(encoding, &footnotes, warn))), caption: self .titles .caption .as_ref() - .map(|caption| Box::new(caption.decode(encoding, &footnotes))), + .map(|caption| Box::new(caption.decode(encoding, &footnotes, warn))), notes: self.table_settings.notes.decode_optional(encoding), notes_unexpanded: n3_inner .and_then(|inner| inner.notes_unexpanded.decode_optional(encoding)), @@ -388,9 +386,18 @@ struct Footnote { } impl Footnote { - fn decode(&self, encoding: &'static Encoding, footnotes: &pivot::Footnotes) -> pivot::Footnote { - pivot::Footnote::new(self.text.decode(encoding, footnotes)) - .with_marker(self.marker.as_ref().map(|m| m.decode(encoding, footnotes))) + fn decode( + &self, + encoding: &'static Encoding, + footnotes: &pivot::Footnotes, + warn: &mut dyn FnMut(LightWarning), + ) -> pivot::Footnote { + pivot::Footnote::new(self.text.decode(encoding, footnotes, warn)) + .with_marker( + self.marker + .as_ref() + .map(|m| m.decode(encoding, footnotes, warn)), + ) .with_show(self.show > 0) } } @@ -963,10 +970,8 @@ struct N1 { #[br(temp, parse_with(parse_bool))] _x16: bool, _lang: u8, - #[br(parse_with(parse_show), args(|warning| eprintln!("{warning}")))] - show_variables: Option, - #[br(parse_with(parse_show), args(|warning| eprintln!("{warning}")))] - show_values: Option, + show_variables: Show, + show_values: Show, #[br(temp)] _x18: i32, #[br(temp)] @@ -979,19 +984,25 @@ struct N1 { show_caption: bool, } -#[binrw::parser(reader, endian)] -fn parse_show(mut warn: F) -> BinResult> -where - F: FnMut(LightWarning), -{ - match ::read_options(reader, endian, ())? { - 0 => Ok(None), - 1 => Ok(Some(Show::Value)), - 2 => Ok(Some(Show::Label)), - 3 => Ok(Some(Show::Both)), - other => { - warn(LightWarning::UnknownShow(other)); - Ok(None) +#[binread] +#[br(little)] +#[derive(Debug)] +struct Show(u8); + +impl Show { + fn decode(&self, mut warn: F) -> Option + where + F: FnMut(LightWarning), + { + match self.0 { + 0 => None, + 1 => Some(settings::Show::Value), + 2 => Some(settings::Show::Label), + 3 => Some(settings::Show::Both), + other => { + warn(LightWarning::UnknownShow(other)); + None + } } } } @@ -1118,24 +1129,32 @@ impl CustomCurrency { struct ValueNumber { #[br(parse_with(parse_explicit_optional), args(context))] mods: Option, - #[br(parse_with(parse_format), args(context))] format: Format, x: f64, } +#[binread] +#[br(little)] +#[derive(Debug)] +struct Format(u32); + +impl Format { + pub fn decode(&self, warn: &mut dyn FnMut(LightWarning)) -> format::Format { + decode_format(self.0, warn) + } +} + #[binread] #[br(little, import(context: &Context))] #[derive(Debug)] struct ValueVarNumber { #[br(parse_with(parse_explicit_optional), args(context))] mods: Option, - #[br(parse_with(parse_format), args(context))] format: Format, x: f64, var_name: U32String, value_label: U32String, - #[br(parse_with(parse_show), args(|warning| eprintln!("{warning}")))] - show: Option, + show: Show, } #[binread] @@ -1157,12 +1176,10 @@ struct ValueText { struct ValueString { #[br(parse_with(parse_explicit_optional), args(context))] mods: Option, - #[br(parse_with(parse_format), args(context))] format: Format, value_label: U32String, var_name: U32String, - #[br(parse_with(parse_show), args(|warning| eprintln!("{warning}")))] - show: Option, + show: Show, s: U32String, } @@ -1174,8 +1191,7 @@ struct ValueVarName { mods: Option, var_name: U32String, var_label: U32String, - #[br(parse_with(parse_show), args(|warning| eprintln!("{warning}")))] - show: Option, + show: Show, } #[binread] @@ -1253,7 +1269,7 @@ impl BinRead for Value { } } -pub(super) fn decode_format(raw: u32, warn: &mut dyn FnMut(LightWarning)) -> Format { +pub(super) fn decode_format(raw: u32, warn: &mut dyn FnMut(LightWarning)) -> format::Format { if raw == 0 || raw == 0x10000 || raw == 1 { return F40_2; } @@ -1273,30 +1289,32 @@ pub(super) fn decode_format(raw: u32, warn: &mut dyn FnMut(LightWarning)) -> For UncheckedFormat::new(type_, w, d).fix() } -#[binrw::parser(reader, endian)] -fn parse_format(context: &Context) -> BinResult { - Ok(decode_format( - u32::read_options(reader, endian, ())?, - &mut *context.warn.borrow_mut(), - )) -} - impl ValueNumber { - fn decode(&self, encoding: &'static Encoding, footnotes: &pivot::Footnotes) -> value::Value { + fn decode( + &self, + encoding: &'static Encoding, + footnotes: &pivot::Footnotes, + warn: &mut dyn FnMut(LightWarning), + ) -> value::Value { value::Value::new_number((self.x != -f64::MAX).then_some(self.x)) - .with_format(self.format) + .with_format(self.format.decode(warn)) .with_styling(ValueMods::decode_optional(&self.mods, encoding, footnotes)) } } impl ValueVarNumber { - fn decode(&self, encoding: &'static Encoding, footnotes: &pivot::Footnotes) -> value::Value { + fn decode( + &self, + encoding: &'static Encoding, + footnotes: &pivot::Footnotes, + warn: &mut dyn FnMut(LightWarning), + ) -> value::Value { value::Value::new_number((self.x != -f64::MAX).then_some(self.x)) - .with_format(self.format) + .with_format(self.format.decode(warn)) .with_styling(ValueMods::decode_optional(&self.mods, encoding, footnotes)) .with_value_label(self.value_label.decode_optional(encoding)) .with_variable_name(Some(self.var_name.decode(encoding))) - .with_show_value_label(self.show) + .with_show_value_label(self.show.decode(warn)) } } @@ -1313,11 +1331,16 @@ impl ValueText { } impl ValueString { - fn decode(&self, encoding: &'static Encoding, footnotes: &pivot::Footnotes) -> value::Value { + fn decode( + &self, + encoding: &'static Encoding, + footnotes: &pivot::Footnotes, + warn: &mut dyn FnMut(LightWarning), + ) -> value::Value { value::Value::new(pivot::value::ValueInner::Datum(DatumValue { datum: Datum::new_utf8(self.s.decode(encoding)), - format: self.format, - show: self.show, + format: self.format.decode(warn), + show: self.show.decode(warn), honor_small: false, variable: self.var_name.decode_optional(encoding), value_label: self.value_label.decode_optional(encoding), @@ -1327,9 +1350,14 @@ impl ValueString { } impl ValueVarName { - fn decode(&self, encoding: &'static Encoding, footnotes: &pivot::Footnotes) -> value::Value { + fn decode( + &self, + encoding: &'static Encoding, + footnotes: &pivot::Footnotes, + warn: &mut dyn FnMut(LightWarning), + ) -> value::Value { value::Value::new(pivot::value::ValueInner::Variable(VariableValue { - show: self.show, + show: self.show.decode(warn), var_name: self.var_name.decode(encoding), variable_label: self.var_label.decode_optional(encoding), })) @@ -1349,12 +1377,17 @@ impl ValueFixedText { } impl ValueTemplate { - fn decode(&self, encoding: &'static Encoding, footnotes: &pivot::Footnotes) -> value::Value { + fn decode( + &self, + encoding: &'static Encoding, + footnotes: &pivot::Footnotes, + warn: &mut dyn FnMut(LightWarning), + ) -> value::Value { value::Value::new(pivot::value::ValueInner::Template(TemplateValue { args: self .args .iter() - .map(|argument| argument.decode(encoding, footnotes)) + .map(|argument| argument.decode(encoding, footnotes, warn)) .collect(), localized: self.template.decode(encoding), id: self @@ -1367,15 +1400,20 @@ impl ValueTemplate { } impl Value { - fn decode(&self, encoding: &'static Encoding, footnotes: &pivot::Footnotes) -> value::Value { + fn decode( + &self, + encoding: &'static Encoding, + footnotes: &pivot::Footnotes, + warn: &mut dyn FnMut(LightWarning), + ) -> value::Value { match self { - Value::Number(number) => number.decode(encoding, footnotes), - Value::VarNumber(var_number) => var_number.decode(encoding, footnotes), + Value::Number(number) => number.decode(encoding, footnotes, warn), + Value::VarNumber(var_number) => var_number.decode(encoding, footnotes, warn), Value::Text(text) => text.decode(encoding, footnotes), - Value::String(string) => string.decode(encoding, footnotes), - Value::VarName(var_name) => var_name.decode(encoding, footnotes), + Value::String(string) => string.decode(encoding, footnotes, warn), + Value::VarName(var_name) => var_name.decode(encoding, footnotes, warn), Value::FixedText(fixed_text) => fixed_text.decode(encoding, footnotes), - Value::Template(template) => template.decode(encoding, footnotes), + Value::Template(template) => template.decode(encoding, footnotes, warn), } } } @@ -1415,10 +1453,11 @@ impl Argument { &self, encoding: &'static Encoding, footnotes: &pivot::Footnotes, + warn: &mut dyn FnMut(LightWarning), ) -> Vec { self.0 .iter() - .map(|value| value.decode(encoding, footnotes)) + .map(|value| value.decode(encoding, footnotes, warn)) .collect() } } @@ -1634,8 +1673,14 @@ struct Category { } impl Category { - fn decode(&self, encoding: &'static Encoding, footnotes: &Footnotes, group: &mut pivot::Group) { - let name = self.name.decode(encoding, footnotes); + fn decode( + &self, + encoding: &'static Encoding, + footnotes: &Footnotes, + group: &mut pivot::Group, + warn: &mut dyn FnMut(LightWarning), + ) { + let name = self.name.decode(encoding, footnotes, warn); match &self.child { Child::Leaf { leaf_index: _ } => { group.push(pivot::Leaf::new(name)); @@ -1645,7 +1690,7 @@ impl Category { subcategories, } => { for subcategory in subcategories { - subcategory.decode(encoding, footnotes, group); + subcategory.decode(encoding, footnotes, group, warn); } } Child::Group { @@ -1654,7 +1699,7 @@ impl Category { } => { let mut subgroup = Group::new(name).with_label_shown(); for subcategory in subcategories { - subcategory.decode(encoding, footnotes, &mut subgroup); + subcategory.decode(encoding, footnotes, &mut subgroup, warn); } group.push(subgroup); } diff --git a/rust/pspp/src/spv/read/structure.rs b/rust/pspp/src/spv/read/structure.rs index e4f5239590..ba10e98fbf 100644 --- a/rust/pspp/src/spv/read/structure.rs +++ b/rust/pspp/src/spv/read/structure.rs @@ -1,8 +1,6 @@ use std::{ - cell::RefCell, io::{BufRead, BufReader, Cursor, Read, Seek}, mem::take, - rc::Rc, }; use anyhow::Context; @@ -22,16 +20,28 @@ use crate::{ spv::{ Error, legacy_bin::LegacyBin, - read::{ - TableType, Warning, WarningDetails, - legacy_xml::Visualization, - light::{LightTable, LightWarning}, - }, + read::{TableType, Warning, WarningDetails, legacy_xml::Visualization, light::LightTable}, }, }; +/// 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, + + /// 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, } @@ -39,7 +49,7 @@ impl StructureMember { pub fn read( reader: R, member_name: &str, - warn: &Rc>>, + warn: &mut dyn FnMut(Warning), ) -> Result where R: BufRead, @@ -51,6 +61,7 @@ impl StructureMember { error, })?; Ok(Self { + member_name: member_name.into(), page_setup: heading .page_setup .take() @@ -60,6 +71,7 @@ impl StructureMember { } } +#[derive(Clone, Debug)] pub struct Heading { page_setup: Option, expand: bool, @@ -69,14 +81,15 @@ pub struct Heading { } impl Heading { - pub fn read_items( + pub fn read_items( self, archive: &mut ZipArchive, structure_member: &str, - warn: &Rc>>, + warn: &mut F, ) -> Vec where R: Read + Seek, + F: FnMut(Warning), { let mut items = Vec::new(); for child in self.children { @@ -91,7 +104,7 @@ impl Heading { ); } let result = match container.content { - Content::Table(table) => table.decode(archive, warn), + 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), @@ -114,7 +127,7 @@ impl Heading { let label = take(&mut heading.label); let command_name = take(&mut heading.command_name); heading - .read_items(archive, structure_member, warn) + .read_items(archive, structure_member, &mut *warn) .into_iter() .collect::() .with_show(expand) @@ -129,6 +142,7 @@ impl Heading { } } +#[derive(Clone, Debug)] pub enum HeadingChild { Heading(Heading), Container(Container), @@ -143,6 +157,7 @@ impl HeadingChild { } } +#[derive(Clone, Debug)] pub struct Container { show: bool, page_break_before: bool, @@ -153,6 +168,7 @@ pub struct Container { content: Content, } +#[derive(Clone, Debug)] pub enum Content { Text(Text), Table(Table), @@ -175,6 +191,7 @@ impl Content { } } +#[derive(Clone, Debug)] pub struct Table { subtype: String, table_type: TableType, @@ -195,13 +212,10 @@ impl Table { } } - fn decode( - self, - archive: &mut ZipArchive, - warn: &Rc>>, - ) -> Result + fn decode(self, archive: &mut ZipArchive, mut warn: F) -> Result where R: Read + Seek, + F: FnMut(Warning), { if let Some(xml_member_name) = &self.xml_member { let bin_member_name = &self.bin_member; @@ -236,28 +250,23 @@ impl Table { light.read_to_end(&mut data)?; let mut cursor = Cursor::new(data); - let warn = warn.clone(); - let warning = Rc::new(RefCell::new(Box::new({ - let warn = warn.clone(); - let member_name = member_name.clone(); - move |w| { - (warn.borrow_mut())(Warning { - member: member_name.clone(), - details: WarningDetails::LightWarning(w), - }) - } - }) as Box)); - let table = LightTable::read_args(&mut cursor, (warning.clone(),)).map_err(|e| { + let table = LightTable::read(&mut cursor).map_err(|e| { e.with_message(format!( "While parsing {member_name:?} as light binary SPV member" )) })?; - let pivot_table = table.decode(&mut *warning.borrow_mut()); + let pivot_table = table.decode(&mut |warning| { + warn(Warning { + member: member_name.clone(), + details: WarningDetails::LightWarning(warning), + }) + }); Ok(pivot_table.into_item()) } } } +#[derive(Clone, Debug)] pub struct Image { member: String, } @@ -276,6 +285,7 @@ impl Image { } } +#[derive(Clone, Debug)] pub struct Graph { xml_member: String, data_member: Option, @@ -293,8 +303,6 @@ impl Graph { } mod raw { - use std::{cell::RefCell, rc::Rc}; - use paper_sizes::PaperSize; use serde::Deserialize; @@ -336,7 +344,7 @@ mod raw { pub fn decode( self, structure_member: &str, - warn: &Rc>>, + warn: &mut dyn FnMut(Warning), ) -> super::Heading { let mut children = Vec::new(); for child in self.children { @@ -449,7 +457,7 @@ mod raw { impl PageSetup { pub fn decode( self, - warn: &Rc>>, + warn: &mut dyn FnMut(Warning), structure_member: &str, ) -> page::PageSetup { let mut setup = page::PageSetup::default(); @@ -486,7 +494,7 @@ mod raw { } else if reference_orientation.starts_with("90") { setup.orientation = Orientation::Landscape; } else { - (warn.borrow_mut())(Warning { + warn(Warning { member: structure_member.into(), details: WarningDetails::UnknownOrientation(reference_orientation.clone()), }); diff --git a/rust/pspp/src/spv/read/tests.rs b/rust/pspp/src/spv/read/tests.rs index d7cb877056..c1860153ec 100644 --- a/rust/pspp/src/spv/read/tests.rs +++ b/rust/pspp/src/spv/read/tests.rs @@ -6,7 +6,7 @@ use std::{ use crate::{ output::{Text, pivot::tests::assert_lines_eq}, - spv::ReadOptions, + spv::SpvArchive, }; #[test] @@ -117,8 +117,12 @@ where R: BufRead + Seek + 'static, { let mut warnings = Vec::new(); - let output = match ReadOptions::new(move |warning| warnings.push(warning)).open_reader(spvfile) - { + let result = match SpvArchive::open_reader(spvfile, None) { + Ok(mut archive) => archive.read(|w| warnings.push(w)), + Err(error) => Err(error), + }; + + let output = match result { Ok(spv_file) => { let (items, _page_setup /*XXX*/) = spv_file.into_contents(); -- 2.30.2