work rust
authorBen Pfaff <blp@cs.stanford.edu>
Tue, 16 Dec 2025 03:43:22 +0000 (19:43 -0800)
committerBen Pfaff <blp@cs.stanford.edu>
Tue, 16 Dec 2025 03:43:22 +0000 (19:43 -0800)
rust/pspp/src/format/display.rs
rust/pspp/src/output/drivers/cairo/fsm.rs
rust/pspp/src/output/pivot/look.rs
rust/pspp/src/output/pivot/look_xml.rs
rust/pspp/src/output/pivot/tlo.rs
rust/pspp/src/output/pivot/value.rs
rust/pspp/src/spv/read.rs
rust/pspp/src/spv/read/legacy_bin.rs
rust/pspp/src/spv/read/legacy_xml.rs
rust/pspp/src/spv/read/light.rs
rust/pspp/src/spv/write.rs

index c101ce74645a4329432359f10288e95fb94acca1..1f2907cde2597d3db7b210032c43809ba3cd8308 100644 (file)
@@ -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,
index 277666cca0971e4ddb4aae2ab0a2935be585ecc4..88def4fcdf688113349213996bf56ed1a4147b3b 100644 (file)
@@ -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 {
index 3528a7a4dadf650db4a2f235c1ff2d1ac955f305..4df79828b311ba0e91ae2fa91e177975b0f084ca 100644 (file)
@@ -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,
     },
 }
 
index 6215acd5587177082e53afbfcc8d4a204f8b8c88..a65fe438e24c36d6a774499806c11411adcfd079 100644 (file)
@@ -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,
                 },
index 31a108910fe60d0b36cc361383cae321fad9314f..a7efd856e205fd5fa499560f2b135f80d47d1e2d 100644 (file)
@@ -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,
                 },
index 48a974b48651e959fa405038dd18a7994c678ae5..c129ec91dcbd5ca8274bdd5cc19d31929dcb5a79 100644 (file)
@@ -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<Decimal> {
+        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<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
index 29384b9b621ea3ca00cac87e6bd1a83160f0ccbc..be3ba6ce4e4358d90f4bdb7b36af1144f4734cd0 100644 (file)
@@ -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<dyn FnMut(LightWarning)>,);
+                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())),
index 7398d257347d9227760d5e153985c75a2bce99f5..a28da59cf4856d9fa255058aac20a179af001217 100644 (file)
@@ -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*/),
         }
     }
 
index 04fbea4d69b9f2537a41def101da0728d09dd9c1..1c5de47f69d7181750c31cd2cebf0432337f100a 100644 (file)
@@ -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,
         }
index c557be16259b1f8435c72f75369a3560454b6a38..a8231d102e69c0b636a4f5ab7cc1245f92e75d77 100644 (file)
@@ -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<RefCell<Box<dyn FnMut(LightWarning)>>>,
+}
+
+impl Context {
+    fn new(version: Version, warn: Box<dyn FnMut(LightWarning)>) -> 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<dyn FnMut(LightWarning)>))]
 #[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<Footnote>,
-    #[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<Counted<Sponge>>,
-    #[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<Dimension>,
     axes: Axes,
-    #[br(parse_with(parse_vec), args(header.version))]
+    #[br(parse_with(parse_vec), args(&context))]
     cells: Vec<Cell>,
 }
 
@@ -124,8 +166,8 @@ impl LightTable {
         }
     }
 
-    pub fn decode(&self) -> Result<PivotTable, LightError> {
-        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::<Vec<_>>();
-        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<One>,
-    #[br(args(version))]
+    #[br(args(context))]
     subtype: Value,
     #[br(temp)]
     _2: Optional<One>,
     #[br(magic = b'1')]
-    #[br(args(version))]
+    #[br(args(context))]
     user_title: Value,
     #[br(temp)]
     _3: Optional<One>,
-    #[br(parse_with(parse_explicit_optional), args(version))]
+    #[br(parse_with(parse_explicit_optional), args(context))]
     corner_text: Option<Value>,
-    #[br(parse_with(parse_explicit_optional), args(version))]
+    #[br(parse_with(parse_explicit_optional), args(context))]
     caption: Option<Value>,
 }
 
@@ -311,24 +363,28 @@ where
 }
 
 #[binrw::parser(reader, endian)]
-pub(super) fn parse_vec<T, A>(inner: A, ...) -> BinResult<Vec<T>>
+pub(super) fn parse_vec<'a, T, A>(inner: A, ...) -> BinResult<Vec<T>>
 where
