Names: keep sheet-level names with their sheet

Moving a sheet left its sheet-level names on whatever sheet took its
index, and copying it shifted nothing and copied no names. Moving now
re-indexes name scopes, and a copy gets copies of the sheet's names,
referring to the copy, as in Excel.

Deleting a sheet left names that referred to it pointing at nothing;
they now become #REF!. formulas.deleteName honours scope, and the Name
Manager's Delete passes it.
This commit is contained in:
nihilist-narwhal
2026-10-09 22:52:49 -05:00
parent 13664fe66b
commit b53a190ea1
4 changed files with 93 additions and 4 deletions
+7 -2
View File
@@ -36,7 +36,7 @@ pub fn specs() -> Vec<CommandSpec> {
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<Json> {
fn delete_name(s: &mut Session, p: &Json) -> Result<Json> {
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"));
}
+40 -1
View File
@@ -392,6 +392,13 @@ fn delete_sheet(s: &mut Session, p: &Json) -> Result<Json> {
{
*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<Json> {
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<gridcraft_model::DefinedName> = 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<Json> {
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)
+45
View File
@@ -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>, 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();
+1 -1
View File
@@ -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();