diff --git a/crates/engine/src/cmd/formulas.rs b/crates/engine/src/cmd/formulas.rs index 2bea26cda..0c9eb5c3e 100644 --- a/crates/engine/src/cmd/formulas.rs +++ b/crates/engine/src/cmd/formulas.rs @@ -36,7 +36,7 @@ pub fn specs() -> Vec { has_doc, define_name ), - cmd!("formulas.deleteName", "Delete Name", [], None, "{name, scope?}", has_doc, delete_name), + cmd!("formulas.deleteName", "Delete Name", [], None, "{name, scope?: \"Workbook\"|sheet name (default: every scope)}", has_doc, delete_name), cmd!(query "formulas.nameManager", "Name Manager", ["Formulas", "Defined Names"], Some("Cmd+F3"), "{} → names with values", has_doc, name_manager), cmd!( "formulas.createFromSelection", @@ -264,9 +264,14 @@ fn define_name(s: &mut Session, p: &Json) -> Result { fn delete_name(s: &mut Session, p: &Json) -> Result { let name = str_param(p, "name").ok_or_else(|| bad("formulas.deleteName", "missing `name`"))?.to_string(); + let scope = match str_param(p, "scope") { + None => None, + Some("Workbook") => Some(None), + Some(sh) => Some(Some(s.doc()?.wb.sheet_index(sh).ok_or_else(|| bad("formulas.deleteName", "no such sheet"))?)), + }; edit(s, |cx| { let before = cx.wb.names.len(); - cx.wb.names.retain(|n| !n.name.eq_ignore_ascii_case(&name)); + cx.wb.names.retain(|n| !(n.name.eq_ignore_ascii_case(&name) && scope.is_none_or(|sc| n.scope == sc))); if cx.wb.names.len() == before { return Err(bad("formulas.deleteName", "no such name")); } diff --git a/crates/engine/src/cmd/sheet.rs b/crates/engine/src/cmd/sheet.rs index 837f0806a..a6a3d5c39 100644 --- a/crates/engine/src/cmd/sheet.rs +++ b/crates/engine/src/cmd/sheet.rs @@ -392,6 +392,13 @@ fn delete_sheet(s: &mut Session, p: &Json) -> Result { { *sc -= 1; } + // 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); + if ne != e { + n.formula = gridcraft_formula::print(&ne); + } + } } if cx.wb.active_sheet >= cx.wb.sheets.len() || cx.wb.active_sheet > i { cx.wb.active_sheet = cx.wb.active_sheet.saturating_sub(1).min(cx.wb.sheets.len().saturating_sub(1)); @@ -461,10 +468,31 @@ fn move_sheet(s: &mut Session, p: &Json) -> Result { if copy { let mut sh = (*src).clone(); let base = sh.name.clone(); - let name = (2..).map(|k| format!("{base} ({k})")).find(|nm| cx.wb.sheet_index(nm).is_none()).unwrap_or(base); + let name = (2..).map(|k| format!("{base} ({k})")).find(|nm| cx.wb.sheet_index(nm).is_none()).unwrap_or(base.clone()); sh.name = name.chars().take(31).collect(); sh.tables.clear(); // table names must be unique let at = to.min(n); + // The copy gets its own copies of the sheet's names, referring to the copy. + let copies: Vec = cx + .wb + .names + .iter() + .filter(|nm| nm.scope == Some(i)) + .map(|nm| { + let formula = gridcraft_formula::parse(&nm.formula) + .map(|e| gridcraft_formula::print(&gridcraft_formula::adjust::rename_sheet(e, &base, &sh.name))) + .unwrap_or_else(|_| nm.formula.clone()); + gridcraft_model::DefinedName { scope: Some(at), formula, ..nm.clone() } + }) + .collect(); + for nm in cx.wb.names.iter_mut() { + if let Some(sc) = nm.scope.as_mut() + && *sc >= at + { + *sc += 1; + } + } + cx.wb.names.extend(copies); cx.wb.sheets.insert(at, Arc::new(sh)); cx.wb.active_sheet = at; } else { @@ -472,6 +500,17 @@ fn move_sheet(s: &mut Session, p: &Json) -> Result { let at = to.min(n - 1); cx.wb.sheets.insert(at, sh); cx.wb.active_sheet = at; + // Sheet-level names follow their sheet. + for nm in cx.wb.names.iter_mut() { + if let Some(sc) = nm.scope.as_mut() { + *sc = match *sc { + x if x == i => at, + x if i < x && x <= at => x - 1, + x if at <= x && x < i => x + 1, + x => x, + }; + } + } } cx.structural = true; Ok(Json::Null) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index f3e36cdf4..bd51545bd 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -134,6 +134,51 @@ fn sheets() { assert_eq!(s.execute("cell.get", json!({"cell": "A1"})).unwrap()["formula"], "='My Data'!A1*2"); } +#[test] +fn sheet_operations_keep_names_on_their_sheets() { + let names = |s: &Session| -> Vec<(String, Option, String)> { + let wb = &s.doc().unwrap().wb; + let mut v: Vec<_> = + wb.names.iter().map(|n| (n.name.clone(), n.scope.and_then(|i| wb.sheet(i)).map(|s| s.name.clone()), n.formula.clone())).collect(); + v.sort(); + v + }; + let named = |n: &str, scope: Option<&str>, f: &str| (n.to_string(), scope.map(str::to_string), f.to_string()); + let mut s = s(); + s.execute("home.insertSheet", json!({"name": "Data"})).unwrap(); + s.execute("home.insertSheet", json!({"name": "Notes"})).unwrap(); + s.execute("formulas.defineName", json!({"name": "Rate", "refersTo": "=Data!$A$1", "scope": "Data"})).unwrap(); + s.execute("formulas.defineName", json!({"name": "Rate", "refersTo": "=Notes!$B$1", "scope": "Notes"})).unwrap(); + s.execute("formulas.defineName", json!({"name": "Total", "refersTo": "=Data!$C$1"})).unwrap(); + // Moving Data (index 1) to the end: its names go with it. + s.execute("sheet.move", json!({"sheet": "Data", "to": 2})).unwrap(); + assert_eq!(names(&s), [named("Rate", Some("Data"), "Data!$A$1"), named("Rate", Some("Notes"), "Notes!$B$1"), named("Total", None, "Data!$C$1")]); + // Copying Data to the front copies its sheet-level names, referring to the copy. + s.execute("sheet.move", json!({"sheet": "Data", "to": 0, "copy": true})).unwrap(); + assert_eq!(s.doc().unwrap().wb.sheets[0].name, "Data (2)"); + assert_eq!( + names(&s), + [ + named("Rate", Some("Data"), "Data!$A$1"), + named("Rate", Some("Data (2)"), "'Data (2)'!$A$1"), + named("Rate", Some("Notes"), "Notes!$B$1"), + named("Total", None, "Data!$C$1") + ] + ); + // Deleting Data removes its names; names referring to it become #REF!. + s.execute("home.deleteSheet", json!({"sheet": "Data"})).unwrap(); + assert_eq!( + names(&s), + [named("Rate", Some("Data (2)"), "'Data (2)'!$A$1"), named("Rate", Some("Notes"), "Notes!$B$1"), named("Total", None, "#REF!")] + ); + // Deleting a name in one scope keeps the others. + s.execute("formulas.deleteName", json!({"name": "rate", "scope": "Notes"})).unwrap(); + assert_eq!(names(&s), [named("Rate", Some("Data (2)"), "'Data (2)'!$A$1"), named("Total", None, "#REF!")]); + assert!(s.execute("formulas.deleteName", json!({"name": "Total", "scope": "Notes"})).is_err()); + s.execute("formulas.deleteName", json!({"name": "Total", "scope": "Workbook"})).unwrap(); + assert_eq!(names(&s).len(), 1); +} + #[test] fn xlsx_roundtrip_through_engine() { let mut s = s(); diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index b6c186d2d..dd2791bb9 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1368,7 +1368,7 @@ fn name_manager(app: &mut SheetApp, ui: &mut egui::Ui) { ui.horizontal(|ui| { ui.label(n["scope"].as_str().unwrap_or("")); if ui.small_button("Delete").clicked() { - app.run_or_alert("formulas.deleteName", json!({"name": n["name"]})); + app.run_or_alert("formulas.deleteName", json!({"name": n["name"], "scope": n["scope"]})); } }); ui.end_row();