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

@@ -185,6 +185,43 @@ pub fn names(e: &Expr, out: &mut Vec<String>) {
     }
 }
 
+/// `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<f64, ExprError>) -> Result<f64, ExprError> {
     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"));

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")]

crates/engine/src/cmd/constraints.rs

@@ -93,7 +93,7 @@ pub fn specs() -> Vec<CommandSpec> {
         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<Value> {
             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(),

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();

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::<String>(filter_id)).unwrap_or_default();
     let mut action: Option<Value> = 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<String> {
+    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::<egui::text_edit::TextEditState>(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<String> {
     let mut buf = ui.data_mut(|d| d.get_temp::<String>(id)).unwrap_or_else(|| value.to_string());

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());
+}