Variables: Undo and Redo clear a highlight that names no variable

Undo, Redo and a canceled undo group put back an earlier document but
left the Variables panel highlight as it was. After renaming the
highlighted variable and undoing, the highlight still named the new
name: a variable defined later under that name showed highlighted
without a click, and Delete removed it.

Each of these paths now clears a highlight whose variable the restored
document does not define, as Delete Variables and Load Variable Library
already did.

crates/engine/src/cmd/edit.rs

@@ -498,6 +498,7 @@ fn undo(s: &mut Session, _: &Value) -> Result<Value> {
     super::symboledit::follow_history(st, editing_symbol.as_ref());
     leave_isolation_if_gone(st);
     super::globaledit::follow_history(st);
+    st.drop_stale_variables_highlight();
     Ok(json!({ "undone": label }))
 }
 
@@ -525,6 +526,7 @@ fn redo(s: &mut Session, _: &Value) -> Result<Value> {
     super::symboledit::follow_history(st, editing_symbol.as_ref());
     leave_isolation_if_gone(st);
     super::globaledit::follow_history(st);
+    st.drop_stale_variables_highlight();
     Ok(json!({ "redone": label }))
 }
 

crates/engine/src/cmd/variablecmds.rs

@@ -231,9 +231,7 @@ fn delete(s: &mut Session, p: &Value) -> Result<Value> {
     let n = s.edit("Delete Variables", |d, _| Ok(d.variables.prune(&names)))?;
     // A highlight of a variable that is gone highlights nothing.
     let st = s.doc_mut()?;
-    if st.variables_highlight.as_ref().is_some_and(|h| st.doc.variables.variable(h).is_none()) {
-        st.variables_highlight = None;
-    }
+    st.drop_stale_variables_highlight();
     st.revision += 1;
     Ok(json!({"deleted": n}))
 }

crates/engine/src/cmd/variablecmds/library.rs

@@ -277,9 +277,7 @@ pub(super) fn load_library(s: &mut Session, p: &Value) -> Result<Value> {
     })?;
     // A highlight of a variable that is gone highlights nothing.
     let st = s.doc_mut()?;
