From 6546f0eecd61ba7f8f4dad639d92028861c48617 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo <103679419+marcoarnulfo@users.noreply.github.com> Date: Mon, 19 Oct 2026 08:30:10 +0200 Subject: [PATCH] Parameters Manager: double-click a name to rename the parameter and every expression that uses it (part of #1134) (#1142) --- crates/constraints/src/expr.rs | 44 ++++++++++++++ crates/constraints/src/lib.rs | 27 ++++++++ crates/engine/src/cmd/constraints.rs | 6 +- crates/engine/src/cmd/constraints_tests.rs | 36 +++++++++++ crates/ui-egui/src/parametric.rs | 43 ++++++++++++- crates/ui-egui/tests/parameters_rename.rs | 71 ++++++++++++++++++++++ 6 files changed, 224 insertions(+), 3 deletions(-) create mode 100644 crates/ui-egui/tests/parameters_rename.rs diff --git a/crates/constraints/src/expr.rs b/crates/constraints/src/expr.rs index cf83ad07e..67f87c247 100644 --- a/crates/constraints/src/expr.rs +++ b/crates/constraints/src/expr.rs @@ -185,6 +185,43 @@ pub fn names(e: &Expr, out: &mut Vec) { } } +/// `text` with every whole-name use of `old` replaced by `new`; numbers (`1e3`), other names +/// (`d10` for `d1`) and spacing are left as written. +pub fn rename(text: &str, old: &str, new: &str) -> String { + let b = text.as_bytes(); + let mut out = String::with_capacity(text.len()); + let mut i = 0; + while let Some(&c) = b.get(i) { + let start = i; + if c.is_ascii_digit() || c == b'.' { + while b.get(i).is_some_and(|c| c.is_ascii_digit() || *c == b'.') { + i += 1; + } + if b.get(i).is_some_and(|c| *c == b'e' || *c == b'E') { + let mut j = i + 1; + if b.get(j).is_some_and(|c| *c == b'+' || *c == b'-') { + j += 1; + } + if b.get(j).is_some_and(u8::is_ascii_digit) { + i = j; + while b.get(i).is_some_and(u8::is_ascii_digit) { + i += 1; + } + } + } + } else if c.is_ascii_alphabetic() || c == b'_' { + while b.get(i).is_some_and(|c| c.is_ascii_alphanumeric() || *c == b'_') { + i += 1; + } + } else { + i += text.get(i..).and_then(|r| r.chars().next()).map_or(1, char::len_utf8); + } + let token = text.get(start..i).unwrap_or_default(); + out.push_str(if token == old { new } else { token }); + } + out +} + /// Evaluate with a variable lookup. pub fn eval_with(e: &Expr, lookup: &mut dyn FnMut(&str) -> Result) -> Result { let v = match e { @@ -314,6 +351,13 @@ mod tests { assert!(chain.value("p0").is_err()); } + #[test] + fn rename_replaces_whole_names_only() { + assert_eq!(rename("d1+d10*d1 / (d1_x+2e1)", "d1", "w"), "w+d10*w / (d1_x+2e1)"); + assert_eq!(rename("1e2+e", "e", "x"), "1e2+x"); + assert_eq!(rename("é+a", "a", "b"), "é+b"); + } + #[test] fn names_valid() { assert!(valid_name("d1") && valid_name("_w") && !valid_name("1d") && !valid_name("a-b") && !valid_name("") && !valid_name("pi")); diff --git a/crates/constraints/src/lib.rs b/crates/constraints/src/lib.rs index 0f5ae03b9..00ba351bb 100644 --- a/crates/constraints/src/lib.rs +++ b/crates/constraints/src/lib.rs @@ -699,6 +699,33 @@ pub fn set_parameter(d: &mut Drawing, name: &str, expr_text: &str, opts: &SolveO Ok(report) } +/// Rename a parameter (dimensional constraint name or user parameter) and every expression that +/// uses it. Geometry doesn't change. On error the drawing is unchanged. +pub fn rename_parameter(d: &mut Drawing, old: &str, new: &str) -> Result<(), SolveError> { + if !name_in_use(d, old) { + return Err(SolveError::Other(format!("no parameter `{old}`"))); + } + if !expr::valid_name(new) { + return Err(SolveError::Other(format!("invalid parameter name `{new}`"))); + } + if new != old && name_in_use(d, new) { + return Err(SolveError::Other(format!("`{new}` is already in use"))); + } + for c in d.constraints.iter_mut().filter(|c| c.kind.is_dimensional()) { + if c.name == old { + c.name = new.to_string(); + } + c.expr = expr::rename(&c.expr, old, new); + } + for p in &mut d.parametric.parameters { + if p.name == old { + p.name = new.to_string(); + } + p.expr = expr::rename(&p.expr, old, new); + } + Ok(()) +} + /// Where to draw a constraint's glyphs (one anchor per reference). #[derive(Clone, Debug, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] diff --git a/crates/engine/src/cmd/constraints.rs b/crates/engine/src/cmd/constraints.rs index d8089bdc6..df37ecd25 100644 --- a/crates/engine/src/cmd/constraints.rs +++ b/crates/engine/src/cmd/constraints.rs @@ -93,7 +93,7 @@ pub fn specs() -> Vec { CommandSpec::new("parameters", "Parameters Manager", run_parameters) .menu(&["Tools", "Parametric", "Parameters Manager"]) .alias(&["par", "parametersclose"]) - .params("{} lists; {name, expr | value, description?} sets (new name = user parameter) and re-solves; {delete: name}"), + .params("{} lists; {name, expr | value, description?} sets (new name = user parameter) and re-solves; {delete: name}; {name, rename: newName} renames it in every expression"), CommandSpec::new("constraintsettings", "Constraint Settings", run_constraintsettings) .menu(&["Tools", "Parametric", "Constraint Settings"]) .alias(&["csettings"]) @@ -873,6 +873,10 @@ fn run_parameters(s: &mut Session, p: &Value) -> Result { return Err(EngineError::Other(format!("`{name}` is used by `{}`", row.name))); } *s.doc_mut()? = d; + } else if let (Some(old), Some(to)) = (str_param(p, "name"), str_param(p, "rename")) { + let mut d = s.doc()?.clone(); + cs::rename_parameter(&mut d, old.trim(), to.trim()).map_err(err)?; + *s.doc_mut()? = d; } else if let Some(name) = str_param(p, "name") { let expr = match (str_param(p, "expr"), p.get("value").and_then(Value::as_f64)) { (Some(e), _) => e.to_string(), diff --git a/crates/engine/src/cmd/constraints_tests.rs b/crates/engine/src/cmd/constraints_tests.rs index 547e37543..83e13e86e 100644 --- a/crates/engine/src/cmd/constraints_tests.rs +++ b/crates/engine/src/cmd/constraints_tests.rs @@ -56,6 +56,42 @@ fn json_geometric_and_dimensional_constraints() { assert_eq!(inspect["glyphs"][0]["kind"], "Horizontal"); } +#[test] +fn renaming_a_parameter_rewrites_the_expressions_that_use_it() { + let mut s = Session::new(); + let a = line(&mut s, [0.0, 0.0], [10.0, 0.0]); + let b = line(&mut s, [0.0, 5.0], [10.0, 5.0]); + s.execute("parameters", &json!({ "name": "user1", "expr": "4" })).unwrap(); + s.execute("dcaligned", &json!({ "h1": a, "expr": "user1*2" })).unwrap(); + s.execute("dcaligned", &json!({ "h1": b, "expr": "d1+1" })).unwrap(); + let names = |s: &Session| -> Vec<(String, String)> { + cadcraft_constraints::parameter_table(s.doc().unwrap()).into_iter().map(|r| (r.name, r.expr)).collect() + }; + + s.execute("parameters", &json!({ "name": "user1", "rename": "width" })).unwrap(); + assert_eq!(names(&s), vec![("d1".into(), "width*2".into()), ("d2".into(), "d1+1".into()), ("width".into(), "4".into())]); + assert!((ends(&s, &a).0.dist(ends(&s, &a).1) - 8.0).abs() < 1e-6); + + // A dimensional constraint's name too, and only the whole name: d1 is not part of d10. + s.execute("parameters", &json!({ "name": "d10", "expr": "3" })).unwrap(); + s.execute("parameters", &json!({ "name": "d2", "expr": "d1+d10" })).unwrap(); + s.execute("parameters", &json!({ "name": "d1", "rename": "base" })).unwrap(); + assert_eq!( + names(&s), + vec![("base".into(), "width*2".into()), ("d2".into(), "base+d10".into()), ("width".into(), "4".into()), ("d10".into(), "3".into())] + ); + + // Taken and invalid names are refused and change nothing; undo takes a rename back. + let before = s.doc().unwrap().clone(); + for to in ["width", "d10", "1x", "pi", ""] { + assert!(s.execute("parameters", &json!({ "name": "base", "rename": to })).is_err(), "{to}"); + } + assert!(s.execute("parameters", &json!({ "name": "nothing", "rename": "x" })).is_err()); + assert_eq!(*s.doc().unwrap(), before); + s.undo().unwrap(); + assert_eq!(names(&s)[0], ("d1".into(), "width*2".into())); +} + #[test] fn edits_are_resolved_and_erase_purges() { let mut s = Session::new(); diff --git a/crates/ui-egui/src/parametric.rs b/crates/ui-egui/src/parametric.rs index e05c1131e..83a4328bd 100644 --- a/crates/ui-egui/src/parametric.rs +++ b/crates/ui-egui/src/parametric.rs @@ -574,6 +574,9 @@ pub fn parameters_dialog(app: &mut CadApp, ctx: &egui::Context, open: &mut bool) let filter_id = egui::Id::new("par_filter"); let mut filter = ctx.data_mut(|d| d.get_temp::(filter_id)).unwrap_or_default(); let mut action: Option = None; + // The name being renamed in place: (old name, text typed so far). + let rename_id = egui::Id::new("par_rename"); + let mut renaming = ctx.data_mut(|d| d.get_temp::<(String, String)>(rename_id)); egui::Window::new(crate::tl!("Parameters Manager")) .id(egui::Id::new("Parameters Manager")) .open(open) @@ -606,7 +609,16 @@ pub fn parameters_dialog(app: &mut CadApp, ctx: &egui::Context, open: &mut bool) ui.label(egui::RichText::new(crate::i18n::t(title)).color(t.text_dim).small()); ui.end_row(); for r in group { - ui.label(&r.name); + if let Some((old, buf)) = renaming.as_mut().filter(|(old, _)| *old == r.name) { + if let Some(name) = rename_field(ui, rename_id.with("te"), old, buf) { + if !name.is_empty() && name != *old { + action = Some(json!({ "name": old.clone(), "rename": name })); + } + renaming = None; + } + } else if ui.add(egui::Label::new(&r.name).selectable(false).sense(egui::Sense::click())).double_clicked() { + renaming = Some((r.name.clone(), r.name.clone())); + } let id = egui::Id::new(("par_expr", &r.name)); if let Some(v) = expr_field(ui, id, &r.expr) { action = Some(json!({ "name": r.name, "expr": v })); @@ -634,12 +646,39 @@ pub fn parameters_dialog(app: &mut CadApp, ctx: &egui::Context, open: &mut bool) }); }); }); - ctx.data_mut(|d| d.insert_temp(filter_id, filter)); + ctx.data_mut(|d| { + d.insert_temp(filter_id, filter); + if let Some(r) = renaming { + d.insert_temp(rename_id, r); + } else { + d.remove_temp::<(String, String)>(rename_id); + } + }); if let Some(p) = action { let _ = app.run("parameters", p); } } +/// The in-place name editor: focused with the old name selected when it opens, so typing goes +/// here and not to the command line. `Some` when it closes: the new name on Enter, `""` otherwise. +fn rename_field(ui: &mut egui::Ui, id: egui::Id, old: &str, buf: &mut String) -> Option { + let opening = egui::TextEdit::load_state(ui.ctx(), id).is_none(); + let resp = ui.add(egui::TextEdit::singleline(buf).id(id).desired_width(120.0)); + if opening { + resp.request_focus(); + if let Some(mut state) = egui::TextEdit::load_state(ui.ctx(), id) { + let all = egui::text::CCursorRange::two(egui::text::CCursor::new(0), egui::text::CCursor::new(old.chars().count())); + state.cursor.set_char_range(Some(all)); + state.store(ui.ctx(), id); + } + } + resp.lost_focus().then(|| { + let name = if ui.input(|i| i.key_pressed(egui::Key::Enter)) { buf.trim().to_string() } else { String::new() }; + ui.ctx().data_mut(|d| d.remove::(id)); + name + }) +} + /// A text field committing on Enter / focus loss when changed. pub fn expr_field(ui: &mut egui::Ui, id: egui::Id, value: &str) -> Option { let mut buf = ui.data_mut(|d| d.get_temp::(id)).unwrap_or_else(|| value.to_string()); diff --git a/crates/ui-egui/tests/parameters_rename.rs b/crates/ui-egui/tests/parameters_rename.rs new file mode 100644 index 000000000..0754bbcb0 --- /dev/null +++ b/crates/ui-egui/tests/parameters_rename.rs @@ -0,0 +1,71 @@ +//! Double-clicking a name in the Parameters Manager edits it in place, as in AutoCAD (#1134): +//! the keys go to the name field, not to the command line, and Enter renames the parameter. + +use cadcraft_engine::Session; +use cadcraft_ui_egui::{CadApp, Services}; +use egui::{Event, Modifiers, PointerButton, vec2}; +use egui_kittest::Harness; +use egui_kittest::kittest::Queryable; +use serde_json::json; + +fn harness() -> Harness<'static, CadApp> { + let mut app = CadApp::new(Session::default(), Services::default()); + app.run("parameters", json!({ "name": "user1", "expr": "4" })).unwrap(); + app.run("parameters", json!({ "name": "user2", "expr": "user1*2" })).unwrap(); + app.ui.dialog = Some("parameters".into()); + // 60 frames a second: the default step (0.25 s) spreads two clicks past the double-click delay. + let mut h = Harness::builder().with_size(vec2(1280.0, 760.0)).with_step_dt(1.0 / 60.0).build_ui_state( + |ui, app: &mut CadApp| { + app.logic(&ui.ctx().clone()); + app.ui(ui); + }, + app, + ); + h.run_steps(5); + h +} + +fn double_click_label(h: &mut Harness<'_, CadApp>, label: &str) { + let pos = h.get_by_label(label).rect().center(); + h.event(Event::PointerMoved(pos)); + h.run_steps(2); + for _ in 0..2 { + for pressed in [true, false] { + h.event(Event::PointerButton { pos, button: PointerButton::Primary, pressed, modifiers: Modifiers::NONE }); + h.run_steps(1); + } + } + h.run_steps(2); +} + +fn parameters(h: &Harness<'_, CadApp>) -> Vec<(String, String)> { + let d = h.state().session.doc().unwrap(); + d.parametric.parameters.iter().map(|p| (p.name.clone(), p.expr.clone())).collect() +} + +#[test] +fn double_click_types_a_new_name_and_enter_renames() { + let mut h = harness(); + double_click_label(&mut h, "user1"); + h.event(Event::Text("width".into())); + h.run_steps(2); + assert_eq!(h.state().cmd.buffer, "", "the command line got the keys"); + + h.key_press(egui::Key::Enter); + h.run_steps(3); + assert_eq!(parameters(&h), vec![("width".into(), "4".into()), ("user2".into(), "width*2".into())]); + assert!(h.query_by_label("width").is_some(), "the table shows the new name"); + assert!(h.state().cmd.buffer.is_empty()); +} + +#[test] +fn escape_leaves_the_name_as_it_was() { + let mut h = harness(); + double_click_label(&mut h, "user1"); + h.event(Event::Text("x".into())); + h.run_steps(2); + h.key_press(egui::Key::Escape); + h.run_steps(3); + assert_eq!(parameters(&h), vec![("user1".into(), "4".into()), ("user2".into(), "user1*2".into())]); + assert!(h.state().cmd.buffer.is_empty()); +}