From 2ea16f45415d9fe962fea77b2b2a2b2184603338 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Tue, 6 Oct 2026 17:47:24 -0400 Subject: [PATCH] =?UTF-8?q?Panics=20=C2=B7=20P14=20Tables:=20pivots,=20gro?= =?UTF-8?q?upings=20and=20merges=20that=20name=20what=20their=20data=20los?= =?UTF-8?q?t=20log=20instead=20of=20panicking?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/tables/pivot.rs | 25 +++++++++++++++++-------- src/tables/sheets.rs | 8 ++++---- src/tables/table.rs | 12 +++++++----- src/tables/tests/mod.rs | 1 + src/tables/tests/stale.rs | 31 +++++++++++++++++++++++++++++++ tasks/panics.md | 3 ++- 6 files changed, 62 insertions(+), 18 deletions(-) create mode 100644 src/tables/tests/stale.rs diff --git a/src/tables/pivot.rs b/src/tables/pivot.rs index dda19b6..ff88080 100644 --- a/src/tables/pivot.rs +++ b/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| { figure.map_or("—".to_string(), |figure| { @@ -205,6 +213,7 @@ impl RenderOnce for PivotTable { .child(head) .children(body) .child(totals) + .into_any_element() } } diff --git a/src/tables/sheets.rs b/src/tables/sheets.rs index 58cb5ea..6715708 100644 --- a/src/tables/sheets.rs +++ b/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 } diff --git a/src/tables/table.rs b/src/tables/table.rs index 36dc5c0..ccf4a68 100644 --- a/src/tables/table.rs +++ b/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); diff --git a/src/tables/tests/mod.rs b/src/tables/tests/mod.rs index 7793f78..0fff119 100644 --- a/src/tables/tests/mod.rs +++ b/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::{ diff --git a/src/tables/tests/stale.rs b/src/tables/tests/stale.rs new file mode 100644 index 0000000..ad8d7aa --- /dev/null +++ b/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) -> 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(); +} diff --git a/tasks/panics.md b/tasks/panics.md index 83b3521..3f33ed8 100644 --- a/tasks/panics.md +++ b/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.