git.lucas.co / cce-designer
graphic design tool
git clone https://git.lucas.co/cce-designer.git

commit21470d3930235c3b93309da9caa17e6eb66e3447
parent2365ab31d4
authorLucas Galante <lsgalante12@gmail.com>
date2026-09-29 19:31
feat: a rename is undoable

A structure step holds the nodes renamed and what each was called, and
puts a name back by renaming, so the wires and the expression paths
that name the node follow it back, and the active camera with them.
MCP's rename_node wrote one of the two copies of the active camera's
name; it goes through set_active_camera now.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

 CLAUDE.md           | 18 ++++++++---
 src/edit_history.rs | 59 ++++++++++++++++++++++++++++++-----
 src/main.rs         | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 src/window.rs       |  4 ++-
 4 files changed, 156 insertions(+), 15 deletions(-)

diff --git a/CLAUDE.md b/CLAUDE.md
index 2cd414e..cf9cf85 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -140,8 +140,8 @@ them back in the order they were made:
   pane's write-back (`sync_parameters_to_project`), the row menu
   (`run_param_action`, whose work is `run_param_action_unrecorded`), MCP's
   `set_param`, and Reset Parameters.
-- **Structure**: nodes added, removed and moved, wires made and broken,
-  the display and bypass flags. Recorded by NOTICING:
+- **Structure**: nodes added, removed, moved and renamed, wires made and
+  broken, the display and bypass flags. Recorded by NOTICING:
   `record_structure_changes` compares the tree with how it stood at the
   last look (`State::structure_base`) and what differs is the step. It
   runs at the end of `process_window_event` and of `apply_action`, and
@@ -183,14 +183,22 @@ The rules:
   then this (`Application::undo` for the chord, `Action::Undo` for the
   palette) — so the order ACROSS the three is by owner and not by time. A
   focused text box is ahead of all of them, in the toolkit's runner.
-- **A rename is not recorded.** It rewrites the expression paths and
-  wires that name the node, anywhere in the tree, and a step that put the
-  name back without them would leave them naming nothing.
+- **A rename is put back by renaming.** A structure step holds the
+  nodes renamed as (id, the name it was), and `restore` runs
+  `rename_node_in_tree` on each, so the wires and the expression paths
+  that name the node, anywhere in the tree, are written back with it —
+  writing the name alone would leave them naming nothing. The active
+  camera is held by id across the step. The names go back AFTER the wires
+  the step holds: the other way round, what is filed for redo is those
+  wires as the rename had just left them, and redo puts the old name on
+  them. A name a sibling has taken since is not taken twice; the node
+  keeps the one it has.
 - It is not cce-ui's `History` because that has no way to look at a step
   before taking it, and what is filed for redo is the current state of
   what the step names.
 
 `the_graph_is_undone_a_step_at_a_time`,
+`a_rename_is_undone_with_what_names_the_node`,
 `a_parameter_edit_is_undone_a_gesture_at_a_time` and
 `reset_parameters_is_undone_and_redone` are the tests.
 
diff --git a/src/edit_history.rs b/src/edit_history.rs
index 420a945..e90b1aa 100644
--- a/src/edit_history.rs
+++ b/src/edit_history.rs
@@ -9,8 +9,8 @@
 //!   something changed them: a row of the params pane, the row menu,
 //!   `set_param`, or a command that rewrites the lot (Reset Parameters).
 //!   Recorded by the writers.
-//! - **Structure** — nodes added, nodes removed, nodes moved, wires made
-//!   and broken, the display and bypass flags. Recorded by NOTICING: the
+//! - **Structure** — nodes added, nodes removed, nodes moved, nodes
+//!   renamed, wires made and broken, the display and bypass flags. Recorded by NOTICING: the
 //!   tree is compared with how it stood at the last look
 //!   (`State::structure_base`) once an event or an action has been
 //!   applied, and what differs is the step. There are a dozen writers of
