diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index b7a5ea709..41c2420d2 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -262,7 +262,7 @@ pub fn save_bytes_as(wb: &Workbook, path: &str, kind: FileKind) -> Result wb.sheet(sheet).map(|sh| csv(wb, sh, wb.csv_delimiter.unwrap_or(b','))).transpose().map(Option::unwrap_or_default), FileKind::Tsv => wb.sheet(sheet).map(|sh| csv(wb, sh, b'\t')).transpose().map(Option::unwrap_or_default), - FileKind::Json => serde_json::to_vec_pretty(wb).map_err(|e| EngineError::Other(e.to_string())), + FileKind::Json => crate::macros::json_bytes(wb), FileKind::Html => to_html(wb, sheet).map(String::into_bytes), } } diff --git a/crates/engine/src/macros/mod.rs b/crates/engine/src/macros/mod.rs index 96f6ad988..5819e79fd 100644 --- a/crates/engine/src/macros/mod.rs +++ b/crates/engine/src/macros/mod.rs @@ -101,7 +101,7 @@ pub fn with_names(wb: &Workbook) -> crate::Result> { // Only the numbered items are this build's: a hidden `_gc_macro__` name with some // other key stays, as `load` leaves it. let stored = |n: &DefinedName| macro_item(n).is_some_and(|(_, i)| i.is_some()); - if wb.macros.is_empty() && !wb.names.iter().any(stored) { + if wb.macros.is_empty() && wb.macros_unread.is_empty() && !wb.names.iter().any(stored) { return Ok(None); } let mut out = wb.clone(); @@ -115,23 +115,63 @@ pub fn with_names(wb: &Workbook) -> crate::Result> { // exactly what `set_many` would refuse, and no arithmetic depends on a number from the file. let blocked: HashSet = out.names.iter().filter(|n| !n.hidden && n.scope.is_none()).filter_map(|n| stash::item_key(&n.name)).collect(); let free = (0..=u64::MAX).map(|i| format!("{MACRO_KEY}{i}")).filter(|key| !blocked.contains(key)); - let items: Vec<(String, String)> = wb.macros.iter().zip(free).map(|(m, key)| (key, macro_json(m).to_string())).collect(); + // Macros in a newer format follow the readable ones, written back as the file held them. + let texts = wb.macros.iter().map(|m| macro_json(m).to_string()).chain(wb.macros_unread.iter().map(Json::to_string)); + let items: Vec<(String, String)> = texts.zip(free).map(|(text, key)| (key, text)).collect(); stash::set_many(&mut out, &items)?; // An empty list of its own, not a copy of the shared one cleared. out.macros = gridcraft_model::Macros::default(); + out.macros_unread = std::sync::Arc::default(); Ok(Some(out)) } -/// After a file is opened: moves macros out of hidden names into `Workbook::macros` and drops -/// macros that are damaged, over the limits or in a newer format. Returns a warning for what was dropped. +/// The JSON format's bytes for `wb`: the workbook, with the macros in a newer format put back into +/// the `macros` list as the file held them. +pub fn json_bytes(wb: &Workbook) -> crate::Result> { + let bytes = if wb.macros_unread.is_empty() { + serde_json::to_vec_pretty(wb) + } else { + serde_json::to_value(wb).and_then(|mut v| { + if let Some(list) = v.as_object_mut().map(|o| o.entry("macros").or_insert_with(|| Json::Array(vec![]))).and_then(Json::as_array_mut) { + list.extend(wb.macros_unread.iter().cloned()); + } + serde_json::to_vec_pretty(&v) + }) + }; + bytes.map_err(|e| crate::EngineError::Other(e.to_string())) +} + +/// Whether `v` is a macro in a newer format than this build reads. +fn is_newer(v: &Json) -> bool { + v.get("v").and_then(Json::as_u64).is_some_and(|found| found > u64::from(gridcraft_model::MACRO_VERSION)) +} + +/// Keeps a macro in a newer format unread, if it is small enough to save back +/// ([`stash::MAX_ITEM_CHARS`] characters of JSON, the size every xlsx save can write). +fn keep_unread(unread: &mut Vec, v: Json, bad: &mut usize) { + if v.to_string().chars().count() <= stash::MAX_ITEM_CHARS { + unread.push(v); + } else { + *bad = bad.saturating_add(1); + } +} + +/// After a file is opened: moves macros out of hidden names into `Workbook::macros`, keeps +/// macros in a newer format unread (`Workbook::macros_unread`), and drops macros that are damaged +/// or over the limits. Returns a warning for what was kept unread and one for what was dropped. pub fn load(wb: &mut Workbook) -> Vec { let mut warnings = Vec::new(); let mut bad = 0usize; + let mut unread = Vec::new(); // Entries of the JSON format's field, each read on its own so a damaged one drops alone. let read = std::mem::take(&mut wb.macros_read); bad = bad.saturating_add(read.len().saturating_sub(gridcraft_model::MAX_MACROS)); - for v in read.iter().take(gridcraft_model::MAX_MACROS) { - match parse_macro(v) { + for v in read.into_iter().take(gridcraft_model::MAX_MACROS) { + if is_newer(&v) { + keep_unread(&mut unread, v, &mut bad); + continue; + } + match parse_macro(&v) { Ok(m) => wb.macros.push(m), Err(_) => bad += 1, } @@ -155,7 +195,12 @@ pub fn load(wb: &mut Workbook) -> Vec { let mut item = Workbook::new(); for ((_, key), names) in stored.into_iter().take(gridcraft_model::MAX_MACROS) { item.names = names; - match stash::get_json(&item, &key).ok_or_else(|| "unreadable".to_string()).and_then(|v| parse_macro(&v)) { + let v = stash::get_json(&item, &key); + if let Some(v) = v.as_ref().filter(|v| is_newer(v)) { + keep_unread(&mut unread, v.clone(), &mut bad); + continue; + } + match v.ok_or_else(|| "unreadable".to_string()).and_then(|v| parse_macro(&v)) { Ok(m) => found.push(m), Err(_) => bad += 1, } @@ -183,9 +228,25 @@ pub fn load(wb: &mut Workbook) -> Vec { Err(_) => bad += 1, } } + // Readable and unread macros together stay within the limit, the readable ones first. + let room = gridcraft_model::MAX_MACROS.saturating_sub(wb.macros.len()); + bad = bad.saturating_add(unread.len().saturating_sub(room)); + unread.truncate(room); + if !unread.is_empty() { + let one = unread.len() == 1; + warnings.push(format!( + "{} macro{} in the workbook {} made by a newer version of GridCraft: this version can't show or run {}, and saving keeps {} unchanged.", + unread.len(), + if one { "" } else { "s" }, + if one { "was" } else { "were" }, + if one { "it" } else { "them" }, + if one { "it" } else { "them" }, + )); + } + wb.macros_unread = std::sync::Arc::new(unread); if bad > 0 { warnings.push(format!( - "{bad} macro{} in the workbook couldn't be opened: {} damaged, made by a newer version of GridCraft, or over the limits ({} macros, {} steps each, Repeat and If nested {} deep).", + "{bad} macro{} in the workbook couldn't be opened: {} damaged or over the limits ({} macros, {} steps each, Repeat and If nested {} deep).", if bad == 1 { "" } else { "s" }, if bad == 1 { "it is" } else { "they are" }, gridcraft_model::MAX_MACROS, diff --git a/crates/engine/src/macros/tests.rs b/crates/engine/src/macros/tests.rs index 2d55cfe09..80c1b0fb6 100644 --- a/crates/engine/src/macros/tests.rs +++ b/crates/engine/src/macros/tests.rs @@ -427,10 +427,11 @@ fn a_damaged_json_macro_drops_alone() { } #[test] -fn a_macro_in_a_newer_format_is_dropped_not_rewritten() { +fn a_macro_in_a_newer_format_is_kept_unread_and_saved_back_unchanged() { // A newer build's macro (`v` 2, with a field this build doesn't know) would lose that field - // here and be saved back as if complete, so it is dropped with the usual warning instead. - // Version 1 and a missing version are read as before. + // if it were read here, so it is never read: it is kept as the file held it, saved back + // unchanged, and the newer build gets it back whole. Version 1 and a missing version are + // read as before. let future = json!({"name": "Future", "v": MACRO_VERSION + 1, "fromALaterBuild": {"x": 1}, "steps": [{"command": "cell.set"}]}); let current = json!({"name": "Current", "v": MACRO_VERSION, "steps": [{"command": "cell.set"}]}); let missing = json!({"name": "Missing"}); @@ -443,17 +444,24 @@ fn a_macro_in_a_newer_format_is_dropped_not_rewritten() { let names: Vec<&str> = back.macros.iter().map(|m| m.name.as_str()).collect(); assert_eq!(names, ["Current", "Missing"], "{file}"); assert!(back.macros.iter().all(|m| m.v == MACRO_VERSION), "{file}"); - assert!(warnings.len() == 1 && warnings[0].starts_with("1 macro ") && warnings[0].contains("newer version"), "{file}: {warnings:?}"); - // Saving and opening again leaves no reduced copy of it behind. + assert_eq!(*back.macros_unread, std::slice::from_ref(&future), "{file}"); + assert!( + warnings.len() == 1 && warnings[0].starts_with("1 macro ") && warnings[0].contains("newer version") && warnings[0].contains("unchanged"), + "{file}: {warnings:?}" + ); + // Saved and opened again, it is still there, whole, and the others are as they were. let (again, warnings) = crate::io::open_bytes(file, &crate::io::save_bytes(back, file).unwrap()).unwrap(); - assert!(warnings.is_empty(), "{file}: {warnings:?}"); + assert_eq!(warnings.len(), 1, "{file}: {warnings:?}"); assert_eq!(again.macros, back.macros, "{file}"); + assert_eq!(again.macros_unread, back.macros_unread, "{file}"); }; // The JSON format's field. let mut v: serde_json::Value = serde_json::from_slice(&crate::io::save_bytes(&Workbook::new(), "book.json").unwrap()).unwrap(); v["macros"] = json!([future, current, missing]); let (back, warnings) = crate::io::open_bytes("book.json", v.to_string().as_bytes()).unwrap(); check("book.json", &back, &warnings); + let saved: serde_json::Value = serde_json::from_slice(&crate::io::save_bytes(&back, "book.json").unwrap()).unwrap(); + assert!(saved["macros"].as_array().unwrap().contains(&future), "the file holds it as it was"); // Hidden names in xlsx. let mut wb = Workbook::new(); for (i, m) in [&future, ¤t, &missing].into_iter().enumerate() { @@ -465,6 +473,42 @@ fn a_macro_in_a_newer_format_is_dropped_not_rewritten() { check("book.xlsx", &back, &warnings); } +#[test] +fn kept_macros_share_the_save_limits_and_the_numbering() { + // A newer macro too large for the file is left out (keeping it would make every xlsx save + // fail), and readable and unread macros together stay within the macro limit, readable ones + // first. Kept ones are numbered after the readable ones, around a person's visible name. + let newer = |name: &str| json!({"name": name, "v": MACRO_VERSION + 1}); + let mut big = newer("Big"); + big["pad"] = json!(long(crate::cmd::stash::MAX_ITEM_CHARS)); + let mut file: serde_json::Value = serde_json::from_slice(&crate::io::save_bytes(&Workbook::new(), "book.json").unwrap()).unwrap(); + let mut list = vec![big, newer("Kept"), json!({"name": "Read"})]; + file["macros"] = json!(list); + let (mut wb, warnings) = crate::io::open_bytes("book.json", file.to_string().as_bytes()).unwrap(); + assert_eq!(*wb.macros_unread, [newer("Kept")]); + assert_eq!(warnings.len(), 2, "one kept, one left out: {warnings:?}"); + wb.names.push(visible("_gc_macro_1_0")); + let (raw, back) = { + let bytes = crate::io::save_bytes(&wb, "book.xlsx").unwrap(); + let raw = names_of(&gridcraft_xlsx::read_xlsx(&bytes).unwrap().0); + (raw, crate::io::open_bytes("book.xlsx", &bytes).unwrap().0) + }; + assert_eq!(raw, ["_gc_macro_1_0", "_gc_macro_0_0", "_gc_macro_2_0"]); + assert_eq!((back.macros.len(), back.macros_unread.len()), (1, 1)); + // Past the limit the rest are left out; and when a JSON file's list is full, an unread item + // it also holds in hidden names is the one left out: readable macros come first. + list = (0..MAX_MACROS).map(|i| json!({"name": format!("M{i}")})).collect(); + let mut with_item = Workbook::new(); + with_item.names.push(hidden("_gc_macro_0_0", &newer("InNames").to_string())); + let mut file: serde_json::Value = serde_json::from_slice(&crate::io::save_bytes(&with_item, "book.json").unwrap()).unwrap(); + for extra in [None, Some(newer("Over"))] { + file["macros"] = json!(list.iter().cloned().chain(extra).collect::>()); + let (wb, warnings) = crate::io::open_bytes("book.json", file.to_string().as_bytes()).unwrap(); + assert_eq!((wb.macros.len(), wb.macros_unread.len()), (MAX_MACROS, 0)); + assert!(warnings.len() == 1 && warnings[0].contains("couldn't be opened"), "{warnings:?}"); + } +} + #[test] fn many_macros_with_one_name_all_survive_save_and_reopen() { // MAX_MACROS macros with the same name, at the length limit, so the suffixes run from " (2)" diff --git a/crates/model/src/workbook.rs b/crates/model/src/workbook.rs index 5a47b75ed..bafca88f4 100644 --- a/crates/model/src/workbook.rs +++ b/crates/model/src/workbook.rs @@ -188,6 +188,11 @@ pub struct Workbook { /// Never written. #[serde(rename = "macros", default, skip_serializing, deserialize_with = "crate::macros::entries")] pub macros_read: Vec, + /// Macros in a newer format than this build reads, as the file held them. They are never read, + /// listed or run; a save writes them back unchanged, so the newer build gets them back whole. + /// Shared between snapshots, as nothing changes them. + #[serde(skip)] + pub macros_unread: Arc>, /// Unmodelled file content kept for saving (package root and workbook level). Not in JSON. #[serde(skip)] pub preserved: Option>, @@ -236,6 +241,7 @@ impl Workbook { csv_delimiter: None, macros: crate::macros::Macros::default(), macros_read: vec![], + macros_unread: Arc::default(), preserved: None, source_dir: None, source_path: None, diff --git a/docs/macros.md b/docs/macros.md index 3fbd61b2d..ea3156c28 100644 --- a/docs/macros.md +++ b/docs/macros.md @@ -66,10 +66,12 @@ is someone's own name: opening a file leaves it alone, and saving stores the mac it does not use. Each macro carries its format version, `v` (1 in this version; a missing `v` reads as 1). When a -file is opened, a macro that is damaged, over the limits or in a newer format (a `v` above 1) is -left out with one warning, and the rest of the workbook and its other macros open. A newer -macro is never read in part, so a save can't write it back with its newer fields missing; a -macro that was left out is not in the saved file. +file is opened, a macro in a newer format (a `v` above 1) is kept unread, with a warning: this +version can't show or run it, and saving writes it back exactly as the file held it, so the newer +version gets it back whole. It is never read in part. Kept macros count toward the 1,000-macro +limit after the readable ones, and one too large for an xlsx file is left out. A macro that is +damaged or over the limits is left out with one warning, and the rest of the workbook and its +other macros open; a macro that was left out is not in the saved file. ## Automate scripts and VBA