Integration: #451 qualify the run-stop row through value_ref

A stored cell with no value and no formula falls through to spill in
value_ref; the scan now asks value_ref for the row where the stored run
stops. Docs corrected; tests cover a style-only stored cell over a spill
value.
This commit is contained in:
aiongg
2026-10-22 02:34:58 +00:00
parent ed4b4e1ca7
commit 6735e9e7c0
2 changed files with 29 additions and 13 deletions
+26 -9
View File
@@ -188,7 +188,7 @@ impl Sheet {
/// empty (or `from_row > cap`), same as a cell-by-cell `value_ref` scan would find, but
/// without calling `value_ref` once per row: `CellStore::contiguous_value_run` walks each
/// band's row map directly for the common case (a run of stored cells), only falling back to
/// a per-row `spill` check where the stored run stops short of `cap`. Used by the fill-handle
/// a per-row `value_ref` check where the stored run stops short of `cap`. Used by the fill-handle
/// double-click, where `col` can be densely populated for very long stretches.
pub fn last_contiguous_nonempty(&self, col: u32, from_row: u32, cap: u32) -> u32 {
if from_row > cap {
@@ -203,10 +203,10 @@ impl Sheet {
match self.cells.contiguous_value_run(c, cap) {
Some(last) => row = last + 1,
None => {
// A stored cell decides the row on its own, even with an empty value (see
// `contiguous_value_run`'s doc comment): only an unstored row falls through
// to `spill`, exactly like `value_ref`.
let qualifies = self.cells.get(c).is_none() && self.spill.get(&c).is_some_and(|v| !v.is_empty());
// The run stopped short of `row`: ask `value_ref` for this one row, which also
// covers a stored cell with no value and no formula (a formatted spill child)
// falling through to `spill`.
let qualifies = self.value_ref(c).is_some_and(|v| !v.is_empty());
if qualifies {
row += 1;
} else {
@@ -505,6 +505,12 @@ mod tests {
// Stored but blank: does not count (matches `has_value`/`value_ref`).
s.set_value(CellRef::new(r, 0), Value::Text("".into()));
}
4 if rng.below(2) == 0 => {
// A style-only stored cell over a spill value: `value_ref` falls through
// to the spill value, so the row counts.
s.spill.insert(CellRef::new(r, 0), Value::Number(r as f64));
s.set_cell(CellRef::new(r, 0), Cell { style: StyleId(1), ..Default::default() });
}
_ => {} // gap
}
}
@@ -536,16 +542,27 @@ mod tests {
let mut s = Sheet::new("S");
s.set_value(CellRef::new(0, 1), Value::Number(1.0));
s.set_value(CellRef::new(1, 1), Value::Number(2.0));
// row 2: a formula (e.g. `=""`) whose cached value is empty. A stored cell always
// decides the row on its own, exactly like `value_ref`: this does NOT count, same as
// `value_ref(...).is_some_and(|v| !v.is_empty())` would say for it, even though it has
// a formula (unlike `CellStore::has`, used for Ctrl+arrow navigation, which would).
// row 2: a formula (e.g. `=""`) whose cached value is empty. It does not count, same as
// `value_ref(...).is_some_and(|v| !v.is_empty())` says for it, even though it has a
// formula (unlike `CellStore::has`, used for Ctrl+arrow navigation, which counts it).
s.set_cell(CellRef::new(2, 1), Cell { formula: Some(Arc::new(Formula::new("\"\""))), ..Default::default() });
s.set_value(CellRef::new(3, 1), Value::Number(4.0)); // would extend the run if row 2 didn't stop it
assert_eq!(s.last_contiguous_nonempty(1, 0, 100), 1);
assert_eq!(s.last_contiguous_nonempty(1, 2, 100), 1); // from_row itself is the stop
}
#[test]
fn last_contiguous_nonempty_spill_extends_a_style_only_stored_row() {
let mut s = Sheet::new("S");
s.set_value(CellRef::new(0, 1), Value::Number(1.0));
// row 1: a formatted spill child: stored with a style only, value from `spill`.
s.spill.insert(CellRef::new(1, 1), Value::Number(2.0));
s.set_cell(CellRef::new(1, 1), Cell { style: StyleId(1), ..Default::default() });
s.set_value(CellRef::new(2, 1), Value::Number(3.0));
assert_eq!(s.last_contiguous_nonempty(1, 0, 100), 2);
assert_eq!(s.last_contiguous_nonempty(1, 1, 100), 2);
}
#[test]
fn last_contiguous_nonempty_spill_extends_an_unstored_row() {
let mut s = Sheet::new("S");
+3 -4
View File
@@ -132,10 +132,9 @@ impl CellStore {
/// yet, for Ctrl+arrow navigation): this matches `Sheet::value_ref`, whose callers test the
/// `Value` it returns with `!v.is_empty()` regardless of whether a formula produced it — a
/// formula cell with an empty cached value (`=""`, or not yet recalculated) is "empty" to
/// them. A stored cell always decides the row on its own either way (`value_ref` never falls
/// through to `spill` once a cell is stored there, whether or not its value is empty), so
/// `last_contiguous_nonempty` only needs to check `spill` where this returns `None`, i.e.
/// no cell is stored at `from.col` for that row at all.
/// them. A stored cell with no value and no formula (for example a formatted spill child) does
/// fall through to `spill` in `value_ref`, so `last_contiguous_nonempty` asks `value_ref`
/// about the row where this run stops.
///
/// Walks each band's row map with `BTreeMap::range` (one in-order pass per band) instead of
/// one `get` per row, so a long, densely-populated run costs roughly one tree-traversal step