@@ -92,6 +92,10 @@ pub struct LevelBefore {
 #[derive(Clone)]
 pub struct StructureStep {
     pub levels: Vec<LevelBefore>,
+    /// Nodes that had another name: (id, the name it was). Put back by
+    /// renaming, not by writing the name, so that what names the node —
+    /// wires, expression paths anywhere in the tree — follows it back.
+    pub renames: Vec<(String, String)>,
     pub what: String,
 }
 
@@ -200,6 +204,8 @@ fn ids_tell_apart(nodes: &[FsNode]) -> bool {
 pub struct Difference {
     /// The structure that changed, as it WAS: a step's content.
     pub levels: Vec<LevelBefore>,
+    /// The nodes renamed: (id, the name it was).
+    pub renames: Vec<(String, String)>,
     /// Whether anything differs at all, a parameter's value included —
     /// which is no step, and is when the base has to be taken again.
     pub any: bool,
@@ -217,7 +223,12 @@ impl Difference {
             [one] => one.clone(),
             many => format!("{} nodes", many.len()),
         };
-        if !self.added.is_empty() && self.removed.is_empty() {
+        if !self.renames.is_empty() && self.added.is_empty() && self.removed.is_empty() {
+            // The wires that named the node changed with it, and are part
+            // of the rename.
+            let names: Vec<String> = self.renames.iter().map(|(_, was)| was.clone()).collect();
+            format!("Rename {}", list(&names))
+        } else if !self.added.is_empty() && self.removed.is_empty() {
             format!("Add {}", list(&self.added))
         } else if !self.removed.is_empty() && self.added.is_empty() {
             format!("Delete {}", list(&self.removed))
@@ -236,7 +247,7 @@ impl Difference {
     /// else: a run of alt+hjkl is one step.
     pub fn move_group(&self) -> Option<String> {
         let only_moves = self.added.is_empty() && self.removed.is_empty() && self.wired == 0 && self.flagged == 0;
-        (only_moves && !self.moved.is_empty()).then(|| format!("move\u{0}{}", self.moved.join("\u{0}")))
+        (only_moves && self.renames.is_empty() && !self.moved.is_empty()).then(|| format!("move\u{0}{}", self.moved.join("\u{0}")))
     }
 }
 
@@ -273,6 +284,9 @@ pub fn difference(base: &FsNode, now: &FsNode, out: &mut Difference) {
                 wires: changed_wires,
             });
         }
+        if was.name != is.name {
+            out.renames.push((was.id.clone(), was.name.clone()));
+        }
         out.any |= was.name != is.name
             || was.params.len() != is.params.len()
             || was.params.iter().zip(is.params.iter()).any(|(a, b)| a.name != b.name || !same(a, b));
@@ -342,7 +356,24 @@ pub fn restore(root: &mut FsNode, step: StructureStep) -> StructureStep {
         }
         levels.push(LevelBefore { dir_id: level.dir_id, nodes });
     }
-    StructureStep { levels, what: step.what }
+    // The names last, and by renaming: every wire and every expression
+    // path that names the node is written back with it. After the wires
+    // the step holds, or what is filed for redo would be those wires as
+    // the rename had just left them. Last renamed, first put back.
+    let mut renames = Vec::new();
+    for (id, was) in step.renames.into_iter().rev() {
+        let Some(is) = crate::viewer_state::find_node_by_id(root, &id).map(|n| n.name.clone()) else { continue };
+        // A sibling has taken the name since: two nodes of one name would
+        // leave every wire to either naming both. The node keeps the name
+        // it has.
+        let taken = crate::geometry::find_parent_node(root, &id)
+            .is_some_and(|p| p.children.iter().any(|c| c.id != id && c.name == was));
+        if !taken && crate::geometry::rename_node_in_tree(root, &id, &was) {
+            renames.push((id, is));
+        }
+    }
+    renames.reverse();
+    StructureStep { levels, renames, what: step.what }
 }
 
 impl State {
@@ -378,9 +409,10 @@ impl State {
         if !diff.any {
             return;
         }
-        if !diff.levels.is_empty() {
+        if !diff.levels.is_empty() || !diff.renames.is_empty() {
             let group = diff.move_group();
-            let step = Step::Structure(StructureStep { what: diff.what(), levels: diff.levels });
+            let step =
+                Step::Structure(StructureStep { what: diff.what(), levels: diff.levels, renames: diff.renames });
             match group {
                 Some(group) => self.edit_history.record_grouped(step, group),
                 None => self.edit_history.record(step),
@@ -480,10 +512,23 @@ impl State {
                     .selected_node()
                     .and_then(|i| self.current_dir().children.get(i))
                     .map(|n| n.id.clone());
+                // The active camera is a name, which a rename changes.
+                let camera = self
+                    .current_dir()
+                    .children
+                    .iter()
+                    .find(|c| c.node_type == "camera" && c.name == self.active_camera)
+                    .map(|c| c.id.clone());
                 let inverse = restore(&mut self.fs_root, step);
                 self.edit_history.file(undo, Step::Structure(inverse));
                 self.current_path = self.slots_along(&path);
                 self.current_path2 = self.slots_along(&path2);
+                let camera = camera
+                    .and_then(|id| crate::viewer_state::find_node_by_id(&self.fs_root, &id))
+                    .map(|n| n.name.clone());
+                if let Some(name) = camera {
+                    self.set_active_camera(name);
+                }
                 let slot = selected.and_then(|id| self.current_dir().children.iter().position(|n| n.id == id));
                 self.graph_mut().set_selected_node(slot);
                 self.slots.content2.set_selected_node(None);
diff --git a/src/main.rs b/src/main.rs
index 5c4e7d8..77b8c49 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -6755,9 +6755,13 @@ mod tests {
         assert!(state.run_command("undo"));
         assert_eq!(texts(&state), edited);
 
-        // A rename between the reset and the undo: the step is by id.
+        // A rename between the reset and the undo: the step is by id, and
+        // is reached under the rename, which is a step of its own.
         state.run_command("reset_parameters");
-        state.current_dir_mut().children[slot].name = "ball".to_string();
+        let id = state.current_dir().children[slot].id.clone();
+        crate::geometry::rename_node_in_tree(&mut state.fs_root, &id, "ball");
+        assert!(state.history_step(true));
+        assert_ne!(state.current_dir().children[slot].name, "ball");
         assert!(state.history_step(true));
         assert_eq!(texts(&state), edited);
 
@@ -7017,6 +7021,88 @@ mod tests {
         assert_eq!(state.edit_history.undo_len(), 0, "New Project was recorded as an edit");
     }
 
+    /// A rename is taken back with everything that named the node: the
+    /// wires to it, the expression paths through it wherever they stand,
+    /// and the active camera.
+    #[test]
+    fn a_rename_is_undone_with_what_names_the_node() {
+        use crate::app::McpAction;
+        let mut state = State::new(false);
+        let mut redraw = false;
+        let add = |state: &mut State, template: &str, x: f32| {
+            state
+                .apply_action(McpAction::AddNode { template_name: template.into(), name: None, x, y: 9.0 }, &mut false)
+                .unwrap();
+            state.current_dir().children.len() - 1
+        };
+        let sphere = state.current_dir().children.iter().position(|c| c.node_type == "sphere").unwrap();
+        let old = state.current_dir().children[sphere].name.clone();
+        let normal = add(&mut state, "Normal", 3.0);
+        let embryo = add(&mut state, "Embryo", 5.0);
+        state.apply_action(McpAction::SetParam { slot: normal, name: "Input".into(), value: old.clone() }, &mut redraw).unwrap();
+        // An expression a level down, reaching up and across to the sphere.
+        let reference = format!("ch(\"../../{old}/Radius\") * 2");
+        let inside = state.current_dir().children[embryo]
+            .children
+            .iter()
+            .position(|c| !c.params.is_empty())
+            .expect("the Embryo has a child with parameters");
+        {
+            let inner = &mut state.current_dir_mut().children[embryo].children[inside];
+            let p = &mut inner.params[0];
+            p.set_text(reference.clone());
+            p.set_expr(true);
+        }
+        let camera = state.current_dir().children.iter().position(|c| c.node_type == "camera").unwrap();
+        let camera_name = state.current_dir().children[camera].name.clone();
+        state.set_active_camera(camera_name.clone());
+        state.record_structure_changes();
+        state.edit_history.clear();
+
+        let names = |state: &State| -> (String, String, String, String, String) {
+            let dir = state.current_dir();
+            (
+                dir.children[sphere].name.clone(),
+                dir.children[normal].params.iter().find(|p| p.name == "Input").unwrap().text().to_string(),
+                dir.children[embryo].children[inside].params[0].text().to_string(),
+                state.active_camera.clone(),
+                state.viewport().active_camera.clone(),
+            )
+        };
+        let before = names(&state);
+        assert_eq!(before.2, reference);
+
+        state.apply_action(McpAction::RenameNode { slot: sphere, new_name: "Ball".into() }, &mut redraw).unwrap();
+        state.apply_action(McpAction::RenameNode { slot: camera, new_name: "lens".into() }, &mut redraw).unwrap();
+        let after = names(&state);
+        assert_eq!(after.0, "ball");
+        assert_eq!(after.1, "ball", "the wire followed the rename");
+        assert!(after.2.contains("../../ball/Radius"), "{}", after.2);
+        assert_eq!((after.3.as_str(), after.4.as_str()), ("lens", "lens"), "both copies of the camera's name");
+        assert_eq!(state.edit_history.undo_len(), 2, "each rename is one step, its wires with it");
+
+        assert!(state.run_command("undo"));
+        assert!(state.last_status_text.contains(&format!("Undo Rename {camera_name}")), "{}", state.last_status_text);
+        assert_eq!(names(&state).3, camera_name);
+        assert_eq!(names(&state).4, camera_name);
+        assert!(state.run_command("undo"));
+        assert_eq!(names(&state), before);
+        assert!(state.run_command("redo"));
+        assert!(state.run_command("redo"));
+        assert_eq!(names(&state), after);
+
+        // A name taken since is not taken twice: the node keeps its own.
+        assert!(state.run_command("undo"));
+        assert!(state.run_command("undo"));
+        state.apply_action(McpAction::RenameNode { slot: sphere, new_name: "ball".into() }, &mut redraw).unwrap();
+        state.apply_action(McpAction::RenameNode { slot: normal, new_name: old.clone() }, &mut redraw).unwrap();
+        state.edit_history.take(true);
+        assert!(state.history_step(true), "the step is taken");
+        let dir = state.current_dir();
+        assert_eq!(dir.children[sphere].name, "ball", "the sphere took a name its sibling has");
+        assert_eq!(dir.children[normal].name, old);
+    }
+
     /// New Project from the palette starts a project. The command named a
     /// label no arm dispatched, so the row ran and nothing happened.
     #[test]
diff --git a/src/window.rs b/src/window.rs
index 29ebab3..db2673b 100644
--- a/src/window.rs
+++ b/src/window.rs
@@ -502,7 +502,9 @@ impl State {
                     // the expressions anywhere in the tree, the active camera.
                     crate::geometry::rename_node_in_tree(&mut state.fs_root, &id, &new_name);
                     if state.active_camera == old_name {
-                        state.active_camera = new_name.clone();
+                        // Both copies of the name: the viewport's routes
+                        // the wheel by its own.
+                        state.set_active_camera(new_name.clone());
                     }
                     state.sync_nodes();
                     // Connections reference nodes by name (Input params), so a