Merge pull request #152 from nihilist-narwhal/fix-whole-sheet-selection

Whole-sheet selection: Fill and Remove Duplicates no longer run out of memory or time

crates/engine/src/cmd/data.rs

@@ -112,7 +112,8 @@ fn data_range(s: &Session, p: &Json) -> Result<RangeRef> {
     }
     let sel = d.selection.current();
     if !sel.is_single() {
-        let used = sh.used_range().unwrap_or(sel);
+        // An empty sheet has no data to work on: just the active cell.
+        let Some(used) = sh.used_range() else { return Ok(RangeRef::cell(d.selection.active)) };
         return Ok(sel.intersection(&used).unwrap_or(sel));
     }
     Ok(current_region(sh, d.selection.active))
@@ -668,6 +669,9 @@ fn remove_duplicates(s: &mut Session, p: &Json) -> Result<Json> {
     let r = data_range(s, p)?;
     let d = s.doc()?;
     let sh = d.wb.active().ok_or(EngineError::NoDocument)?;
+    // Rows and columns past the data are blank: only the part with data is compared (whole
+    // columns compared a million blank rows and reported them as removed duplicates).
+    let r = sh.used_range().and_then(|u| r.intersection(&u)).unwrap_or(RangeRef::cell(r.start));
     let header = bool_param(p, "header").unwrap_or_else(|| guess_header(&d.wb, sh, r));
     let cols: Vec<u32> = p
         .get("columns")

crates/engine/src/cmd/edit.rs

@@ -1078,6 +1078,16 @@ fn clear(s: &mut Session, p: &Json, what: &str) -> Result<Json> {
 fn fill_dir(s: &mut Session, p: &Json, dr: i64, dc: i64) -> Result<Json> {
     let sheet = target_sheet(s, p)?;
     let mut r = target_range(s, p)?;
+    // Whole rows or columns fill only where the sheet has content (nothing on an empty sheet).
+    if r.is_full_cols() || r.is_full_rows() {
+        match s.doc()?.wb.sheet(sheet).and_then(|sh| sh.used_range()).and_then(|u| r.intersection(&u)) {
+            Some(u) => r = u,
+            None => return Ok(Json::Null),
+        }
+    }
+    if r.count() > 5_000_000 {
+        return Err(bad("edit.fill", "range too large"));
+    }
     // With one row selected, Fill Down copies from the row above (Excel behaviour).
     if dr == 1 && r.height() == 1 {
         r.start.row = r.start.row.saturating_sub(1);
@@ -1094,8 +1104,6 @@ fn fill_dir(s: &mut Session, p: &Json, dr: i64, dc: i64) -> Result<Json> {
     } else {
         RangeRef::new(CellRef::new(r.start.row, r.end.col), r.end)
     };
-    let used = s.doc()?.wb.sheet(sheet).and_then(|sh| sh.used_range());
-    let r = if r.is_full_cols() || r.is_full_rows() { used.and_then(|u| r.intersection(&u)).unwrap_or(r) } else { r };
     edit(s, |cx| {
         crate::fill::fill(cx, sheet, src, r, crate::fill::FillMode::Copy)?;
         Ok(Json::Null)
@@ -1112,8 +1120,8 @@ fn auto_fill(s: &mut Session, p: &Json) -> Result<Json> {
         "values" => crate::fill::FillMode::ValuesOnly,
         _ => crate::fill::FillMode::Series,
     };
-    if target.count() > 5_000_000 {
-        return Err(bad("edit.autoFill", "target too large"));
+    if target.count() > 5_000_000 || src.count() > 5_000_000 {
+        return Err(bad("edit.autoFill", "range too large"));
     }
     edit(s, |cx| {
         crate::fill::fill(cx, sheet, src, target, mode)?;

crates/engine/src/tests.rs

@@ -959,6 +959,27 @@ fn every_command_survives_empty_params() {
     }
 }
 
+#[test]
+fn whole_sheet_selection_stays_within_the_data() {
+    // On an empty sheet with everything selected these filled or scanned every cell of the
+    // sheet, running out of memory or time.
+    let mut s = s();
+    s.execute("edit.selectAll", json!({})).unwrap();
+    for cmd in ["edit.fillDown", "edit.fillRight", "edit.fillUp", "edit.fillLeft", "data.removeDuplicates"] {
+        s.execute(cmd, json!({})).unwrap();
+    }
+    s.execute("range.setValues", json!({"range": "A1", "values": [[1, 2], [3, 4], [5, 6]]})).unwrap();
+    s.execute("selection.set", json!({"range": "A1:XFD1048576"})).unwrap();
+    s.execute("edit.fillUp", json!({})).unwrap();
+    assert_eq!(v(&s, "A1"), Value::Number(5.0));
+    assert!(s.execute("edit.fillDown", json!({"range": "A1:Z1000000"})).is_err(), "too large to fill");
+    assert!(s.execute("edit.autoFill", json!({"source": "A1:XFD1048576", "target": "A1"})).is_err(), "too large to fill");
+    s.execute("range.setValues", json!({"range": "D1", "values": [[1], [1], [2]]})).unwrap();
+    let r = s.execute("data.removeDuplicates", json!({"range": "D:D", "header": false})).unwrap();
+    assert_eq!((r["removed"].clone(), r["remaining"].clone()), (json!(1), json!(2)));
+    assert!(s.execute("data.removeDuplicates", json!({"range": "A1:XFD1048576", "header": false})).is_ok());
+}
+
 #[test]
 fn parity_counts() {
     let (done, total) = crate::catalog::parity();