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

@@ -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)

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]