mirror of
https://github.com/storytold/gridcraft.git
synced 2026-10-31 06:06:48 -06:00
Merge branch 'main' into fix/1419-clear-skips-filtered-rows
This commit is contained in:
@@ -19,6 +19,7 @@ concurrency:
|
||||
|
||||
jobs:
|
||||
build:
|
||||
timeout-minutes: 60
|
||||
name: Build Flatpak repository
|
||||
# Pre-releases (rc builds) never reach the stable branch: a published pre-release is skipped
|
||||
# here, and the validation step below refuses a manual tag with a pre-release suffix.
|
||||
@@ -161,6 +162,7 @@ jobs:
|
||||
include-hidden-files: true
|
||||
|
||||
deploy:
|
||||
timeout-minutes: 60
|
||||
name: Deploy Pages
|
||||
needs: build
|
||||
runs-on: ubuntu-24.04
|
||||
|
||||
@@ -1260,6 +1260,9 @@ fn clear(s: &mut Session, p: &Json, what: &str) -> Result<Json> {
|
||||
.sheet(sheet)
|
||||
.map(|sh| sh.cells.iter_range(r).filter(|(c, _)| !skip.contains(&c.row)).map(|(c, x)| (c, x.clone())).collect())
|
||||
.unwrap_or_default();
|
||||
// Report every stored cell we touch, even beyond a million coordinates in a sparse
|
||||
// selection. Removed formula anchors must also reach calc so their spills disappear.
|
||||
cx.changed.extend(cells.iter().map(|(c, _)| (sheet, *c)));
|
||||
let mut cells = cells;
|
||||
if what == "contents" {
|
||||
for (_, cell) in cells.iter_mut() {
|
||||
@@ -1299,10 +1302,24 @@ fn clear(s: &mut Session, p: &Json, what: &str) -> Result<Json> {
|
||||
sh.hyperlinks.retain(|c, _| !cleared(*c));
|
||||
}
|
||||
if matches!(what, "all" | "formats") {
|
||||
sh.merges.retain(|m| !r.contains_range(m) || skip.range(m.start.row..=m.end.row).next().is_some());
|
||||
}
|
||||
for c in r.iter().take(1_000_000) {
|
||||
cx.changed.push((sheet, c));
|
||||
let mut unmerged = false;
|
||||
sh.merges.retain(|m| {
|
||||
let keep = !r.contains_range(m) || skip.range(m.start.row..=m.end.row).next().is_some();
|
||||
unmerged |= !keep;
|
||||
keep
|
||||
});
|
||||
if unmerged {
|
||||
// An empty merge can block a spill without storing any cells in this range.
|
||||
// As in Unmerge, retry blocked formula anchors that could reach the freed area.
|
||||
let reach = RangeRef::new(CellRef::new(0, 0), r.end);
|
||||
let blocked: Vec<_> = sh
|
||||
.cells
|
||||
.iter_range(reach)
|
||||
.filter(|(_, cell)| cell.formula.is_some() && cell.value == Value::Error(gridcraft_core::CellError::Spill))
|
||||
.map(|(c, _)| (sheet, c))
|
||||
.collect();
|
||||
cx.changed.extend(blocked);
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(Json::Null)
|
||||
|
||||
@@ -0,0 +1,87 @@
|
||||
//! Clearing a large sparse selection must invalidate every changed cell (issue #1032).
|
||||
|
||||
use gridcraft_core::{CellError, CellRef, Value};
|
||||
use gridcraft_engine::Session;
|
||||
use serde_json::json;
|
||||
|
||||
fn value(s: &Session, address: &str) -> Value {
|
||||
s.doc().unwrap().wb.active().unwrap().value(CellRef::parse(address).unwrap())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn large_clear_updates_distant_direct_and_transitive_dependents() {
|
||||
for command in ["edit.clearContents", "edit.clearAll"] {
|
||||
let mut s = Session::new();
|
||||
s.new_workbook();
|
||||
for (cell, input) in [("A1000001", "7"), ("B1", "=A1000001"), ("C1", "=B1*2")] {
|
||||
s.execute("cell.set", json!({"cell": cell, "input": input})).unwrap();
|
||||
}
|
||||
assert_eq!((value(&s, "B1"), value(&s, "C1")), (Value::Number(7.0), Value::Number(14.0)));
|
||||
|
||||
// B1 stretches the used range to row 1; the changed source lies just past the old cap.
|
||||
s.execute(command, json!({"range": "A1:A1000001"})).unwrap();
|
||||
assert_eq!(value(&s, "A1000001"), Value::Empty, "{command}");
|
||||
assert_eq!((value(&s, "B1"), value(&s, "C1")), (Value::Number(0.0), Value::Number(0.0)), "{command}");
|
||||
|
||||
s.execute("edit.undo", json!({})).unwrap();
|
||||
assert_eq!(value(&s, "A1000001"), Value::Number(7.0), "{command} undo");
|
||||
assert_eq!((value(&s, "B1"), value(&s, "C1")), (Value::Number(7.0), Value::Number(14.0)), "{command} undo");
|
||||
s.execute("edit.redo", json!({})).unwrap();
|
||||
assert_eq!(value(&s, "A1000001"), Value::Empty, "{command} redo");
|
||||
assert_eq!((value(&s, "B1"), value(&s, "C1")), (Value::Number(0.0), Value::Number(0.0)), "{command} redo");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn large_clear_removes_a_distant_formulas_spill_and_updates_its_dependent() {
|
||||
for command in ["edit.clearContents", "edit.clearAll"] {
|
||||
let mut s = Session::new();
|
||||
s.new_workbook();
|
||||
s.execute("cell.set", json!({"cell": "A1000001", "input": "=SEQUENCE(2)"})).unwrap();
|
||||
s.execute("cell.set", json!({"cell": "B1", "input": "=A1000002"})).unwrap();
|
||||
assert_eq!((value(&s, "A1000002"), value(&s, "B1")), (Value::Number(2.0), Value::Number(2.0)));
|
||||
|
||||
// The spill lies outside the selection, but must disappear when its anchor is cleared.
|
||||
s.execute(command, json!({"range": "A1:A1000001"})).unwrap();
|
||||
assert_eq!(value(&s, "A1000001"), Value::Empty, "{command}");
|
||||
assert_eq!(value(&s, "A1000002"), Value::Empty, "{command}");
|
||||
assert_eq!(value(&s, "B1"), Value::Number(0.0), "{command}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn clearing_an_empty_merge_unblocks_a_spill() {
|
||||
for command in ["edit.clearFormats", "edit.clearAll"] {
|
||||
let mut s = Session::new();
|
||||
s.new_workbook();
|
||||
s.execute("cell.set", json!({"cell": "A1", "input": "=SEQUENCE(3)"})).unwrap();
|
||||
// Keep the empty merged area inside the used range after the spill becomes blocked.
|
||||
s.execute("cell.set", json!({"cell": "C4", "input": "1"})).unwrap();
|
||||
s.execute("home.mergeCells", json!({"range": "A2:B3"})).unwrap();
|
||||
assert_eq!(value(&s, "A1"), Value::Error(CellError::Spill));
|
||||
|
||||
s.execute(command, json!({"range": "A2:B3"})).unwrap();
|
||||
assert_eq!((value(&s, "A1"), value(&s, "A3")), (Value::Number(1.0), Value::Number(3.0)), "{command}");
|
||||
assert!(s.doc().unwrap().wb.active().unwrap().merges.is_empty(), "{command}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn clearing_a_distant_styled_blocker_unblocks_a_spill() {
|
||||
let mut s = Session::new();
|
||||
s.new_workbook();
|
||||
s.execute("cell.set", json!({"cell": "A1", "input": "=SEQUENCE(100)"})).unwrap();
|
||||
s.execute("cell.set", json!({"cell": "A90", "input": "x"})).unwrap();
|
||||
s.execute("home.bold", json!({"range": "A90", "on": true})).unwrap();
|
||||
assert_eq!(value(&s, "A1"), Value::Error(CellError::Spill));
|
||||
|
||||
// The cleared blocker stays stored for its style and lies beyond the local 64-row scan.
|
||||
s.execute("edit.clearContents", json!({"range": "A2:A100"})).unwrap();
|
||||
assert_eq!(value(&s, "A1"), Value::Number(1.0));
|
||||
assert_eq!(value(&s, "A90"), Value::Number(90.0));
|
||||
assert_eq!(value(&s, "A100"), Value::Number(100.0));
|
||||
let wb = &s.doc().unwrap().wb;
|
||||
let cell = wb.active().unwrap().cell(CellRef::parse("A90").unwrap()).unwrap();
|
||||
assert_eq!(cell.value, Value::Empty);
|
||||
assert!(wb.styles.get(cell.style).font.bold);
|
||||
}
|
||||
@@ -12,7 +12,7 @@ pub mod printer;
|
||||
|
||||
pub use ast::{Anchor, BinOp, Expr, RefKind, Reference, SheetSel, StructItem, StructRef, TrimMode, UnOp};
|
||||
pub use input::{FormulaLocale, parse_input, print_input};
|
||||
pub use parser::{ParseError, normalize_function_name, parse};
|
||||
pub use parser::{MAX_FORMULA_LEN, ParseError, normalize_function_name, parse};
|
||||
pub use printer::{print, print_r1c1, quote_sheet};
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -17,6 +17,11 @@ pub struct ParseError {
|
||||
|
||||
pub(crate) const MAX_DEPTH: usize = 256;
|
||||
|
||||
/// Longest formula accepted, in characters: Excel's limit for formula contents. It also bounds
|
||||
/// the depth of the trees long operator chains build (`1+1+…`), which dropping and evaluating
|
||||
/// walk recursively; without it a formula of a few hundred kilobytes in a file overflows the stack.
|
||||
pub const MAX_FORMULA_LEN: usize = 8_192;
|
||||
|
||||
/// Parses formula text. A leading `=` (or `+`/`-` as Lotus users type) is accepted and skipped
|
||||
/// for `=`.
|
||||
pub fn parse(src: &str) -> Result<Expr, ParseError> {
|
||||
@@ -25,6 +30,9 @@ pub fn parse(src: &str) -> Result<Expr, ParseError> {
|
||||
|
||||
pub(crate) fn parse_syntax(src: &str, locale: crate::FormulaLocale) -> Result<Expr, ParseError> {
|
||||
let body = src.strip_prefix('=').unwrap_or(src);
|
||||
if body.len() > MAX_FORMULA_LEN && body.chars().count() > MAX_FORMULA_LEN {
|
||||
return Err(ParseError { msg: format!("formula is longer than {MAX_FORMULA_LEN} characters"), pos: 0 });
|
||||
}
|
||||
let toks = tokenize_locale(body, locale).map_err(|e| ParseError { msg: e.msg, pos: e.pos })?;
|
||||
let toks = significant_spaces(toks);
|
||||
let mut p = Parser { toks, i: 0, depth: 0, input: locale == crate::FormulaLocale::Es };
|
||||
@@ -160,11 +168,18 @@ impl Parser {
|
||||
return Err(self.err("formula is nested too deeply"));
|
||||
}
|
||||
let mut lhs = self.prefix(allow_union)?;
|
||||
// Postfix operators (`%`, invoking a result) wrap `lhs` without recursing: they count
|
||||
// towards the nesting limit, so `1%%%…` can't build a tree deeper than it.
|
||||
let mut postfix = 0;
|
||||
loop {
|
||||
if *self.peek() == Tok::Op("%") {
|
||||
if 70 < min_bp {
|
||||
break;
|
||||
}
|
||||
postfix += 1;
|
||||
if self.depth + postfix > MAX_DEPTH {
|
||||
return Err(self.err("formula is nested too deeply"));
|
||||
}
|
||||
self.next();
|
||||
lhs = Expr::Unary(UnOp::Percent, Box::new(lhs));
|
||||
continue;
|
||||
@@ -174,6 +189,10 @@ impl Parser {
|
||||
if let Expr::Name(_) = lhs {
|
||||
// A name followed by '(' was lexed as Func; this is `(name)(…)` only.
|
||||
}
|
||||
postfix += 1;
|
||||
if self.depth + postfix > MAX_DEPTH {
|
||||
return Err(self.err("formula is nested too deeply"));
|
||||
}
|
||||
self.next();
|
||||
let args = self.args()?;
|
||||
lhs = Expr::Invoke(Box::new(lhs), args);
|
||||
|
||||
@@ -366,3 +366,38 @@ fn prints_long_flat_chain_without_deep_recursion() {
|
||||
assert_eq!(text, vec!["1"; 20001].join("+"));
|
||||
assert_eq!(r1c1, text);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hostile_formulas_are_errors_not_stack_overflows() {
|
||||
// A small stack: parsing must fail cleanly instead of building trees deeper than the nesting
|
||||
// limit, which dropping, printing and evaluating walk recursively.
|
||||
let handle = std::thread::Builder::new()
|
||||
.stack_size(512 * 1024)
|
||||
.spawn(|| {
|
||||
let percents = format!("1{}", "%".repeat(1_000));
|
||||
let invokes = format!("LAMBDA(x,x)(1){}", "(1)".repeat(1_000));
|
||||
let long_chain = format!("1{}", "+1".repeat(100_000));
|
||||
let huge_percents = format!("1{}", "%".repeat(1_000_000));
|
||||
(parse(&percents).is_err(), parse(&invokes).is_err(), parse(&long_chain).is_err(), parse(&huge_percents).is_err())
|
||||
})
|
||||
.expect("spawn");
|
||||
assert_eq!(handle.join().expect("parsing must not overflow the stack"), (true, true, true, true));
|
||||
// Within the limits these still parse.
|
||||
assert!(parse(&format!("1{}", "%".repeat(100))).is_ok());
|
||||
assert!(parse("LAMBDA(x,x)(1)(2)").is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn formulas_longer_than_excel_allows_are_refused() {
|
||||
let at_limit = format!("1{}", "+1".repeat((crate::MAX_FORMULA_LEN - 1) / 2));
|
||||
assert_eq!(at_limit.chars().count(), crate::MAX_FORMULA_LEN - 1);
|
||||
assert!(parse(&at_limit).is_ok());
|
||||
assert!(parse(&format!("={at_limit}")).is_ok(), "the leading = does not count");
|
||||
let over = format!("{at_limit}+1");
|
||||
let err = parse(&over).unwrap_err();
|
||||
assert!(err.msg.contains("8192"), "{}", err.msg);
|
||||
// Characters, not bytes: 8,000 two-byte characters in a text literal are within the limit.
|
||||
let text = format!("\"{}\"", "é".repeat(8_000));
|
||||
assert!(text.len() > crate::MAX_FORMULA_LEN);
|
||||
assert!(parse(&text).is_ok());
|
||||
}
|
||||
|
||||
@@ -227,3 +227,32 @@ fn hyperlinks_past_the_cell_cap_are_reported() {
|
||||
assert_eq!(wb.sheet(0).unwrap().hyperlinks.len(), 3);
|
||||
assert!(rep.warnings.is_empty(), "{:?}", rep.warnings);
|
||||
}
|
||||
|
||||
/// Formula text built to overflow a recursive parser or tree walk (a million `%`, a 200,000-term
|
||||
/// sum): the cells keep their formula text and cached value, nothing overflows the stack.
|
||||
#[test]
|
||||
fn hostile_formula_text_does_not_overflow_the_stack() {
|
||||
let percents = format!("1{}", "%".repeat(1_000_000));
|
||||
let chain = format!("1{}", "+1".repeat(100_000));
|
||||
let body = format!(r#"<sheetData><row r="1"><c r="A1"><f>{percents}</f><v>3</v></c><c r="B1"><f>{chain}</f><v>4</v></c></row></sheetData>"#);
|
||||
let names = format!(r#"<definedNames><definedName name="Deep">{percents}</definedName></definedNames>"#);
|
||||
let bytes = minimal(&body, &[], "", &names);
|
||||
let handle = std::thread::Builder::new()
|
||||
.stack_size(1024 * 1024)
|
||||
.spawn(move || {
|
||||
let (wb, _) = read_xlsx(&bytes).expect("readable");
|
||||
let s = wb.sheet(0).expect("sheet");
|
||||
let a1 = s.cell(gridcraft_core::CellRef::new(0, 0)).and_then(|c| c.formula.clone()).expect("formula kept");
|
||||
let b1 = s.cell(gridcraft_core::CellRef::new(0, 1)).and_then(|c| c.formula.clone()).expect("formula kept");
|
||||
let kept = (a1.expr.is_none(), b1.expr.is_none(), a1.text.len(), s.value(gridcraft_core::CellRef::new(0, 0)));
|
||||
assert_eq!(wb.names.first().map(|n| n.formula.len()), Some(1_000_001));
|
||||
let out = write_xlsx(&wb).expect("writable");
|
||||
drop(wb);
|
||||
(kept, !out.is_empty())
|
||||
})
|
||||
.expect("spawn");
|
||||
let ((a_unparsed, b_unparsed, a_len, a_value), written) = handle.join().expect("no stack overflow");
|
||||
assert!(a_unparsed && b_unparsed && written);
|
||||
assert_eq!(a_len, 1_000_001);
|
||||
assert_eq!(a_value, gridcraft_core::Value::Number(3.0));
|
||||
}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
# File-format parity
|
||||
|
||||
> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** trivial (XLSX chart sheets are kept on save (#727); previously minor: CSV save keeps the separator it was opened with) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4
|
||||
> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-14 · **Change:** trivial (formulas over Excel's 8,192 characters, and `%` or call chains past the nesting limit, are parse errors instead of stack overflows when a file is opened; previously trivial: XLSX chart sheets are kept on save (#727)) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4
|
||||
|
||||
Every format Excel opens or saves, with GridCraft's read and write support, fidelity and tests.
|
||||
Part of [target-app-parity.md](target-app-parity.md). **File formats: ~50% ready, 80–120 h**
|
||||
@@ -85,6 +85,7 @@ refresh. Counted under features (Get & Transform, ~10% ready), not here.
|
||||
|
||||
| Date | Change | Summary |
|
||||
|---|---|---|
|
||||
| 2026-10-14 | trivial | Formula parser: text longer than Excel's 8,192 characters, and postfix chains (`1%%%…`, `f(1)(1)…`) past the 256-level nesting limit, are parse errors (the cell keeps its formula text and cached value) instead of trees deep enough to overflow the stack when a hostile file is opened, saved or dropped |
|
||||
| 2026-10-12 | trivial | XLSX chart sheets are kept on save (#727) |
|
||||
| 2026-10-12 | trivial | CSV and TSV export is capped at 512 MiB and refuses a larger used range with an error (#528) |
|
||||
| 2026-10-12 | minor | `.xls` import reads the calculation settings (CalcMode, CalcIter, CalcCount, CalcDelta, CalcPrecision), so circular models calculate instead of showing `#CIRC!` (#724) |
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
# Function parity: worksheet functions and formulas
|
||||
|
||||
> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** minor (USDOLLAR added as a compatibility function; MIRR, COUNTIF and INDIRECT edge cases match Excel 365) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4
|
||||
> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-13 · **Change:** minor (clearing large sparse ranges recalculates every changed cell's dependents, #1032) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4
|
||||
|
||||
Excel's worksheet functions, category by category, against GridCraft's function library. Part of
|
||||
[target-app-parity.md](target-app-parity.md); gaps feed [gaps.md](gaps.md).
|
||||
@@ -76,6 +76,7 @@ PIVOTBY, REGEXTEST…) are all present.
|
||||
| Structured references (`Table1[Col]`, `[#Totals]`, `@`; the unbracketed `[[#This Row],Col]` some writers emit) | done |
|
||||
| Defined names (workbook and sheet scope), Create from Selection | done |
|
||||
| Dynamic arrays, spill, `#SPILL!`, spill operator `A1#` | done; stale-spill bugs (#133, #139; PRs #148, #157) |
|
||||
| Recalculation after clearing large sparse ranges | every stored cell changed by Clear is reported to recalculation, without a million-coordinate cutoff; removed formula spills are cleared, and removing empty merges still unblocks spills (#1032) |
|
||||
| Implicit intersection `@`, legacy CSE array formulas | done; legacy array range fill (PR #156) |
|
||||
| Trim references `A1:.E10` (trailing), `A1.:E10` (leading), `A1.:.E10` (both), also on whole rows/columns (`A:.A`) | done; evaluated as `TRIMRANGE(range, code, code)`, adjusted on insert/delete; files store `_xlfn.TRIMRANGE(range,code,code)` (Excel's own file spelling unconfirmed in public docs), `_xlfn._TRO_ALL/_TRO_LEADING/_TRO_TRAILING(...)` read too |
|
||||
| LET, LAMBDA, named LAMBDAs, ISOMITTED, MAP/REDUCE/SCAN/BYROW/BYCOL/MAKEARRAY | done |
|
||||
@@ -108,6 +109,7 @@ workbooks; results checked in as data, not Excel files) and fixing what it finds
|
||||
|
||||
| Date | Change | Summary |
|
||||
|---|---|---|
|
||||
| 2026-10-13 | minor | Clear reports changed stored cells sparsely, including those beyond the first million coordinates; dependent formulas and removed spills update, while clearing empty merges still unblocks spills (#1032) |
|
||||
| 2026-10-12 | minor | USDOLLAR added as a Compatibility function (not on Microsoft's list, so the counts don't change); MIRR with no positive flow is -1, COUNTIF's bare `"<="` / `">="` skip empty strings, INDIRECT with `a1` = FALSE gives `#REF!` for A1-style text (#738) |
|
||||
| 2026-10-11 | minor | TEXT / number formats fixed against Excel-cached results: fraction literals and zero fractions, `0` denominators zero padded, `?` pads the thousands separator, booleans use the text section (226 mismatches → 0 in 352 TEXT cells) |
|
||||
| 2026-10-11 | minor | Iterative calculation: circular references iterate up to Maximum Iterations / Maximum Change when enabled |
|
||||
|
||||
Reference in New Issue
Block a user