From 2fce1a9b420d44ff95fea1dba7f35748ce039807 Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Sun, 11 Oct 2026 21:38:58 -0400 Subject: [PATCH] Objects: every kind follows edits; chart series follow cut-moves and sheet deletes Structural edits and sorts kept three hand-indexed lists (charts, images, shapes) in step in two places, and slicers were left out of both, so a slicer stayed put when rows were inserted above it. Chart series were rewritten for inserted and deleted lines and for sheet renames, but not for cut-moves or sheet deletes, which left them pointing at the old cells. - Inserting and deleting rows and columns and sorting walk `Sheet::object_anchors` and `object_anchors_mut`, one fixed order over every kind, so slicers now follow their cells by their anchor mode like the other objects, and a new kind cannot be forgotten in one of the passes. - `rewrite_cell_links` is the one place that rewrites the references floating objects hold (chart series values, categories and names, and the source block a chart was made from). `rewrite_all_formulas` calls it, which covers inserting and deleting lines and cells and cut-moves on one sheet or across sheets; renaming and deleting a sheet call it too, so a series into a deleted sheet becomes #REF!, as a formula does. - Copying a sheet gives its charts, pictures, shapes and slicers fresh ids, keeping ids unique across the workbook, which object selection and the `{kind, id}` commands rely on. Tests: every kind through row and column inserts and deletes, an absolute shape, a sort and undo; a slicer through inserted rows; chart series through a cut-move, a sheet rename, a sheet delete and undo; ids after a sheet copy. --- ROADMAP.md | 2 +- crates/engine/src/cmd/data/sort.rs | 38 +++------ crates/engine/src/cmd/edit.rs | 2 +- crates/engine/src/cmd/edit/paste_special.rs | 28 +++++++ crates/engine/src/cmd/sheet.rs | 44 +++-------- crates/engine/src/cmd/sheet/tabs.rs | 25 +++--- crates/engine/src/tests/objects.rs | 87 +++++++++++++++++++++ 7 files changed, 155 insertions(+), 71 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 43af2299b..8afe566c6 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -135,7 +135,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | What landed | |---|---| -| 2026-10-31 | Floating objects: charts, pictures, shapes and slicers share one selection with a frame of eight resize handles (each moves only its own edges, also on right-to-left sheets); Delete or Backspace deletes the selected object and Escape drops the selection; `ui.inspect` reports `selectedObject` and `ui.object.select` selects any object (#190, #213) | +| 2026-10-31 | Floating objects: charts, pictures, shapes and slicers share one selection with a frame of eight resize handles (each moves only its own edges, also on right-to-left sheets); Delete or Backspace deletes the selected object and Escape drops the selection; slicers follow row and column edits and sorts like the other objects; chart series follow cut-moves and sheet renames and deletes; a copied sheet gives its objects fresh ids; `ui.inspect` reports `selectedObject` and `ui.object.select` selects any object (#190, #213) | | 2026-10-25 | AutoSum over a selection does what Excel does: a selected row gets one total to its right (in its last cell if that is empty), not a one-cell total under each cell; a block whose last column is empty gets each row's total there, one whose last row is empty each column's, and with both the corner gets the total of the block; otherwise totals go under each column as before | | 2026-10-24 | Macro interface ([macros.md](docs/macros.md#in-the-window)): an Automate › Macros group (Record Macro, which turns into Stop Recording, Use Relative References, Run Macro with recent macros); "Recording · N steps" with Stop in the status bar; a Record Macro dialog (checked name, free Ctrl+Shift shortcut, description); the Macros dialog lists macros beside Automate scripts and a VBA project, with Run, Step Into, Edit, Create, Delete and Options; Ctrl+Shift+letter macro shortcuts; questions that offer Run, Overwrite, Shift Down or Shift Right; a step editor with a breakpoint gutter, drag to reorder, a command picker, reference boxes that switch between relative tokens and absolute references, Inputs and Output tabs, a debug bar (Check, Run, Step, Continue, Run to Here, Stop with keep or undo) and grid overlays for inputs, writes, conflicts and the current step | | 2026-10-24 | Go To lists the named ranges of every sheet (`Sheet2!Name` for a name scoped to another sheet; hidden names, constants and formulas left out) and the references entered before, a double-click goes to an entry, and the Reference box takes several names or references separated by commas (`Price, Type`, `B14:C22,F19:G30`) | diff --git a/crates/engine/src/cmd/data/sort.rs b/crates/engine/src/cmd/data/sort.rs index a36527650..d335a7695 100644 --- a/crates/engine/src/cmd/data/sort.rs +++ b/crates/engine/src/cmd/data/sort.rs @@ -195,8 +195,7 @@ fn do_sort(s: &mut Session, sheet: usize, r: RangeRef, header: bool, keys: Vec = - sh.images.iter().map(|o| o.anchor).chain(sh.charts.iter().map(|o| o.anchor)).chain(sh.shapes.iter().map(|o| o.anchor)).collect(); + let objs: Vec = sh.object_anchors().copied().collect(); let descs: Vec<(u32, u32, f32, f32)> = objs .iter() .map(|a| { @@ -242,33 +241,22 @@ fn do_sort(s: &mut Session, sheet: usize, r: RangeRef, header: bool, keys: Vec o1 { - continue; - } - let na = sorted_anchor(shm, a, desc, by_cols, b0, b1, &dst_of); - if na != a { - if i < ni { - if let Some(o) = shm.images.get_mut(i) { - o.anchor = na; - } - } else if i < ni + nch { - if let Some(o) = shm.charts.get_mut(i - ni) { - o.anchor = na; - } - } else if let Some(o) = shm.shapes.get_mut(i - ni - nch) { - o.anchor = na; - } - } + let new_anchors: Vec = objs + .into_iter() + .zip(descs) + .map(|(a, desc)| { + let other = if by_cols { a.cell.row } else { a.cell.col }; + if other < o0 || other > o1 { a } else { sorted_anchor(shm, a, desc, by_cols, b0, b1, &dst_of) } + }) + .collect(); + for (a, n) in shm.object_anchors_mut().zip(new_anchors) { + *a = n; } for (c, _) in moved { cx.changed.push((sheet, c)); diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 27e217474..4b5c98a8b 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -35,7 +35,7 @@ use go_to::go_to_special; #[cfg(test)] use go_to::{GO_TO_MAX_AREAS, GO_TO_RECENT, reference_parts}; use paste_special::paste_special; -pub(crate) use paste_special::rewrite_all_formulas; +pub(crate) use paste_special::{rewrite_all_formulas, rewrite_cell_links}; use selection::{select_all, select_col, select_region, select_row, selection_move, selection_next, selection_set}; pub fn specs() -> Vec { diff --git a/crates/engine/src/cmd/edit/paste_special.rs b/crates/engine/src/cmd/edit/paste_special.rs index 0a6d14247..6c3f6baa4 100644 --- a/crates/engine/src/cmd/edit/paste_special.rs +++ b/crates/engine/src/cmd/edit/paste_special.rs @@ -355,4 +355,32 @@ pub(crate) fn rewrite_all_formulas(wb: &mut gridcraft_model::Workbook, target: & n.formula = gridcraft_formula::print(&new); } } + rewrite_cell_links(wb, |host, expr| gridcraft_formula::adjust::adjust(expr, host, target, e)); +} + +/// Runs `f` over every reference a floating object holds (a chart's series values, categories +/// and name, and the source block it was made from). Every edit that moves references calls it: +/// inserting and deleting lines and cells and cut-moves (through [`rewrite_all_formulas`]), and +/// renaming and deleting sheets. `f` maps a reference held by an object on the sheet named `host` +/// to its new form; text that does not parse is kept. A source block that no longer exists is +/// dropped. +pub(crate) fn rewrite_cell_links(wb: &mut gridcraft_model::Workbook, f: impl Fn(&str, gridcraft_formula::Expr) -> gridcraft_formula::Expr) { + for si in 0..wb.sheets.len() { + let Some(host) = wb.sheets.get(si).filter(|s| !s.charts.is_empty()).map(|s| s.name.clone()) else { continue }; + let Some(sh) = wb.sheet_mut(si) else { continue }; + let fix = |text: &str| -> String { + match gridcraft_formula::parse(text) { + Ok(expr) => gridcraft_formula::print(&f(&host, expr)), + Err(_) => text.to_string(), + } + }; + for ch in sh.charts.iter_mut() { + ch.source = ch.source.as_deref().map(fix).filter(|s| !s.contains("#REF!")); + for se in ch.series.iter_mut() { + se.values = fix(&se.values); + se.categories = se.categories.as_deref().map(fix); + se.name = se.name.as_deref().map(fix); + } + } + } } diff --git a/crates/engine/src/cmd/sheet.rs b/crates/engine/src/cmd/sheet.rs index 32122abd6..fa9c915c7 100644 --- a/crates/engine/src/cmd/sheet.rs +++ b/crates/engine/src/cmd/sheet.rs @@ -141,14 +141,7 @@ fn shift_sheet_features(sh: &mut Sheet, axis: Axis, at: u32, count: u32, insert: }; // Floating objects: snapshot their bottom-right corners against the pre-shift geometry, so a // re-fit after the shift measures against the grid the object actually lands on. - let obj_corners: Vec<(u32, u32, f32, f32)> = sh - .charts - .iter() - .map(|o| &o.anchor) - .chain(sh.images.iter().map(|o| &o.anchor)) - .chain(sh.shapes.iter().map(|o| &o.anchor)) - .map(|a| anchor_corner(sh, a)) - .collect(); + let obj_corners: Vec<(u32, u32, f32, f32)> = sh.object_anchors().map(|a| anchor_corner(sh, a)).collect(); // Line info. let lines = if axis == Axis::Rows { &mut sh.rows } else { &mut sh.cols }; let old = std::mem::take(lines); @@ -193,22 +186,13 @@ fn shift_sheet_features(sh: &mut Sheet, axis: Axis, at: u32, count: u32, insert: std::mem::take(&mut sh.cell_pictures).into_iter().filter_map(|(c, v)| map_cell(c).filter(CellRef::is_valid).map(|c| (c, v))).collect(); sh.comments = std::mem::take(&mut sh.comments).into_iter().filter_map(|(c, v)| map_cell(c).map(|c| (c, v))).collect(); sh.hyperlinks = std::mem::take(&mut sh.hyperlinks).into_iter().filter_map(|(c, v)| map_cell(c).map(|c| (c, v))).collect(); - // Floating objects follow their anchor lines per their [`AnchorMode`]. Corners were snapshotted - // as charts ++ images ++ shapes, matching the three slices below. New anchors are computed - // against the post-shift grid first (the re-fit needs its sizes), then written back. - let anchored = |ui: usize, a: Anchor| -> Anchor { obj_corners.get(ui).map_or(a, |&corner| shift_anchor(sh, a, corner, axis, at, &map_line)) }; - let new_charts: Vec = sh.charts.iter().enumerate().map(|(i, o)| anchored(i, o.anchor)).collect(); - let new_images: Vec = sh.images.iter().enumerate().map(|(i, o)| anchored(new_charts.len() + i, o.anchor)).collect(); - let base = new_charts.len() + new_images.len(); - let new_shapes: Vec = sh.shapes.iter().enumerate().map(|(i, o)| anchored(base + i, o.anchor)).collect(); - for (o, a) in sh.charts.iter_mut().zip(new_charts) { - o.anchor = a; - } - for (o, a) in sh.images.iter_mut().zip(new_images) { - o.anchor = a; - } - for (o, a) in sh.shapes.iter_mut().zip(new_shapes) { - o.anchor = a; + // Floating objects follow their anchor lines per their [`AnchorMode`]. New anchors are + // computed against the post-shift grid first (the re-fit needs its sizes), then written back + // in the same `object_anchors` order the corners were snapshotted in. + let new_anchors: Vec = + sh.object_anchors().zip(&obj_corners).map(|(a, &corner)| shift_anchor(sh, *a, corner, axis, at, &map_line)).collect(); + for (a, n) in sh.object_anchors_mut().zip(new_anchors) { + *a = n; } sh.sparklines.retain_mut(|sp| match map_cell(sp.cell) { Some(c) => { @@ -231,7 +215,8 @@ fn shift_sheet_features(sh: &mut Sheet, axis: Axis, at: u32, count: u32, insert: } } -/// Rewrites every formula reference (and chart series) for a structural edit on `target`. +/// Rewrites every formula reference (chart series included), sparkline source and PivotTable +/// source for a structural edit on `target`. pub(super) fn rewrite(cx: &mut Ctx, target: &str, e: &Edit) { cx.edits.push((target.to_string(), e.clone())); let wb = &mut cx.wb; @@ -245,15 +230,6 @@ pub(super) fn rewrite(cx: &mut Ctx, target: &str, e: &Edit) { Err(_) => text.to_string(), } }; - for ch in sh.charts.iter_mut() { - // The source block moves with its cells; one that was deleted is dropped. - ch.source = ch.source.as_deref().map(fix).filter(|s| !s.contains("#REF!")); - for se in ch.series.iter_mut() { - se.values = fix(&se.values); - se.categories = se.categories.as_deref().map(fix); - se.name = se.name.as_deref().map(fix); - } - } for sp in sh.sparklines.iter_mut() { sp.source = fix(&sp.source); } diff --git a/crates/engine/src/cmd/sheet/tabs.rs b/crates/engine/src/cmd/sheet/tabs.rs index 13956e35c..2f87294bb 100644 --- a/crates/engine/src/cmd/sheet/tabs.rs +++ b/crates/engine/src/cmd/sheet/tabs.rs @@ -63,6 +63,8 @@ pub(super) fn delete_sheet(s: &mut Session, p: &Json) -> Result { } } } + // Chart series into the deleted sheet become #REF!, like formulas. + crate::cmd::edit::rewrite_cell_links(&mut cx.wb, |_, e| gridcraft_formula::adjust::delete_sheet(e, &name, &order)); cx.wb.names.retain(|n| n.scope != Some(i)); for n in cx.wb.names.iter_mut() { if let Some(sc) = n.scope.as_mut() @@ -116,17 +118,7 @@ pub(super) fn rename_sheet(s: &mut Session, p: &Json) -> Result { } } } - let fix = |t: &str| { - gridcraft_formula::parse(t) - .map(|e| gridcraft_formula::print(&gridcraft_formula::adjust::rename_sheet(e, &old, &name))) - .unwrap_or_else(|_| t.to_string()) - }; for ch in sh.charts.iter_mut() { - for se in ch.series.iter_mut() { - se.values = fix(&se.values); - se.categories = se.categories.as_deref().map(fix); - se.name = se.name.as_deref().map(fix); - } // A PivotChart follows its PivotTable's sheet. if let Some((sheet, pivot)) = ch.pivot.as_deref().and_then(crate::pivot::parse_chart_link) && sheet.eq_ignore_ascii_case(&old) @@ -135,6 +127,7 @@ pub(super) fn rename_sheet(s: &mut Session, p: &Json) -> Result { } } } + crate::cmd::edit::rewrite_cell_links(&mut cx.wb, |_, e| gridcraft_formula::adjust::rename_sheet(e, &old, &name)); for n in cx.wb.names.iter_mut() { if let Ok(e) = gridcraft_formula::parse_name(&n.formula) { n.formula = gridcraft_formula::print(&gridcraft_formula::adjust::rename_sheet(e, &old, &name)); @@ -170,6 +163,18 @@ pub(super) fn move_sheet(s: &mut Session, p: &Json) -> Result { sh.name = name; sh.tables.clear(); // table names must be unique sh.preserved = None; // kept file content (form controls, OLE objects…) stays with the original + // Object ids are unique across the workbook (selection and the `{kind, id}` commands + // rely on it), so the copied objects get fresh ones. + let mut next = cx.wb.next_object_id(); + let mut fresh = || { + let id = next; + next = next.saturating_add(1); + id + }; + sh.charts.iter_mut().for_each(|o| o.id = fresh()); + sh.images.iter_mut().for_each(|o| o.id = fresh()); + sh.shapes.iter_mut().for_each(|o| o.id = fresh()); + sh.slicers.iter_mut().for_each(|o| o.id = fresh()); let at = to.min(n); // The copy gets its own copies of the sheet's names, referring to the copy. let copies: Vec = cx diff --git a/crates/engine/src/tests/objects.rs b/crates/engine/src/tests/objects.rs index ea4dfb0e8..d0c2eba44 100644 --- a/crates/engine/src/tests/objects.rs +++ b/crates/engine/src/tests/objects.rs @@ -103,3 +103,90 @@ fn hostile_object_params_are_refused() { let a = sheet(&s, 0).object_anchor(o).unwrap(); assert!(a.dx.is_finite() && a.dy.is_finite() && a.width <= 10000.0 && a.height >= 0.0, "{a:?}"); } + +fn cell_of(s: &Session, o: ObjRef) -> String { + sheet(s, 0).object_anchor(o).unwrap().cell.a1() +} + +#[test] +fn every_kind_follows_row_and_column_edits_and_sorts() { + let mut s = s(); + let objs = every_kind(&mut s); + // Anchors: chart D2, picture D18, shape H18, slicer L2. + s.execute("object.setAnchorMode", json!({"kind": "shape", "id": objs[2].id, "mode": "absolute"})).unwrap(); + let cells = |s: &Session| objs.map(|o| cell_of(s, o)); + assert_eq!(cells(&s), ["D2", "D18", "H18", "L2"]); + s.execute("home.insertRows", json!({"rows": "10:11"})).unwrap(); + assert_eq!(cells(&s), ["D2", "D20", "H18", "L2"], "rows inserted above move the objects below them; an absolute shape stays"); + s.execute("home.insertColumns", json!({"cols": "C:C"})).unwrap(); + assert_eq!(cells(&s), ["E2", "E20", "H18", "M2"]); + s.execute("home.deleteColumns", json!({"cols": "C:C"})).unwrap(); + s.execute("home.deleteRows", json!({"rows": "10:11"})).unwrap(); + assert_eq!(cells(&s), ["D2", "D18", "H18", "L2"]); + // Sorting moves a picture anchored inside the sorted block with its row. + s.execute("range.setValues", json!({"range": "T1", "values": [["k"], [3], [1], [2]]})).unwrap(); + let pic = ObjRef::new(ObjKind::Image, super::add_image(&mut s, "T2", AnchorMode::MoveAndSize)); + s.execute("selection.set", json!({"range": "T1:T4"})).unwrap(); + s.execute("data.sortAscending", json!({"header": true, "column": "T"})).unwrap(); + assert_eq!(cell_of(&s, pic), "T4", "the row holding 3 sorts last"); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(cell_of(&s, pic), "T2"); +} + +#[test] +fn slicers_follow_inserted_rows_like_other_objects() { + let mut s = s(); + let objs = every_kind(&mut s); + s.execute("home.insertRows", json!({"rows": "1:2"})).unwrap(); + assert_eq!(cell_of(&s, objs[3]), "L4"); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(cell_of(&s, objs[3]), "L2"); +} + +#[test] +fn chart_series_follow_cut_moves_renames_and_sheet_deletes() { + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [[3], [5], [2], [8]]})).unwrap(); + let chart = id_of(&s.execute("insert.chart", json!({"range": "A1:A4", "type": "column"})).unwrap(), "chart"); + let values = |s: &Session| sheet(s, 0).charts.iter().find(|c| c.id == chart).map(|c| c.series[0].values.clone()).unwrap_or_default(); + s.execute("edit.cut", json!({"range": "A1:A4"})).unwrap(); + s.execute("edit.paste", json!({"at": "F2"})).unwrap(); + assert!(values(&s).contains("$F$2:$F$5"), "a cut-move takes the series with the cells: {}", values(&s)); + s.execute("home.insertSheet", json!({"name": "Other"})).unwrap(); + s.execute("range.setValues", json!({"range": "A1", "values": [[1], [2], [3]]})).unwrap(); + s.execute("sheet.activate", json!({"sheet": 0})).unwrap(); + std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb).sheet_mut(0).unwrap().charts[0].series[0].values = "Other!$A$1:$A$3".into(); + s.execute("sheet.rename", json!({"sheet": 1, "name": "Inputs"})).unwrap(); + assert_eq!(values(&s), "Inputs!$A$1:$A$3"); + s.execute("home.deleteSheet", json!({"sheet": 1})).unwrap(); + assert!(values(&s).contains("#REF!"), "a series into a deleted sheet becomes #REF!: {}", values(&s)); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(values(&s), "Inputs!$A$1:$A$3"); +} + +#[test] +fn a_copied_sheet_gives_its_objects_fresh_ids() { + let mut s = s(); + let objs = every_kind(&mut s); + s.execute("sheet.move", json!({"sheet": 0, "to": 1, "copy": true})).unwrap(); + let wb = &s.doc().unwrap().wb; + let ids_of = |sh: &Sheet| -> Vec { + sh.charts + .iter() + .map(|c| c.id) + .chain(sh.images.iter().map(|c| c.id)) + .chain(sh.shapes.iter().map(|c| c.id)) + .chain(sh.slicers.iter().map(|c| c.id)) + .collect() + }; + let mut ids: Vec = wb.sheets.iter().flat_map(|sh| ids_of(sh)).collect(); + assert_eq!(ids.len(), 2 * objs.len()); + ids.sort_unstable(); + ids.dedup(); + assert_eq!(ids.len(), 2 * objs.len(), "every object id stays unique across the workbook"); + let copy = sheet(&s, 1); + for (o, id) in objs.iter().zip(ids_of(copy)) { + assert_eq!(copy.object_ref(id).map(|c| c.kind), Some(o.kind)); + } + assert!(s.doc().unwrap().wb.next_object_id() > *ids.last().unwrap_or(&0)); +}