From: Ben Pfaff <blp@cs.stanford.edu>
Date: Wed, 19 Aug 2026 00:10:02 +0000 (-0700)
Subject: wokr on cleaning up types
X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c0eec4181b341b4fd858b792fdeef98da9926754;p=pspp

wokr on cleaning up types
---

diff --git a/rust/pspp/src/spv/read/graph.rs b/rust/pspp/src/spv/read/graph.rs
index 5984568fab..88cf30c204 100644
--- a/rust/pspp/src/spv/read/graph.rs
+++ b/rust/pspp/src/spv/read/graph.rs
@@ -125,6 +125,9 @@ pub enum GraphError {
     /// Graph has no `coordinates` element.
     NoCoordinates,
 
+    /// Graph has multiple `{0}` child elements.
+    DuplicateGraphChild(&'static str),
+
     /// Graph references missing variable `{0}`.
     MissingVariable(String),
 }
@@ -384,12 +387,13 @@ impl Visualization {
         styles: &'a HashMap<String, Style>,
     ) -> Result<Vec<Element<'a>>, GraphError> {
         let elements = graph
-            .elements()
+            .elements
+            .iter()
             .map(|(raw, graphic)| {
                 Element::new(
                     raw,
                     locale,
-                    graphic,
+                    *graphic,
                     series,
                     bin_widths,
                     axis_domains,
@@ -397,11 +401,7 @@ impl Visualization {
                 )
             })
             .collect::<Result<Vec<_>, _>>()?;
-        let coordinates = graph
-            .coordinates()
-            .next()
-            .ok_or(GraphError::NoCoordinates)?;
-        let dimensions = coordinates.dimensions().collect_vec();
+        let dimensions = graph.coordinates.dimensions().collect_vec();
         for (index, axis) in [Axis3::X, Axis3::Y, Axis3::Z].into_iter().enumerate() {
             let mut domain = if let Some(dimension) = dimensions.get(index)
                 && let Some(domain_name) = &dimension.domain
@@ -667,7 +667,8 @@ impl Visualization {
 
         let facet_levels = graph.facet_levels();
         let faceting = graph
-            .faceting()
+            .faceting
+            .as_ref()
             .map(|f| f.decode(&series, &facet_levels))
             .unwrap_or_default();
 
@@ -2874,33 +2875,32 @@ impl<'a> Element<'a> {
     }
 }
 
-#[derive(Deserialize, Debug)]
-#[serde(rename_all = "camelCase")]
+#[derive(Debug)]
 struct Graph {
-    #[serde(rename = "@id")]
     id: Option<String>,
-
-    #[serde(rename = "@cellStyle")]
     cell_style: Option<String>,
-
-    #[serde(rename = "$value")]
-    children: Vec<GraphChild>,
+    locations: Vec<Location>,
+    faceting: Option<Faceting>,
+    facet_layout: Option<FacetLayout>,
+    coordinates: Coordinates,
+    elements: Vec<(RawElement, Graphic)>,
 }
 
 impl Graph {
-    fn coordinates(&self) -> impl Iterator<Item = &Coordinates> {
-        self.children.iter().filter_map(|child| match child {
-            GraphChild::Coordinates(coordinates) => Some(coordinates),
-            _ => None,
-        })
+    fn facet_levels(&self) -> HashMap<usize, &FacetLevel> {
+        self.facet_layout
+            .iter()
+            .flat_map(|facet_layout| facet_layout.facet_levels())
+            .map(|facet_level| (facet_level.level, facet_level))
+            .collect()
     }
 
     fn x_dimension(&self) -> Option<&Dimension> {
-        self.coordinates().next()?.dimensions().next()
+        self.coordinates.dimensions().next()
     }
 
     fn y_dimension(&self) -> Option<&Dimension> {
-        self.coordinates().next()?.dimensions().skip(1).next()
+        self.coordinates.dimensions().skip(1).next()
     }
 
     fn axis_dimension(&self, axis: Axis2) -> Option<&Dimension> {
@@ -2909,6 +2909,80 @@ impl Graph {
             Axis2::Y => self.y_dimension(),
         }
     }
+}
+
+impl<'de> Deserialize<'de> for Graph {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        let raw_graph = RawGraph::deserialize(deserializer)?;
+        let mut locations = Vec::new();
+        let mut faceting = None;
+        let mut facet_layout = None;
+        let mut coordinates = None;
+        let mut elements = Vec::new();
+        for child in raw_graph.children {
+            match child {
+                GraphChild::Interval(interval) => elements.push((interval, Graphic::Interval)),
+                GraphChild::Point(point) => elements.push((point, Graphic::Point)),
+                GraphChild::Line(line) => elements.push((line, Graphic::Line)),
+                GraphChild::Polygon(polygon) => elements.push((polygon, Graphic::Polygon)),
+                GraphChild::Area(area) => elements.push((area, Graphic::Area)),
+                GraphChild::Schema(schema) => elements.push((schema, Graphic::Schema)),
+                GraphChild::Edge(edge) => elements.push((edge, Graphic::Edge)),
+                GraphChild::Path(path) => elements.push((path, Graphic::Path)),
+                GraphChild::Location(location) => locations.push(location),
+                GraphChild::Faceting(value) => {
+                    if faceting.replace(value).is_some() {
+                        return Err(D::Error::duplicate_field("faceting"));
+                    }
+                }
+                GraphChild::FacetLayout(value) => {
+                    if facet_layout.replace(value).is_some() {
+                        return Err(D::Error::duplicate_field("facetLayout"));
+                    }
+                }
+                GraphChild::Coordinates(value) => {
+                    if coordinates.replace(value).is_some() {
+                        return Err(D::Error::duplicate_field("coordinates"));
+                    }
+                }
+                GraphChild::LineGuide => (),
+            };
+        }
+        Ok(Self {
+            id: raw_graph.id,
+            cell_style: raw_graph.cell_style,
+            locations,
+            faceting,
+            facet_layout,
+            coordinates: coordinates.ok_or_else(|| D::Error::missing_field("coordinates"))?,
+            elements,
+        })
+    }
+}
+
+#[derive(Deserialize, Debug)]
+#[serde(rename_all = "camelCase")]
+struct RawGraph {
+    #[serde(rename = "@id")]
+    id: Option<String>,
+
+    #[serde(rename = "@cellStyle")]
+    cell_style: Option<String>,
+
+    #[serde(rename = "$value")]
+    children: Vec<GraphChild>,
+}
+
+impl RawGraph {
+    fn coordinates(&self) -> impl Iterator<Item = &Coordinates> {
+        self.children.iter().filter_map(|child| match child {
+            GraphChild::Coordinates(coordinates) => Some(coordinates),
+            _ => None,
+        })
+    }
 
     fn elements(&self) -> impl Iterator<Item = (&RawElement, Graphic)> {
         self.children.iter().filter_map(|child| child.element())
@@ -2917,18 +2991,9 @@ impl Graph {
     fn faceting(&self) -> Option<&Faceting> {
         self.children.iter().find_map(|child| child.faceting())
     }
-
-    fn facet_levels(&self) -> HashMap<usize, &FacetLevel> {
-        self.children
-            .iter()
-            .flat_map(|child| child.facet_layout())
-            .flat_map(|facet_layout| facet_layout.facet_levels())
-            .map(|facet_level| (facet_level.level, facet_level))
-            .collect()
-    }
 }
 
-impl Locations for Graph {
+impl Locations for RawGraph {
     fn locations(&self) -> Vec<&Location> {
         self.children
             .iter()
@@ -4303,7 +4368,7 @@ impl<'a> FrameType<'a> {
         match self {
             FrameType::Visualization(visualization) => visualization.locations(),
             FrameType::Container(container) => container.locations(),
-            FrameType::Graph(graph) => graph.locations(),
+            FrameType::Graph(graph) => graph.locations.iter().collect(),
             FrameType::LabelFrame(label_frame) => label_frame.locations(),
             FrameType::Legend(legend) => legend.locations(),
         }