-    for<'a> T: BinRead<Args<'a> = A>,
+    T: BinRead<Args<'a> = A>,
     A: Clone,
     T: 'static,
 {
     let count = u32::read_options(reader, endian, ())? as usize;
-    <Vec<T>>::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<Value>,
     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<Zero>,
-    #[br(args(version))]
+    #[br(args(context))]
     areas: [Area; 8],
 }
 
@@ -389,7 +445,7 @@ fn parse_color() -> BinResult<Color> {
 }
 
 #[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<Optional<N0>>,
-    #[br(if(version == Version::V3))]
+    #[br(if(context.version == Version::V3))]
     v3: Option<Counted<FormatsV3>>,
 }
 
@@ -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<Show>,
-    #[br(parse_with(parse_show))]
+    #[br(parse_with(parse_show), args(|warning| eprintln!("{warning}")))]
     show_values: Option<Show>,
     #[br(temp)]
     _x18: i32,
@@ -922,14 +982,17 @@ struct N1 {
 }
 
 #[binrw::parser(reader, endian)]
-fn parse_show() -> BinResult<Option<Show>> {
+fn parse_show<F>(mut warn: F) -> BinResult<Option<Show>>
+where
+    F: FnMut(LightWarning),
+{
     match <u8>::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<CC, Option<Box<NumberStyle>>> {
+    fn decode(
+        &self,
+        encoding: &'static Encoding,
+        warn: &mut dyn FnMut(LightWarning),
+    ) -> EnumMap<CC, Option<Box<NumberStyle>>> {
         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<ValueMods>,
-    #[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<ValueMods>,
-    #[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<Show>,
 }
 
 #[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<ValueMods>,
     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<ValueMods>,
-    #[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<Show>,
     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<ValueMods>,
     var_name: U32String,
     var_label: U32String,
-    #[br(parse_with(parse_show))]
+    #[br(parse_with(parse_show), args(|warning| eprintln!("{warning}")))]
     show: Option<Show>,
 }
 
 #[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<ValueMods>,
     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<ValueMods>,
     template: U32String,
-    #[br(parse_with(parse_vec), args(version))]
+    #[br(parse_with(parse_vec), args(context))]
     args: Vec<Argument>,
 }
 
@@ -1136,7 +1210,7 @@ enum Value {
 }
 
 impl BinRead for Value {
-    type Args<'a> = (Version,);
+    type Args<'a> = (&'a Context,);
 
     fn read_options<R: Read + Seek>(
         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<Format> {
-    Ok(decode_format(u32::read_options(reader, endian, ())?))
+fn parse_format(context: &Context) -> BinResult<Format> {
+    Ok(decode_format(
+        u32::read_options(reader, endian, ())?,
+        &mut *context.warn.borrow_mut(),
+    ))
 }
 
 impl ValueNumber {
@@ -1305,16 +1382,16 @@ impl Value {
 struct Argument(Vec<Value>);
 
 impl BinRead for Argument {
-    type Args<'a> = (Version,);
+    type Args<'a> = (&'a Context,);
 
     fn read_options<R: Read + Seek>(
         reader: &mut R,
         endian: Endian,
-        (version,): (Version,),
+        context: Self::Args<'_>,
     ) -> BinResult<Self> {
         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<i16>,
     #[br(parse_with(parse_vec))]
     subscripts: Vec<U32String>,
-    #[br(if(version == Version::V1))]
+    #[br(if(context.version == Version::V1))]
     v1: Option<ValueModsV1>,
-    #[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<Category>,
 }
 
 #[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<Box<Category>>,
     },
 }
@@ -1625,21 +1704,20 @@ impl Axes {
     fn decode(
         &self,
         dimensions: Vec<pivot::Dimension>,
-    ) -> Result<Vec<(Axis3, pivot::Dimension)>, LightError> {
+    ) -> Result<Vec<(Axis3, pivot::Dimension)>, (LightWarning, Vec<pivot::Dimension>)> {
         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<Item = (Axis3, usize)> {
@@ -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<Zero>,
-    #[br(args(version))]
+    #[br(args(context))]
     value: Value,
 }
index b275f7ce2b58e366eb3752ce829520254347e61e..74266feb9e849358925c9bb641c6c3855e5e3d7f 100644 (file)
@@ -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<f64> {
+    /// Returns the decimal offset for [HorzAlign::Decimal], or `None` for other
+    /// horizontal alignments.
+    pub fn decimal_offset(&self) -> Option<f64> {
         match *self {
-            HorzAlign::Decimal { offset, .. } => Some(offset),
+            HorzAlign::Decimal { offset } => Some(offset),
             _ => None,
         }
     }