diff --git a/crates/engine/src/cmd.rs b/crates/engine/src/cmd.rs index 00cc008ca..8092968d6 100644 --- a/crates/engine/src/cmd.rs +++ b/crates/engine/src/cmd.rs @@ -492,7 +492,7 @@ fn check_arrays(old: &Workbook, new: &Workbook) -> Result<()> { continue; } for c in sh.cells.diff(&after.cells) { - let filled = after.cell(c).is_some_and(|x| x.formula.is_some() || !x.value.is_empty()); + let filled = after.cell(c).is_some_and(gridcraft_model::Cell::has_content); if !filled || !sh.spill.contains_key(&c) || same_content(sh.cell(c), after.cell(c)) { continue; } diff --git a/crates/engine/src/cmd/analysis.rs b/crates/engine/src/cmd/analysis.rs index 134294542..9cd10e50f 100644 --- a/crates/engine/src/cmd/analysis.rs +++ b/crates/engine/src/cmd/analysis.rs @@ -1788,7 +1788,7 @@ fn write_report(s: &mut Session, p: &Json, tool: &str, g: Grid) -> Result { for row in rect.start.row..=rect.end.row { for (c, cell) in sh.cells.row(row, rect.start.col, rect.end.col) { - if !cell.value.is_empty() || cell.formula.is_some() { + if cell.has_content() { occupied.push(CellRef::new(row, c)); } } diff --git a/crates/engine/src/cmd/insert.rs b/crates/engine/src/cmd/insert.rs index 04dc1844e..84b262152 100644 --- a/crates/engine/src/cmd/insert.rs +++ b/crates/engine/src/cmd/insert.rs @@ -308,7 +308,7 @@ fn insert_table(s: &mut Session, p: &Json) -> Result { let sh = cx.sheet_mut(sheet)?; // The header row pushes the data down one row: never off the end of the sheet. let last = RangeRef::new(CellRef::new(MAX_ROWS - 1, r.start.col), CellRef::new(MAX_ROWS - 1, r.end.col)); - if sh.cells.iter_range(last).any(|(_, x)| x.formula.is_some() || !x.value.is_empty()) { + if sh.cells.iter_range(last).any(|(_, x)| x.has_content()) { return Err(EngineError::Other("To prevent possible loss of data, cells can't be pushed off the end of the worksheet.".into())); } sh.cells.shift_rows_in_cols(r.start.col, r.end.col, r.start.row, 1); diff --git a/crates/engine/src/cmd/sheet.rs b/crates/engine/src/cmd/sheet.rs index e2d4b77d5..2d4b9259f 100644 --- a/crates/engine/src/cmd/sheet.rs +++ b/crates/engine/src/cmd/sheet.rs @@ -521,7 +521,7 @@ fn insert_cells(s: &mut Session, p: &Json) -> Result { } else { RangeRef::new(CellRef::new(r.start.row.max(MAX_ROWS.saturating_sub(r.height())), r.start.col), CellRef::new(MAX_ROWS - 1, r.end.col)) }; - if sh.cells.iter_range(doomed).any(|(_, x)| x.formula.is_some() || !x.value.is_empty()) { + if sh.cells.iter_range(doomed).any(|(_, x)| x.has_content()) { return Err(EngineError::Other("To prevent possible loss of data, GridCraft cannot shift nonblank cells off of the worksheet.".into())); } } diff --git a/crates/engine/src/pivot.rs b/crates/engine/src/pivot.rs index 1e76c1215..bfaccace4 100644 --- a/crates/engine/src/pivot.rs +++ b/crates/engine/src/pivot.rs @@ -228,7 +228,7 @@ pub fn refresh(wb: &mut Workbook, sheet: usize, idx: usize, replace: bool) -> Re } if !replace { for (c, cell) in sh.cells.iter_range(area) { - if (!cell.value.is_empty() || cell.formula.is_some()) && !pt.last_range.is_some_and(|r| r.contains(c)) { + if cell.has_content() && !pt.last_range.is_some_and(|r| r.contains(c)) { return Err(format!( "There's already data in {}!{}. Do you want to replace it? (Run again with replace: true to overwrite.)", quote_sheet(&sh.name), diff --git a/crates/engine/src/selection.rs b/crates/engine/src/selection.rs index 4df6fbe4a..0e00e4869 100644 --- a/crates/engine/src/selection.rs +++ b/crates/engine/src/selection.rs @@ -190,7 +190,7 @@ pub fn jump(sheet: &Sheet, from: CellRef, dr: i64, dc: i64) -> CellRef { /// The contiguous data region around `c` (Ctrl+A / Ctrl+Shift+8, AutoSum, sort, tables). pub fn current_region(sheet: &Sheet, c: CellRef) -> RangeRef { - let filled = |c: CellRef| sheet.cell(c).is_some_and(|x| !x.value.is_empty() || x.formula.is_some()) || sheet.spill.contains_key(&c); + let filled = |c: CellRef| sheet.cell(c).is_some_and(gridcraft_model::Cell::has_content) || sheet.spill.contains_key(&c); // Whether a row (or column) has a filled cell between two columns (rows), inclusive. let row_has = |row: u32, a: u32, b: u32| (a..=b).any(|col| filled(CellRef::new(row, col))); let col_has = |col: u32, a: u32, b: u32| (a..=b).any(|row| filled(CellRef::new(row, col))); diff --git a/crates/engine/src/tests/python_cells.rs b/crates/engine/src/tests/python_cells.rs index 913956af8..63cf1d92b 100644 --- a/crates/engine/src/tests/python_cells.rs +++ b/crates/engine/src/tests/python_cells.rs @@ -484,3 +484,49 @@ fn an_edit_that_would_make_python_code_too_long_is_refused() { assert_eq!(code(&s, 0, "A5").unwrap(), a5); assert_eq!(code(&s, 1, "C1"), None); } + +/// Makes `cell` a Python cell whose cached value is empty, as a file's `PY` formula with no cached +/// value reads. +fn plant_empty(s: &mut Session, cell: &str) { + plant(s, 0, cell, "x = 1"); + let d = s.doc_mut().unwrap(); + let mut wb = (*d.wb).clone(); + if let Some(c) = wb.sheet_mut(0).unwrap().cells.get_mut(CellRef::parse(cell).unwrap()) { + c.value = Value::Empty; + } + d.wb = Arc::new(wb); +} + +#[test] +fn python_code_with_an_empty_value_counts_as_content() { + // A report or a PivotTable asks before replacing it. + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [["Region", "Sales"], ["East", 1], ["West", 2], ["East", 3]]})).unwrap(); + plant_empty(&mut s, "D2"); + let report = json!({"tool": "Descriptive Statistics", "range": "B1:B4", "labels": true, "output": "D1"}); + assert!(s.execute("data.analysisToolPak", report).is_err(), "the report asks for overwrite"); + plant_empty(&mut s, "H4"); + let pivot = s.execute("insert.pivotTable", json!({"destination": "Sheet1!H3", "rows": ["Region"], "values": ["Sales"]})); + assert!(pivot.is_err_and(|e| e.to_string().contains("already data"))); + assert_eq!((code(&s, 0, "D2"), code(&s, 0, "H4")), (Some("x = 1".into()), Some("x = 1".into())), "the code is kept"); + // Cells aren't pushed off the sheet over it. + plant_empty(&mut s, "K1048576"); + assert!(s.execute("home.insertCells", json!({"range": "K1", "shift": "down"})).is_err()); + assert_eq!(code(&s, 0, "K1048576"), Some("x = 1".into())); + // A table without a header row doesn't push it off the sheet either. + s.execute("range.setValues", json!({"range": "X1", "values": [[1], [2]]})).unwrap(); + plant_empty(&mut s, "X1048576"); + assert!(s.execute("insert.table", json!({"range": "X1:X2", "header": false})).is_err()); + assert_eq!(code(&s, 0, "X1048576"), Some("x = 1".into())); + // It is part of the data region. + s.execute("range.setValues", json!({"range": "P1", "values": [[1], [""], [3]]})).unwrap(); + plant_empty(&mut s, "P2"); + let sh = s.doc().unwrap().wb.sheet(0).unwrap().clone(); + assert_eq!(crate::selection::current_region(&sh, CellRef::parse("P1").unwrap()).a1(), "P1:P3"); + // Pasted into part of a legacy array, it is refused like any content. + s.execute("selection.set", json!({"range": "R1:R3"})).unwrap(); + s.execute("cell.set", json!({"input": "=5", "array": true})).unwrap(); + plant_empty(&mut s, "T1"); + s.execute("edit.copy", json!({"range": "T1"})).unwrap(); + assert!(s.execute("edit.paste", json!({"at": "R2"})).is_err_and(|e| e.to_string().contains("part of an array"))); +}