diff --git a/crates/engine/src/macros/mod.rs b/crates/engine/src/macros/mod.rs index 6b2150e66..b74fc7e99 100644 --- a/crates/engine/src/macros/mod.rs +++ b/crates/engine/src/macros/mod.rs @@ -20,6 +20,10 @@ const MACRO_KEY: &str = "macro_"; /// Reads a macro from JSON, as `macro.save` and files give it. A command step may also use the /// short form `{"command": "cell.set", "params": {…}}`. pub fn parse_macro(v: &Json) -> Result { + // A newer format is refused before its steps are read, so the message says why. + if let Some(found) = v.get("v").and_then(Json::as_u64) { + gridcraft_model::check_version(found)?; + } let mut v = v.clone(); if let Some(steps) = v.get_mut("steps") { normalize_steps(steps, 1)?; @@ -99,7 +103,7 @@ pub fn with_names(wb: &Workbook) -> crate::Result> { } /// After a file is opened: moves macros out of hidden names into `Workbook::macros` and drops -/// macros that are damaged or over the limits. Returns a warning for what was dropped. +/// macros that are damaged, over the limits or in a newer format. Returns a warning for what was dropped. pub fn load(wb: &mut Workbook) -> Vec { let mut warnings = Vec::new(); let mut bad = 0usize; @@ -156,7 +160,7 @@ pub fn load(wb: &mut Workbook) -> Vec { } if bad > 0 { warnings.push(format!( - "{bad} macro{} in the workbook couldn't be opened: {} damaged or over the limits ({} macros, {} steps each, Repeat and If nested {} deep).", + "{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).", 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 9d5b90a37..63f8cef76 100644 --- a/crates/engine/src/macros/tests.rs +++ b/crates/engine/src/macros/tests.rs @@ -278,3 +278,42 @@ fn a_damaged_json_macro_drops_alone() { assert!(warnings.len() == 1 && warnings[0].starts_with("3 macros"), "{warnings:?}"); assert_eq!(back.sheet(0).unwrap().value(gridcraft_core::CellRef::new(0, 0)), gridcraft_core::Value::text("kept")); } + +#[test] +fn a_macro_in_a_newer_format_is_dropped_not_rewritten() { + // 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. + 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"}); + let err = parse_macro(&future).unwrap_err(); + assert!(err.contains("newer version") && err.contains("format 2"), "{err}"); + assert!(parse_macro(&json!({"name": "Huge", "v": u64::from(u32::MAX) + 1})).unwrap_err().contains("newer version")); + // A macro built in code goes through the same check on every path. + assert!(Macro { name: "Built".into(), v: MACRO_VERSION + 1, ..Macro::default() }.check_limits().is_err()); + let check = |file: &str, back: &Workbook, warnings: &[String]| { + 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. + let (again, warnings) = crate::io::open_bytes(file, &crate::io::save_bytes(back, file).unwrap()).unwrap(); + assert!(warnings.is_empty(), "{file}: {warnings:?}"); + assert_eq!(again.macros, back.macros, "{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); + // Hidden names in xlsx. + let mut wb = Workbook::new(); + for (i, m) in [&future, ¤t, &missing].into_iter().enumerate() { + wb.names.push(hidden(&format!("_gc_macro_{i}_0"), &m.to_string())); + } + let bytes = gridcraft_xlsx::write_xlsx(&wb).unwrap(); + let (back, warnings) = crate::io::open_bytes("book.xlsx", &bytes).unwrap(); + assert!(back.names.is_empty(), "{:?}", names_of(&back)); + check("book.xlsx", &back, &warnings); +} diff --git a/crates/model/src/macros.rs b/crates/model/src/macros.rs index 39f799615..8dc657f90 100644 --- a/crates/model/src/macros.rs +++ b/crates/model/src/macros.rs @@ -63,9 +63,10 @@ impl Macro { count(&self.steps) } - /// Checks the size limits ([`MAX_MACRO_STEPS`], [`MAX_MACRO_DEPTH`], name and text lengths) and - /// that the name has no control characters. + /// Checks the format version ([`check_version`]), the size limits ([`MAX_MACRO_STEPS`], + /// [`MAX_MACRO_DEPTH`], name and text lengths) and that the name has no control characters. pub fn check_limits(&self) -> Result<(), String> { + check_version(u64::from(self.v))?; let name = self.name.trim(); if name.is_empty() { return Err("A macro needs a name.".into()); @@ -119,6 +120,15 @@ impl Macro { } } +/// Refuses a format version newer than [`MACRO_VERSION`]: this build would drop the fields it +/// doesn't know, and a save would then write the rest back as if it were the whole macro. +pub fn check_version(v: u64) -> Result<(), String> { + if v > u64::from(MACRO_VERSION) { + return Err(format!("This macro was made by a newer version of GridCraft (macro format {v}); this version reads format {MACRO_VERSION}.")); + } + Ok(()) +} + /// Reads the `macros` field of a JSON workbook as raw entries, whatever they hold, so they can be /// checked one at a time. pub(crate) fn entries<'de, D: serde::Deserializer<'de>>(d: D) -> Result, D::Error> { diff --git a/docs/macros.md b/docs/macros.md index 99050b0aa..1c755e329 100644 --- a/docs/macros.md +++ b/docs/macros.md @@ -49,6 +49,12 @@ Macros are saved in the workbook: as a field in GridCraft's JSON format, and in defined names (`_gc_macro__`, the macro's JSON split into string literals within Excel's limits), which other spreadsheet applications keep but do not run. +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. + ## Automate scripts and VBA Automate scripts (`automate.recordActions`, `newScript`, `allScripts`, `runScript`,