Variables: variable.duplicate matches an exact id before a name

Duplicate group copies each member by id; an earlier variable named like that id was copied instead. Review item on #2087.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Model-Effort: Opus 5.5 drove and wrote (review fix, no brief)
This commit is contained in:
Trevor BrownandClaude Opus 5.5 committed 2026-11-10 01:49:05 -05:00
1 parent 4313a60fb6
commit 815cbf9fa1
2 files changed
+16 -1

No files matched your search

+4 -1
View File
@@ -184,7 +184,10 @@ fn duplicate(s: &mut Session, p: &Value) -> Result<Value> {
let cmd = "variable.duplicate";
let v = req_string(cmd, p, "variable")?;
let doc = s.doc_mut()?;
let pos = doc.variables.variables.iter().position(|x| x.id == v || x.name == v).ok_or_else(|| bad(cmd, "no such variable"))?;
// An exact id first, then a name: an earlier variable named like another's id must not be the
// one copied.
let vars = &doc.variables.variables;
let pos = vars.iter().position(|x| x.id == v).or_else(|| vars.iter().position(|x| x.name == v)).ok_or_else(|| bad(cmd, "no such variable"))?;
let mut copy = doc.variables.variables.get(pos).cloned().ok_or_else(|| bad(cmd, "no such variable"))?;
let taken = |n: &str| doc.variables.variables.iter().any(|x| x.collection == copy.collection && x.name == n);
let name = (1..1000)
@@ -47,6 +47,18 @@ fn rename_describe_scope_and_duplicate() {
assert_eq!(vars.get(&id).unwrap().scopes, brand.scopes);
}
#[test]
fn duplicate_by_id_copies_that_variable_even_when_an_earlier_one_is_named_like_its_id() {
let mut s = Session::new();
tokens(&mut s);
let brand = s.doc().unwrap().variables.get("brand").unwrap().id.clone();
// The first variable in the list takes the later one's id as its name.
ex(&mut s, "variable.update", json!({"variable": "space/sm", "name": brand.clone()}));
let id = ex(&mut s, "variable.duplicate", json!({"variable": brand.clone()}))["id"].as_str().unwrap().to_string();
let vars = &s.doc().unwrap().variables;
assert_eq!(vars.get(&id).unwrap().name, "brand copy");
}
#[test]
fn modes_and_collections_rename_and_delete() {
let mut s = Session::new();