From: Ben Pfaff <blp@cs.stanford.edu>
Date: Fri, 2 Jan 2026 03:57:31 +0000 (-0800)
Subject: cleanup
X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c8abd87fe3c8323c26c0f478f3d55474c446bfbb;p=pspp

cleanup
---

diff --git a/rust/pspp/src/spv/read/legacy_xml.rs b/rust/pspp/src/spv/read/legacy_xml.rs
index 99fd8f6be2..5f742f8c8d 100644
--- a/rust/pspp/src/spv/read/legacy_xml.rs
+++ b/rust/pspp/src/spv/read/legacy_xml.rs
@@ -14,6 +14,7 @@
 // You should have received a copy of the GNU General Public License along with
 // this program.  If not, see <http://www.gnu.org/licenses/>.
 
+#![warn(dead_code)]
 use std::{
     cell::{Cell, RefCell},
     collections::{BTreeMap, HashMap},
@@ -38,8 +39,8 @@ use crate::{
     output::pivot::{
         self, Axis2, Axis3, Category, CategoryLocator, Dimension, Group, Leaf, Length, PivotTable,
         look::{
-            self, Area, AreaStyle, BorderStyle, BoxBorder, CellStyle, Color, HeadingRegion,
-            HorzAlign, Look, RowParity, Stroke, VertAlign,
+            self, Area, AreaStyle, CellStyle, Color, HeadingRegion, HorzAlign, Look, RowParity,
+            Stroke, VertAlign,
         },
         value::Value,
     },
@@ -194,23 +195,33 @@ pub enum LegacyXmlWarning {
         /// Value expression.
         value: String,
     },
+
+    /// Unsupported applyToConverse.
+    UnsupportedApplyToConverse,
 }
 
 #[derive(Deserialize, Debug)]
 #[serde(rename_all = "camelCase")]
 pub struct Visualization {
     /// In format `YYYY-MM-DD`.
+    // XXX parse this
     #[serde(rename = "@date")]
-    date: String,
+    _date: String,
+
     // Locale used for output, e.g. `en-US`.
     #[serde(rename = "@lang")]
     lang: String,
+
     /// Localized title of the pivot table.
+    ///
+    /// This is repeated in the table's labels, and that form supports
+    /// footnotes, so we don't read this one.
     #[serde(rename = "@name")]
-    name: String,
+    _name: String,
+
     /// Base style for the pivot table.
     #[serde(rename = "@style")]
-    style: Ref<Style>,
+    _style: Ref<Style>,
 
     #[serde(rename = "$value")]
     children: Vec<VisChild>,
@@ -769,28 +780,17 @@ impl Visualization {
                 fn decode(
                     &self,
                     intersect: &Intersect,
-                    look: &mut Look,
+                    look: &Look,
                     series: &BTreeMap<&str, Series>,
                     dims: &mut [Dim],
                     data: &mut HashMap<Vec<usize>, Value>,
                     footnotes: &pivot::Footnotes,
                     has_cell_footnotes: bool,
                 ) {
-                    let mut wheres = Vec::new();
-                    for child in &intersect.children {
-                        match child {
-                            IntersectChild::Where(w) => wheres.push(w),
-                            IntersectChild::IntersectWhere(_) => {
-                                // Presumably we should do something (but we don't).
-                            }
-                            IntersectChild::Alternating | IntersectChild::Empty => (),
-                        }
-                    }
-
                     match self.target_type {
                         TargetType::MajorTicks => {
                             // Formatting for individual row or column labels.
-                            for w in &wheres {
+                            for w in intersect.wheres() {
                                 let Some(s) = series.get(w.variable.as_str()) else {
                                     continue;
                                 };
@@ -825,7 +825,7 @@ impl Visualization {
                             // Formatting for individual cells or groups of them
                             // with some dimensions in common.
                             let mut include = vec![HashSet::new(); dims.len()];
-                            for w in &wheres {
+                            for w in intersect.wheres() {
                                 let Some(s) = series.get(w.variable.as_str()) else {
                                     continue;
                                 };
@@ -879,26 +879,19 @@ impl Visualization {
                     }
                 }
             }
-            //let mut targets = scp.sets.iter().filter_map(|set| set.as_set_format());
-            let mut targets = Vec::new();
-            for set in &scp.sets {
-                match set {
-                    Set::SetFormat(sf) => {
-                        if let Some(target_type) =
-                            TargetType::from_id(&sf.target, graph, &major_ticks)
-                        {
-                            targets.push(Target { sf, target_type });
-                        }
-                    }
-                    Set::Other => (),
-                }
-            }
 
-            match (
-                scp.union_.as_ref(),
-                scp.apply_to_converse.unwrap_or_default(),
-            ) {
-                (Some(union_), false) => {
+            let targets = scp
+                .sets
+                .iter()
+                .filter_map(|set| set.as_set_format())
+                .filter_map(|sf| {
+                    TargetType::from_id(&sf.target, graph, &major_ticks)
+                        .map(|target_type| Target { sf, target_type })
+                })
+                .collect::<Vec<_>>();
+
+            if let Some(union_) = &scp.union_ {
+                if !scp.apply_to_converse {
                     for intersect in &union_.intersects {
                         for target in &targets {
                             target.decode(
@@ -912,30 +905,26 @@ impl Visualization {
                             );
                         }
                     }
+                } else {
+                    // Not seen in the corpus.
+                    warn(LegacyXmlWarning::UnsupportedApplyToConverse);
                 }
-                (Some(_), true) => {
-                    // Not implemented, not seen in the corpus.
-                }
-                (None, true) => {
-                    for target in &targets {
-                        if target.target_type == TargetType::Labeling {
-                            for value in data.values_mut() {
-                                Style::apply_to_value(
-                                    value,
-                                    Some(&target.sf),
-                                    None,
-                                    None,
-                                    &look.areas[Area::Data(RowParity::Even)],
-                                    &footnotes,
-                                    cell_footnotes.is_some(),
-                                );
-                            }
+            } else if scp.apply_to_converse {
+                for target in &targets {
+                    if target.target_type == TargetType::Labeling {
+                        for value in data.values_mut() {
+                            Style::apply_to_value(
+                                value,
+                                Some(&target.sf),
+                                None,
+                                None,
+                                &look.areas[Area::Data(RowParity::Even)],
+                                &footnotes,
+                                cell_footnotes.is_some(),
+                            );
                         }
                     }
                 }
-                (None, false) => {
-                    // Appears to be used to set the font for something—but what?
-                }
             }
         }
 
@@ -965,8 +954,6 @@ impl Visualization {
 pub struct Series {
     name: String,
     label: Option<String>,
-    format: crate::format::Format,
-    remapped: bool,
     values: Vec<DataValue>,
     map: Map,
     affixes: Vec<Affix>,
@@ -987,8 +974,6 @@ impl Series {
         Self {
             name,
             label: None,
-            format: F8_0,
-            remapped: false,
             values,
             map,
             affixes: Vec::new(),
@@ -996,9 +981,6 @@ impl Series {
             dimension_index: Default::default(),
         }
     }
-    fn with_format(self, format: crate::format::Format) -> Self {
-        Self { format, ..self }
-    }
     fn with_label(self, label: Option<String>) -> Self {
         Self { label, ..self }
     }
@@ -1061,8 +1043,6 @@ struct SourceVariable {
     #[serde(rename = "@labelVariable")]
     label_variable: Option<Ref<SourceVariable>>,
 
-    #[serde(default, rename = "extension")]
-    extensions: Vec<VariableExtension>,
     format: Option<Format>,
     string_format: Option<StringFormat>,
 }
@@ -1103,7 +1083,6 @@ impl SourceVariable {
         series.insert(
             &self.id,
             Series::new(self.id.clone(), data, map)
-                .with_format(format)
                 .with_affixes(affixes)
                 .with_label(self.label.clone()),
         );
@@ -1120,14 +1099,10 @@ struct DerivedVariable {
     /// An expression that defines the variable's value.
     #[serde(rename = "@value")]
     value: String,
-    #[serde(default, rename = "extension")]
-    extensions: Vec<VariableExtension>,
     format: Option<Format>,
     string_format: Option<StringFormat>,
     #[serde(default, rename = "valueMapEntry")]
     value_map: Vec<ValueMapEntry>,
-    #[serde(rename = "@dependsOn")]
-    depends_on: Option<String>,
 }
 
 impl DerivedVariable {
@@ -1197,23 +1172,6 @@ impl DerivedVariable {
     }
 }
 
-#[derive(Deserialize, Debug)]
-#[serde(rename = "extension", rename_all = "camelCase")]
-struct VariableExtension {
-    #[serde(rename = "@from", default)]
-    from: String,
-}
-
-#[derive(Deserialize, Debug)]
-#[serde(rename_all = "camelCase")]
-struct UserSource {
-    #[serde(rename = "@id")]
-    id: String,
-
-    #[serde(rename = "@missing")]
-    missing: Option<Missing>,
-}
-
 #[derive(Deserialize, Debug)]
 #[serde(rename_all = "camelCase")]
 struct VariableReference {
@@ -1221,13 +1179,6 @@ struct VariableReference {
     reference: String,
 }
 
-#[derive(Copy, Clone, Debug, PartialEq, Eq, Deserialize)]
-#[serde(rename_all = "camelCase")]
-enum Missing {
-    Listwise,
-    Pairwise,
-}
-
 #[derive(Deserialize, Debug)]
 #[serde(rename_all = "camelCase")]
 struct StringFormat {
@@ -1821,18 +1772,6 @@ struct Style {
 }
 
 impl Style {
-    fn border(&self, which: BoxBorder) -> Option<BorderStyle> {
-        let (stroke, color) = match which {
-            BoxBorder::Left => (self.border_left, self.border_left_color),
-            BoxBorder::Top => (self.border_top, self.border_top_color),
-            BoxBorder::Right => (self.border_right, self.border_right_color),
-            BoxBorder::Bottom => (self.border_bottom, self.border_bottom_color),
-        };
-        stroke
-            .zip(color)
-            .map(|(stroke, color)| BorderStyle { stroke, color })
-    }
-
     fn apply_to_value(
         value: &mut Value,
         sf: Option<&SetFormat>,
@@ -1966,10 +1905,6 @@ impl Style {
         }
         updated
     }
-
-    fn decode_area(fg: Option<&Style>, bg: Option<&Style>, out: &mut AreaStyle) {
-        Self::decode(fg, bg, &mut out.cell_style, &mut out.font_style);
-    }
 }
 
 #[derive(Copy, Clone, Debug, PartialEq, Eq, Deserialize)]
@@ -2056,81 +1991,17 @@ impl From<LabelLocation> for VertAlign {
 #[derive(Deserialize, Debug)]
 #[serde(rename_all = "camelCase")]
 struct Graph {
-    #[serde(rename = "@id")]
-    id: Option<String>,
-
     #[serde(rename = "@cellStyle")]
     cell_style: Ref<Style>,
 
-    #[serde(rename = "@style")]
-    style: Ref<Style>,
-
-    #[serde(rename = "location")]
-    locations: Vec<Location>,
-    coordinates: Coordinates,
     faceting: Faceting,
     facet_layout: FacetLayout,
     interval: Interval,
 }
 
-#[derive(Deserialize, Debug)]
-#[serde(rename_all = "camelCase")]
-struct Coordinates;
-
-#[derive(Deserialize, Debug)]
-#[serde(rename_all = "camelCase")]
-struct Location {
-    /// The part of the table being located.
-    #[serde(rename = "@part")]
-    part: Part,
-
-    /// How the location is determined.
-    #[serde(rename = "@method")]
-    method: Method,
-
-    /// Minimum size.
-    #[serde(rename = "@min")]
-    min: Option<Length>,
-
-    /// Maximum size.
-    #[serde(rename = "@max")]
-    max: Option<Length>,
-
-    /// An element to attach to. Required when method is attach or same, not
-    /// observed otherwise.
-    #[serde(rename = "@target")]
-    target: Option<String>,
-
-    #[serde(rename = "@value")]
-    value: Option<String>,
-}
-
-#[derive(Copy, Clone, Debug, PartialEq, Eq, Deserialize)]
-#[serde(rename_all = "camelCase")]
-enum Part {
-    Height,
-    Width,
-    Top,
-    Bottom,
-    Left,
-    Right,
-}
-
-#[derive(Copy, Clone, Debug, PartialEq, Eq, Deserialize)]
-#[serde(rename_all = "camelCase")]
-enum Method {
-    SizeToContent,
-    Attach,
-    Fixed,
-    Same,
-}
-
 #[derive(Deserialize, Debug)]
 #[serde(rename_all = "camelCase")]
 struct Faceting {
-    #[serde(rename = "@id")]
-    id: Option<String>,
-
     #[serde(default, rename = "$value")]
     children: Vec<FacetingChild>,
 }
@@ -2216,18 +2087,11 @@ struct Layer {
 
     #[serde(rename = "@value")]
     value: String,
-
-    #[serde(rename = "@visible")]
-    visible: Option<bool>,
-
-    #[serde(rename = "@titleVisible")]
-    title_visible: Option<bool>,
 }
 
 #[derive(Deserialize, Debug)]
 #[serde(rename_all = "camelCase")]
 struct FacetLayout {
-    table_layout: TableLayout,
     #[serde(rename = "$value")]
     children: Vec<FacetLayoutChild>,
 }
@@ -2237,26 +2101,15 @@ struct FacetLayout {
 enum FacetLayoutChild {
     SetCellProperties(SetCellProperties),
     FacetLevel(FacetLevel),
-}
-
-#[derive(Deserialize, Debug)]
-#[serde(rename_all = "camelCase")]
-struct TableLayout {
-    #[serde(rename = "@verticalTitlesInCorner")]
-    vertical_titles_in_corner: bool,
-
-    #[serde(rename = "@style")]
-    style: Option<Ref<Style>>,
+    #[serde(other)]
+    Other,
 }
 
 #[derive(Deserialize, Debug)]
 #[serde(rename_all = "camelCase")]
 struct SetCellProperties {
-    #[serde(rename = "@id")]
-    id: Option<String>,
-
-    #[serde(rename = "@applyToConverse")]
-    apply_to_converse: Option<bool>,
+    #[serde(rename = "@applyToConverse", default)]
+    apply_to_converse: bool,
 
     #[serde(rename = "$value")]
     sets: Vec<Set>,
@@ -2279,14 +2132,21 @@ struct Intersect {
     children: Vec<IntersectChild>,
 }
 
+impl Intersect {
+    fn wheres(&self) -> impl Iterator<Item = &Where> {
+        self.children.iter().filter_map(|child| match child {
+            IntersectChild::Where(w) => Some(w),
+            IntersectChild::Other => None,
+        })
+    }
+}
+
 #[derive(Deserialize, Debug)]
 #[serde(rename_all = "camelCase")]
 enum IntersectChild {
     Where(Where),
-    IntersectWhere(IntersectWhere),
-    Alternating,
     #[serde(other)]
-    Empty,
+    Other,
 }
 
 #[derive(Deserialize, Debug)]
@@ -2298,16 +2158,6 @@ struct Where {
     include: String,
 }
 
-#[derive(Deserialize, Debug)]
-#[serde(rename_all = "camelCase")]
-struct IntersectWhere {
-    #[serde(rename = "@variable")]
-    variable: String,
-
-    #[serde(rename = "@variable2")]
-    variable2: String,
-}
-
 #[derive(Deserialize, Debug)]
 #[serde(rename_all = "camelCase")]
 enum Set {
@@ -2615,12 +2465,6 @@ impl Label {
             LabelChild::DescriptionGroup(_) => &[],
         }
     }
-
-    fn decode_style(&self, area_style: &mut AreaStyle, styles: &HashMap<&str, &Style>) {
-        let fg = self.style.get(styles);
-        let bg = self.text_frame_style.as_ref().and_then(|r| r.get(styles));
-        Style::decode_area(fg, bg, area_style);
-    }
 }
 #[derive(Copy, Clone, Debug, PartialEq, Eq, Deserialize, Enum)]
 #[serde(rename_all = "camelCase")]
@@ -2701,9 +2545,6 @@ struct LabelFrame {
     #[serde(rename = "@style")]
     style: Option<Ref<Style>>,
 
-    #[serde(rename = "location")]
-    locations: Vec<Location>,
-
     label: Option<Label>,
     paragraph: Option<Paragraph>,
 }
@@ -2751,8 +2592,6 @@ struct Container {
 
     #[serde(default, rename = "extension")]
     extensions: Option<ContainerExtension>,
-    #[serde(default)]
-    locations: Vec<Location>,
     #[serde(rename = "labelFrame")]
     #[serde(default)]
     label_frames: Vec<LabelFrame>,