From: Ben Pfaff Date: Fri, 9 Jan 2026 01:03:29 +0000 (-0800) Subject: fixes X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=460f9bbc89ef1fb3c0009c08ef1345f8a446f0cb;p=pspp fixes --- diff --git a/rust/pspp/src/format.rs b/rust/pspp/src/format.rs index f180434b73..571c485c6c 100644 --- a/rust/pspp/src/format.rs +++ b/rust/pspp/src/format.rs @@ -16,9 +16,9 @@ use std::{ fmt::{Debug, Display, Formatter, Result as FmtResult, Write}, - ops::{Not, RangeInclusive}, + ops::{Index, Not, RangeInclusive}, str::{Chars, FromStr}, - sync::LazyLock, + sync::{Arc, LazyLock}, }; use chrono::{Datelike, Local}; @@ -550,6 +550,12 @@ pub const PCT40_1: Format = Format { d: 1, }; +pub const DOLLAR40_2: Format = Format { + type_: Type::Dollar, + w: 40, + d: 2, +}; + pub const F8_0: Format = Format { type_: Type::F, w: 8, @@ -1001,6 +1007,39 @@ impl Display for Epoch { } } +#[derive(Clone, Debug, Default, Serialize)] +pub struct CustomCurrencies { + map: Option>>>>, +} + +impl CustomCurrencies { + pub fn new() -> Self { + Self::default() + } + + pub fn set(&mut self, cc: CC, number_style: NumberStyle) { + if &number_style != &self[cc] { + Arc::make_mut(&mut self.map.get_or_insert_default())[cc] = Some(Box::new(number_style)); + } + } +} + +impl Index for CustomCurrencies { + type Output = NumberStyle; + + fn index(&self, index: CC) -> &Self::Output { + if let Some(map) = &self.map + && let Some(number_style) = &map[index] + { + &**number_style + } else { + static DEFAULT: LazyLock = + LazyLock::new(|| NumberStyle::new(Decimal::Dot, false)); + &DEFAULT + } + } +} + #[derive(Clone, Debug, Default, Serialize)] pub struct Settings { /// Epoch for 2-digit years. @@ -1018,7 +1057,7 @@ pub struct Settings { pub leading_zero_pct: bool, /// Custom currency styles. - pub ccs: EnumMap>>, + pub ccs: CustomCurrencies, } #[derive(Copy, Clone, Enum)] @@ -1084,7 +1123,7 @@ impl NumberStyles { Type::Dot => self.dot.get(settings), Type::Dollar => self.dollar.get(settings), Type::Pct => self.pct.get(settings), - Type::CC(cc) => settings.ccs[cc].as_deref().unwrap_or(&self.default), + Type::CC(cc) => &settings.ccs[cc], _ => &self.default, } } @@ -1092,7 +1131,7 @@ impl NumberStyles { impl Settings { pub fn with_cc(mut self, cc: CC, style: NumberStyle) -> Self { - self.ccs[cc] = Some(Box::new(style)); + self.ccs.set(cc, style); self } pub fn with_leading_zero(self, leading_zero: bool) -> Self { @@ -1118,7 +1157,7 @@ impl Settings { /// A numeric output style. This can express numeric formats in /// [Category::Basic] and [Category::Custom]. -#[derive(Clone, Debug, Serialize)] +#[derive(Clone, Debug, Serialize, Eq)] pub struct NumberStyle { pub neg_prefix: Affix, pub prefix: Affix, @@ -1145,6 +1184,18 @@ pub struct NumberStyle { pub extra_bytes: usize, } +impl PartialEq for NumberStyle { + fn eq(&self, other: &Self) -> bool { + self.neg_prefix == other.neg_prefix + && self.prefix == other.prefix + && self.suffix == other.suffix + && self.neg_suffix == other.neg_suffix + && self.decimal == other.decimal + && self.grouping == other.grouping + && self.leading_zero == other.leading_zero + } +} + impl Display for NumberStyle { /// Display this number style in the format used for custom currency. /// @@ -1231,7 +1282,7 @@ impl NumberStyle { } } -#[derive(Clone, Debug, Serialize)] +#[derive(Clone, Debug, Serialize, Eq)] pub struct Affix { /// String contents of affix. pub s: String, @@ -1241,6 +1292,12 @@ pub struct Affix { pub width: usize, } +impl PartialEq for Affix { + fn eq(&self, other: &Self) -> bool { + self.s == other.s + } +} + impl From for Affix where T: Into, diff --git a/rust/pspp/src/output/pivot.rs b/rust/pspp/src/output/pivot.rs index a2cd1401a7..933ab3473f 100644 --- a/rust/pspp/src/output/pivot.rs +++ b/rust/pspp/src/output/pivot.rs @@ -338,9 +338,7 @@ impl PivotTable { /// Returns this pivot table with the specified decimal point. pub fn with_decimal(mut self, decimal: Decimal) -> Self { - if self.style.settings.decimal != decimal { - Arc::make_mut(&mut self.style.settings).decimal = decimal; - } + self.style.settings.decimal = decimal; self } @@ -586,7 +584,7 @@ impl From<&PivotTable> for ValueOptions { show_variables: value.style.show_variables, small: value.style.small, footnote_marker_type: value.style.look.footnote_marker_type, - settings: value.style.settings.clone(), + settings: value.style.settings.clone().with_leading_zero_pct(true), } } } @@ -1528,7 +1526,7 @@ pub struct PivotTableStyle { pub sizing: EnumMap>>, /// Format settings. - pub settings: Arc, + pub settings: FormatSettings, /// Numeric grouping character (usually `.` or `,`). pub grouping: Option, diff --git a/rust/pspp/src/output/pivot/testdata/template_formats_dollar.expected b/rust/pspp/src/output/pivot/testdata/template_formats_dollar.expected new file mode 100644 index 0000000000..b432a60307 --- /dev/null +++ b/rust/pspp/src/output/pivot/testdata/template_formats_dollar.expected @@ -0,0 +1,9 @@ +$0.50 +╭──┬──┬──┬──╮ +│ │a1│a2│a3│ +├──┼──┼──┼──┤ +│b1│ 0│ 1│ 2│ +│b2│ 3│ 4│ 5│ +│b3│ 6│ 7│ 8│ +╰──┴──┴──┴──╯ +No leading zero inside template: $.50 diff --git a/rust/pspp/src/output/pivot/testdata/template_formats_f.expected b/rust/pspp/src/output/pivot/testdata/template_formats_f.expected new file mode 100644 index 0000000000..e57876f4b0 --- /dev/null +++ b/rust/pspp/src/output/pivot/testdata/template_formats_f.expected @@ -0,0 +1,9 @@ +.50 +╭──┬──┬──┬──╮ +│ │a1│a2│a3│ +├──┼──┼──┼──┤ +│b1│ 0│ 1│ 2│ +│b2│ 3│ 4│ 5│ +│b3│ 6│ 7│ 8│ +╰──┴──┴──┴──╯ +No leading zero inside template: .50 diff --git a/rust/pspp/src/output/pivot/testdata/template_formats_pct.expected b/rust/pspp/src/output/pivot/testdata/template_formats_pct.expected new file mode 100644 index 0000000000..ef4b12295e --- /dev/null +++ b/rust/pspp/src/output/pivot/testdata/template_formats_pct.expected @@ -0,0 +1,9 @@ +0.5% +╭──┬──┬──┬──╮ +│ │a1│a2│a3│ +├──┼──┼──┼──┤ +│b1│ 0│ 1│ 2│ +│b2│ 3│ 4│ 5│ +│b3│ 6│ 7│ 8│ +╰──┴──┴──┴──╯ +No leading zero inside template: .5% diff --git a/rust/pspp/src/output/pivot/tests.rs b/rust/pspp/src/output/pivot/tests.rs index ad73c77a9c..d5a8993203 100644 --- a/rust/pspp/src/output/pivot/tests.rs +++ b/rust/pspp/src/output/pivot/tests.rs @@ -18,25 +18,32 @@ use std::{fmt::Display, fs::File, path::Path, sync::Arc}; use enum_map::EnumMap; -use crate::output::{ - Text, - drivers::{ - Driver, - cairo::{CairoConfig, CairoDriver}, - html::HtmlDriver, - spv::SpvDriver, - }, - pivot::{ - Axis2, Class, Dimension, Footnote, FootnoteMarkerPosition, FootnoteMarkerType, Footnotes, - Group, PivotTable, - look::{ - Area, Border, BorderStyle, Color, HeadingRegion, HorzAlign, LabelPosition, Look, - RowColBorder, Stroke, +use crate::{ + format::{DOLLAR40_2, F40_2, Format, PCT40_1}, + output::{ + Text, + drivers::{ + Driver, + cairo::{CairoConfig, CairoDriver}, + html::HtmlDriver, + spv::SpvDriver, + }, + pivot::{ + Axis2, Class, Dimension, Footnote, FootnoteMarkerPosition, FootnoteMarkerType, + Footnotes, Group, PivotTable, + look::{ + Area, Border, BorderStyle, Color, HeadingRegion, HorzAlign, LabelPosition, Look, + RowColBorder, Stroke, + }, + value::TemplateValue, }, }, }; -use super::{Axis3, value::Value}; +use super::{ + Axis3, + value::{Value, ValueInner}, +}; #[test] fn color() { @@ -430,6 +437,37 @@ fn title_and_caption() { assert_rendering("no_title_or_caption", &pivot_table); } +/// Tests a peculiarity of [Value] formatting: ordinarily, `PCT` and `DOLLAR` +/// are formatted with a leading zero (0.123 instead of .123), but it is omitted +/// if the [Value] is nested inside a template. +fn template_formats(format: Format) -> PivotTable { + let value = Value::new_number(Some(0.5)).with_format(format); + let value_in_template = Value::new(ValueInner::Template(TemplateValue { + localized: "No leading zero inside template: ^1".into(), + args: vec![vec![value.clone()]], + id: Some("id".into()), + })); + + d2("Title", [Axis3::X, Axis3::Y], None) + .with_title(value) + .with_caption(value_in_template) +} + +#[test] +fn template_formats_dollar() { + assert_rendering("template_formats_dollar", &template_formats(DOLLAR40_2)); +} + +#[test] +fn template_formats_pct() { + assert_rendering("template_formats_pct", &template_formats(PCT40_1)); +} + +#[test] +fn template_formats_f() { + assert_rendering("template_formats_f", &template_formats(F40_2)); +} + fn footnote_table(show_f0: bool) -> PivotTable { let mut footnotes = Footnotes::new(); let f0 = footnotes.push( diff --git a/rust/pspp/src/output/pivot/value.rs b/rust/pspp/src/output/pivot/value.rs index 1f9c21bdce..aad9a9543d 100644 --- a/rust/pspp/src/output/pivot/value.rs +++ b/rust/pspp/src/output/pivot/value.rs @@ -987,6 +987,11 @@ impl TemplateValue { (input, "") } + // Arguments are formatted without leading zeros for `PCT` and `DOLLAR`. + // (I don't know why.) + let mut options = display.options.clone(); + options.settings.leading_zero_pct = false; + let mut iter = self.localized.chars(); while let Some(c) = iter.next() { match c { @@ -1004,7 +1009,7 @@ impl TemplateValue { && let Some(arg) = self.args.get(index) && let Some(arg) = arg.first() { - write!(f, "{}", arg.display(&display.options))?; + write!(f, "{}", arg.display(&options))?; } iter = rest.chars(); } @@ -1023,7 +1028,7 @@ impl TemplateValue { if !a.is_empty() { (a, '%') } else { (b, '^') }; while !args.is_empty() && let n_consumed = - self.inner_template(display, f, template, escape, args)? + self.inner_template(&options, f, template, escape, args)? && n_consumed > 0 { args = &args[n_consumed..]; @@ -1040,7 +1045,7 @@ impl TemplateValue { fn inner_template<'a>( &self, - display: &DisplayValue<'a>, + options: &ValueOptions, f: &mut std::fmt::Formatter<'_>, template: &str, escape: char, @@ -1062,7 +1067,7 @@ impl TemplateValue { && let Some(arg) = args.get(index) { args_consumed = args_consumed.max(index + 1); - write!(f, "{}", arg.display(&display.options))?; + write!(f, "{}", arg.display(options))?; } } c => write!(f, "{c}")?, @@ -1323,7 +1328,7 @@ pub struct ValueOptions { pub footnote_marker_type: FootnoteMarkerType, /// Settings for formatting [Datum]s. - pub settings: Arc, + pub settings: format::Settings, } impl Default for ValueOptions { diff --git a/rust/pspp/src/settings.rs b/rust/pspp/src/settings.rs index cc424a5783..21224a757d 100644 --- a/rust/pspp/src/settings.rs +++ b/rust/pspp/src/settings.rs @@ -110,7 +110,7 @@ pub struct Settings { pub commands: Compatibility, pub global: Compatibility, pub syntax: Compatibility, - pub formats: Arc, + pub formats: FormatSettings, pub endian: EndianSettings, pub small: f64, pub show_values: Show, diff --git a/rust/pspp/src/spv/read/light.rs b/rust/pspp/src/spv/read/light.rs index 0c627fed0b..46413b3027 100644 --- a/rust/pspp/src/spv/read/light.rs +++ b/rust/pspp/src/spv/read/light.rs @@ -19,8 +19,8 @@ use enum_map::{EnumMap, enum_map}; use crate::{ data::Datum, format::{ - CC, Decimal, Decimals, Epoch, F40, F40_2, NumberStyle, Settings, Type, UncheckedFormat, - Width, + CustomCurrencies, Decimal, Decimals, Epoch, F40, F40_2, NumberStyle, Settings, Type, + UncheckedFormat, Width, }, output::pivot::{ self, Axis, Axis2, Axis3, FootnoteMarkerPosition, FootnoteMarkerType, Footnotes, Group, @@ -227,7 +227,7 @@ impl LightTable { &self.formats.column_widths, n2.map_or(&[], |x2| &x2.row_heights), ), - settings: Arc::new(Settings { + settings: Settings { epoch: self.formats.y0.epoch(), decimal: self.formats.y0.decimal(warn), leading_zero: if let Some(y1) = y1 { @@ -237,7 +237,7 @@ impl LightTable { }, leading_zero_pct: true, ccs: self.formats.custom_currency.decode(encoding, warn), - }), + }, grouping: { let grouping = self.formats.y0.grouping; b",.' ".contains(&grouping).then_some(grouping as char) @@ -1121,13 +1121,13 @@ impl CustomCurrency { &self, encoding: &'static Encoding, warn: &mut dyn FnMut(LightWarning), - ) -> EnumMap>> { - let mut ccs = EnumMap::default(); + ) -> CustomCurrencies { + let mut ccs = CustomCurrencies::default(); for (cc, string) in enum_iterator::all().zip(&self.ccs) { let string = string.decode(encoding); if !string.is_empty() { if let Ok(style) = NumberStyle::from_str(&string) { - ccs[cc] = Some(Box::new(style)); + ccs.set(cc, style); } else { warn(LightWarning::InvalidCustomCurrency(string)); }