-    if st.variables_highlight.as_ref().is_some_and(|h| st.doc.variables.variable(h).is_none()) {
-        st.variables_highlight = None;
-    }
+    st.drop_stale_variables_highlight();
     st.revision += 1;
     let skipped: Vec<Value> = lib.other.iter().map(|(name, t)| json!({"name": name, "trait": t})).collect();
     let mut out = json!({

crates/engine/src/doc_state.rs

@@ -352,6 +352,14 @@ impl DocState {
             self.revision += 1;
         }
     }
+    /// Clear the Variables panel highlight when the document defines no variable of that name
+    /// (deleted, or taken out by Undo or Redo), so a variable defined later under that name is
+    /// not highlighted without a click.
+    pub(crate) fn drop_stale_variables_highlight(&mut self) {
+        if self.variables_highlight.as_ref().is_some_and(|h| self.doc.variables.variable(h).is_none()) {
+            self.variables_highlight = None;
+        }
+    }
     /// Count the document as modified, as if never saved (a document restored by Data Recovery).
     pub fn mark_unsaved(&mut self) {
         self.saved_doc = Arc::new(Document::new(1.0, 1.0));

crates/engine/src/session/execute.rs

@@ -219,6 +219,7 @@ impl Session {
                     st.interaction = interaction;
                     st.history = history;
                     cmd::globaledit::follow_history(st);
+                    st.drop_stale_variables_highlight();
                     st.revision += 1;
                 }
                 if let Some(i) = active.and_then(|uid| self.docs.iter().position(|d| d.uid == uid)) {

crates/engine/src/session/undo.rs

@@ -102,6 +102,7 @@ impl Session {
                 deselect_hidden(&self.menu, &st.doc, &mut st.selection);
                 st.sync_new_art();
                 cmd::globaledit::follow_history(st);
+                st.drop_stale_variables_highlight();
                 st.revision += 1;
                 // The journal drops only the canceled groups' own entries and keeps the ones made in other documents meanwhile.
                 journal.extend(g.journal);
@@ -230,6 +231,7 @@ impl Session {
         st.doc = doc;
         st.selection = selection;
         cmd::globaledit::follow_history(st);
+        st.drop_stale_variables_highlight();
         deselect_hidden(&menu, &st.doc, &mut st.selection);
         st.history.recost_last(&st.doc);
         st.revision += 1;

crates/engine/src/tests/variables.rs

@@ -893,6 +893,49 @@ fn highlight_follows_rename_and_clears_with_delete() {
     assert!(s.doc().unwrap().variables_highlight.is_none(), "a highlight of a deleted variable highlights nothing");
 }
 
+/// Undo of a rename takes the new name away, so the highlight that followed the rename goes
+/// too: a variable defined later under that name is not highlighted, and Delete without names
+/// does not delete it.
+#[test]
+fn undoing_a_rename_clears_the_highlight_of_the_new_name() {
+    let mut s = session();
+    s.execute("variable.define", &json!({"name": "A", "kind": "text"})).unwrap();
+    s.execute("variable.highlight", &json!({"variable": "A"})).unwrap();
+    s.execute("variable.rename", &json!({"name": "A", "newName": "B"})).unwrap();
+    s.execute("edit.undo", &json!({})).unwrap();
+    assert!(s.doc().unwrap().variables_highlight.is_none());
+    s.execute("variable.define", &json!({"name": "B", "kind": "visibility"})).unwrap();
+    assert!(s.doc().unwrap().variables_highlight.is_none(), "B was never clicked");
+    assert!(s.execute("variable.delete", &json!({})).is_err(), "no variable highlighted");
+    let vars = s.execute("variable.list", &json!({})).unwrap();
+    assert_eq!(vars["variables"].as_array().unwrap().len(), 2);
+}
+
+/// Undo of New Variable, Redo of Delete Variables and a canceled undo group each leave the
+/// highlighted name undefined, and the highlight goes with it.
+#[test]
+fn undo_redo_and_a_canceled_group_clear_a_highlight_of_a_variable_they_remove() {
+    let mut s = session();
+    s.execute("variable.define", &json!({"name": "N", "kind": "text"})).unwrap();
+    s.execute("variable.highlight", &json!({"variable": "N"})).unwrap();
+    s.execute("edit.undo", &json!({})).unwrap();
+    assert!(s.doc().unwrap().variables_highlight.is_none(), "undo of New Variable");
+
+    s.execute("edit.redo", &json!({})).unwrap();
+    s.execute("variable.delete", &json!({"names": ["N"]})).unwrap();
+    s.execute("edit.undo", &json!({})).unwrap();
+    s.execute("variable.highlight", &json!({"variable": "N"})).unwrap();
+    s.execute("edit.redo", &json!({})).unwrap();
+    assert!(s.doc().unwrap().variables_highlight.is_none(), "redo of Delete Variables");
+
+    s.execute("edit.undoGroup.begin", &json!({})).unwrap();
+    s.execute("variable.define", &json!({"name": "G", "kind": "text"})).unwrap();
+    s.execute("variable.highlight", &json!({"variable": "G"})).unwrap();
+    s.execute("edit.undoGroup.end", &json!({"cancel": true})).unwrap();
+    assert!(s.doc().unwrap().doc.variables.variable("G").is_none(), "the cancel took G back");
+    assert!(s.doc().unwrap().variables_highlight.is_none(), "a canceled undo group");
+}
+
 /// A row that hides an object must be able to show it again, and hiding it on the artboard
 /// (as the Layers panel does) is what a capture records: a visibility variable reads and
 /// writes its object's own visibility, which therefore never makes the object off limits.