Panics · P14 Tables: pivots, groupings and merges that name what their data lost log instead of panicking

src/tables/pivot.rs

@@ -130,16 +130,24 @@ impl PivotTable {
 
 impl RenderOnce for PivotTable {
     fn render(self, _: &mut Window, cx: &mut App) -> impl IntoElement {
-        let field = |name: &SharedString| {
-            self.fields
-                .iter()
-                .position(|known| known == name)
-                .unwrap_or_else(|| panic!("no field {name} to pivot by"))
-        };
-        let (down, across, value) = (field(&self.by.0), field(&self.by.1), field(&self.by.2));
-        let table = pivot(&self.records, down, across, value, self.how);
+        let field = |name: &SharedString| self.fields.iter().position(|known| known == name);
         let theme = cx.theme();
         let colors = &theme.colors;
+        let (Some(down), Some(across), Some(value)) =
+            (field(&self.by.0), field(&self.by.1), field(&self.by.2))
+        else {
+            let (down, across, value) = &self.by;
+            log::error!(
+                "pivot table: {down}, {across} or {value} is not among {:?}",
+                self.fields
+            );
+            return div()
+                .text_size(theme.text_size(TextSize::Sm))
+                .text_color(colors.danger)
+                .child(format!("No field to pivot by {down}, {across} and {value}"))
+                .into_any_element();
+        };
+        let table = pivot(&self.records, down, across, value, self.how);
         let decimals = self.decimals;
         let reads = |figure: Option<f64>| {
             figure.map_or("—".to_string(), |figure| {
@@ -205,6 +213,7 @@ impl RenderOnce for PivotTable {
             .child(head)
             .children(body)
             .child(totals)
+            .into_any_element()
     }
 }
 

src/tables/sheets.rs

@@ -113,10 +113,10 @@ impl Spreadsheet {
             from.0 <= to.0 && from.1 <= to.1,
             "a merge runs from its top-left to its bottom-right"
         );
-        assert!(
-            to.0 < self.size.0 && to.1 < self.size.1,
-            "a merge stays inside the sheet"
-        );
+        if to.0 >= self.size.0 || to.1 >= self.size.1 {
+            log::error!("sheet: a merge to {to:?} leaves {:?}; skipped", self.size);
+            return self;
+        }
         self.merges.push(Merge { from, to });
         self
     }

src/tables/table.rs

@@ -191,11 +191,13 @@ impl RenderOnce for DataTable {
                 lines.push(Line::Detail { ix });
             }
         };
-        match self
-            .group_by
-            .as_ref()
-            .map(|key| index(key).unwrap_or_else(|| panic!("no column {key} to group by")))
-        {
+        match self.group_by.as_ref().and_then(|key| {
+            let col = index(key);
+            if col.is_none() {
+                log::error!("data table {id:?}: no column {key} to group by; ungrouped");
+            }
+            col
+        }) {
             Some(col) => {
                 for (name, members) in groups(&rows, &order, col) {
                     let shut = folded.contains(&name);

src/tables/tests/mod.rs

@@ -11,6 +11,7 @@ use gpui::{
 
 mod clipboard;
 mod columns;
+mod stale;
 
 use super::{Column, DataTable, FilterBuilder, FilterRule, Row};
 use crate::{

src/tables/tests/stale.rs

@@ -0,0 +1,31 @@
+use gpui::{Context, IntoElement, ParentElement, Render, Styled, TestAppContext, Window, div, px};
+
+use crate::{
+    tables::{Aggregate, Column, DataTable, PivotTable, Row, Spreadsheet},
+    theme::Theme,
+};
+
+/// A pivot, a grouping and a merge that name what their data no longer holds.
+struct Stale;
+
+impl Render for Stale {
+    fn render(&mut self, _: &mut Window, _: &mut Context<Self>) -> impl IntoElement {
+        let pivot = PivotTable::new(["region", "sales"], [vec!["Europe".into(), 4.0.into()]])
+            .pivot("region", "quarter", "sales", Aggregate::Sum);
+        let table = DataTable::new("table", [Column::new("name", "Name")])
+            .rows(vec![Row::new("a", ["Ann".into()])])
+            .group_by("region");
+        let sheet = Spreadsheet::new("sheet", 2, 2)
+            .merge((0, 0), (5, 1))
+            .w(px(300.0))
+            .h(px(120.0));
+        div().w(px(400.0)).child(pivot).child(table).child(sheet)
+    }
+}
+
+#[gpui::test]
+fn a_pivot_grouping_or_merge_past_its_data_draws(cx: &mut TestAppContext) {
+    cx.update(Theme::init);
+    let (_, cx) = cx.add_window_view(|_, _| Stale);
+    cx.run_until_parked();
+}

tasks/panics.md

@@ -28,7 +28,7 @@ Each item reads every site in its folders, fixes what the rules name, updates ca
 - [x] P11 Loading & Motion — `src/motion`
 - [x] P12 Data Display — `src/data_display`
 - [x] P13 Lists & Trees — `src/lists`
-- [ ] P14 Tables — `src/tables`
+- [x] P14 Tables — `src/tables`
 - [ ] P15 Charts — `src/charts`
 - [ ] P16 Finance — `src/finance`
 - [ ] P17 Code Editor — `src/editor`
@@ -78,3 +78,4 @@ One line per item: what changed, what stayed and why, review rounds.
 - P11: a buffer at or behind the value draws none; `slide` was fixed in P05. Kept: values in 0..=1, segment and skeleton counts (one value each), builder misuse, reorder's permutation. Review: 1 round.
 - P12: a featured column past the columns features none; a gauge past its scale pegs; usage parts past the total fill it and repeat hues past eight; stars above max log and fill. Kept: one-value contracts, a comparison row's shape, construction. Review: 3 rounds (round 2 met an edit that had not applied).
 - P13: a directory listing's selected name gone from its entries picks none. Kept: one-value contracts; activation, folder split, single-select sections and tree rows hold by construction. Review: 1 round.
+- P14: a pivot field missing from its data draws an error line; a gone group-by column leaves the table ungrouped; a merge past the sheet is skipped; each logs. Kept: row shapes of one snapshot, one-value contracts, builder misuse, construction. Review: 1 round.