mirror of
https://github.com/storytold/gridcraft.git
synced 2026-10-24 10:46:50 -06:00
Memory: undo steps share picture and macro bytes instead of copying them
Every edit copies the workbook for its undo step (copy-on-write: sheets and 64-row bands of cells are shared). A floating picture's bytes (Image.data) and the VBA project (Workbook.vba_project) were plain Vec<u8>, so each edit on a sheet with a picture, and every edit in a workbook with macros, kept another full copy of them, up to the 100 steps of the history. They are Arc<Vec<u8>> now, as cell pictures already are, with base64 helpers for serde (b64::shared, b64::opt). JSON and XLSX are unchanged. Measured on macOS 27 (Apple M3 Max), gridcraft-cli, kernel peak footprint: a 19 MB picture and 50 cell edits on its sheet, 992 -> 22 MB; 10 edits, 215 -> 21 MB. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
bb685e3275
commit
a09d3f6934
+3
-1
@@ -2,7 +2,7 @@
|
||||
|
||||
**Stage: alpha** · next: beta, ~25 points of "ready for real work" and ~160–240 h away
|
||||
|
||||
> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** minor (rows auto-fit oversized fonts on open and whole-row/column formatting) · **Target:** Microsoft Excel (Microsoft 365)
|
||||
> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-16 · **Change:** minor (undo steps share picture and macro bytes) · **Target:** Microsoft Excel (Microsoft 365)
|
||||
|
||||
GridCraft aims at full Microsoft Excel parity, and at being better in places: faster, open (XLSX
|
||||
is the native format), fully scriptable by agents (MCP, control channel, CLI), and on the web.
|
||||
@@ -132,6 +132,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in
|
||||
|
||||
| Date | What landed |
|
||||
|---|---|
|
||||
| 2026-10-16 | Memory: undo steps share a sheet's picture bytes and the workbook's VBA project instead of copying them: 50 edits on a sheet with a 20 MB picture keep 22 MB instead of 992 MB |
|
||||
| 2026-10-12 | Rows auto-fit oversized fonts: opening a file and whole-row/column formatting grow non-custom rows to their tallest font (#833) |
|
||||
| 2026-10-12 | Number formats: parsing a format code is linear in its length (long runs of `[`, `,` or `/` no longer take quadratic time), and an unclosed `[` is a literal when splitting sections as well as when tokenizing (#810) |
|
||||
| 2026-10-12 | Number formats: a fraction denominator with 65,536 zeros no longer crashes; format codes are capped at 255 characters like Excel (longer codes display as General, `TEXT` gives `#VALUE!`) (#807) |
|
||||
@@ -193,6 +194,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in
|
||||
|
||||
| Date | Change | Summary |
|
||||
|---|---|---|
|
||||
| 2026-10-16 | minor | Progress log: undo steps share picture and macro bytes |
|
||||
| 2026-10-12 | minor | Rows auto-fit oversized fonts on open and whole-row/column formatting (#833); progress log row |
|
||||
| 2026-10-12 | minor | Progress log: multi-area row/column delete (#758) |
|
||||
| 2026-10-12 | trivial | Progress log: XLSX chart sheets kept on save (#727) |
|
||||
|
||||
@@ -977,7 +977,7 @@ fn insert_picture(s: &mut Session, p: &Json) -> Result<Json> {
|
||||
// Excel inserts pictures as "Move but don't size with cells" (`editAs="oneCell"`).
|
||||
mode: anchor_mode_param(p).unwrap_or(gridcraft_model::AnchorMode::MoveOnly),
|
||||
},
|
||||
data,
|
||||
data: data.into(),
|
||||
mime,
|
||||
alt: str_param(p, "alt").unwrap_or("").into(),
|
||||
};
|
||||
|
||||
@@ -2000,7 +2000,7 @@ mod tests {
|
||||
wb.sheet_mut(0).unwrap().images.push(Image {
|
||||
id,
|
||||
anchor: Anchor { cell: CellRef::new(5, 5), dx: 0.0, dy: 0.0, width: 40.0, height: 40.0, mode: gridcraft_model::AnchorMode::MoveAndSize },
|
||||
data: png,
|
||||
data: png.into(),
|
||||
mime: "image/png".into(),
|
||||
alt: String::new(),
|
||||
});
|
||||
|
||||
@@ -517,7 +517,7 @@ mod tests {
|
||||
#[test]
|
||||
fn macros_are_saved_only_into_xlsm() {
|
||||
let mut wb = Workbook::new();
|
||||
wb.vba_project = Some(b"vba".to_vec());
|
||||
wb.vba_project = Some(b"vba".to_vec().into());
|
||||
let has_macros = |path: &str| {
|
||||
let bytes = save_bytes(&wb, path).unwrap();
|
||||
gridcraft_xlsx::read_xlsx(&bytes).unwrap().0.vba_project.is_some()
|
||||
|
||||
@@ -2549,7 +2549,7 @@ fn add_image(s: &mut Session, cell: &str, mode: gridcraft_model::AnchorMode) ->
|
||||
wb.sheet_mut(0).unwrap().images.push(gridcraft_model::Image {
|
||||
id,
|
||||
anchor: gridcraft_model::Anchor { cell: CellRef::parse(cell).unwrap(), dx: 0.0, dy: 0.0, width: 40.0, height: 40.0, mode },
|
||||
data: tiny_png(),
|
||||
data: tiny_png().into(),
|
||||
mime: "image/png".into(),
|
||||
alt: String::new(),
|
||||
});
|
||||
@@ -2560,6 +2560,35 @@ fn image_cell(s: &Session, id: u32) -> Option<CellRef> {
|
||||
s.doc().unwrap().wb.active().unwrap().images.iter().find(|i| i.id == id).map(|i| i.anchor.cell)
|
||||
}
|
||||
|
||||
/// Every undo step holds a copy of the workbook. A picture's bytes and the VBA project are shared
|
||||
/// between those copies instead of copied into each: a 20 MB picture used to cost another 20 MB
|
||||
/// for every cell typed on its sheet, up to 100 steps (2 GB).
|
||||
#[test]
|
||||
fn undo_steps_share_picture_and_macro_bytes() {
|
||||
let mut s = s();
|
||||
s.execute("insert.picture", json!({"base64": crate::io::base64_encode(&tiny_png()), "at": "C3"})).unwrap();
|
||||
std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb).vba_project = Some(b"vba".to_vec().into());
|
||||
for i in 1..=3 {
|
||||
s.execute("cell.set", json!({"cell": format!("A{i}"), "input": i.to_string()})).unwrap();
|
||||
}
|
||||
let d = s.doc().unwrap();
|
||||
let live = &d.wb.active().unwrap().images[0].data;
|
||||
let vba = d.wb.vba_project.as_ref().unwrap();
|
||||
assert_eq!(**live, tiny_png());
|
||||
assert!(d.undo.len() >= 3);
|
||||
for step in d.undo.iter().filter(|h| !h.wb.active().unwrap().images.is_empty()) {
|
||||
assert!(std::sync::Arc::ptr_eq(&step.wb.active().unwrap().images[0].data, live), "{}", step.label);
|
||||
}
|
||||
for step in d.undo.iter().rev().take(3) {
|
||||
assert!(std::sync::Arc::ptr_eq(step.wb.vba_project.as_ref().unwrap(), vba), "{}", step.label);
|
||||
}
|
||||
// Undo and redo still give the picture back as it was.
|
||||
s.execute("edit.undo", json!({"steps": 3})).unwrap();
|
||||
assert_eq!(*s.doc().unwrap().wb.active().unwrap().images[0].data, tiny_png());
|
||||
s.execute("edit.redo", json!({})).unwrap();
|
||||
assert_eq!(*s.doc().unwrap().wb.active().unwrap().images[0].data, tiny_png());
|
||||
}
|
||||
|
||||
/// Issue #5: images anchored in cells follow their row when the sheet is sorted.
|
||||
#[test]
|
||||
fn sort_moves_anchored_images_with_their_rows() {
|
||||
|
||||
+22
-4
@@ -49,22 +49,40 @@ pub fn deserialize<'de, D: Deserializer<'de>>(d: D) -> Result<Vec<u8>, D::Error>
|
||||
decode(&s).ok_or_else(|| serde::de::Error::custom("invalid base64"))
|
||||
}
|
||||
|
||||
/// Base64 for optional byte blobs (VBA projects): `null`/missing stays `None`.
|
||||
/// Base64 for byte blobs shared through `Arc` (floating pictures), so copies of a workbook, such as
|
||||
/// its undo steps, share one payload.
|
||||
pub mod shared {
|
||||
use std::sync::Arc;
|
||||
|
||||
use serde::{Deserializer, Serializer};
|
||||
|
||||
pub fn serialize<S: Serializer>(v: &Arc<Vec<u8>>, s: S) -> Result<S::Ok, S::Error> {
|
||||
super::serialize(v, s)
|
||||
}
|
||||
|
||||
pub fn deserialize<'de, D: Deserializer<'de>>(d: D) -> Result<Arc<Vec<u8>>, D::Error> {
|
||||
super::deserialize(d).map(Arc::new)
|
||||
}
|
||||
}
|
||||
|
||||
/// Base64 for optional shared byte blobs (VBA projects): `null`/missing stays `None`.
|
||||
pub mod opt {
|
||||
use std::sync::Arc;
|
||||
|
||||
use serde::{Deserialize, Deserializer, Serializer};
|
||||
|
||||
pub fn serialize<S: Serializer>(v: &Option<Vec<u8>>, s: S) -> Result<S::Ok, S::Error> {
|
||||
pub fn serialize<S: Serializer>(v: &Option<Arc<Vec<u8>>>, s: S) -> Result<S::Ok, S::Error> {
|
||||
match v {
|
||||
Some(data) => s.serialize_str(&super::encode(data)),
|
||||
None => s.serialize_none(),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn deserialize<'de, D: Deserializer<'de>>(d: D) -> Result<Option<Vec<u8>>, D::Error> {
|
||||
pub fn deserialize<'de, D: Deserializer<'de>>(d: D) -> Result<Option<Arc<Vec<u8>>>, D::Error> {
|
||||
let s = Option::<String>::deserialize(d)?;
|
||||
match s {
|
||||
None => Ok(None),
|
||||
Some(s) => super::decode(&s).map(Some).ok_or_else(|| serde::de::Error::custom("invalid base64")),
|
||||
Some(s) => super::decode(&s).map(|v| Some(Arc::new(v))).ok_or_else(|| serde::de::Error::custom("invalid base64")),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -508,9 +508,10 @@ pub struct Chart {
|
||||
pub struct Image {
|
||||
pub id: u32,
|
||||
pub anchor: Anchor,
|
||||
/// Encoded bytes (PNG/JPEG), base64 in JSON.
|
||||
#[serde(with = "crate::b64")]
|
||||
pub data: Vec<u8>,
|
||||
/// Encoded bytes (PNG/JPEG), base64 in JSON. Shared, so a copy of the sheet (an undo step)
|
||||
/// doesn't copy the picture.
|
||||
#[serde(with = "crate::b64::shared")]
|
||||
pub data: std::sync::Arc<Vec<u8>>,
|
||||
pub mime: String,
|
||||
pub alt: String,
|
||||
}
|
||||
|
||||
@@ -106,9 +106,10 @@ pub struct Workbook {
|
||||
pub cell_styles: Vec<(String, crate::style::Style)>,
|
||||
/// VBA project (`xl/vbaProject.bin`) carried through unchanged. GridCraft does not run
|
||||
/// macros; the blob is preserved so that saving a macro-enabled workbook never destroys the
|
||||
/// macros it arrived with. base64 in JSON. `None` = the workbook has no macros.
|
||||
/// macros it arrived with. base64 in JSON. `None` = the workbook has no macros. Shared, so a
|
||||
/// copy of the workbook (every edit's undo step) doesn't copy it.
|
||||
#[serde(default, with = "crate::b64::opt")]
|
||||
pub vba_project: Option<Vec<u8>>,
|
||||
pub vba_project: Option<Arc<Vec<u8>>>,
|
||||
/// Field separator of the CSV this workbook was read from, so saving it as CSV writes the same
|
||||
/// separator back (a `;` file stays a `;` file). `None` = not read from CSV: CSV saves use `,`.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
|
||||
@@ -750,7 +750,7 @@ fn column_autocomplete_can_be_turned_off() {
|
||||
/// An .xlsm carrying a VBA project, saved to bytes.
|
||||
fn macro_workbook_bytes() -> Vec<u8> {
|
||||
let mut wb = gridcraft_engine::model::Workbook::new();
|
||||
wb.vba_project = Some(b"\xd0\xcf\x11\xe0\xa1\xb1\x1a\xe1 fake OLE2 vbaProject".to_vec());
|
||||
wb.vba_project = Some(b"\xd0\xcf\x11\xe0\xa1\xb1\x1a\xe1 fake OLE2 vbaProject".to_vec().into());
|
||||
gridcraft_engine::io::save_bytes(&wb, "macros.xlsm").unwrap()
|
||||
}
|
||||
|
||||
@@ -798,7 +798,7 @@ fn plain_open_warning_becomes_an_info_notice() {
|
||||
fn macros_dialog_lists_vba_and_scripts() {
|
||||
let mut s = blank();
|
||||
let mut wb = gridcraft_engine::model::Workbook::new();
|
||||
wb.vba_project = Some(vec![0xd0, 0xcf, 0x11, 0xe0, 0xa1, 0xb1, 0x1a, 0xe1]);
|
||||
wb.vba_project = Some(vec![0xd0, 0xcf, 0x11, 0xe0, 0xa1, 0xb1, 0x1a, 0xe1].into());
|
||||
s.active_mut().unwrap().wb = std::sync::Arc::new(wb);
|
||||
let mut h = harness(s);
|
||||
h.state_mut().open_dialog("macros", json!({}));
|
||||
@@ -814,7 +814,7 @@ fn macros_dialog_lists_vba_and_scripts() {
|
||||
fn status_bar_shows_macro_badge() {
|
||||
let mut s = blank();
|
||||
let mut wb = gridcraft_engine::model::Workbook::new();
|
||||
wb.vba_project = Some(vec![0xd0, 0xcf, 0x11, 0xe0, 0xa1, 0xb1, 0x1a, 0xe1]);
|
||||
wb.vba_project = Some(vec![0xd0, 0xcf, 0x11, 0xe0, 0xa1, 0xb1, 0x1a, 0xe1].into());
|
||||
s.active_mut().unwrap().wb = std::sync::Arc::new(wb);
|
||||
// Rendering the status bar must not panic with a VBA project present.
|
||||
let mut h = harness(s);
|
||||
|
||||
@@ -156,7 +156,7 @@ pub fn read_drawing(cx: &mut Ctx<'_>, part: &str, sheet: &mut Sheet) -> Result<(
|
||||
};
|
||||
let alt = obj.path(&["nvPicPr", "cNvPr"]).and_then(|c| c.attr("descr")).unwrap_or("").to_string();
|
||||
let id = cx.next_object_id();
|
||||
sheet.images.push(Image { id, anchor, data, mime: mime_for(&target).to_string(), alt });
|
||||
sheet.images.push(Image { id, anchor, data: data.into(), mime: mime_for(&target).to_string(), alt });
|
||||
}
|
||||
"graphicFrame" => {
|
||||
let Some(gd) = obj.path(&["graphic", "graphicData"]) else { continue };
|
||||
|
||||
@@ -331,7 +331,7 @@ pub fn read_xlsx(bytes: &[u8]) -> Result<(Workbook, ReadReport), IoError> {
|
||||
if let Some(r) = find_rel(&wb_rels, "vbaProject").cloned() {
|
||||
match cx.pkg.read(&r.target) {
|
||||
Ok(Some(data)) => {
|
||||
wb.vba_project = Some(data);
|
||||
wb.vba_project = Some(data.into());
|
||||
cx.warn("macros (VBA project) were kept but not executed");
|
||||
}
|
||||
Ok(None) => cx.warn("macros (VBA project) reference a missing part and were dropped"),
|
||||
|
||||
@@ -502,7 +502,7 @@ pub fn write_sheet(wb: &Workbook, si: usize, selected: bool, out: &mut Out) -> (
|
||||
let ext = image_ext(&img.mime, &img.data);
|
||||
let name = format!("xl/media/image{}.{ext}", out.images);
|
||||
out.default_type(ext, image_content_type(ext));
|
||||
out.part(&name, None, img.data.clone());
|
||||
out.part(&name, None, img.data.to_vec());
|
||||
let rid = drels.add("image", &format!("../media/image{}.{ext}", out.images));
|
||||
objs.push(Obj::Image(img, rid));
|
||||
}
|
||||
|
||||
@@ -381,7 +381,7 @@ fn drawing_with_picture_and_chart() {
|
||||
let (wb, _) = read_xlsx(&bytes).unwrap();
|
||||
let s = wb.sheet(0).unwrap();
|
||||
let img = &s.images[0];
|
||||
assert_eq!(img.data, png);
|
||||
assert_eq!(*img.data, png);
|
||||
assert_eq!(img.mime, "image/png");
|
||||
assert_eq!(img.alt, "logo");
|
||||
assert_eq!(img.anchor.cell, at("B3"));
|
||||
|
||||
@@ -326,7 +326,7 @@ fn sample() -> Workbook {
|
||||
id: 10,
|
||||
// A non-default anchor mode, to prove it survives an xlsx round-trip.
|
||||
anchor: Anchor { cell: at("C10"), dx: 5.0, dy: 4.0, width: 120.0, height: 80.0, mode: AnchorMode::MoveOnly },
|
||||
data: png,
|
||||
data: png.into(),
|
||||
mime: "image/png".into(),
|
||||
alt: "Logo \"x\"".into(),
|
||||
});
|
||||
@@ -737,7 +737,7 @@ fn vba_project_roundtrip() {
|
||||
// carries the macro-enabled content type plus the vbaProject relationship.
|
||||
let vba = b"\xd0\xcf\x11\xe0\xa1\xb1\x1a\xe1 fake OLE2 vbaProject".to_vec();
|
||||
let mut wb = Workbook::new();
|
||||
wb.vba_project = Some(vba.clone());
|
||||
wb.vba_project = Some(vba.clone().into());
|
||||
let bytes = write_xlsx(&wb).unwrap();
|
||||
assert_eq!(write_xlsx(&wb).unwrap(), bytes, "output is deterministic");
|
||||
|
||||
@@ -756,7 +756,7 @@ fn vba_project_roundtrip() {
|
||||
assert!(rels.contains("relationships/vbaProject"), "vbaProject relationship: {rels}");
|
||||
|
||||
let (back, rep) = read_xlsx(&bytes).unwrap();
|
||||
assert_eq!(back.vba_project.as_deref(), Some(vba.as_slice()), "vbaProject survives the read");
|
||||
assert_eq!(back.vba_project.as_deref(), Some(&vba), "vbaProject survives the read");
|
||||
assert!(rep.warnings.iter().any(|w| w.contains("not executed")), "{:?}", rep.warnings);
|
||||
|
||||
// A workbook without macros stays macro-free and free of VBA warnings.
|
||||
|
||||
@@ -404,7 +404,7 @@ pub fn write_xlsx(wb: &Workbook, kind: XlsxKind) -> Result<Vec<u8>, IoError> {
|
||||
// A macro-free package can't hold a VBA project (Excel refuses such a file).
|
||||
if let Some(data) = wb.vba_project.as_ref().filter(|_| kind.is_macro_enabled()) {
|
||||
wb_rels.add("vbaProject", "vbaProject.bin");
|
||||
out.part("xl/vbaProject.bin", Some(CT_VBA), data.clone());
|
||||
out.part("xl/vbaProject.bin", Some(CT_VBA), data.to_vec());
|
||||
}
|
||||
out.part("xl/_rels/workbook.xml.rels", None, wb_rels.xml().into_bytes());
|
||||
|
||||
|
||||
Reference in New Issue
Block a user