From 985257b1662bd9d0668363e994be4c5d15e69ddf Mon Sep 17 00:00:00 2001 From: nihilist-narwhal <240798405+nihilist-narwhal@users.noreply.github.com> Date: Sat, 10 Oct 2026 05:30:49 -0500 Subject: [PATCH 1/2] Tables: typing in the header row renames the column Editing a table's header cell changed only the cell: the column kept its old name, so =SUM(Table1[a]) still worked after the header read "zz", and saving wrote the old names back into the header row, undoing the edit on reopening. As in Excel, the column now takes the new header; an emptied header becomes ColumnN and a repeated one gets a number ("a2"), written back into the cell, and structured references follow the new name. --- crates/engine/src/cmd/mod.rs | 6 +- crates/engine/src/tables.rs | 132 ++++++++++++++++++++++++++++++++++- 2 files changed, 136 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 397a05cbd..14f7d729e 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -213,7 +213,11 @@ pub struct Ctx<'a> { pub(crate) fn edit(s: &mut Session, f: impl FnOnce(&mut Ctx) -> Result) -> Result { let d = s.doc_mut()?; - commit(d, f) + commit(d, |cx| { + let r = f(cx)?; + crate::tables::sync_headers(cx); + Ok(r) + }) } pub(crate) fn commit(d: &mut DocState, f: impl FnOnce(&mut Ctx) -> Result) -> Result { diff --git a/crates/engine/src/tables.rs b/crates/engine/src/tables.rs index 62cb59c9e..8036aa103 100644 --- a/crates/engine/src/tables.rs +++ b/crates/engine/src/tables.rs @@ -1,7 +1,7 @@ //! Table styles: how a cell inside a table looks (our own palettes, keyed by the familiar //! `TableStyleLight1…21 / Medium1…28 / Dark1…11` names so files round-trip). -use gridcraft_core::CellRef; +use gridcraft_core::{CellRef, RangeRef}; use gridcraft_model::*; /// (family, number) from a style name, e.g. `TableStyleMedium2` → ("Medium", 2). @@ -147,3 +147,133 @@ pub fn gallery() -> Vec { } v } + +/// Editing a table's header row renames the column, as in Excel: an empty or repeated header +/// gets a unique name written back into the cell, and structured references follow the rename. +pub(crate) fn sync_headers(cx: &mut crate::cmd::Ctx) { + if cx.wb.sheets.iter().all(|s| s.tables.is_empty()) { + return; + } + // (sheet, table name, table range, old column name, new column name) + let mut renames: Vec<(usize, String, RangeRef, String, String)> = Vec::new(); + let heads: Vec<(usize, RangeRef)> = (0..cx.wb.sheets.len()) + .flat_map(|si| cx.wb.sheet(si).into_iter().flat_map(move |sh| sh.tables.iter().filter(|t| t.header_row).map(move |t| (si, t.range)))) + .map(|(si, r)| (si, RangeRef::new(r.start, CellRef::new(r.start.row, r.end.col)))) + .collect(); + // A structural edit can move anything into a header row: then every header is checked. + let changed: std::collections::BTreeSet<(usize, CellRef)> = if cx.structural { + heads.iter().flat_map(|(s, r)| r.iter().map(move |c| (*s, c))).collect() + } else { + cx.changed.iter().filter(|(si, c)| heads.iter().any(|(s, r)| s == si && r.contains(*c))).copied().collect() + }; + // Per table: lower-case column name → column, and the next number to try for a repeated name + // (both kept up to date, so a wide header row stays linear). + type Names = (std::collections::HashMap, std::collections::HashMap); + let mut names: std::collections::HashMap<(usize, usize), Names> = std::collections::HashMap::new(); + for (si, c) in changed { + let Some(sh) = cx.wb.sheet(si) else { continue }; + let Some(ti) = sh.tables.iter().position(|t| t.header_row && c.row == t.range.start.row && t.range.contains(c)) else { continue }; + let Some(t) = sh.tables.get(ti) else { continue }; + let col = (c.col - t.range.start.col) as usize; + let Some(old) = t.columns.get(col).map(|x| x.name.clone()) else { continue }; + let text = crate::display::cell_text(&cx.wb, sh, c); + let is_text = sh.value(c).is_text(); + if text == old && is_text { + continue; + } + let (taken, next) = names + .entry((si, ti)) + .or_insert_with(|| (t.columns.iter().enumerate().map(|(i, x)| (x.name.to_lowercase(), i)).collect(), Default::default())); + if taken.get(&old.to_lowercase()) == Some(&col) { + taken.remove(&old.to_lowercase()); + } + let base = if text.trim().is_empty() { format!("Column{}", col + 1) } else { text.clone() }; + let k = next.entry(base.to_lowercase()).or_insert(2); + let mut name = base.clone(); + while taken.contains_key(&name.to_lowercase()) { + name = format!("{base}{k}"); + *k += 1; + } + taken.insert(name.to_lowercase(), col); + let (table, range) = (t.name.clone(), t.range); + let Some(sh) = cx.wb.sheet_mut(si) else { continue }; + if let Some(x) = sh.tables.get_mut(ti).and_then(|t| t.columns.get_mut(col)) { + x.name = name.clone(); + } + // Headers are text: a number typed there, or a generated name, is written back as text. + if name != text || !is_text { + let style = sh.style_id(c); + sh.cells.set(c, Cell { value: gridcraft_core::Value::text(name.as_str()), style, ..Default::default() }); + } + if !old.eq_ignore_ascii_case(&name) { + renames.push((si, table, range, old, name)); + } + } + if renames.is_empty() { + return; + } + // Structured references follow the new names. + for i in 0..cx.wb.sheets.len() { + let formulas: Vec<(CellRef, std::sync::Arc)> = + cx.wb.sheets.get(i).map(|s| s.cells.iter().filter_map(|(c, x)| x.formula.clone().map(|f| (c, f))).collect()).unwrap_or_default(); + for (c, f) in formulas { + let Some(e) = f.expr() else { continue }; + let mut hit = false; + let ne = e.map(&mut |x| match x { + gridcraft_formula::Expr::Struct(mut st) => { + for (si, table, range, old, name) in &renames { + let this = if st.table.is_empty() { i == *si && range.contains(c) } else { st.table.eq_ignore_ascii_case(table) }; + if this { + for end in [&mut st.col_start, &mut st.col_end].into_iter().flatten() { + if end.eq_ignore_ascii_case(old) { + *end = name.clone(); + hit = true; + } + } + } + } + gridcraft_formula::Expr::Struct(st) + } + o => o, + }); + if hit && let Some(cell) = cx.wb.sheet_mut(i).and_then(|s| s.cells.get_mut(c)) { + cell.formula = Some(std::sync::Arc::new(Formula::from_expr(ne))); + } + } + } + cx.structural = true; +} + +#[cfg(test)] +mod tests { + use gridcraft_core::Value; + use serde_json::json; + + use crate::Session; + + #[test] + fn editing_a_header_renames_the_column() { + let mut s = Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A1", "values": [["a", "b", "c"], [1, 2, 3]]})).unwrap(); + s.execute("insert.table", json!({"range": "A1:C2", "header": true})).unwrap(); + s.execute("cell.set", json!({"cell": "E1", "input": "=SUM(Table1[a])"})).unwrap(); + s.execute("cell.set", json!({"cell": "A1", "input": "zz"})).unwrap(); + s.execute("edit.clearContents", json!({"range": "B1"})).unwrap(); + s.execute("cell.set", json!({"cell": "C1", "input": "ZZ"})).unwrap(); + let get = |s: &mut Session, a: &str| s.execute("cell.get", json!({"cell": a})).unwrap(); + assert_eq!(get(&mut s, "E1")["formula"], "=SUM(Table1[zz])"); + assert_eq!(get(&mut s, "E1")["value"], json!(1.0)); + let d = s.doc().unwrap(); + let sh = d.wb.sheet(0).unwrap(); + let names: Vec<&str> = sh.tables[0].columns.iter().map(|c| c.name.as_str()).collect(); + assert_eq!(names, ["zz", "Column2", "ZZ2"]); + assert_eq!(sh.value(gridcraft_core::CellRef::new(0, 1)), Value::from("Column2")); + assert_eq!(sh.value(gridcraft_core::CellRef::new(0, 2)), Value::from("ZZ2")); + s.execute("range.setValues", json!({"range": "A1", "values": [["x", "x", 5]]})).unwrap(); + let sh = s.doc().unwrap().wb.sheet(0).unwrap(); + let names: Vec<&str> = sh.tables[0].columns.iter().map(|c| c.name.as_str()).collect(); + assert_eq!(names, ["x", "x2", "5"]); + assert_eq!(sh.value(gridcraft_core::CellRef::new(0, 2)), Value::from("5")); + } +} From 801119182436dca51b6a76cb2ed33171235ef9b1 Mon Sep 17 00:00:00 2001 From: nihilist-narwhal <240798405+nihilist-narwhal@users.noreply.github.com> Date: Sat, 10 Oct 2026 05:30:49 -0500 Subject: [PATCH 2/2] Go To: a table name selects the table's data Go To (and the Name Box) with a table's name selected the whole table, header and totals rows included. In Excel a table's name stands for its data rows, as in formulas (=SUM(Table1) adds only the data). --- crates/engine/src/cmd/edit.rs | 3 ++- crates/engine/src/tables.rs | 9 +++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index cffe15430..b68507515 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -478,7 +478,8 @@ fn go_to(s: &mut Session, p: &Json) -> Result { if let Some(n) = d.wb.name(reference, sheet) { n.formula.clone() } else if let Some((si, ti)) = d.wb.table(reference) { - let t = d.wb.sheet(si).and_then(|sh| sh.tables.get(ti)).map(|t| t.range.a1()).unwrap_or_default(); + // As in Excel, a table's name stands for its data, without the header and totals rows. + let t = d.wb.sheet(si).and_then(|sh| sh.tables.get(ti)).map(|t| t.data_range().unwrap_or(t.range).a1()).unwrap_or_default(); format!("'{}'!{}", d.wb.sheet(si).map(|s| s.name.clone()).unwrap_or_default(), t) } else { return Err(EngineError::Other("Reference isn't valid.".into())); diff --git a/crates/engine/src/tables.rs b/crates/engine/src/tables.rs index 8036aa103..4036555a4 100644 --- a/crates/engine/src/tables.rs +++ b/crates/engine/src/tables.rs @@ -276,4 +276,13 @@ mod tests { assert_eq!(names, ["x", "x2", "5"]); assert_eq!(sh.value(gridcraft_core::CellRef::new(0, 2)), Value::from("5")); } + + #[test] + fn go_to_a_table_selects_its_data() { + let mut s = Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A1", "values": [["a", "b"], [1, 2], [3, 4]]})).unwrap(); + s.execute("insert.table", json!({"range": "A1:B3", "header": true})).unwrap(); + assert_eq!(s.execute("edit.goTo", json!({"reference": "Table1"})).unwrap()["selection"], "A2:B3"); + } }