From 10838300f1bcfc69236ebe950542ce6601723baa Mon Sep 17 00:00:00 2001
From: Dan Green <danngreen1@gmail.com>
Date: Thu, 29 Oct 2026 07:05:25 -0700
Subject: [PATCH] Libraries: an add that cannot be placed makes no "My Library"

With no library yet, library.add made and saved "My Library" before it
looked for the group or item to place the new one by, so a bad `group`,
`before` or `after` failed but left an empty library behind. The new
library is now kept only once the item is in it.
---
 crates/engine/src/cmd/library.rs   | 25 +++++++++++++------------
 crates/engine/src/tests/library.rs | 24 ++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs
index 47a5f6e98..70aa9561f 100644
--- a/crates/engine/src/cmd/library.rs
+++ b/crates/engine/src/cmd/library.rs
@@ -808,14 +808,6 @@ fn delete(s: &mut Session, p: &Value) -> Result<Value> {
     ok()
 }
 
-/// The library `p` names, else the current one, else a new "My Library".
-fn target_library(s: &mut Session, p: &Value, cmd: &str) -> Result<String> {
-    if p.get("library").is_some_and(|v| !v.is_null()) || s.libraries.current().is_some() {
-        return library_id(s, p, cmd);
-    }
-    s.libraries.create("My Library")
-}
-
 /// `base`, or `base 2`, `base 3`… the first that `taken` doesn't hold.
 fn unique(base: &str, taken: impl Fn(&str) -> bool) -> String {
     std::iter::once(base.to_string()).chain((2..).map(|n| format!("{base} {n}"))).find(|n| !taken(n)).unwrap_or_else(|| base.to_string())
@@ -838,10 +830,9 @@ fn add(s: &mut Session, p: &Value) -> Result<Value> {
         }
     };
     let place = Placement::of(p, C)?;
-    let id = target_library(s, p, C)?;
     let kind_name = str_param(p, "kind").unwrap_or_default().to_string();
     let item_kind = kind.item_kind();
-    let r = s.libraries.change(&id, |lib| {
+    let put = |lib: &mut Library| {
         let full = |n: usize| if n >= MAX_ITEMS { Err(bad(C, format!("the library holds {MAX_ITEMS} items of this kind already"))) } else { Ok(()) };
         Ok(match item {
             Item::Graphic(mut g) => {
@@ -901,8 +892,18 @@ fn add(s: &mut Session, p: &Value) -> Result<Value> {
                 placed(lib, &place, ItemRef { kind: item_kind, item: name.clone() }, json!({ "name": name, "existing": false }))?
             }
         })
-    })?;
-    let mut r = r;
+    };
+    // Into the library `p` names, else the current one, else a new "My Library", which is kept
+    // only once the item is in it (a `group` or item to place it by that it lacks makes none).
+    let (id, mut r) = if p.get("library").is_some_and(|v| !v.is_null()) || s.libraries.current().is_some() {
+        let id = library_id(s, p, C)?;
+        let r = s.libraries.change(&id, put)?;
+        (id, r)
+    } else {
+        let mut lib = Library { name: "My Library".to_string(), ..Default::default() };
+        let r = put(&mut lib)?;
+        (s.libraries.insert(lib)?, r)
+    };
     r["library"] = json!(id);
     r["kind"] = json!(kind_name);
     Ok(r)
diff --git a/crates/engine/src/tests/library.rs b/crates/engine/src/tests/library.rs
index 34672008a..92c780e29 100644
--- a/crates/engine/src/tests/library.rs
+++ b/crates/engine/src/tests/library.rs
@@ -724,6 +724,30 @@ fn a_new_item_is_added_where_it_is_dropped() {
     assert_eq!(order(&mut s, "graphics", "id"), [g]);
 }
 
+/// With no library yet, an add that can't be placed makes no "My Library", in the session or in
+/// the library folder; one that can makes it.
+#[cfg(not(target_arch = "wasm32"))]
+#[test]
+fn an_add_that_cannot_be_placed_makes_no_library() {
+    let folder = vectorcraft_testkit::temp_dir("library-add-unplaced").to_string_lossy().to_string();
+    let mut s = session();
+    s.libraries.set_dir(Some(folder.clone()));
+    for bad in [json!({"kind": "fillColor", "color": "#808080", "group": "Nope"}), json!({"kind": "fillColor", "color": "#808080", "after": "Nope"})]
+    {
+        assert!(s.execute("library.add", &bad).is_err(), "{bad}");
+        assert_eq!(run(&mut s, "library.list", json!({}))["libraries"], json!([]), "{bad}");
+    }
+    let mut t = session();
+    t.libraries.set_dir(Some(folder.clone()));
+    assert_eq!(run(&mut t, "library.list", json!({}))["libraries"], json!([]), "no file was written");
+    let r = run(&mut s, "library.add", json!({"kind": "fillColor", "color": "#808080"}));
+    let list = run(&mut s, "library.list", json!({}));
+    assert_eq!((list["current"].clone(), list["libraries"][0]["name"].clone()), (r["library"].clone(), json!("My Library")));
+    let mut u = session();
+    u.libraries.set_dir(Some(folder));
+    assert_eq!(run(&mut u, "library.list", json!({}))["libraries"].as_array().map(Vec::len), Some(1));
+}
+
 /// A library goes out to a file with its art and groups and comes back as a new library, numbered
 /// when its name is taken; the graphic it brings places in a document (#926).
 #[test]