shifathosn
/
gridcraft
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #183 from MarekSpodBiedry/issue-140 

Shrink 3-D sheet spans when an end sheet is deleted (#140)
This commit is contained in:
Brandon Thomas 2026-10-10 15:46:31 -04:00
parent cda29c5b73 47210cde01
commit d031c8f37e
4 changed files with 62 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
crates/engine/src/cmd/sheet.rs
View File

@ -460,6 +460,7 @@ fn delete_sheet(s: &mut Session, p: &Json) -> Result<Json> {
return Err(EngineError::Other("A workbook must contain at least one visible worksheet.".into()));
}
let name = d.wb.sheet(i).map(|s| s.name.clone()).unwrap_or_default();
let order: Vec<String> = d.wb.sheets.iter().map(|s| s.name.clone()).collect();
let d = s.doc_mut()?;
edit_doc(d, |cx| {
cx.wb.sheets.remove(i);
@ -469,7 +470,7 @@ fn delete_sheet(s: &mut Session, p: &Json) -> Result<Json> {
let Some(sh) = cx.wb.sheet_mut(si) else { continue };
for (c, f) in keys {
if let Some(e) = f.expr() {
let ne = gridcraft_formula::adjust::delete_sheet(e, &name);
let ne = gridcraft_formula::adjust::delete_sheet(e, &name, &order);
if let Some(cell) = sh.cells.get_mut(c) {
cell.formula = Some(Arc::new(gridcraft_model::Formula::from_expr(ne)));
}
@ -485,7 +486,7 @@ fn delete_sheet(s: &mut Session, p: &Json) -> Result<Json> {
}
// Names referring to the sheet become #REF!.
if let Ok(e) = gridcraft_formula::parse(&n.formula) {
let ne = gridcraft_formula::adjust::delete_sheet(e.clone(), &name);
let ne = gridcraft_formula::adjust::delete_sheet(e.clone(), &name, &order);
if ne != e {
n.formula = gridcraft_formula::print(&ne);
}

26
crates/engine/src/tests.rs
View File

@ -764,6 +764,32 @@ fn sheet_operations_keep_names_on_their_sheets() {
assert_eq!(names(&s).len(), 1);
}
#[test]
fn deleting_a_3d_span_end_sheet_keeps_the_rest() {
let book = || {
let mut s = s();
for name in ["Sheet2", "Sheet3", "Results"] {
s.execute("home.insertSheet", json!({"name": name})).unwrap();
}
for (sheet, n) in [("Sheet1", 3), ("Sheet2", 5), ("Sheet3", 7)] {
s.execute("sheet.activate", json!({"sheet": sheet})).unwrap();
s.execute("cell.set", json!({"cell": "A1", "input": n.to_string()})).unwrap();
}
s.execute("sheet.activate", json!({"sheet": "Results"})).unwrap();
s.execute("cell.set", json!({"cell": "A1", "input": "=SUM(Sheet1:Sheet3!A1)"})).unwrap();
s.execute("cell.set", json!({"cell": "B1", "input": "=Sheet1!A1"})).unwrap();
s
};
let gone = Value::Error(gridcraft_core::CellError::Ref);
for (deleted, total, direct) in [("Sheet1", 12.0, gone), ("Sheet3", 8.0, Value::Number(3.0)), ("Sheet2", 10.0, Value::Number(3.0))] {
let mut s = book();
s.execute("home.deleteSheet", json!({"sheet": deleted})).unwrap();
s.execute("sheet.activate", json!({"sheet": "Results"})).unwrap();
assert_eq!(v(&s, "A1"), Value::Number(total), "deleting {deleted}");
assert_eq!(v(&s, "B1"), direct, "deleting {deleted}");
}
}
#[test]
fn xlsx_roundtrip_through_engine() {
let mut s = s();

22
crates/formula/src/adjust.rs
View File

@ -229,10 +229,28 @@ pub fn rename_sheet(e: Expr, old: &str, new: &str) -> Expr {
})
}
/// References to a deleted sheet become `#REF!`.
pub fn delete_sheet(e: Expr, name: &str) -> Expr {
/// References to a deleted sheet become `#REF!`. `order` lists the sheet names (before the deletion) in tab order:
/// a 3-D span that loses an end sheet shrinks to the neighbouring surviving sheet, as in Excel.
pub fn delete_sheet(e: Expr, name: &str, order: &[String]) -> Expr {
let pos = |n: &str| order.iter().position(|o| o.eq_ignore_ascii_case(n));
e.map(&mut |x| match x {
Expr::Ref(r) if r.sheet_name().is_some_and(|n| n.eq_ignore_ascii_case(name)) => Expr::Error(CellError::Ref),
Expr::Ref(mut r) => {
if let SheetSel::Span(a, b) = &r.sheet
&& (a.eq_ignore_ascii_case(name) || b.eq_ignore_ascii_case(name))
{
let (Some(ia), Some(ib)) = (pos(a), pos(b)) else { return Expr::Ref(r) };
if ia == ib {
return Expr::Error(CellError::Ref);
}
// The deleted end moves one sheet towards the other end; the span keeps its direction.
let inward = |from: usize, to: usize| order.get(if from < to { from + 1 } else { from.saturating_sub(1) });
let (na, nb) = if a.eq_ignore_ascii_case(name) { (inward(ia, ib), order.get(ib)) } else { (order.get(ia), inward(ib, ia)) };
let (Some(na), Some(nb)) = (na, nb) else { return Expr::Error(CellError::Ref) };
r.sheet = if na.eq_ignore_ascii_case(nb) { SheetSel::Named(na.clone()) } else { SheetSel::Span(na.clone(), nb.clone()) };
}
Expr::Ref(r)
}
other => other,
})
}

14
crates/formula/src/tests.rs
View File

@ -65,7 +65,19 @@ fn spill_operator() {
assert!(parse("A1:B2#").is_err());
assert!(parse("Name#").is_err());
// Deleting the anchor's sheet leaves #REF!.
assert_eq!(print(&crate::adjust::delete_sheet(parse("SUM(Data!A1#)").unwrap(), "Data")), "SUM(#REF!)");
assert_eq!(print(&crate::adjust::delete_sheet(parse("SUM(Data!A1#)").unwrap(), "Data", &[])), "SUM(#REF!)");
}
#[test]
fn delete_sheet_shrinks_3d_span() {
let order: Vec<String> = ["Sheet1", "Sheet2", "Sheet3", "Results"].iter().map(|s| s.to_string()).collect();
let del = |f: &str, n: &str| print(&crate::adjust::delete_sheet(parse(f).unwrap(), n, &order));
assert_eq!(del("SUM(Sheet1:Sheet3!A1)", "Sheet1"), "SUM(Sheet2:Sheet3!A1)");
assert_eq!(del("SUM(Sheet1:Sheet3!A1)", "Sheet3"), "SUM(Sheet1:Sheet2!A1)");
assert_eq!(del("SUM(Sheet3:Sheet1!A1)", "Sheet1"), "SUM(Sheet3:Sheet2!A1)");
assert_eq!(del("SUM(Sheet1:Sheet3!A1)", "Sheet2"), "SUM(Sheet1:Sheet3!A1)");
assert_eq!(del("SUM(Sheet1:Sheet2!A1)", "Sheet1"), "SUM(Sheet2!A1)");
assert_eq!(del("Sheet1!A1+Sheet3!A1", "Sheet1"), "#REF!+Sheet3!A1");
}
#[test]