Merge pull request #156 from nihilist-narwhal/fix-legacy-array-formulas

Legacy array formulas: fill the range they were entered in

# Conflicts:
#	crates/engine/src/cmd/edit.rs
#	crates/ui-egui/src/formula_bar.rs

crates/calc/src/recalc.rs

@@ -432,6 +432,12 @@ impl PassHost<'_> {
             let mut ev = Evaluator::new(self, k.0, k.1);
             ev.value(&expr)
         };
+        // A legacy array formula fills the range it was entered in.
+        let legacy = self.wb.sheet(k.0).and_then(|s| s.cell(k.1)).and_then(|c| c.formula.as_ref()).and_then(|f| f.array);
+        let v = match legacy {
+            Some(range) => fit_to_range(v, range),
+            None => v,
+        };
         self.depth -= 1;
         self.in_progress.remove(&k);
         self.pending.remove(&k);
@@ -468,6 +474,30 @@ impl PassHost<'_> {
     }
 }
 
+/// A legacy (Ctrl+Shift+Enter) array formula's result fitted to the range it was entered in, as
+/// Excel does: a single row or column repeats across the range, a single value fills it, cells
+/// past the result show `#N/A`, and a result larger than the range is cut off.
+fn fit_to_range(v: Value, range: RangeRef) -> Value {
+    if range.count() > crate::eval::MAX_CELLS {
+        return Value::Error(CellError::Num);
+    }
+    let (h, w) = (range.height() as usize, range.width() as usize);
+    let Value::Array(a) = v else {
+        return if h * w == 1 { v } else { Array::new(h, w, vec![v; h * w]).map(Value::from).unwrap_or(Value::Error(CellError::Value)) };
+    };
+    if h * w == 1 {
+        return a.data.first().cloned().unwrap_or_default();
+    }
+    let mut data = Vec::with_capacity(h * w);
+    for r in 0..h {
+        for c in 0..w {
+            let (r, c) = (if a.rows == 1 { 0 } else { r }, if a.cols == 1 { 0 } else { c });
+            data.push(a.get(r, c).cloned().unwrap_or(Value::Error(CellError::NA)));
+        }
+    }
+    Array::new(h, w, data).map(Value::from).unwrap_or(Value::Error(CellError::Value))
+}
+
 impl Host for PassHost<'_> {
     fn workbook(&self) -> &Workbook {
         self.wb

crates/calc/src/tests.rs

@@ -319,6 +319,33 @@ fn names_and_sheets() {
     assert_eq!(t.num("A1"), 5.0);
 }
 
+#[test]
+fn legacy_array_formulas_fill_their_range() {
+    let mut t = T::new();
+    t.set("A1", "1");
+    t.set("A2", "2");
+    let cse = |t: &mut T, at: &str, formula: &str, range: &str| {
+        let mut f = Formula::new(formula);
+        f.array = gridcraft_core::RangeRef::parse(range);
+        t.wb.sheet_mut(0).unwrap().cells.set(c(at), Cell::formula(f));
+        t.calc.cells_changed(&mut t.wb, &[(0, c(at))]);
+    };
+    // Past the result: #N/A. A single column repeats across the range.
+    cse(&mut t, "C1", "=A1:A2*2", "C1:D3");
+    cse(&mut t, "F1", "=A1:A2*2", "F1");
+    cse(&mut t, "G1", "=5", "G1:G2");
+    assert_eq!(t.num("C2"), 4.0);
+    assert_eq!(t.num("D2"), 4.0);
+    assert_eq!(t.get("C3"), Value::Error(CellError::NA));
+    // A smaller range shows only part of the result and never spills.
+    assert_eq!(t.num("F1"), 2.0);
+    assert_eq!(t.get("F2"), Value::Empty);
+    assert_eq!(t.num("G2"), 5.0);
+    // A range too large to fill is an error, not an allocation of every cell.
+    cse(&mut t, "H1", "=1", "H1:XFD1048576");
+    assert_eq!(t.get("H1"), Value::Error(CellError::Num));
+}
+
 #[test]
 fn text_function() {
     let mut t = T::new();

crates/engine/src/cmd/edit.rs

@@ -213,6 +213,13 @@ fn cell_set(s: &mut Session, p: &Json) -> Result<Json> {
                 std::sync::Arc::make_mut(f).array = Some(range);
             }
             let sh = cx.sheet_mut(sheet)?;
+            // The array takes the whole range: other cells in it lose their contents.
+            for c in range.iter().skip(1).take(1_000_000) {
+                if let Some(old) = sh.cells.get(c).filter(|x| !x.value.is_empty() || x.formula.is_some()) {
+                    let style = old.style;
+                    sh.set_cell(c, Cell { style, ..Cell::default() });
+                }
+            }
             sh.set_cell(range.start, cell);
             cx.touch(sheet, range.start);
             return Ok(Json::Null);
@@ -1365,6 +1372,22 @@ fn begin_edit(s: &mut Session, _: &Json) -> Result<Json> {
 mod tests {
     use super::*;
 
+    #[test]
+    fn legacy_array_entry() {
+        let mut s = Session::new();
+        s.new_workbook();
+        s.execute("range.setValues", json!({"range": "B2", "values": [[1], [2]]})).unwrap();
+        s.execute("cell.set", json!({"cell": "C4", "input": "old"})).unwrap();
+        s.execute("selection.set", json!({"range": "C2:C4"})).unwrap();
+        s.execute("cell.set", json!({"cell": "C2", "input": "=B2:B3*2", "array": true})).unwrap();
+        let get = |s: &mut Session, a: &str| s.execute("cell.get", json!({"cell": a})).unwrap();
+        assert_eq!(get(&mut s, "C3")["value"], json!(4.0));
+        // Past the result Excel shows #N/A; the old content of the range is replaced.
+        assert_eq!(get(&mut s, "C4")["text"], "#N/A");
+        let bar = s.doc().unwrap().wb.active().unwrap().cell(CellRef::parse("C2").unwrap()).map(|c| c.bar_text());
+        assert_eq!(bar.as_deref(), Some("{=B2:B3*2}"));
+    }
+
     #[test]
     fn wildcards() {
         assert!(wildcard("hello", "h*o"));

crates/model/src/cell.rs

@@ -85,4 +85,12 @@ impl Cell {
             v => v.display(),
         }
     }
+    /// What the formula bar shows when not editing: [`input_text`](Self::input_text), with a
+    /// legacy array formula in braces (`{=A1:A2*2}`).
+    pub fn bar_text(&self) -> String {
+        match &self.formula {
+            Some(f) if f.array.is_some() => format!("{{={}}}", f.text),
+            _ => self.input_text(),
+        }
+    }
 }

crates/ui-egui/src/formula_bar.rs

@@ -73,8 +73,10 @@ fn active_input(app: &SheetApp) -> String {
         if c.formula.is_some() && d.wb.styles.get(c.style).protection.hidden && sh.is_protected() {
             return String::new();
         }
-        return if c.formula.is_some() {
-            crate::formula_locale::display(&c.input_text(), app.ui.language.formula_locale(), &d.wb, d.wb.active_sheet)
+        return if let Some(f) = &c.formula {
+            let shown = crate::formula_locale::display(&c.input_text(), app.ui.language.formula_locale(), &d.wb, d.wb.active_sheet);
+            // A legacy array formula shows in braces (`{=A1:A2*2}`), as `Cell::bar_text` does.
+            if f.array.is_some() { format!("{{{shown}}}") } else { shown }
         } else {
             c.input_text()
         };

crates/xlsx/src/sheet_read.rs

@@ -29,8 +29,8 @@ struct Shared {
 struct RowState {
     next_row: u32,
     shared: HashMap<u32, Shared>,
-    /// Dynamic-array spill ranges: cached values inside them (other than the anchor) are not
-    /// stored, so the formula can spill again.
+    /// Dynamic-array spill ranges and legacy array ranges: cached values inside them (other than
+    /// the anchor) are not stored, so the formula can fill them again.
     dynamic: Vec<RangeRef>,
     dropped: u64,
     rows: u64,
@@ -416,11 +416,10 @@ fn read_formula(cx: &mut Ctx<'_>, st: &mut RowState, f: &El, pos: CellRef, dynam
             }
             let mut fm = Formula::new(text);
             let range = f.attr("ref").and_then(RangeRef::parse).unwrap_or(RangeRef::cell(pos));
-            if dynamic {
-                if !range.is_single() {
-                    st.dynamic.push(range);
-                }
-            } else {
+            if !range.is_single() {
+                st.dynamic.push(range);
+            }
+            if !dynamic {
                 fm.array = Some(range);
             }
             Some(fm)

crates/xlsx/src/sheet_write.rs

@@ -100,7 +100,8 @@ pub fn write_sheet(wb: &Workbook, si: usize, selected: bool, out: &mut Out) -> (
         .map(|(a, r)| (*a, *r))
         .collect();
     let mut extra: BTreeMap<CellRef, Value> = BTreeMap::new();
-    for (a, r) in &dynamic {
+    // Values inside spills and legacy array ranges.
+    for (a, r) in sheet.spill_ranges.iter().filter(|(_, r)| !r.is_single()) {
         for c in r.iter().take(1_000_000) {
             if c != *a
                 && let Some(v) = sheet.spill.get(&c)

crates/xlsx/src/tests/fixtures.rs

@@ -17,8 +17,8 @@ fn shared_formulas_and_value_types() {
         <row r="2"><c r="A2"><v>2</v></c><c r="B2"><f t="shared" si="0"/><v>5</v></c></row>
         <row r="3"><c r="A3" t="inlineStr"><is><r><t>rich </t></r><r><rPr><b/></rPr><t>text</t></r><rPh><t>ignored</t></rPh></is></c><c r="B3"><f t="shared" si="0"/></c></row>
         <row r="4"><c r="A4" t="b"><v>1</v></c><c r="B4" t="e"><v>#DIV/0!</v></c><c r="C4" t="str"><f>_xlfn.CONCAT("a","b")</f><v>ab</v></c><c r="D4" t="d"><v>2024-01-01T12:00:00</v></c></row>
-        <row r="5"><c r="A5"><f t="array" ref="A5:B6">A1:B2*2</f><v>2</v></c><c r="C5" cm="1"><f t="array" ref="C5:C7">_xlfn.SEQUENCE(3)</f><v>1</v></c></row>
-        <row r="6"><c r="C6"><v>2</v></c></row>
+        <row r="5"><c r="A5"><f t="array" ref="A5:B6">A1:B2*2</f><v>2</v></c><c r="B5"><v>6</v></c><c r="C5" cm="1"><f t="array" ref="C5:C7">_xlfn.SEQUENCE(3)</f><v>1</v></c></row>
+        <row r="6"><c r="A6"><v>4</v></c><c r="C6"><v>2</v></c></row>
         <row><c><v>9</v></c><c><v>10</v></c></row>
     </sheetData>"#;
     let (wb, rep) = read_xlsx(&minimal(body, &[], "", "")).unwrap();
@@ -35,6 +35,7 @@ fn shared_formulas_and_value_types() {
     assert_eq!(s.value(at("D4")), Value::Number(45292.5));
     let a5 = s.cell(at("A5")).unwrap().formula.clone().unwrap();
     assert_eq!(a5.array, Some(RangeRef::parse("A5:B6").unwrap()));
+    assert!(s.cell(at("B5")).is_none() && s.cell(at("A6")).is_none(), "a legacy array's cached values are dropped too");
     let c5 = s.cell(at("C5")).unwrap().formula.clone().unwrap();
     assert_eq!(c5.array, None, "dynamic arrays are not legacy CSE arrays");
     assert_eq!(c5.text, "SEQUENCE(3)");