Merge pull request #149 from nihilist-narwhal/fix-table-header-edits

Tables: typing in the header row renames the column; Go To a table selects its data

# Conflicts:
#	crates/engine/src/tables.rs

crates/engine/src/cmd/edit.rs

@@ -478,7 +478,8 @@ fn go_to(s: &mut Session, p: &Json) -> Result<Json> {
         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()));

crates/engine/src/cmd/mod.rs

@@ -240,7 +240,11 @@ pub struct Ctx<'a> {
 
 pub(crate) fn edit<R>(s: &mut Session, f: impl FnOnce(&mut Ctx) -> Result<R>) -> Result<R> {
     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<R>(d: &mut DocState, f: impl FnOnce(&mut Ctx) -> Result<R>) -> Result<R> {

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).
@@ -149,7 +149,7 @@ pub fn gallery() -> Vec<String> {
 }
 
 #[cfg(test)]
-mod tests {
+mod style_tests {
     use super::*;
 
     #[test]
@@ -161,3 +161,142 @@ mod tests {
         }
     }
 }
+
+/// 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<String, usize>, std::collections::HashMap<String, usize>);
+    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<Formula>)> =
+            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"));
+    }
+
+    #[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");
+    }
+}