From 64e76d3bde306e3887f9a535f949c7c9034c402d Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Mon, 15 Dec 2025 19:43:22 -0800 Subject: [PATCH] work --- rust/pspp/src/format/display.rs | 16 +- rust/pspp/src/output/drivers/cairo/fsm.rs | 28 +- rust/pspp/src/output/pivot/look.rs | 4 - rust/pspp/src/output/pivot/look_xml.rs | 2 - rust/pspp/src/output/pivot/tlo.rs | 2 - rust/pspp/src/output/pivot/value.rs | 16 +- rust/pspp/src/spv/read.rs | 43 +-- rust/pspp/src/spv/read/legacy_bin.rs | 2 +- rust/pspp/src/spv/read/legacy_xml.rs | 3 +- rust/pspp/src/spv/read/light.rs | 304 ++++++++++++++-------- rust/pspp/src/spv/write.rs | 23 +- 11 files changed, 272 insertions(+), 171 deletions(-) diff --git a/rust/pspp/src/format/display.rs b/rust/pspp/src/format/display.rs index c101ce7464..1f2907cde2 100644 --- a/rust/pspp/src/format/display.rs +++ b/rust/pspp/src/format/display.rs @@ -278,6 +278,10 @@ where ..self } } + + pub fn decimal(&self) -> Decimal { + self.settings.number_style(self.format.type_).decimal + } fn fmt_binary(&self, f: &mut Formatter) -> FmtResult { let output = self.to_binary().unwrap(); for b in output { @@ -290,15 +294,15 @@ where let style = self.settings.number_style(self.format.type_); if self.format.type_ != Type::E && number.abs() < 1.5 * power10(self.format.w()) { let rounder = Rounder::new(style, number, self.format.d); - if self.decimal(f, &rounder, style, true)? - || self.scientific(f, number, style, true)? - || self.decimal(f, &rounder, style, false)? + if self.number_decimal(f, &rounder, style, true)? + || self.number_scientific(f, number, style, true)? + || self.number_decimal(f, &rounder, style, false)? { return Ok(()); } } - if !self.scientific(f, number, style, false)? { + if !self.number_scientific(f, number, style, false)? { self.overflow(f)?; } Ok(()) @@ -374,7 +378,7 @@ where Ok(()) } - fn decimal( + fn number_decimal( &self, f: &mut Formatter<'_>, rounder: &Rounder, @@ -463,7 +467,7 @@ where Ok(false) } - fn scientific( + fn number_scientific( &self, f: &mut Formatter<'_>, number: f64, diff --git a/rust/pspp/src/output/drivers/cairo/fsm.rs b/rust/pspp/src/output/drivers/cairo/fsm.rs index 277666cca0..88def4fcdf 100644 --- a/rust/pspp/src/output/drivers/cairo/fsm.rs +++ b/rust/pspp/src/output/drivers/cairo/fsm.rs @@ -327,28 +327,24 @@ impl<'a, 'b> DrawCell<'a, 'b> { }; layout.set_font_description(Some(font)); - let (body, suffixes) = self.display().split_suffixes(); - let horz_align = self.horz_align(&body); + let (body_display, suffixes) = self.display().split_suffixes(); + let horz_align = self.horz_align(&body_display); - let (mut body, mut attrs) = if let Some(markup) = body.markup() { + let (mut body, mut attrs) = if let Some(markup) = body_display.markup() { let (body, attrs) = Markup::to_pango(markup, self.substitutions); (body, Some(attrs)) } else { - (avoid_decimal_split(body.to_string()), None) + (avoid_decimal_split(body_display.to_string()), None) }; - match horz_align { - HorzAlign::Decimal { offset, decimal } if !self.rotate => { - let decimal_position = if let Some(position) = body.rfind(char::from(decimal)) { - layout.set_text(&body[position..]); - layout.set_width(-1); - layout.size().0 as isize - } else { - 0 - }; - bb[Axis2::X].end -= pxf_to_xr(offset).saturating_sub(decimal_position); - } - _ => (), + if let Some(decimal_offset) = horz_align.decimal_offset() + && !self.rotate + && let Some(decimal) = body_display.decimal() + && let Some(index) = body.rfind(char::from(decimal)) + { + layout.set_text(&body[index..]); + layout.set_width(-1); + bb[Axis2::X].end -= pxf_to_xr(decimal_offset).saturating_sub(layout.size().0 as isize); } if self.font_style.underline { diff --git a/rust/pspp/src/output/pivot/look.rs b/rust/pspp/src/output/pivot/look.rs index 3528a7a4da..4df79828b3 100644 --- a/rust/pspp/src/output/pivot/look.rs +++ b/rust/pspp/src/output/pivot/look.rs @@ -40,7 +40,6 @@ use quick_xml::{DeError, de::from_str}; use serde::{Deserialize, Serialize, de::Visitor, ser::SerializeStruct}; use crate::{ - format::Decimal, output::pivot::{ Axis2, FootnoteMarkerPosition, FootnoteMarkerType, TableProperties, tlo::parse_tlo, }, @@ -720,9 +719,6 @@ pub enum HorzAlign { Decimal { /// Decimal offset from the right side of the cell, in 1/96" units. offset: f64, - - /// Decimal character. - decimal: Decimal, }, } diff --git a/rust/pspp/src/output/pivot/look_xml.rs b/rust/pspp/src/output/pivot/look_xml.rs index 6215acd558..a65fe438e2 100644 --- a/rust/pspp/src/output/pivot/look_xml.rs +++ b/rust/pspp/src/output/pivot/look_xml.rs @@ -20,7 +20,6 @@ use enum_map::enum_map; use serde::{Deserialize, de::Visitor}; use crate::{ - format::Decimal, output::pivot::{ Axis2, FootnoteMarkerPosition, FootnoteMarkerType, look::{ @@ -215,7 +214,6 @@ impl CellStyle { TextAlignment::Center => Some(HorzAlign::Center), TextAlignment::Decimal => Some(HorzAlign::Decimal { offset: self.decimal_offset.as_px_f64(), - decimal: Decimal::Dot, }), TextAlignment::Mixed => None, }, diff --git a/rust/pspp/src/output/pivot/tlo.rs b/rust/pspp/src/output/pivot/tlo.rs index 31a108910f..a7efd856e2 100644 --- a/rust/pspp/src/output/pivot/tlo.rs +++ b/rust/pspp/src/output/pivot/tlo.rs @@ -17,7 +17,6 @@ use std::{fmt::Debug, io::Cursor}; use crate::{ - format::Decimal, output::pivot::{ Axis2, FootnoteMarkerPosition, FootnoteMarkerType, look::{self, Border, BoxBorder, HeadingRegion, LabelPosition, RowColBorder}, @@ -347,7 +346,6 @@ impl look::AreaStyle { 2 => Some(HorzAlign::Center), 4 => Some(HorzAlign::Decimal { offset: style.decimal_offset as f64 / (72.0 * 20.0) * 96.0, - decimal: Decimal::Comma, }), _ => None, }, diff --git a/rust/pspp/src/output/pivot/value.rs b/rust/pspp/src/output/pivot/value.rs index 48a974b486..c129ec91dc 100644 --- a/rust/pspp/src/output/pivot/value.rs +++ b/rust/pspp/src/output/pivot/value.rs @@ -26,7 +26,7 @@ use crate::{ calendar::{date_time_to_pspp, time_to_pspp}, data::{ByteString, Datum, EncodedString, WithEncoding}, - format::{DATETIME40_0, F8_2, F40, Format, TIME40_0, Type, UncheckedFormat}, + format::{DATETIME40_0, Decimal, F8_2, F40, Format, TIME40_0, Type, UncheckedFormat}, output::pivot::{ Footnote, FootnoteMarkerType, look::{CellStyle, FontStyle}, @@ -527,6 +527,15 @@ impl<'a> DisplayValue<'a> { self.inner.is_empty() && self.subscripts.is_empty() && self.footnotes.is_empty() } + /// Returns the character that the formatted value would use for a decimal + /// point if it has one, or `None` if the value isn't a datum value and + /// therefore doesn't have a decimal point. + pub fn decimal(&self) -> Option { + self.inner + .as_datum_value() + .map(|datum_value| datum_value.decimal()) + } + fn small(&self) -> f64 { self.options.small } @@ -727,6 +736,11 @@ impl DatumValue { Ok(()) } + /// Returns the decimal point used in the formatted value, if any. + pub fn decimal(&self) -> Decimal { + self.datum.display(self.format).decimal() + } + /// Serializes this value to `serializer` in the "bare" manner described for /// [BareValue]. pub fn serialize_bare(&self, serializer: S) -> Result diff --git a/rust/pspp/src/spv/read.rs b/rust/pspp/src/spv/read.rs index 29384b9b62..be3ba6ce4e 100644 --- a/rust/pspp/src/spv/read.rs +++ b/rust/pspp/src/spv/read.rs @@ -38,7 +38,7 @@ use crate::{ html::Document, legacy_bin::LegacyBin, legacy_xml::Visualization, - light::{LightError, LightTable}, + light::{LightTable, LightWarning}, }, }; @@ -195,9 +195,6 @@ pub enum Error { /// {0} BinrwError(#[from] binrw::Error), - /// {0} - LightError(#[from] LightError), - /// {0} CairoError(#[from] cairo::IoError), } @@ -263,9 +260,12 @@ impl Heading { ); } let item = match container.content { - ContainerContent::Table(table) => { - table.decode(archive, structure_member).unwrap() /* XXX*/ - } + ContainerContent::Table(table) => table + .decode(archive, structure_member) + .unwrap_or_else(|error| { + new_error_item(format!("Error reading table: {error}")) + .with_spv_info(SpvInfo::new(structure_member).with_error()) + }), ContainerContent::Graph(graph) => graph.decode(structure_member), ContainerContent::Text(container_text) => Text::new( match container_text.text_type { @@ -278,12 +278,18 @@ impl Heading { .into_item() .with_command_name(container_text.command_name) .with_spv_info(SpvInfo::new(structure_member)), - ContainerContent::Image(image) => { - image.decode(archive, structure_member).unwrap() - } /*XXX*/, - ContainerContent::Object(object) => { - object.decode(archive, structure_member).unwrap() - } /*XXX*/, + ContainerContent::Image(image) => image + .decode(archive, structure_member) + .unwrap_or_else(|error| { + new_error_item(format!("Error reading image: {error}")) + .with_spv_info(SpvInfo::new(structure_member).with_error()) + }), + ContainerContent::Object(object) => object + .decode(archive, structure_member) + .unwrap_or_else(|error| { + new_error_item(format!("Error reading object: {error}")) + .with_spv_info(SpvInfo::new(structure_member).with_error()) + }), ContainerContent::Model => new_error_item("models not yet implemented") .with_spv_info(SpvInfo::new(structure_member).with_error()), ContainerContent::Tree => new_error_item("trees not yet implemented") @@ -667,12 +673,19 @@ impl Table { let mut data = Vec::with_capacity(light.size() as usize); light.read_to_end(&mut data)?; let mut cursor = Cursor::new(data); - let table = LightTable::read(&mut cursor).map_err(|e| { + + let table = LightTable::read_args( + &mut cursor, + (Box::new(|warning| println!("{warning}")),), /*XXX*/ + ) + .map_err(|e| { e.with_message(format!( "While parsing {member_name:?} as light binary SPV member" )) })?; - let pivot_table = table.decode()?; + let mut b = + (Box::new(|warning| println!("{warning}")) as Box,); + let pivot_table = table.decode(&mut b.0); Ok(pivot_table.into_item().with_spv_info( SpvInfo::new(structure_member) .with_members(SpvMembers::Light(self.table_structure.data_path.clone())), diff --git a/rust/pspp/src/spv/read/legacy_bin.rs b/rust/pspp/src/spv/read/legacy_bin.rs index 7398d25734..a28da59cf4 100644 --- a/rust/pspp/src/spv/read/legacy_bin.rs +++ b/rust/pspp/src/spv/read/legacy_bin.rs @@ -93,7 +93,7 @@ impl DataValue { }; match format_map.get(&f) { Some(format) => *format, - None => decode_format(f as u32), + None => decode_format(f as u32, &mut |_| () /*XXX*/), } } diff --git a/rust/pspp/src/spv/read/legacy_xml.rs b/rust/pspp/src/spv/read/legacy_xml.rs index 04fbea4d69..1c5de47f69 100644 --- a/rust/pspp/src/spv/read/legacy_xml.rs +++ b/rust/pspp/src/spv/read/legacy_xml.rs @@ -32,7 +32,7 @@ use serde::Deserialize; use crate::{ data::{Datum, EncodedString}, - format::{self, Decimal::Dot, F8_0, F40_2, Type, UncheckedFormat}, + format::{self, F8_0, F40_2, Type, UncheckedFormat}, output::pivot::{ self, Axis2, Axis3, Category, CategoryLocator, Dimension, Group, Leaf, Length, PivotTable, look::{ @@ -2006,7 +2006,6 @@ impl TextAlignment { TextAlignment::Center => Some(HorzAlign::Center), TextAlignment::Decimal => Some(HorzAlign::Decimal { offset: decimal_offset.unwrap_or_default().as_px_f64(), - decimal: Dot, }), TextAlignment::Mixed => None, } diff --git a/rust/pspp/src/spv/read/light.rs b/rust/pspp/src/spv/read/light.rs index c557be1625..a8231d102e 100644 --- a/rust/pspp/src/spv/read/light.rs +++ b/rust/pspp/src/spv/read/light.rs @@ -1,7 +1,9 @@ use std::{ + cell::RefCell, fmt::Debug, io::{Read, Seek}, ops::Deref, + rc::Rc, str::FromStr, sync::Arc, }; @@ -18,8 +20,8 @@ use enum_map::{EnumMap, enum_map}; use crate::{ data::Datum, format::{ - CC, Decimal, Decimals, Epoch, F40, Format, NumberStyle, Settings, Type, UncheckedFormat, - Width, + CC, Decimal, Decimals, Epoch, F40, F40_2, Format, NumberStyle, Settings, Type, + UncheckedFormat, Width, }, output::pivot::{ self, Axis2, Axis3, FootnoteMarkerPosition, FootnoteMarkerType, Footnotes, Group, @@ -34,14 +36,35 @@ use crate::{ settings::Show, }; +/// A warning decoding a light detail member. #[derive(Debug, Display, thiserror::Error)] -pub enum LightError { +pub enum LightWarning { + /// Unknown encoding {0:?}. + UnknownEncoding(String), + + /// Unknown "show" value {0}. + UnknownShow(u8), + + /// Invalid decimal point {0:?}. + InvalidDecimal(char), + + /// Invalid custom currency definition {0:?}. + InvalidCustomCurrency(String), + + /// Invalid format type {0}. + InvalidFormat(u16), + /// Expected {expected} dimensions along axes, found {actual} dimensions ({n_layers} layers + {n_rows} rows + {n_columns} columns). WrongAxisCount { + /// Expected number of dimensions. expected: usize, + /// Actual number of dimensions. actual: usize, + /// Actual number of layer dimensions. n_layers: usize, + /// Actual number of row dimensions. n_rows: usize, + /// Actual number of column dimensions. n_columns: usize, }, @@ -52,16 +75,35 @@ pub enum LightError { DuplicateDimensionIndex(usize), } +struct Context { + version: Version, + warn: Rc>>, +} + +impl Context { + fn new(version: Version, warn: Box) -> Self { + Self { + version: version, + warn: Rc::new(RefCell::new(Box::new(warn))), + } + } + fn warn(&self, warning: LightWarning) { + (self.warn.borrow_mut())(warning); + } +} + #[binread] -#[br(little)] +#[br(little, import(warn: Box))] #[derive(Debug)] pub struct LightTable { header: Header, - #[br(args(header.version))] + #[br(temp, calc(Context::new(header.version, warn)))] + context: Context, + #[br(args(&context))] titles: Titles, - #[br(parse_with(parse_vec), args(header.version))] + #[br(parse_with(parse_vec), args(&context))] footnotes: Vec, - #[br(args(header.version))] + #[br(args(&context))] areas: Areas, #[br(parse_with(parse_counted))] borders: Borders, @@ -71,12 +113,12 @@ pub struct LightTable { table_settings: TableSettings, #[br(if(header.version == Version::V1), temp)] _ts: Option>, - #[br(args(header.version))] + #[br(args(&context))] formats: Formats, - #[br(parse_with(parse_vec), args(header.version))] + #[br(parse_with(parse_vec), args(&context))] dimensions: Vec, axes: Axes, - #[br(parse_with(parse_vec), args(header.version))] + #[br(parse_with(parse_vec), args(&context))] cells: Vec, } @@ -124,8 +166,8 @@ impl LightTable { } } - pub fn decode(&self) -> Result { - let encoding = self.formats.encoding(); + pub fn decode(&self, warn: &mut dyn FnMut(LightWarning)) -> PivotTable { + let encoding = self.formats.encoding(warn); let n1 = self.formats.n1(); let n2 = self.formats.n2(); @@ -163,7 +205,17 @@ impl LightTable { } }) .collect::>(); - let pivot_table = PivotTable::new(self.axes.decode(dimensions)?) + let dimensions = match self.axes.decode(dimensions) { + Ok(dimensions) => dimensions, + Err((warning, dimensions)) => { + warn(warning); + dimensions + .into_iter() + .map(|dimension| (Axis3::Y, dimension)) + .collect() + } + }; + let pivot_table = PivotTable::new(dimensions) .with_style(PivotTableStyle { look: Arc::new(self.decode_look(encoding)), rotate_inner_column_labels: self.header.rotate_inner_column_labels, @@ -179,9 +231,9 @@ impl LightTable { ), settings: Settings { epoch: self.formats.y0.epoch(), - decimal: self.formats.y0.decimal(), + decimal: self.formats.y0.decimal(warn), leading_zero: y1.map_or(false, |y1| y1.include_leading_zero), - ccs: self.formats.custom_currency.decode(encoding), + ccs: self.formats.custom_currency.decode(encoding, warn), }, grouping: { let grouping = self.formats.y0.grouping; @@ -222,7 +274,7 @@ impl LightTable { }) .with_footnotes(footnotes) .with_data(cells); - Ok(pivot_table) + pivot_table } } @@ -263,25 +315,25 @@ enum Version { } #[binread] -#[br(little, import(version: Version))] +#[br(little, import(context: &Context))] #[derive(Debug)] struct Titles { - #[br(args(version))] + #[br(args(context))] title: Value, #[br(temp)] _1: Optional, - #[br(args(version))] + #[br(args(context))] subtype: Value, #[br(temp)] _2: Optional, #[br(magic = b'1')] - #[br(args(version))] + #[br(args(context))] user_title: Value, #[br(temp)] _3: Optional, - #[br(parse_with(parse_explicit_optional), args(version))] + #[br(parse_with(parse_explicit_optional), args(context))] corner_text: Option, - #[br(parse_with(parse_explicit_optional), args(version))] + #[br(parse_with(parse_explicit_optional), args(context))] caption: Option, } @@ -311,24 +363,28 @@ where } #[binrw::parser(reader, endian)] -pub(super) fn parse_vec(inner: A, ...) -> BinResult> +pub(super) fn parse_vec<'a, T, A>(inner: A, ...) -> BinResult> where - for<'a> T: BinRead = A>, + T: BinRead = A>, A: Clone, T: 'static, { let count = u32::read_options(reader, endian, ())? as usize; - >::read_options(reader, endian, VecArgs { count, inner }) + let mut vec = Vec::with_capacity(count); + for _ in 0..count { + vec.push(T::read_options(reader, endian, inner.clone())?); + } + Ok(vec) } #[binread] -#[br(little, import(version: Version))] +#[br(little, import(context: &Context))] #[derive(Debug)] struct Footnote { - #[br(args(version))] + #[br(args(context))] text: Value, #[br(parse_with(parse_explicit_optional))] - #[br(args(version))] + #[br(args(context))] marker: Option, show: i32, } @@ -342,12 +398,12 @@ impl Footnote { } #[binread] -#[br(little, import(version: Version))] +#[br(little, import(context: &Context))] #[derive(Debug)] struct Areas { #[br(temp)] _1: Optional, - #[br(args(version))] + #[br(args(context))] areas: [Area; 8], } @@ -389,7 +445,7 @@ fn parse_color() -> BinResult { } #[binread] -#[br(little, import(version: Version))] +#[br(little, import(context: &Context))] #[derive(Debug)] struct Area { #[br(temp)] @@ -412,7 +468,7 @@ struct Area { alt_fg: Color, #[br(parse_with(parse_color))] alt_bg: Color, - #[br(if(version == Version::V3))] + #[br(if(context.version == Version::V3))] margins: Margins, } @@ -774,7 +830,7 @@ where #[binread] #[br(little)] -#[br(import(version: Version))] +#[br(import(context: &Context))] #[derive(Debug)] struct Formats { #[br(parse_with(parse_vec))] @@ -789,9 +845,9 @@ struct Formats { _x9: bool, y0: Y0, custom_currency: CustomCurrency, - #[br(if(version == Version::V1))] + #[br(if(context.version == Version::V1))] v1: Counted>, - #[br(if(version == Version::V3))] + #[br(if(context.version == Version::V3))] v3: Option>, } @@ -819,13 +875,17 @@ impl Formats { self.y1().map(|y1| &y1.charset) } - fn encoding(&self) -> &'static Encoding { - // XXX We should probably warn for unknown encodings below - if let Some(charset) = self.charset() - && let Some(encoding) = Encoding::for_label(&charset.string) - { - encoding - } else if let Ok(locale) = str::from_utf8(&self.locale.string) + fn encoding(&self, warn: &mut dyn FnMut(LightWarning)) -> &'static Encoding { + if let Some(charset) = self.charset() { + if let Some(encoding) = Encoding::for_label(&charset.string) { + return encoding; + } + warn(LightWarning::UnknownEncoding( + String::from_utf8_lossy(&charset.string).into_owned(), + )); + } + + if let Ok(locale) = str::from_utf8(&self.locale.string) && let Some(dot) = locale.find('.') && let Some(encoding) = Encoding::for_label(locale[dot + 1..].as_bytes()) { @@ -905,9 +965,9 @@ struct N1 { #[br(temp, parse_with(parse_bool))] _x16: bool, _lang: u8, - #[br(parse_with(parse_show))] + #[br(parse_with(parse_show), args(|warning| eprintln!("{warning}")))] show_variables: Option, - #[br(parse_with(parse_show))] + #[br(parse_with(parse_show), args(|warning| eprintln!("{warning}")))] show_values: Option, #[br(temp)] _x18: i32, @@ -922,14 +982,17 @@ struct N1 { } #[binrw::parser(reader, endian)] -fn parse_show() -> BinResult> { +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)), - _ => { - // XXX warn about invalid value + other => { + warn(LightWarning::UnknownShow(other)); Ok(None) } } @@ -1008,9 +1071,15 @@ impl Y0 { } } - fn decimal(&self) -> Decimal { - // XXX warn about bad decimal point? - Decimal::try_from(self.decimal as char).unwrap_or_default() + fn decimal(&self, warn: &mut dyn FnMut(LightWarning)) -> Decimal { + let c = self.decimal as char; + match Decimal::try_from(c) { + Ok(decimal) => decimal, + Err(_) => { + warn(LightWarning::InvalidDecimal(c)); + Decimal::default() + } + } } } @@ -1023,13 +1092,18 @@ struct CustomCurrency { } impl CustomCurrency { - fn decode(&self, encoding: &'static Encoding) -> EnumMap>> { + fn decode( + &self, + encoding: &'static Encoding, + warn: &mut dyn FnMut(LightWarning), + ) -> EnumMap>> { let mut ccs = EnumMap::default(); for (cc, string) in enum_iterator::all().zip(&self.ccs) { - if let Ok(style) = NumberStyle::from_str(&string.decode(encoding)) { + let string = string.decode(encoding); + if let Ok(style) = NumberStyle::from_str(&string) { ccs[cc] = Some(Box::new(style)); } else { - // XXX warning + warn(LightWarning::InvalidCustomCurrency(string)); } } ccs @@ -1037,37 +1111,37 @@ impl CustomCurrency { } #[binread] -#[br(little, import(version: Version))] +#[br(little, import(context: &Context))] #[derive(Debug)] struct ValueNumber { - #[br(parse_with(parse_explicit_optional), args(version))] + #[br(parse_with(parse_explicit_optional), args(context))] mods: Option, - #[br(parse_with(parse_format))] + #[br(parse_with(parse_format), args(context))] format: Format, x: f64, } #[binread] -#[br(little, import(version: Version))] +#[br(little, import(context: &Context))] #[derive(Debug)] struct ValueVarNumber { - #[br(parse_with(parse_explicit_optional), args(version))] + #[br(parse_with(parse_explicit_optional), args(context))] mods: Option, - #[br(parse_with(parse_format))] + #[br(parse_with(parse_format), args(context))] format: Format, x: f64, var_name: U32String, value_label: U32String, - #[br(parse_with(parse_show))] + #[br(parse_with(parse_show), args(|warning| eprintln!("{warning}")))] show: Option, } #[binread] -#[br(little, import(version: Version))] +#[br(little, import(context: &Context))] #[derive(Debug)] struct ValueText { local: U32String, - #[br(parse_with(parse_explicit_optional), args(version))] + #[br(parse_with(parse_explicit_optional), args(context))] mods: Option, id: U32String, c: U32String, @@ -1076,51 +1150,51 @@ struct ValueText { } #[binread] -#[br(little, import(version: Version))] +#[br(little, import(context: &Context))] #[derive(Debug)] struct ValueString { - #[br(parse_with(parse_explicit_optional), args(version))] + #[br(parse_with(parse_explicit_optional), args(context))] mods: Option, - #[br(parse_with(parse_format))] + #[br(parse_with(parse_format), args(context))] format: Format, value_label: U32String, var_name: U32String, - #[br(parse_with(parse_show))] + #[br(parse_with(parse_show), args(|warning| eprintln!("{warning}")))] show: Option, s: U32String, } #[binread] -#[br(little, import(version: Version))] +#[br(little, import(context: &Context))] #[derive(Debug)] struct ValueVarName { - #[br(parse_with(parse_explicit_optional), args(version))] + #[br(parse_with(parse_explicit_optional), args(context))] mods: Option, var_name: U32String, var_label: U32String, - #[br(parse_with(parse_show))] + #[br(parse_with(parse_show), args(|warning| eprintln!("{warning}")))] show: Option, } #[binread] -#[br(little, import(version: Version))] +#[br(little, import(context: &Context))] #[derive(Debug)] struct ValueFixedText { local: U32String, - #[br(parse_with(parse_explicit_optional), args(version))] + #[br(parse_with(parse_explicit_optional), args(context))] mods: Option, id: U32String, c: U32String, } #[binread] -#[br(little, import(version: Version))] +#[br(little, import(context: &Context))] #[derive(Debug)] struct ValueTemplate { - #[br(parse_with(parse_explicit_optional), args(version))] + #[br(parse_with(parse_explicit_optional), args(context))] mods: Option, template: U32String, - #[br(parse_with(parse_vec), args(version))] + #[br(parse_with(parse_vec), args(context))] args: Vec, } @@ -1136,7 +1210,7 @@ enum Value { } impl BinRead for Value { - type Args<'a> = (Version,); + type Args<'a> = (&'a Context,); fn read_options( reader: &mut R, @@ -1177,9 +1251,9 @@ impl BinRead for Value { } } -pub(super) fn decode_format(raw: u32) -> Format { +pub(super) fn decode_format(raw: u32, warn: &mut dyn FnMut(LightWarning)) -> Format { if raw == 0 || raw == 0x10000 || raw == 1 { - return Format::new(Type::F, 40, 2).unwrap(); + return F40_2; } let raw_type = (raw >> 16) as u16; @@ -1188,7 +1262,7 @@ pub(super) fn decode_format(raw: u32) -> Format { } else if let Ok(type_) = Type::try_from(raw_type) { type_ } else { - // XXX warn + warn(LightWarning::InvalidFormat(raw_type)); Type::F }; let w = ((raw >> 8) & 0xff) as Width; @@ -1198,8 +1272,11 @@ pub(super) fn decode_format(raw: u32) -> Format { } #[binrw::parser(reader, endian)] -fn parse_format() -> BinResult { - Ok(decode_format(u32::read_options(reader, endian, ())?)) +fn parse_format(context: &Context) -> BinResult { + Ok(decode_format( + u32::read_options(reader, endian, ())?, + &mut *context.warn.borrow_mut(), + )) } impl ValueNumber { @@ -1305,16 +1382,16 @@ impl Value { struct Argument(Vec); impl BinRead for Argument { - type Args<'a> = (Version,); + type Args<'a> = (&'a Context,); fn read_options( reader: &mut R, endian: Endian, - (version,): (Version,), + context: Self::Args<'_>, ) -> BinResult { let count = u32::read_options(reader, endian, ())? as usize; if count == 0 { - Ok(Self(vec![Value::read_options(reader, endian, (version,))?])) + Ok(Self(vec![Value::read_options(reader, endian, context)?])) } else { let zero = u32::read_options(reader, endian, ())?; assert_eq!(zero, 0); @@ -1323,7 +1400,7 @@ impl BinRead for Argument { endian, VecArgs { count, - inner: (version,), + inner: context, }, )?; Ok(Self(values)) @@ -1345,16 +1422,16 @@ impl Argument { } #[binread] -#[br(little, import(version: Version))] +#[br(little, import(context: &Context))] #[derive(Debug)] struct ValueMods { #[br(parse_with(parse_vec))] refs: Vec, #[br(parse_with(parse_vec))] subscripts: Vec, - #[br(if(version == Version::V1))] + #[br(if(context.version == Version::V1))] v1: Option, - #[br(if(version == Version::V3), parse_with(parse_counted))] + #[br(if(context.version == Version::V3), parse_with(parse_counted))] v3: ValueModsV3, } @@ -1401,15 +1478,18 @@ impl ValueMods { bg: font_style.bg, size: (font_style.size as i32) * 4 / 3, }); - let cell_style = self.v3.style_pair.cell_style.as_ref().map(|cell_style| { - look::CellStyle { + let cell_style = self + .v3 + .style_pair + .cell_style + .as_ref() + .map(|cell_style| look::CellStyle { horz_align: match cell_style.halign { 0 => Some(HorzAlign::Center), 2 => Some(HorzAlign::Left), 4 => Some(HorzAlign::Right), 6 => Some(HorzAlign::Decimal { offset: cell_style.decimal_offset, - decimal: Decimal::Dot, /*XXX*/ }), _ => None, }, @@ -1422,8 +1502,7 @@ impl ValueMods { Axis2::X => [cell_style.left_margin as i32, cell_style.right_margin as i32], Axis2::Y => [cell_style.top_margin as i32, cell_style.bottom_margin as i32], }, - } - }); + }); ValueStyle { cell_style, font_style, @@ -1521,10 +1600,10 @@ struct CellStyle { #[binread] #[br(little)] -#[br(import(version: Version))] +#[br(import(context: &Context))] #[derive(Debug)] struct Dimension { - #[br(args(version))] + #[br(args(context))] name: Value, #[br(temp)] _x1: u8, @@ -1538,17 +1617,17 @@ struct Dimension { hide_all_labels: bool, #[br(magic(1u8), temp)] _dim_index: i32, - #[br(parse_with(parse_vec), args(version))] + #[br(parse_with(parse_vec), args(context))] categories: Vec, } #[binread] -#[br(little, import(version: Version))] +#[br(little, import(context: &Context))] #[derive(Debug)] struct Category { - #[br(args(version))] + #[br(args(context))] name: Value, - #[br(args(version))] + #[br(args(context))] child: Child, } @@ -1582,7 +1661,7 @@ impl Category { } #[binread] -#[br(little, import(version: Version))] +#[br(little, import(context: &Context))] #[derive(Debug)] enum Child { Leaf { @@ -1598,7 +1677,7 @@ enum Child { merge: bool, #[br(temp, magic(b"\0\x01"))] _x23: i32, - #[br(magic(-1i32), parse_with(parse_vec), args(version))] + #[br(magic(-1i32), parse_with(parse_vec), args(context))] subcategories: Vec>, }, } @@ -1625,21 +1704,20 @@ impl Axes { fn decode( &self, dimensions: Vec, - ) -> Result, LightError> { + ) -> Result, (LightWarning, Vec)> { let n = self.layers.len() + self.rows.len() + self.columns.len(); if n != dimensions.len() { - /* XXX warn - return Err(LightError::WrongAxisCount { + // Warn, then treat all of the dimensions as rows. + return Err(( + LightWarning::WrongAxisCount { expected: dimensions.len(), actual: n, n_layers: self.layers.len(), n_rows: self.rows.len(), n_columns: self.columns.len(), - });*/ - return Ok(dimensions - .into_iter() - .map(|dimension| (Axis3::Y, dimension)) - .collect()); + }, + dimensions, + )); } fn axis_dims(axis: Axis3, dimensions: &[u32]) -> impl Iterator { @@ -1652,9 +1730,9 @@ impl Axes { .chain(axis_dims(Axis3::X, &self.columns)) { if index >= n { - return Err(LightError::InvalidDimensionIndex { index, n }); + return Err((LightWarning::InvalidDimensionIndex { index, n }, dimensions)); } else if axes[index].is_some() { - return Err(LightError::DuplicateDimensionIndex(index)); + return Err((LightWarning::DuplicateDimensionIndex(index), dimensions)); } axes[index] = Some(axis); } @@ -1667,12 +1745,12 @@ impl Axes { } #[binread] -#[br(little, import(version: Version))] +#[br(little, import(context: &Context))] #[derive(Debug)] struct Cell { index: u64, - #[br(if(version == Version::V1), temp)] + #[br(if(context.version == Version::V1), temp)] _zero: Optional, - #[br(args(version))] + #[br(args(context))] value: Value, } diff --git a/rust/pspp/src/spv/write.rs b/rust/pspp/src/spv/write.rs index b275f7ce2b..74266feb9e 100644 --- a/rust/pspp/src/spv/write.rs +++ b/rust/pspp/src/spv/write.rs @@ -134,10 +134,16 @@ where let table_name = format!("{table_id:011}_lightTableData.bin"); self.writer - .start_file(&table_name, SimpleFileOptions::default())?; // XXX - self.writer.write_all(&content)?; // XXX + .start_file(&table_name, SimpleFileOptions::default())?; + self.writer.write_all(&content)?; self.container(structure, item, "vtb:table", |element| { + let subtype = pivot_table.subtype().display(pivot_table).to_string(); + let type_ = match subtype.as_str() { + "Note" | "Notes" => "note", + "Warning" | "Warnings" => "warning", + _ => "table", + }; element .with_attribute(("tableId", Cow::from(table_id.to_string()))) .with_attribute(( @@ -148,11 +154,8 @@ where .as_ref() .map_or("", |s| s.as_str()), )) - .with_attribute(("type", "table" /*XXX*/)) - .with_attribute(( - "subType", - Cow::from(pivot_table.subtype().display(pivot_table).to_string()), - )) + .with_attribute(("type", type_)) + .with_attribute(("subType", Cow::from(subtype))) .write_inner_content(|w| { w.create_element("vtb:tableStructure") .write_inner_content(|w| { @@ -1079,9 +1082,11 @@ impl HorzAlign { } } - fn decimal_offset(&self) -> Option { + /// Returns the decimal offset for [HorzAlign::Decimal], or `None` for other + /// horizontal alignments. + pub fn decimal_offset(&self) -> Option { match *self { - HorzAlign::Decimal { offset, .. } => Some(offset), + HorzAlign::Decimal { offset } => Some(offset), _ => None, } } -- 2.30.2