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

Name Manager: New… and Edit… open the name dialog and return to the manager (#238) 

dialogs::show put the Name Manager back into app.dialog after its body
had opened the New Name dialog, so New… did nothing. A dialog opened from
another one now takes over and the opener waits behind it (back), coming
back when the child closes; Go To still hands over to Go To Special.

The Name Manager gets Excel's New…/Edit…/Delete buttons over a selectable
list. formulas.defineName takes oldName/oldScope to edit a name in place;
a renamed name is renamed in cell formulas and other names' definitions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
vangel76 2026-10-11 12:09:19 +02:00
parent 9f896c1249
commit a11446f2f0
4 changed files with 231 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
crates/engine/src/cmd/formulas.rs
View File

@ -1,7 +1,7 @@
//! Formulas tab: AutoSum, Insert Function, names, auditing, calculation.
use gridcraft_core::{CellRef, RangeRef};
use gridcraft_model::{CalcMode, DefinedName};
use gridcraft_model::{CalcMode, DefinedName, Formula};
use serde_json::{Value as Json, json};
use super::*;
@ -32,7 +32,7 @@ pub fn specs() -> Vec<CommandSpec> {
"Define Name…",
["Formulas", "Defined Names"],
None,
"{name, refersTo?: \"=Sheet1!$A$1:$A$10\" (default: the selection), scope?: \"Workbook\"|sheet name, comment?}",
"{name, refersTo?: \"=Sheet1!$A$1:$A$10\" (default: the selection), scope?: \"Workbook\"|sheet name, comment?, oldName?, oldScope?} (oldName: edit that name (Name Manager › Edit); a new name also renames it in formulas)",
has_doc,
define_name
),
@ -265,7 +265,32 @@ fn define_name(s: &mut Session, p: &Json) -> Result<Json> {
return Err(EngineError::Other("There's a problem with this formula.".into()));
}
let comment = str_param(p, "comment").unwrap_or("").to_string();
// Editing an existing name (Name Manager › Edit…): it must exist, and its new name must not
// collide with another name in the target scope.
let old = match str_param(p, "oldName") {
Some(old_name) => {
let old_scope = match str_param(p, "oldScope") {
None | Some("Workbook") => None,
Some(sh) => Some(d.wb.sheet_index(sh).ok_or_else(|| bad("formulas.defineName", "no such sheet"))?),
};
if !d.wb.names.iter().any(|n| n.name.eq_ignore_ascii_case(old_name) && n.scope == old_scope) {
return Err(bad("formulas.defineName", "no such name to edit"));
}
let same = old_name.eq_ignore_ascii_case(&name) && old_scope == scope;
if !same && d.wb.names.iter().any(|n| n.name.eq_ignore_ascii_case(&name) && n.scope == scope) {
return Err(EngineError::Other("The name entered already exists. Enter a unique name.".into()));
}
Some((old_name.to_string(), old_scope))
}
None => None,
};
edit(s, |cx| {
if let Some((old_name, old_scope)) = &old {
cx.wb.names.retain(|n| !(n.name.eq_ignore_ascii_case(old_name) && n.scope == *old_scope));
if !old_name.eq_ignore_ascii_case(&name) {
rename_name_in_formulas(&mut cx.wb, old_name, *old_scope, &name);
}
}
cx.wb.names.retain(|n| !(n.name.eq_ignore_ascii_case(&name) && n.scope == scope));
cx.wb.names.push(DefinedName { name: name.clone(), scope, formula: refers.clone(), comment: comment.clone(), hidden: false });
cx.structural = true;
@ -273,6 +298,51 @@ fn define_name(s: &mut Session, p: &Json) -> Result<Json> {
})
}
/// Formulas that use a renamed name follow it: cell formulas and other names' definitions, on
/// the sheets where `old` resolves to the renamed name (its own sheet for a sheet-scoped name;
/// every sheet without a same-named sheet-level name for a workbook-level one).
fn rename_name_in_formulas(wb: &mut gridcraft_model::Workbook, old: &str, old_scope: Option<usize>, new: &str) {
let shadowed: Vec<usize> = wb.names.iter().filter(|n| n.name.eq_ignore_ascii_case(old)).filter_map(|n| n.scope).collect();
let applies = |sheet: usize| match old_scope {
Some(s) => s == sheet,
None => !shadowed.contains(&sheet),
};
// The renamed expression, or `None` when the name doesn't occur in it.
let rename = |e: gridcraft_formula::Expr| {
let mut hit = false;
let out = e.map(&mut |x| match x {
gridcraft_formula::Expr::Name(n) if n.eq_ignore_ascii_case(old) => {
hit = true;
gridcraft_formula::Expr::Name(new.to_string())
}
o => o,
});
hit.then_some(out)
};
for i in 0..wb.sheets.len() {
if !applies(i) {
continue;
}
let Some(sh) = wb.sheet_mut(i) else { continue };
let formulas: Vec<(CellRef, std::sync::Arc<Formula>)> = sh.cells.iter().filter_map(|(c, x)| x.formula.clone().map(|f| (c, f))).collect();
for (c, f) in formulas {
let Some(e) = f.expr().and_then(rename) else { continue };
let mut nf = Formula::from_expr(e);
nf.array = f.array;
if let Some(cell) = sh.cells.get_mut(c) {
cell.formula = Some(std::sync::Arc::new(nf));
}
}
}
for n in wb.names.iter_mut() {
if n.scope.is_none_or(applies)
&& let Some(e) = gridcraft_formula::parse(&n.formula).ok().and_then(rename)
{
n.formula = gridcraft_formula::print(&e);
}
}
}
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") {

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

@ -52,6 +52,26 @@ fn entering_empty_cells_preserves_whole_sheet_formatting() {
}
}
#[test]
fn editing_a_name_renames_it_in_formulas() {
// Name Manager › Edit…: `oldName` edits the name in place; formulas that use it follow.
let mut s = s();
s.execute("cell.set", json!({"cell": "A1", "input": "21"})).unwrap();
s.execute("formulas.defineName", json!({"name": "Price", "refersTo": "=Sheet1!$A$1"})).unwrap();
s.execute("formulas.defineName", json!({"name": "Other", "refersTo": "=1"})).unwrap();
s.execute("cell.set", json!({"cell": "B1", "input": "=Price*2"})).unwrap();
let err = s.execute("formulas.defineName", json!({"name": "Other", "refersTo": "=Sheet1!$A$1", "oldName": "Price"}));
assert!(err.is_err(), "renaming onto another existing name is refused");
s.execute("formulas.defineName", json!({"name": "Cost", "refersTo": "=Sheet1!$A$1", "comment": "unit", "oldName": "Price"})).unwrap();
let d = s.doc().unwrap();
let names: Vec<&str> = d.wb.names.iter().map(|n| n.name.as_str()).collect();
assert_eq!(names, ["Other", "Cost"]);
let b1 = d.wb.active().unwrap().cell(CellRef::parse("B1").unwrap()).and_then(|c| c.formula.clone()).unwrap();
assert_eq!(b1.text, "Cost*2");
assert_eq!(v(&s, "B1"), Value::number(42.0));
assert!(s.execute("formulas.defineName", json!({"name": "X", "oldName": "Missing"})).is_err());
}
#[test]
fn entry_and_text_paste_use_inherited_formats_and_cell_overrides() {
let mut s = s();

140
crates/ui-egui/src/dialogs.rs
View File

@ -33,6 +33,8 @@ pub struct Dialog {
pub result: Option<Json>,
picture_inbox: Option<crate::Inbox>,
picture_target: Option<(u64, usize)>,
/// The dialog this one was opened from (Name Manager › New…): it comes back when this closes.
back: Option<Box<Dialog>>,
}
impl Dialog {
@ -53,6 +55,7 @@ impl Dialog {
result: None,
picture_inbox: None,
picture_target: None,
back: None,
}
}
@ -69,6 +72,7 @@ impl Dialog {
result: None,
picture_inbox: None,
picture_target: None,
back: None,
}
}
@ -353,18 +357,32 @@ impl Dialog {
],
json!({"percent": app.session.active().and_then(|d| d.wb.active().map(|s| s.zoom)).unwrap_or(100)}),
),
"defineName" => Dialog::form(
"defineName",
"New Name",
"formulas.defineName",
vec![
Text { key: "name", label: "Name:" },
Text { key: "scope", label: "Scope (Workbook or sheet name):" },
Text { key: "comment", label: "Comment:" },
Text { key: "refersTo", label: "Refers to:" },
],
json!({"scope": "Workbook", "refersTo": app.session.active().and_then(|d| d.wb.active().map(|s| format!("={}!{}", gridcraft_engine::formula::quote_sheet(&s.name), abs(&d.selection.current()))))}),
),
"defineName" => {
// Name Manager › Edit… passes the name being edited (`oldName`, `oldScope`) and
// its current fields; New… and Define Name start from the selection.
let editing = params.get("oldName").and_then(Json::as_str).is_some();
let selection = app.session.active().and_then(|d| {
d.wb.active().map(|s| format!("={}!{}", gridcraft_engine::formula::quote_sheet(&s.name), abs(&d.selection.current())))
});
let mut defaults = json!({"scope": "Workbook", "refersTo": selection});
for key in ["name", "scope", "comment", "refersTo", "oldName", "oldScope"] {
if let Some(v) = params.get(key).filter(|v| v.is_string()) {
defaults[key] = v.clone();
}
}
Dialog::form(
"defineName",
if editing { "Edit Name" } else { "New Name" },
"formulas.defineName",
vec![
Text { key: "name", label: "Name:" },
Text { key: "scope", label: "Scope (Workbook or sheet name):" },
Text { key: "comment", label: "Comment:" },
Text { key: "refersTo", label: "Refers to:" },
],
defaults,
)
}
"dataValidation" => Dialog::form(
"dataValidation",
"Data Validation",
@ -636,10 +654,12 @@ fn build_params(d: &Dialog) -> Json {
}
}
"defineName" => {
if d.values.get("scope").and_then(Json::as_str) == Some("Workbook")
&& let Some(m) = v.as_object_mut()
{
m.remove("scope");
if let Some(m) = v.as_object_mut() {
for key in ["scope", "oldScope"] {
if m.get(key).and_then(Json::as_str) == Some("Workbook") {
m.remove(key);
}
}
}
}
_ => {}
@ -677,7 +697,7 @@ pub fn show(app: &mut SheetApp, ctx: &egui::Context) {
"find" => find(app, ui, &mut d),
"goTo" => go_to(app, ui, &mut d, &mut confirm),
"sort" => sort(app, ui, &mut d, &mut confirm),
"nameManager" => name_manager(app, ui),
"nameManager" => name_manager(app, ui, &mut d, &mut open),
"manageRules" => manage_rules(app, ui),
"commandSearch" => command_search(app, ui, &mut d, &mut confirm),
"goalSeek" => goal_seek(app, ui, &mut d, &mut confirm),
@ -883,8 +903,21 @@ pub fn show(app: &mut SheetApp, ctx: &egui::Context) {
} else if confirm {
open = false;
}
if let Some(child) = app.dialog.as_mut() {
// The body opened another dialog (Name Manager › New…). It takes over; this one waits
// behind it and comes back when it closes, as Excel returns to the Name Manager. Go To
// hands over to Go To Special for good, as in Excel.
if open && d.name != "goTo" {
child.back = Some(Box::new(d));
} else if child.back.is_none() {
child.back = d.back.take();
}
return;
}
if open {
app.dialog = Some(d);
} else if let Some(back) = d.back.take() {
app.dialog = Some(*back);
}
}
@ -1485,30 +1518,61 @@ fn sort(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dialog, confirm: &mut boo
}
}
fn name_manager(app: &mut SheetApp, ui: &mut egui::Ui) {
fn name_manager(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dialog, open: &mut bool) {
let list = app.session.run("formulas.nameManager", json!({})).unwrap_or(Json::Null);
egui::Grid::new("names").striped(true).num_columns(4).show(ui, |ui| {
ui.strong("Name");
ui.strong("Value");
ui.strong("Refers To");
ui.strong("Scope");
ui.end_row();
for n in list.as_array().cloned().unwrap_or_default() {
ui.label(n["name"].as_str().unwrap_or(""));
ui.label(n["value"].to_string().chars().take(30).collect::<String>());
ui.label(n["refersTo"].as_str().unwrap_or(""));
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"], "scope": n["scope"]}));
}
});
ui.end_row();
let names = list.as_array().cloned().unwrap_or_default();
let selected = names.get(d.list_index).cloned();
let field = |n: &Json, k: &str| n.get(k).and_then(Json::as_str).unwrap_or("").to_string();
ui.horizontal(|ui| {
if ui.button("New…").clicked() {
app.open_dialog("defineName", json!({}));
}
if ui.add_enabled(selected.is_some(), egui::Button::new("Edit…")).clicked()
&& let Some(n) = &selected
{
app.open_dialog(
"defineName",
json!({
"name": field(n, "name"), "scope": field(n, "scope"), "comment": field(n, "comment"),
"refersTo": field(n, "refersTo"), "oldName": field(n, "name"), "oldScope": field(n, "scope"),
}),
);
}
if ui.add_enabled(selected.is_some(), egui::Button::new("Delete")).clicked()
&& let Some(n) = &selected
{
app.run_or_alert("formulas.deleteName", json!({"name": field(n, "name"), "scope": field(n, "scope")}));
}
});
ui.add_space(4.0);
egui::ScrollArea::vertical().max_height(260.0).show(ui, |ui| {
egui::Grid::new("names").striped(true).num_columns(4).show(ui, |ui| {
ui.strong("Name");
ui.strong("Value");
ui.strong("Refers To");
ui.strong("Scope");
ui.end_row();
if names.is_empty() {
ui.label(egui::RichText::new("No names defined").italics());
ui.end_row();
}
for (i, n) in names.iter().enumerate() {
if ui.selectable_label(i == d.list_index, field(n, "name")).clicked() {
d.list_index = i;
}
ui.label(n.get("value").map(|v| v.to_string()).unwrap_or_default().chars().take(30).collect::<String>());
ui.label(field(n, "refersTo"));
ui.label(field(n, "scope"));
ui.end_row();
}
});
});
ui.add_space(8.0);
ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| {
if ui.button(" Close ").clicked() {
*open = false;
}
});
if ui.button("New…").clicked() {
app.open_dialog("defineName", json!({}));
}
}
fn manage_rules(app: &mut SheetApp, ui: &mut egui::Ui) {

37
crates/ui-egui/tests/interaction.rs
View File

@ -1,6 +1,7 @@
//! UI interaction tests: real egui input through the whole app (no GPU needed).
use egui::{Event, Key, Modifiers, PointerButton};
use egui_kittest::kittest::Queryable as _;
use gridcraft_engine::Session;
use gridcraft_engine::core::{CellRef, Value};
use gridcraft_ui_egui::{SheetApp, grid::Geo};
@ -213,6 +214,42 @@ fn language_switch_translates_the_ribbon_and_persists() {
assert_eq!(h.state().ui.language, Language::En);
}
#[test]
fn name_manager_new_and_edit_open_the_name_dialog_and_come_back() {
// #238: New… in the Name Manager did nothing (the manager put itself back over the dialog it
// had just opened).
let mut h = harness(blank());
h.state_mut().open_dialog("nameManager", json!({}));
h.run_steps(4);
h.get_by_label("New…").click();
h.run_steps(2);
assert_eq!(h.state().dialog.as_ref().map(|d| d.title.as_str()), Some("New Name"));
if let Some(d) = h.state_mut().dialog.as_mut() {
d.values.insert("name".into(), json!("Total"));
d.values.insert("refersTo".into(), json!("=Sheet1!$A$1"));
}
h.run_steps(1);
key(&mut h, Key::Enter, Modifiers::NONE);
h.run_steps(2);
let names = |h: &egui_kittest::Harness<'static, SheetApp>| {
h.state().session.active().unwrap().wb.names.iter().map(|n| n.name.clone()).collect::<Vec<_>>()
};
assert_eq!(names(&h), ["Total"]);
assert_eq!(h.state().dialog.as_ref().map(|d| d.name()), Some("nameManager"), "OK returns to the Name Manager");
h.get_by_label("Edit…").click();
h.run_steps(2);
assert_eq!(h.state().dialog.as_ref().map(|d| d.title.as_str()), Some("Edit Name"));
if let Some(d) = h.state_mut().dialog.as_mut() {
d.values.insert("name".into(), json!("GrandTotal"));
}
h.run_steps(1);
key(&mut h, Key::Enter, Modifiers::NONE);
h.run_steps(2);
assert_eq!(names(&h), ["GrandTotal"], "Edit renames instead of adding a second name");
assert_eq!(h.state().dialog.as_ref().map(|d| d.name()), Some("nameManager"));
}
#[test]
fn dialogs_open_and_close() {
let mut h = harness(blank());