mirror of
https://github.com/storytold/gridcraft.git
synced 2026-11-03 00:41:56 -06:00
Engine: an edit that would make Python code too long is refused
Structural edits, sheet renames and deletes, and a cut to another sheet rewrite the xl() references in Python code, and could make a cell's code longer than the 8,192-character limit that python.set enforces. Opening the saved file then dropped that code to its value. Such an edit is now refused with a message naming the cell, and nothing changes; keeping the old text would have left a reference the edit had moved.
This commit is contained in:
@@ -145,6 +145,7 @@ pub(super) fn paste_special(s: &mut Session, p: &Json) -> Result<Json> {
|
||||
}
|
||||
moved_formulas = taken;
|
||||
}
|
||||
let dest_name = cx.wb.sheet(dest_sheet).map(|sh| sh.name.clone()).unwrap_or_default();
|
||||
for (tr, tc, r, c) in tiled_cells(src.height(), src.width(), tiles_r, tiles_c) {
|
||||
let sc = CellRef::new(src.start.row + r, src.start.col + c);
|
||||
let (dr, dc) = if transpose { (c, r) } else { (r, c) };
|
||||
@@ -189,10 +190,10 @@ pub(super) fn paste_special(s: &mut Session, p: &Json) -> Result<Json> {
|
||||
// A Python cell's code travels with it. Cut to another sheet, it keeps reading the
|
||||
// cells its `xl()` references named on its original sheet, as a moved formula does:
|
||||
// they are qualified with that sheet before the workbook-wide move adjustment below.
|
||||
let moved_py = |cell: &Cell| -> Option<std::sync::Arc<gridcraft_model::PyCode>> {
|
||||
let py = cell.py.clone()?;
|
||||
let moved_py = |cell: &Cell| -> Result<Option<std::sync::Arc<gridcraft_model::PyCode>>> {
|
||||
let Some(py) = cell.py.clone() else { return Ok(None) };
|
||||
if !cut_move || dest_sheet == clip.sheet {
|
||||
return Some(py);
|
||||
return Ok(Some(py));
|
||||
}
|
||||
let qualified = gridcraft_model::python::rewrite_xl(&py.code, |reference| {
|
||||
let e = gridcraft_formula::parse(reference).ok()?.map(&mut |x| match x {
|
||||
@@ -204,10 +205,14 @@ pub(super) fn paste_special(s: &mut Session, p: &Json) -> Result<Json> {
|
||||
});
|
||||
Some(gridcraft_formula::print(&e))
|
||||
});
|
||||
Some(match qualified {
|
||||
Some(code) => std::sync::Arc::new(gridcraft_model::PyCode { code, ..(*py).clone() }),
|
||||
Ok(Some(match qualified {
|
||||
Some(code) => {
|
||||
let moved = gridcraft_model::PyCode { code, ..(*py).clone() };
|
||||
check_python_fits(&moved, &dest_name, dest)?;
|
||||
std::sync::Arc::new(moved)
|
||||
}
|
||||
None => py,
|
||||
})
|
||||
}))
|
||||
};
|
||||
if link {
|
||||
let sheet_prefix = if dest_sheet == clip.sheet && same_doc {
|
||||
@@ -226,7 +231,7 @@ pub(super) fn paste_special(s: &mut Session, p: &Json) -> Result<Json> {
|
||||
new_picture = src_picture.clone();
|
||||
new.formula = shift(&sc_cell);
|
||||
new.value = if new.formula.is_some() { Value::Empty } else { sc_cell.value.clone() };
|
||||
new.py = moved_py(&sc_cell);
|
||||
new.py = moved_py(&sc_cell)?;
|
||||
let st = map_style(sc_cell.style, &mut cx.wb);
|
||||
new.style = if what == What::AllExceptBorders {
|
||||
let borders = cx.wb.styles.get(old.style).borders;
|
||||
@@ -239,7 +244,7 @@ pub(super) fn paste_special(s: &mut Session, p: &Json) -> Result<Json> {
|
||||
new_picture = src_picture.clone();
|
||||
new.formula = shift(&sc_cell);
|
||||
new.value = if new.formula.is_some() { Value::Empty } else { sc_cell.value.clone() };
|
||||
new.py = moved_py(&sc_cell);
|
||||
new.py = moved_py(&sc_cell)?;
|
||||
if what == What::FormulasAndNumberFormats {
|
||||
let nf = src_wb.styles.get(sc_cell.style).num_fmt.clone();
|
||||
new.style = cx.wb.styles.derive(old.style, |s| s.num_fmt = nf);
|
||||
@@ -323,7 +328,7 @@ pub(super) fn paste_special(s: &mut Session, p: &Json) -> Result<Json> {
|
||||
} else {
|
||||
gridcraft_formula::adjust::Edit::MoveToSheet { from: src, to_sheet: dest_name, to_row: at.row, to_col: at.col }
|
||||
};
|
||||
rewrite_all_formulas(&mut cx.wb, &src_sheet_name, &e);
|
||||
rewrite_all_formulas(&mut cx.wb, &src_sheet_name, &e)?;
|
||||
cx.structural = true;
|
||||
}
|
||||
let end = at.offset_clamped((tiles_r * h) as i64 - 1, (tiles_c * w) as i64 - 1);
|
||||
@@ -347,7 +352,7 @@ pub(super) fn paste_special(s: &mut Session, p: &Json) -> Result<Json> {
|
||||
}
|
||||
|
||||
/// Applies a reference adjustment to every cell formula and defined name in the workbook.
|
||||
pub(crate) fn rewrite_all_formulas(wb: &mut gridcraft_model::Workbook, target: &str, e: &gridcraft_formula::adjust::Edit) {
|
||||
pub(crate) fn rewrite_all_formulas(wb: &mut gridcraft_model::Workbook, target: &str, e: &gridcraft_formula::adjust::Edit) -> Result<()> {
|
||||
use gridcraft_formula::adjust::Edit;
|
||||
// Rows and columns inserted or deleted on the target sheet move its legacy array ranges too
|
||||
// (a cut moves its arrays itself).
|
||||
@@ -386,14 +391,19 @@ pub(crate) fn rewrite_all_formulas(wb: &mut gridcraft_model::Workbook, target: &
|
||||
n.formula = gridcraft_formula::print(&new);
|
||||
}
|
||||
}
|
||||
rewrite_python_refs(wb, &|host, expr| gridcraft_formula::adjust::adjust(expr, host, target, e));
|
||||
rewrite_python_refs(wb, &|host, expr| gridcraft_formula::adjust::adjust(expr, host, target, e))
|
||||
}
|
||||
|
||||
/// Rewrites the `xl("…")` literals in every Python cell with `f`, which maps a reference held on
|
||||
/// sheet `host` to its new form. Every reference-moving edit (insert and delete of lines or cells,
|
||||
/// cut-move, sheet rename and delete) runs it, as it adjusts formulas. A literal is only replaced
|
||||
/// when the reference it names changed, and text that doesn't parse is kept.
|
||||
pub(crate) fn rewrite_python_refs(wb: &mut gridcraft_model::Workbook, f: &impl Fn(&str, gridcraft_formula::Expr) -> gridcraft_formula::Expr) {
|
||||
/// when the reference it names changed, and text that doesn't parse is kept. A rewrite that would
|
||||
/// make a cell's code longer than one cell holds (a longer sheet name, `Z1` becoming `AA1`) refuses
|
||||
/// the whole edit, which then changes nothing: see [`check_python_fits`].
|
||||
pub(crate) fn rewrite_python_refs(
|
||||
wb: &mut gridcraft_model::Workbook,
|
||||
f: &impl Fn(&str, gridcraft_formula::Expr) -> gridcraft_formula::Expr,
|
||||
) -> Result<()> {
|
||||
for si in 0..wb.sheets.len() {
|
||||
let Some(sh) = wb.sheets.get(si) else { continue };
|
||||
let host = sh.name.clone();
|
||||
@@ -407,6 +417,7 @@ pub(crate) fn rewrite_python_refs(wb: &mut gridcraft_model::Workbook, f: &impl F
|
||||
(after != before).then_some(after)
|
||||
});
|
||||
if let Some(code) = changed {
|
||||
check_python_fits(&gridcraft_model::PyCode { code: code.clone(), object: py.object, ..Default::default() }, &host, c)?;
|
||||
new_code.push((c, code));
|
||||
}
|
||||
}
|
||||
@@ -422,4 +433,20 @@ pub(crate) fn rewrite_python_refs(wb: &mut gridcraft_model::Workbook, f: &impl F
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Refuses Python code an edit has made longer than one cell holds: the limit `python.set`
|
||||
/// enforces and opening a file applies (code over it is dropped to its value on open). The edit
|
||||
/// is refused rather than made, so no code is lost and none keeps a reference the edit moved.
|
||||
pub(crate) fn check_python_fits(py: &gridcraft_model::PyCode, sheet: &str, at: CellRef) -> Result<()> {
|
||||
if py.fits() {
|
||||
return Ok(());
|
||||
}
|
||||
Err(EngineError::Other(format!(
|
||||
"The edit would make the Python code in {}!{} longer than Excel's {}-character formula limit, so nothing was changed. Shorten the code first.",
|
||||
gridcraft_formula::quote_sheet(sheet),
|
||||
at.a1(),
|
||||
gridcraft_model::python::MAX_FORMULA_CHARS
|
||||
)))
|
||||
}
|
||||
|
||||
@@ -301,7 +301,7 @@ fn insert_table(s: &mut Session, p: &Json) -> Result<Json> {
|
||||
let e = Edit::InsertCells { axis: Axis::Rows, range: RangeRef::new(r.start, CellRef::new(r.start.row, r.end.col)) };
|
||||
shift_cell_features(sh, &e);
|
||||
let name = sh.name.clone();
|
||||
rewrite(cx, &name, &e);
|
||||
rewrite(cx, &name, &e)?;
|
||||
let sh = cx.sheet_mut(sheet)?;
|
||||
for (i, c) in (r.start.col..=r.end.col).enumerate() {
|
||||
sh.set_value(CellRef::new(r.start.row, c), gridcraft_core::Value::text(format!("Column{}", i + 1)));
|
||||
|
||||
@@ -60,7 +60,7 @@ pub(super) fn insert_table(s: &mut Session, p: &Json) -> Result<Json> {
|
||||
let e = Edit::InsertCells { axis: Axis::Rows, range: RangeRef::new(r.start, CellRef::new(r.start.row, r.end.col)) };
|
||||
shift_cell_features(sh, &e);
|
||||
let name = sh.name.clone();
|
||||
rewrite(cx, &name, &e);
|
||||
rewrite(cx, &name, &e)?;
|
||||
let sh = cx.sheet_mut(sheet)?;
|
||||
for (i, c) in (r.start.col..=r.end.col).enumerate() {
|
||||
sh.set_value(CellRef::new(r.start.row, c), gridcraft_core::Value::text(format!("Column{}", i + 1)));
|
||||
|
||||
@@ -232,10 +232,10 @@ 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`.
|
||||
pub(super) fn rewrite(cx: &mut Ctx, target: &str, e: &Edit) {
|
||||
pub(super) fn rewrite(cx: &mut Ctx, target: &str, e: &Edit) -> Result<()> {
|
||||
cx.edits.push((target.to_string(), e.clone()));
|
||||
let wb = &mut cx.wb;
|
||||
super::edit::rewrite_all_formulas(wb, target, e);
|
||||
super::edit::rewrite_all_formulas(wb, target, e)?;
|
||||
for si in 0..wb.sheets.len() {
|
||||
let host = wb.sheets.get(si).map(|s| s.name.clone()).unwrap_or_default();
|
||||
let Some(sh) = wb.sheet_mut(si) else { continue };
|
||||
@@ -270,6 +270,7 @@ pub(super) fn rewrite(cx: &mut Ctx, target: &str, e: &Edit) {
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Inserting or deleting cells moves the copied block away from the stored source range, so, like
|
||||
@@ -327,7 +328,7 @@ fn insert_lines(s: &mut Session, p: &Json, axis: Axis) -> Result<Json> {
|
||||
// New lines take the format of the line before (Excel's "Format Same As Above").
|
||||
inherit_line_format(sh, axis, at, count);
|
||||
}
|
||||
rewrite(cx, &name, &Edit::Insert { axis, at, count });
|
||||
rewrite(cx, &name, &Edit::Insert { axis, at, count })?;
|
||||
cx.structural = true;
|
||||
Ok(Json::Null)
|
||||
})?;
|
||||
@@ -392,7 +393,7 @@ fn delete_lines(s: &mut Session, p: &Json, axis: Axis) -> Result<Json> {
|
||||
}
|
||||
shift_sheet_features(sh, axis, at, count, false);
|
||||
}
|
||||
rewrite(cx, &name, &Edit::Delete { axis, at, count });
|
||||
rewrite(cx, &name, &Edit::Delete { axis, at, count })?;
|
||||
}
|
||||
cx.structural = true;
|
||||
Ok(Json::Null)
|
||||
@@ -444,7 +445,7 @@ fn move_columns(s: &mut Session, p: &Json) -> Result<Json> {
|
||||
sh.cells.shift_cols(before, count as i64);
|
||||
shift_sheet_features(sh, Axis::Cols, before, count, true);
|
||||
}
|
||||
rewrite(cx, &name, &Edit::Insert { axis: Axis::Cols, at: before, count });
|
||||
rewrite(cx, &name, &Edit::Insert { axis: Axis::Cols, at: before, count })?;
|
||||
|
||||
let mut touched = Vec::new();
|
||||
{
|
||||
@@ -482,14 +483,14 @@ fn move_columns(s: &mut Session, p: &Json) -> Result<Json> {
|
||||
for cell in touched {
|
||||
cx.touch(sheet, cell);
|
||||
}
|
||||
rewrite(cx, &name, &Edit::Move { from: source, to_row: 0, to_col: destination });
|
||||
rewrite(cx, &name, &Edit::Move { from: source, to_row: 0, to_col: destination })?;
|
||||
|
||||
{
|
||||
let sh = cx.sheet_mut(sheet)?;
|
||||
sh.cells.shift_cols(delete_at + count, -(count as i64));
|
||||
shift_sheet_features(sh, Axis::Cols, delete_at, count, false);
|
||||
}
|
||||
rewrite(cx, &name, &Edit::Delete { axis: Axis::Cols, at: delete_at, count });
|
||||
rewrite(cx, &name, &Edit::Delete { axis: Axis::Cols, at: delete_at, count })?;
|
||||
*cx.sel = crate::Selection {
|
||||
active: CellRef::new(active_row, final_start),
|
||||
anchor: CellRef::new(active_row, final_start),
|
||||
@@ -534,7 +535,7 @@ fn insert_cells(s: &mut Session, p: &Json) -> Result<Json> {
|
||||
}
|
||||
shift_cell_features(sh, &e);
|
||||
let name = cx.wb.sheet(sheet).map(|s| s.name.clone()).unwrap_or_default();
|
||||
rewrite(cx, &name, &e);
|
||||
rewrite(cx, &name, &e)?;
|
||||
cx.structural = true;
|
||||
Ok(Json::Null)
|
||||
})?;
|
||||
@@ -569,7 +570,7 @@ fn delete_cells(s: &mut Session, p: &Json) -> Result<Json> {
|
||||
shift_cell_features(sh, &e);
|
||||
}
|
||||
// References into the deleted cells become #REF!; the rest move.
|
||||
rewrite(cx, &name, &e);
|
||||
rewrite(cx, &name, &e)?;
|
||||
cx.structural = true;
|
||||
Ok(Json::Null)
|
||||
})?;
|
||||
|
||||
@@ -63,7 +63,7 @@ pub(super) fn delete_sheet(s: &mut Session, p: &Json) -> Result<Json> {
|
||||
}
|
||||
}
|
||||
}
|
||||
crate::cmd::edit::rewrite_python_refs(&mut cx.wb, &|_, e| gridcraft_formula::adjust::delete_sheet(e, &name, &order));
|
||||
crate::cmd::edit::rewrite_python_refs(&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()
|
||||
@@ -141,7 +141,7 @@ pub(super) fn rename_sheet(s: &mut Session, p: &Json) -> Result<Json> {
|
||||
n.formula = gridcraft_formula::print(&gridcraft_formula::adjust::rename_sheet(e, &old, &name));
|
||||
}
|
||||
}
|
||||
crate::cmd::edit::rewrite_python_refs(&mut cx.wb, &|_, e| gridcraft_formula::adjust::rename_sheet(e, &old, &name));
|
||||
crate::cmd::edit::rewrite_python_refs(&mut cx.wb, &|_, e| gridcraft_formula::adjust::rename_sheet(e, &old, &name))?;
|
||||
cx.structural = true;
|
||||
Ok(Json::Null)
|
||||
})
|
||||
|
||||
@@ -430,3 +430,57 @@ fn clearing_pasting_over_or_blocking_a_python_spill_recalculates_its_readers() {
|
||||
s.execute("edit.redo", json!({})).unwrap();
|
||||
assert_eq!(super::v(&s, "B1"), Value::Number(0.0), "redo");
|
||||
}
|
||||
|
||||
/// `prefix` and a comment padded to the longest code that still fits in one cell.
|
||||
fn at_the_limit(prefix: &str) -> String {
|
||||
let with = |n: usize| format!("{prefix}\n#{}", "x".repeat(n));
|
||||
let (mut lo, mut hi) = (0, gridcraft_model::python::MAX_FORMULA_CHARS);
|
||||
while lo < hi {
|
||||
let mid = (lo + hi).div_ceil(2);
|
||||
if PyCode::new(&with(mid)).fits() { lo = mid } else { hi = mid - 1 }
|
||||
}
|
||||
with(lo)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_edit_that_would_make_python_code_too_long_is_refused() {
|
||||
let mut s = s();
|
||||
s.execute("home.insertSheet", json!({})).unwrap();
|
||||
s.execute("sheet.activate", json!({"sheet": 0})).unwrap();
|
||||
let other = s.doc().unwrap().wb.sheet(1).unwrap().name.clone();
|
||||
let refused = |e: crate::EngineError, cell: &str| {
|
||||
let e = e.to_string();
|
||||
assert!(e.contains(&format!("!{cell} longer than Excel's 8192-character")) && e.contains("nothing was changed"), "{e}");
|
||||
};
|
||||
// A sheet rename that lengthens a reference.
|
||||
let a1 = at_the_limit(&format!("a = xl(\"{other}!B2\")"));
|
||||
plant(&mut s, 0, "A1", &a1);
|
||||
refused(s.execute("sheet.rename", json!({"sheet": 1, "name": "A much longer sheet name"})).unwrap_err(), "A1");
|
||||
assert_eq!(code(&s, 0, "A1").unwrap(), a1);
|
||||
assert_eq!(s.doc().unwrap().wb.sheet(1).unwrap().name, other);
|
||||
// One of the same length is made.
|
||||
let same = "Z".repeat(other.chars().count());
|
||||
s.execute("sheet.rename", json!({"sheet": 1, "name": same})).unwrap();
|
||||
assert_eq!(code(&s, 0, "A1").unwrap(), a1.replace(&other, &same));
|
||||
// Code short of the limit still follows each edit.
|
||||
plant(&mut s, 0, "A7", "d = xl(\"Z1\")");
|
||||
s.execute("selection.set", json!({"range": "B10"})).unwrap();
|
||||
s.execute("home.insertColumns", json!({"cols": "Y:Y"})).unwrap();
|
||||
assert_eq!(code(&s, 0, "A7").unwrap(), "d = xl(\"AA1\")");
|
||||
// An inserted column that turns Z1 into AA1.
|
||||
let a3 = at_the_limit("b = xl(\"Z1\")");
|
||||
plant(&mut s, 0, "A3", &a3);
|
||||
s.execute("cell.set", json!({"cell": "D1", "input": "kept"})).unwrap();
|
||||
s.execute("selection.set", json!({"range": "C1"})).unwrap();
|
||||
refused(s.execute("home.insertColumns", json!({"cols": "C:C"})).unwrap_err(), "A3");
|
||||
assert_eq!(code(&s, 0, "A3").unwrap(), a3);
|
||||
assert_eq!(s.doc().unwrap().wb.sheet(0).unwrap().value(CellRef::parse("D1").unwrap()), Value::text("kept"));
|
||||
// A cut to another sheet, which qualifies B9 with the source sheet.
|
||||
let a5 = at_the_limit("c = xl(\"B9\")");
|
||||
plant(&mut s, 0, "A5", &a5);
|
||||
s.execute("edit.cut", json!({"range": "A5"})).unwrap();
|
||||
s.execute("sheet.activate", json!({"sheet": 1})).unwrap();
|
||||
refused(s.execute("edit.paste", json!({"at": "C1"})).unwrap_err(), "C1");
|
||||
assert_eq!(code(&s, 0, "A5").unwrap(), a5);
|
||||
assert_eq!(code(&s, 1, "C1"), None);
|
||||
}
|
||||
|
||||
@@ -160,7 +160,10 @@ characters (the function's syntax, which some files hold), the `_xlfn._xlws.` pr
|
||||
code, which structural edits rewrite like formula references.
|
||||
|
||||
Only those edits rewrite them: inserting or deleting cells, rows and columns, a cut and paste, and
|
||||
renaming or deleting a sheet. Copying, filling and sorting carry the code unchanged, so an `xl()`
|
||||
renaming or deleting a sheet. An edit that would make a cell's code longer than the limit (a
|
||||
longer sheet name, `Z1` becoming `AA1`, a cut to another sheet that qualifies the references) is
|
||||
refused and changes nothing, since code over the limit would be dropped to its value on the next
|
||||
open. Copying, filling and sorting carry the code unchanged, so an `xl()`
|
||||
reference acts as an absolute reference. A sort that moves a Python cell to another row leaves its
|
||||
code reading the cells it named before (a formula moved by the same sort has its relative references
|
||||
shifted), and references to the sorted cells from other cells, formulas and Python code alike, keep
|
||||
@@ -200,7 +203,7 @@ rpm, tarball), FreeBSD, and web (WASM; WebGPU or WebGL2). Release workflows in
|
||||
|
||||
| Date | Change | Summary |
|
||||
|---|---|---|
|
||||
| 2026-11-01 | minor | Python cells as data: `model::PyCode`, the file forms, calc laying out kept results, the limits on open, and what copying, filling and sorting do to `xl()` references |
|
||||
| 2026-11-01 | minor | Python cells as data: `model::PyCode`, the file forms, calc laying out kept results, the limits on open, and what copying, filling and sorting do to `xl()` references; edits that would push code past the limit are refused |
|
||||
| 2026-10-31 | minor | Recently used functions (`recent.rs`, `Prefs::recent_functions`) behind `formulas.recentlyUsed` |
|
||||
| 2026-10-31 | minor | Function Library module (`function_library.rs`); accent icons (`icons::paint_accent`, `widgets::small_button_accent`) |
|
||||
| 2026-10-25 | trivial | The hidden names that hold macros are `_gc_macro_<n>_<k>` |
|
||||
|
||||
Reference in New Issue
Block a user