From c6c54c57e61da673fe26d44cd424e256f8ab87f3 Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Sun, 25 Oct 2026 12:47:09 -0400 Subject: [PATCH] Engine: python.set makes a Python cell; this build stores code and doesn't run it python.set {cell?, code, output?: object|value} makes the cell (the active cell by default) a Python cell, keeping its style and taking a formula and its spill away. `output` is the return type Microsoft documents for PY(): an Excel value (the default) or a Python object; without it a Python cell keeps the one it had. The code must fit in Excel's 8,192-character formula once written as PY(...), quotes doubled and literals joined, so the command checks the encoded formula rather than a character count. A table's header row refuses Python (it holds the column's name, as Excel keeps formulas out of headers), and a locked cell on a protected sheet refuses it through the usual protection check. Nothing runs Python here. Unchanged code keeps its cached result and value; new or changed code shows #BLOCKED!. One undo step takes the cell back. Sheet::clear_spill removes the spilled values inside a range with work bounded by the values stored, not by the range's size. In the interface, F2, a double click or the formula bar on a Python cell edits its code, and a changed text is committed through python.set, which keeps the return type; through cell.set it would have replaced the Python cell with an ordinary cell holding the code as text. Typing over a Python cell replaces it, as typing over a formula does, and committing the code unchanged writes nothing. Deleting all the code clears the cell through cell.set, as an empty entry clears any other cell; code of spaces is still code. Tests: the code and return type in cell.get, #BLOCKED! for new code, cached results kept for unchanged code, the active-cell default, undo, refused params (missing or non-text code, a bad cell, a bad return type), the encoded limit at 8,079 and 8,080 characters and with doubled quotes, a table header, a protected sheet, and a formula's spill taken away. A UI test edits a Python cell's code with F2 and Enter, commits it unchanged, and types over it; another clears a Python cell by deleting all its code, keeps code of spaces, and undoes the clearing. --- crates/engine/src/cmd/mod.rs | 2 + crates/engine/src/cmd/python.rs | 96 ++++++++++++++++ crates/engine/src/tests.rs | 1 + crates/engine/src/tests/python_commands.rs | 91 ++++++++++++++++ crates/model/src/sheet.rs | 13 +++ crates/ui-egui/src/editor.rs | 5 + crates/ui-egui/src/lib.rs | 44 +++++++- crates/ui-egui/tests/it/main.rs | 1 + crates/ui-egui/tests/it/python_cell_edit.rs | 115 ++++++++++++++++++++ 9 files changed, 362 insertions(+), 6 deletions(-) create mode 100644 crates/engine/src/cmd/python.rs create mode 100644 crates/engine/src/tests/python_commands.rs create mode 100644 crates/ui-egui/tests/it/python_cell_edit.rs diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 2f1becd23..2bd2a13bb 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -14,6 +14,7 @@ pub mod insert; pub mod inspect; pub mod pivot; pub mod print; +pub mod python; pub mod review; pub mod sheet; pub mod slicer; @@ -147,6 +148,7 @@ pub fn command_specs() -> &'static [CommandSpec] { v.extend(spelling::specs()); v.extend(solver::specs()); v.extend(pivot::specs()); + v.extend(python::specs()); v.extend(slicer::specs()); v.extend(whatif::specs()); v.extend(forecast::specs()); diff --git a/crates/engine/src/cmd/python.rs b/crates/engine/src/cmd/python.rs new file mode 100644 index 000000000..29288e64e --- /dev/null +++ b/crates/engine/src/cmd/python.rs @@ -0,0 +1,96 @@ +//! Python in cells: making and editing Python cells. This build stores the code and the last +//! result a file cached; it doesn't run Python, so a cell with new or changed code shows +//! `#BLOCKED!`. + +use std::sync::Arc; + +use gridcraft_core::{CellRef, Value}; +use gridcraft_model::{Cell, PyCode, python::MAX_FORMULA_CHARS}; +use serde_json::{Value as Json, json}; + +use super::{CommandSpec, bad, cell_param_on, cmd, edit, has_doc, str_param, target_sheet}; +use crate::{EngineError, Result, Session}; + +pub fn specs() -> Vec { + vec![cmd!( + "python.set", + "Set Python Code", + [], + None, + "{cell?, code, output?: object|value} makes the cell (default: the active cell) a Python cell with this code; the code must fit in Excel's 8,192-character formula once written as PY(…). `output` is the return type: `value` (an Excel value, the default) or `object` (a Python object)", + has_doc, + set + )] +} + +fn target_cell(s: &Session, p: &Json) -> Result<(usize, CellRef)> { + let sheet = target_sheet(s, p)?; + let at = match cell_param_on(s, p, "cell", sheet)? { + Some(c) => c, + None => s.doc()?.selection.active, + }; + Ok((sheet, at)) +} + +fn existing(s: &Session, sheet: usize, at: CellRef) -> Result> { + Ok(s.doc()?.wb.sheet(sheet).and_then(|sh| sh.cell(at)).and_then(|c| c.py.as_deref()).cloned()) +} + +fn output_mode(p: &Json, key: &str) -> Result> { + match str_param(p, key) { + None => Ok(None), + Some(m) if m.eq_ignore_ascii_case("object") => Ok(Some(true)), + Some(m) if m.eq_ignore_ascii_case("value") => Ok(Some(false)), + Some(m) => Err(bad("python.set", format!("`{key}` is `object` or `value`, not `{m}`"))), + } +} + +fn set(s: &mut Session, p: &Json) -> Result { + let (sheet, at) = target_cell(s, p)?; + let code = match p.get("code") { + Some(Json::String(c)) => c.as_str(), + Some(_) => return Err(bad("python.set", "`code` must be text")), + None => return Err(bad("python.set", "missing `code`")), + }; + let mode = output_mode(p, "output")?; + let old = existing(s, sheet, at)?; + let object = mode.or(old.as_ref().map(|o| o.object)).unwrap_or(false); + let mut py = PyCode { object, ..PyCode::new(code) }; + if !py.fits() { + return Err(bad( + "python.set", + format!("the code doesn't fit in one cell: written as PY(…) it would be longer than Excel's {MAX_FORMULA_CHARS}-character formula limit"), + )); + } + // A table header is the column's name, never a formula or code (as in Excel). + let header = + s.doc()?.wb.sheet(sheet).is_some_and(|sh| sh.tables.iter().any(|t| t.header_row && t.range.start.row == at.row && t.range.contains(at))); + if header { + return Err(EngineError::Other(format!("{} is a table header; Python cells can't go in a table's header row", at.a1()))); + } + // Unchanged code keeps its result (and its value); new code has none until it runs. + let keep = old.filter(|o| o.code == py.code); + let value = match &keep { + Some(o) => { + py.output = o.output.clone(); + py.stale = o.stale; + None + } + None => Some(gridcraft_model::python::BLOCKED), + }; + edit(s, |cx| { + let sh = cx.sheet_mut(sheet)?; + let style = sh.style_id(at); + let value = value.unwrap_or_else(|| sh.cell(at).map(|c| c.value.clone()).unwrap_or(Value::Empty)); + // A formula's spill (or the old result's) goes with what the cell held. + if let Some(r) = sh.spill_ranges.remove(&at) { + sh.clear_spill(r); + } + sh.set_cell(at, Cell { value, formula: None, style, py: Some(Arc::new(py)) }); + cx.touch(sheet, at); + Ok(()) + })?; + let d = s.doc()?; + let shown = d.wb.sheet(sheet).map(|sh| sh.value(at)).unwrap_or_default(); + Ok(json!({"cell": at.a1(), "value": crate::cmd::inspect::value_json(&shown)})) +} diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index c9d10e36e..47eada0b4 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -5094,6 +5094,7 @@ fn a_script_cannot_switch_windows() { } mod python_cells; +mod python_commands; mod seams; #[test] diff --git a/crates/engine/src/tests/python_commands.rs b/crates/engine/src/tests/python_commands.rs new file mode 100644 index 000000000..6d0bfac42 --- /dev/null +++ b/crates/engine/src/tests/python_commands.rs @@ -0,0 +1,91 @@ +//! The python.* commands. + +use gridcraft_core::{CellError, Value}; +use serde_json::json; + +use super::{s, v}; + +fn set(s: &mut crate::Session, cell: &str, code: &str) -> serde_json::Value { + s.execute("python.set", json!({"cell": cell, "code": code})).unwrap() +} + +#[test] +fn set_stores_the_code_and_blocks_the_cell_until_it_can_run() { + let mut s = s(); + let r = set(&mut s, "B1", "xl('A1') + 1"); + assert_eq!(r["cell"], "B1"); + assert_eq!(v(&s, "B1"), Value::Error(CellError::Blocked)); + let got = s.execute("cell.get", json!({"cell": "B1"})).unwrap(); + assert_eq!(got["python"]["code"], "xl('A1') + 1"); + assert_eq!(got["python"]["output"], "value"); + assert_eq!(got["input"], "xl('A1') + 1"); + // The active cell by default. + s.execute("selection.set", json!({"cell": "D4"})).unwrap(); + s.execute("python.set", json!({"code": "2"})).unwrap(); + assert_eq!(s.execute("cell.get", json!({"cell": "D4"})).unwrap()["python"]["code"], "2"); + // One undo step takes the cell back. + s.execute("edit.undo", json!({})).unwrap(); + assert!(s.execute("cell.get", json!({"cell": "D4"})).unwrap()["python"].is_null()); +} + +#[test] +fn unchanged_code_keeps_its_cached_result() { + let mut s = s(); + set(&mut s, "A1", "21 * 2"); + // As a file would have cached it. + { + let d = s.doc_mut().unwrap(); + let wb = std::sync::Arc::make_mut(&mut d.wb); + let sh = wb.sheet_mut(0).unwrap(); + let cell = sh.cells.get_mut(gridcraft_core::CellRef::new(0, 0)).unwrap(); + let py = std::sync::Arc::make_mut(cell.py.as_mut().unwrap()); + py.output = Some(gridcraft_model::PyOutput { value: Value::Number(42.0), type_name: "int".into(), preview: "42".into(), convertible: true }); + cell.value = Value::Number(42.0); + } + set(&mut s, "A1", "21 * 2"); + assert_eq!(v(&s, "A1"), Value::Number(42.0), "same code, same result"); + s.execute("python.set", json!({"cell": "A1", "code": "21 * 2", "output": "object"})).unwrap(); + assert_eq!(s.execute("cell.get", json!({"cell": "A1"})).unwrap()["python"]["output"], "object"); + set(&mut s, "A1", "21 * 3"); + assert_eq!(v(&s, "A1"), Value::Error(CellError::Blocked), "new code has no result yet"); + assert_eq!(s.execute("cell.get", json!({"cell": "A1"})).unwrap()["python"]["output"], "object", "the return type stays"); +} + +#[test] +fn set_refuses_bad_params_long_code_headers_and_protected_cells() { + let mut s = s(); + for p in [ + json!({"cell": "A1"}), + json!({"cell": "A1", "code": 5}), + json!({"cell": "not a cell", "code": "1"}), + json!({"cell": "A1", "code": "1", "output": "sideways"}), + ] { + assert!(s.execute("python.set", p.clone()).is_err(), "{p}"); + } + // The limit is the stored PY(…) formula's 8,192 characters. + assert!(s.execute("python.set", json!({"cell": "A1", "code": "y".repeat(8_080)})).is_err()); + s.execute("python.set", json!({"cell": "A1", "code": "y".repeat(8_079)})).unwrap(); + let quoted = format!("{}{}", "\"".repeat(1_200), "y".repeat(5_800)); + let e = s.execute("python.set", json!({"cell": "A2", "code": quoted})).unwrap_err(); + assert!(e.to_string().contains("8192") || e.to_string().contains("8,192"), "{e}"); + // A table header is the column's name. + s.execute("range.setValues", json!({"range": "C1", "values": [["Name", "Score"], ["a", 1]]})).unwrap(); + s.execute("insert.table", json!({"range": "C1:D2", "headers": true})).unwrap(); + assert!(s.execute("python.set", json!({"cell": "D1", "code": "1"})).is_err()); + assert_eq!(v(&s, "D1"), Value::text("Score")); + set(&mut s, "D2", "1"); + // A locked cell on a protected sheet refuses it. + s.execute("review.protectSheet", json!({})).unwrap(); + let e = s.execute("python.set", json!({"cell": "F1", "code": "1"})).unwrap_err(); + assert!(e.to_string().contains("protected"), "{e}"); +} + +#[test] +fn set_over_a_spilling_formula_takes_the_spill_away() { + let mut s = s(); + s.execute("cell.set", json!({"cell": "A1", "input": "=SEQUENCE(3)"})).unwrap(); + assert_eq!(v(&s, "A3"), Value::Number(3.0)); + set(&mut s, "A1", "[1, 2, 3]"); + assert_eq!(v(&s, "A3"), Value::Empty); + assert!(s.doc().unwrap().wb.sheet(0).unwrap().spill_ranges.is_empty()); +} diff --git a/crates/model/src/sheet.rs b/crates/model/src/sheet.rs index 6bbb331f4..433451277 100644 --- a/crates/model/src/sheet.rs +++ b/crates/model/src/sheet.rs @@ -202,6 +202,19 @@ impl Sheet { self.spill_ranges.iter().find(|(anchor, range)| **anchor != c && range.contains(c)).map(|(anchor, _)| *anchor) } + /// Removes the spilled values inside `r`. The work is bounded by the values stored, not by + /// the size of `r`, so a range from a file that claims the whole sheet costs no more than the + /// values it holds. + pub fn clear_spill(&mut self, r: RangeRef) { + if r.count() <= self.spill.len() as u64 { + for c in r.iter() { + self.spill.remove(&c); + } + } else { + self.spill.retain(|c, _| !r.contains(*c)); + } + } + /// Displayed value of a cell: its own value, or a spilled value. pub fn value(&self, c: CellRef) -> Value { match self.cells.get(c) { diff --git a/crates/ui-egui/src/editor.rs b/crates/ui-egui/src/editor.rs index 847afe3ae..8fe3bc7d2 100644 --- a/crates/ui-egui/src/editor.rs +++ b/crates/ui-egui/src/editor.rs @@ -47,6 +47,10 @@ pub struct EditState { /// The cell's text when an F2-style edit began. Committing it unchanged leaves the cell alone /// (no undo step, the workbook stays clean). `None` for an edit started by typing. pub original: Option, + /// The edit began on a Python cell's existing content (F2, a double click or the formula + /// bar), so the text is the cell's code: a change goes in as new code through `python.set`. + /// `false` for an edit started by typing, which replaces the cell as it does a formula. + pub code_edit: bool, } impl EditState { @@ -75,6 +79,7 @@ impl EditState { caret_to_end: true, completion: None, original, + code_edit: false, } } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 55b3bd740..8cfccb68f 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1146,6 +1146,7 @@ impl SheetApp { None => (current.clone(), false), }; let mut ed = editor::EditState::new(d.uid, d.wb.active_sheet(), at, text, replace, from_formula_bar); + ed.code_edit = !replace && sh.cell(at).is_some_and(|c| c.py.is_some()); ed.window = self.session.active_window(); ed.locale = locale; if replace && self.ui.autocomplete && !ed.is_formula() { @@ -1207,12 +1208,17 @@ impl SheetApp { _ => ed.text.clone(), }; // F2 then Enter (or a file command) on an untouched draft: nothing to write. - if !fill_selection && !array && ed.original.as_ref() == Some(&text) { + if (ed.code_edit || (!fill_selection && !array)) && ed.original.as_ref() == Some(&text) { if dr != 0 || dc != 0 { self.move_after_enter(dr, dc); } return true; } + // All the code deleted: the cell is cleared through cell.set, as an empty entry clears any + // other cell. Code of spaces is still code. + if ed.code_edit && !text.is_empty() { + return self.commit_code_edit(ed, text, dr, dc); + } let text = match self.session.active().map(|d| formula_locale::canonical(&text, ed.locale, &d.wb, ed.sheet, ed.cell)) { Some(Ok(text)) => text, Some(Err(error)) => { @@ -1250,11 +1256,9 @@ impl SheetApp { let unchanged = !fill_selection && !array && text.is_empty() - && self - .session - .active() - .and_then(|d| d.wb.sheet(ed.sheet)) - .is_some_and(|sh| sh.cell(ed.cell).is_none_or(|c| c.formula.is_none() && c.value == gridcraft_engine::core::Value::Empty)); + && self.session.active().and_then(|d| d.wb.sheet(ed.sheet)).is_some_and(|sh| { + sh.cell(ed.cell).is_none_or(|c| c.formula.is_none() && c.py.is_none() && c.value == gridcraft_engine::core::Value::Empty) + }); let r = if unchanged { Ok(json!({})) } else if fill_selection { @@ -1279,6 +1283,34 @@ impl SheetApp { } } + /// Commits a changed edit of a Python cell's code through `python.set`, which keeps the cell's + /// return type; `cell.set` would replace the Python cell with an ordinary one holding the code + /// as text. When the command refuses the code, its message shows and the edit stays open. + fn commit_code_edit(&mut self, ed: editor::EditState, code: String, dr: i64, dc: i64) -> bool { + // Point mode can leave another sheet active; the edit belongs to the sheet it started on. + let r = if self.session.active().is_some_and(|d| d.wb.active_sheet() != ed.sheet) { + self.session.run("sheet.activate", json!({"sheet": ed.sheet})) + } else { + Ok(Json::Null) + }; + let r = r.and_then(|_| self.session.run("python.set", json!({"sheet": ed.sheet, "cell": ed.cell.a1(), "code": code}))); + match r { + Ok(_) => { + if dr != 0 || dc != 0 { + self.move_after_enter(dr, dc); + } + self.after_engine(); + true + } + Err(e) => { + self.message = Some(("GridCraft".into(), clean_error(&e))); + self.editor = Some(ed); + self.session.mode = gridcraft_engine::Mode::Edit; + false + } + } + } + /// Commits the edit with an arrow key. The arrow is an ordinary move, so it ends a run of Tabs /// instead of continuing it (Right) or going back to its column like Enter (Down), as in Excel. pub fn commit_edit_arrow(&mut self, dr: i64, dc: i64) -> bool { diff --git a/crates/ui-egui/tests/it/main.rs b/crates/ui-egui/tests/it/main.rs index 53625015e..09abd1097 100644 --- a/crates/ui-egui/tests/it/main.rs +++ b/crates/ui-egui/tests/it/main.rs @@ -73,6 +73,7 @@ mod pivot_show_details; mod pivot_show_details_menu; mod popup_dismissal; mod protect_password; +mod python_cell_edit; mod ribbon_layout; mod right_to_left; mod scroll_position; diff --git a/crates/ui-egui/tests/it/python_cell_edit.rs b/crates/ui-egui/tests/it/python_cell_edit.rs new file mode 100644 index 000000000..0afa43185 --- /dev/null +++ b/crates/ui-egui/tests/it/python_cell_edit.rs @@ -0,0 +1,115 @@ +//! Editing a Python cell's content (F2, the formula bar) edits its code; typing over it replaces +//! it, as typing over a formula does. + +use egui::{Event, Key, Modifiers}; +use gridcraft_engine::Session; +use gridcraft_engine::core::{CellRef, Value}; +use gridcraft_ui_egui::SheetApp; +use serde_json::json; + +type Harness = egui_kittest::Harness<'static, SheetApp>; + +fn harness() -> Harness { + let mut session = Session::new(); + session.new_workbook(); + session.execute("python.set", json!({"cell": "A1", "code": "1 + 1", "output": "object"})).unwrap(); + session.execute("selection.set", json!({"range": "A1"})).unwrap(); + crate::common::harness_with(crate::common::builder().with_size(egui::vec2(1200.0, 800.0)), crate::common::app(session), 4) +} + +fn text(h: &mut Harness, text: &str) { + h.input_mut().events.push(Event::Text(text.into())); + h.run_steps(2); +} + +fn key(h: &mut Harness, key: Key) { + for pressed in [true, false] { + h.input_mut().events.push(Event::Key { key, physical_key: None, pressed, repeat: false, modifiers: Modifiers::NONE }); + } + h.run_steps(2); +} + +/// A1's code and return type (true: a Python object), or `None` when it isn't a Python cell. +fn python(h: &Harness) -> Option<(String, bool)> { + let sh = h.state().session.active().unwrap().wb.active().unwrap(); + sh.cell(CellRef::parse("A1").unwrap()).and_then(|c| c.py.as_deref()).map(|p| (p.code.clone(), p.object)) +} + +fn select_a1(h: &mut Harness) { + h.state_mut().session.execute("selection.set", json!({"range": "A1"})).unwrap(); + h.run_steps(2); +} + +#[test] +fn f2_edits_a_python_cells_code_and_typing_replaces_it() { + let mut h = harness(); + let undo_depth = |h: &Harness| h.state().session.active().unwrap().undo.len(); + let before = undo_depth(&h); + + // F2 and Enter without a change writes nothing. + key(&mut h, Key::F2); + assert_eq!(h.state().editor.as_ref().map(|e| e.text.as_str()), Some("1 + 1")); + key(&mut h, Key::Enter); + assert!(h.state().editor.is_none()); + assert_eq!(undo_depth(&h), before, "an unchanged edit is no undo step"); + assert_eq!(python(&h), Some(("1 + 1".into(), true))); + + // A change is new code for the same Python cell, which keeps its return type. + select_a1(&mut h); + key(&mut h, Key::F2); + text(&mut h, " + 2"); + key(&mut h, Key::Enter); + assert!(h.state().editor.is_none(), "{:?}", h.state().message); + assert_eq!(python(&h), Some(("1 + 1 + 2".into(), true)), "the cell stays a Python cell with the new code"); + + // Typing over the cell replaces it. + select_a1(&mut h); + text(&mut h, "5"); + key(&mut h, Key::Enter); + assert_eq!(python(&h), None); + assert_eq!(h.state().session.active().unwrap().wb.active().unwrap().value(CellRef::parse("A1").unwrap()), Value::Number(5.0)); +} + +#[test] +fn deleting_all_the_code_clears_the_cell() { + let mut h = harness(); + let a1 = |h: &Harness| h.state().session.active().unwrap().wb.active().unwrap().value(CellRef::parse("A1").unwrap()); + // Code of spaces is still code, for the same Python cell. + key(&mut h, Key::F2); + h.state_mut().editor.as_mut().unwrap().text = " ".into(); + assert!(h.state_mut().commit_edit(0, 0, false, false)); + assert_eq!(python(&h), Some((" ".into(), true))); + // No code at all clears the cell, as an empty entry clears any other cell. + select_a1(&mut h); + key(&mut h, Key::F2); + h.state_mut().editor.as_mut().unwrap().text.clear(); + assert!(h.state_mut().commit_edit(0, 0, false, false)); + assert!(h.state().editor.is_none()); + assert_eq!(python(&h), None, "the cell is no longer a Python cell"); + assert_eq!(a1(&h), Value::Empty); + h.state_mut().run("edit.undo", json!({})).unwrap(); + assert_eq!(python(&h), Some((" ".into(), true)), "one undo step brings the code back"); +} + +#[test] +fn deleting_all_the_code_clears_a_python_cell_with_no_cached_value() { + let mut h = harness(); + // A Python cell from a file that cached no value (the value is empty, the code is not). + { + let d = h.state_mut().session.doc_mut().unwrap(); + let mut wb = (*d.wb).clone(); + let i = wb.active_sheet(); + let sh = wb.sheet_mut(i).unwrap(); + let at = CellRef::parse("A1").unwrap(); + let mut cell = sh.cell(at).cloned().unwrap(); + cell.value = Value::Empty; + sh.set_cell(at, cell); + d.wb = std::sync::Arc::new(wb); + } + h.run_steps(2); + key(&mut h, Key::F2); + h.state_mut().editor.as_mut().unwrap().text.clear(); + assert!(h.state_mut().commit_edit(0, 0, false, false)); + assert!(h.state().editor.is_none()); + assert_eq!(python(&h), None, "the code is removed, not kept as an unchanged empty cell"); +}