Engine: python.output switches a Python cell between Python Object and Excel Value

A Python cell's return type had no command of its own: python.set takes
it, but only together with the code. python.output {cell?, mode?} sets
it on its own; its command entry lists Excel's shortcut,
Ctrl+Alt+Shift+M. Without a mode it switches to the other one, so the
shortcut needs no parameters; a cell that isn't a Python cell is an
error. A cached result belongs to the return type it was made for, so
a switch treats it as python.set treats a new return type: the result
goes, the cell shows #BLOCKED! until it runs, and its old spill goes
with what read it recalculated. Both commands write the cell through
one path, so python.output refuses a table header and a locked cell on
a protected sheet as python.set does. Each switch is one undo step, and
setting the mode a cell already has adds none.

python.set now refuses an output that isn't text instead of ignoring it.
The command-sequence fuzzer runs python.output too. A macro recording
keeps the return type a switch without a mode set, as its mode (as Bold
records on), also for a switch inside a recorded script run, so playing
it back sets that type rather than switching again.

Tests: switching without a mode drops the cached result, its spill and
the value its reader saw, the same as python.set with the other return
type; switching back brings no result back; the code is kept; one undo
step per switch, undo brings the result back, and none for the mode a
cell already has; a cell without a result stays #BLOCKED!; the
refusals (no Python cell, a bad or non-text mode, a bad cell, a table
header, a locked cell, a non-text output for python.set), an unlocked
Hidden cell on a protected sheet switches, and a recorded switch played
back on a cell that already has the new type keeps it.

crates/engine/src/cmd/extra/scripts.rs

@@ -138,7 +138,7 @@ pub(super) fn run_script(s: &mut Session, p: &Json) -> Result<Json> {
             let opened_dialog = s.ui_requests.get(before..).is_some_and(|new| new.iter().any(|r| matches!(r, crate::UiRequest::Dialog(..))));
             let changed = wb_before.as_ref().is_some_and(|(uid, wb)| s.docs.iter().any(|d| d.uid == *uid && !std::sync::Arc::ptr_eq(&d.wb, wb)));
             if !opened_dialog || changed {
-                ran_steps.push((id.to_string(), crate::record::with_toggle_state(st["params"].clone(), &result)));
+                ran_steps.push((id.to_string(), crate::record::with_toggle_state(id, st["params"].clone(), &result)));
             }
         }
     }

crates/engine/src/cmd/python.rs

@@ -1,6 +1,6 @@
-//! 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!`.
+//! Python in cells: making and editing Python cells, and their return type. 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;
 
@@ -12,15 +12,26 @@ use super::{CommandSpec, bad, cell_param_on, cmd, edit, has_doc, str_param, targ
 use crate::{EngineError, Result, Session};
 
 pub fn specs() -> Vec<CommandSpec> {
-    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
-    )]
+    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
+        ),
+        cmd!(
+            "python.output",
+            "Python Output",
+            [],
+            Some("Ctrl+Alt+Shift+M"),
+            "{cell?, mode?: object|value} sets the return type of a Python cell (default: the active cell): `object` (a Python object) or `value` (an Excel value). Without `mode` it switches to the other one. The cell keeps its code; a cached result of the old return type goes, as with `python.set`, so the cell shows #BLOCKED! until it runs",
+            has_doc,
+            output
+        ),
+    ]
 }
 
 fn target_cell(s: &Session, p: &Json) -> Result<(usize, CellRef)> {
@@ -36,12 +47,17 @@ fn existing(s: &Session, sheet: usize, at: CellRef) -> Result<Option<PyCode>> {
     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<Option<bool>> {
+fn output_mode(cmd: &str, p: &Json, key: &str) -> Result<Option<bool>> {
+    match p.get(key) {
+        None | Some(Json::Null) => return Ok(None),
+        Some(Json::String(_)) => {}
+        Some(_) => return Err(bad(cmd, format!("`{key}` is `object` or `value`"))),
+    }
     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}`"))),
+        Some(m) => Err(bad(cmd, format!("`{key}` is `object` or `value`, not `{m}`"))),
     }
 }
 
@@ -52,13 +68,22 @@ fn set(s: &mut Session, p: &Json) -> Result<Json> {
         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 mode = output_mode("python.set", 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) };
+    store(s, "python.set", sheet, at, PyCode { object, ..PyCode::new(code) }, old)?;
+    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)}))
+}
+
+/// Writes `py` into the cell, which held `old`: what `python.set` and `python.output` share. The
+/// code must fit in one cell and a table header takes none; a locked cell on a protected sheet is
+/// refused by the edit itself.
+fn store(s: &mut Session, cmd: &str, sheet: usize, at: CellRef, mut py: PyCode, old: Option<PyCode>) -> Result<()> {
     if !py.fits() {
         return Err(bad(
-            "python.set",
+            cmd,
             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"),
         ));
     }
@@ -89,8 +114,24 @@ fn set(s: &mut Session, p: &Json) -> Result<Json> {
         sh.set_cell(at, Cell { value, formula: None, style, py: Some(Arc::new(py)) });
         cx.touch(sheet, at);
         Ok(())
-    })?;
+    })
+}
+
+/// `python.output`: the cell's return type. The code stays; a cached result belongs to the old
+/// return type, so it goes, as with `python.set` and a new return type: the cell shows
+/// `#BLOCKED!` until it runs, and its old spill goes with its readers recalculated.
+fn output(s: &mut Session, p: &Json) -> Result<Json> {
+    let (sheet, at) = target_cell(s, p)?;
+    let Some(old) = existing(s, sheet, at)? else {
+        return Err(EngineError::Other(format!("{} isn't a Python cell, so it has no Python output to change", at.a1())));
+    };
+    let object = output_mode("python.output", p, "mode")?.unwrap_or(!old.object);
+    let mode = |object: bool| if object { "object" } else { "value" };
+    if object != old.object {
+        let py = PyCode { code: old.code.clone(), object, output: None, stale: false };
+        store(s, "python.output", sheet, at, py, Some(old))?;
+    }
     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)}))
+    Ok(json!({"cell": at.a1(), "output": mode(object), "value": crate::cmd::inspect::value_json(&shown)}))
 }

crates/engine/src/lib.rs

@@ -792,7 +792,7 @@ impl Session {
                 // A toggle (Bold) is recorded with the state it set, as Excel records
                 // `Bold = True`: playing it back sets that state rather than flipping it again.
                 let params = match &result {
-                    Ok(r) if params.is_object() => record::with_toggle_state(cmd::journal_params(&params), r),
+                    Ok(r) if params.is_object() => record::with_toggle_state(id, cmd::journal_params(&params), r),
                     _ => cmd::journal_params(&params),
                 };
                 let kept = rec.push(record::RecordedStep { doc, sheet, sheet_name, selection, id: id.to_string(), params });

crates/engine/src/record.rs

@@ -74,15 +74,21 @@ impl Default for Recorder {
 
 /// `params` with the state a toggle set (Bold's `on`, from the command's result), as Excel
 /// records `Bold = True`: playing the step back sets that state rather than flipping it again.
-pub(crate) fn with_toggle_state(mut params: Json, result: &Json) -> Json {
-    if let Some(on @ Json::Bool(_)) = result.get("on") {
+/// `python.output` without `mode` is a toggle too: it keeps the return type it set (the result's
+/// `output`) as its `mode`.
+pub(crate) fn with_toggle_state(id: &str, mut params: Json, result: &Json) -> Json {
+    let state = match id {
+        "python.output" => result.get("output").filter(|o| o.is_string()).map(|o| ("mode", o)),
+        _ => result.get("on").filter(|o| o.is_boolean()).map(|o| ("on", o)),
+    };
+    if let Some((key, state)) = state {
         if params.is_null() {
             params = Json::Object(serde_json::Map::new());
         }
         if let Json::Object(p) = &mut params
-            && !p.contains_key("on")
+            && p.get(key).is_none_or(Json::is_null)
         {
-            p.insert("on".into(), on.clone());
+            p.insert(key.into(), state.clone());
         }
     }
     params

crates/engine/src/tests/python_commands.rs

@@ -89,3 +89,157 @@ fn set_over_a_spilling_formula_takes_the_spill_away() {
     assert_eq!(v(&s, "A3"), Value::Empty);
     assert!(s.doc().unwrap().wb.sheet(0).unwrap().spill_ranges.is_empty());
 }
+
+/// A cached result as a file would have kept it on the cell: `[[1], [2], [3]]`, a list.
+fn cache_list(s: &mut crate::Session, cell: &str) {
+    let at = gridcraft_core::CellRef::parse(cell).unwrap();
+    let d = s.doc_mut().unwrap();
+    let wb = std::sync::Arc::make_mut(&mut d.wb);
+    let sh = wb.sheet_mut(0).unwrap();
+    let py = std::sync::Arc::make_mut(sh.cells.get_mut(at).unwrap().py.as_mut().unwrap());
+    let arr = gridcraft_core::Array::new(3, 1, vec![1.0.into(), 2.0.into(), 3.0.into()]).map(Value::from).unwrap();
+    py.output = Some(gridcraft_model::PyOutput { value: arr, type_name: "list".into(), preview: "[1, 2, 3]".into(), convertible: true });
+}
+
+/// A1 holds `[1, 2, 3]` with its cached list spilled over A1:A3 as Excel values; B1 reads A3.
+fn cached_spill_with_a_reader() -> crate::Session {
+    let mut s = s();
+    set(&mut s, "A1", "[1, 2, 3]");
+    cache_list(&mut s, "A1");
+    s.execute("formulas.calculateNow", json!({})).unwrap();
+    s.execute("cell.set", json!({"cell": "B1", "input": "=SUM(A3)"})).unwrap();
+    assert_eq!(v(&s, "A1"), Value::Number(1.0));
+    assert_eq!(v(&s, "B1"), Value::Number(3.0));
+    s
+}
+
+fn output_of(s: &mut crate::Session, cell: &str) -> serde_json::Value {
+    s.execute("cell.get", json!({"cell": cell})).unwrap()["python"]["output"].clone()
+}
+
+fn undo_depth(s: &mut crate::Session) -> Option<usize> {
+    s.execute("history.list", json!({})).unwrap()["undo"].as_array().map(Vec::len)
+}
+
+#[test]
+fn output_switches_the_return_type_and_drops_a_result_of_the_other_type() {
+    let mut s = cached_spill_with_a_reader();
+    // The mode it already has changes nothing and adds no undo step.
+    let undo = undo_depth(&mut s);
+    let r = s.execute("python.output", json!({"cell": "A1", "mode": "value"})).unwrap();
+    assert_eq!(r["output"], "value");
+    assert_eq!(undo_depth(&mut s), undo);
+    assert_eq!(v(&s, "A3"), Value::Number(3.0), "the kept result still spills");
+    // Without `mode` it switches. The cached list is an Excel value's result: as with python.set
+    // and a new return type, the cell has no result until it runs, and its old spill goes with
+    // its readers recalculated.
+    s.execute("selection.set", json!({"cell": "A1"})).unwrap();
+    let r = s.execute("python.output", json!({})).unwrap();
+    assert_eq!(r["output"], "object");
+    assert_eq!(v(&s, "A1"), Value::Error(CellError::Blocked));
+    assert_eq!(v(&s, "A3"), Value::Empty, "the old spill goes");
+    assert_eq!(v(&s, "B1"), Value::Number(0.0), "and what read it is recalculated");
+    assert_eq!(s.execute("cell.get", json!({"cell": "A1"})).unwrap()["python"]["code"], "[1, 2, 3]", "the code stays");
+    // The same as python.set with the same code and the other return type.
+    let mut by_set = cached_spill_with_a_reader();
+    by_set.execute("python.set", json!({"cell": "A1", "code": "[1, 2, 3]", "output": "object"})).unwrap();
+    for c in ["A1", "A3", "B1"] {
+        assert_eq!(v(&s, c), v(&by_set, c), "{c}");
+    }
+    assert_eq!(output_of(&mut s, "A1"), output_of(&mut by_set, "A1"));
+    // Switching back brings no result back: the dropped one belonged to the old type.
+    s.execute("python.output", json!({"cell": "A1", "mode": "value"})).unwrap();
+    assert_eq!(v(&s, "A1"), Value::Error(CellError::Blocked));
+    assert_eq!(v(&s, "A3"), Value::Empty);
+    // Each switch is one undo step, and undoing the first brings the result and its spill back.
+    s.execute("edit.undo", json!({})).unwrap();
+    assert_eq!(output_of(&mut s, "A1"), "object");
+    s.execute("edit.undo", json!({})).unwrap();
+    assert_eq!(output_of(&mut s, "A1"), "value");
+    assert_eq!(v(&s, "A1"), Value::Number(1.0));
+    assert_eq!(v(&s, "A3"), Value::Number(3.0));
+    assert_eq!(v(&s, "B1"), Value::Number(3.0));
+    // A cell with no result keeps showing #BLOCKED!.
+    set(&mut s, "C1", "x = 1");
+    s.execute("python.output", json!({"cell": "C1"})).unwrap();
+    assert_eq!(v(&s, "C1"), Value::Error(CellError::Blocked));
+    assert_eq!(output_of(&mut s, "C1"), "object");
+}
+
+#[test]
+fn output_refuses_table_headers_and_locked_cells_as_set_does() {
+    let mut s = s();
+    // A Python cell in a table's header row (as a file could hold one): python.set refuses to
+    // put code there, and python.output refuses to change it.
+    s.execute("range.setValues", json!({"range": "C1", "values": [["Name", "Score"], ["a", 1]]})).unwrap();
+    s.execute("insert.table", json!({"range": "C1:D2", "headers": true})).unwrap();
+    {
+        let d = s.doc_mut().unwrap();
+        let wb = std::sync::Arc::make_mut(&mut d.wb);
+        let cell = wb.sheet_mut(0).unwrap().cells.get_mut(gridcraft_core::CellRef::parse("D1").unwrap()).unwrap();
+        cell.py = Some(std::sync::Arc::new(gridcraft_model::PyCode::new("'Score'")));
+    }
+    let e = s.execute("python.output", json!({"cell": "D1"})).unwrap_err();
+    assert!(e.to_string().contains("table header"), "{e}");
+    assert_eq!(output_of(&mut s, "D1"), "value");
+    // A locked cell on a protected sheet refuses it; an unlocked one formatted Hidden takes it,
+    // as it takes python.set.
+    set(&mut s, "F1", "1");
+    set(&mut s, "G1", "2");
+    s.execute("home.formatCells", json!({"range": "G1", "style": {"protection": {"locked": false, "hidden": true}}})).unwrap();
+    s.execute("review.protectSheet", json!({})).unwrap();
+    let e = s.execute("python.output", json!({"cell": "F1"})).unwrap_err();
+    assert!(e.to_string().contains("protected"), "{e}");
+    assert_eq!(output_of(&mut s, "F1"), "value");
+    s.execute("python.set", json!({"cell": "G1", "code": "2"})).unwrap();
+    s.execute("python.output", json!({"cell": "G1"})).unwrap();
+    assert_eq!(output_of(&mut s, "G1"), "object");
+}
+
+#[test]
+fn output_refuses_cells_without_python_and_bad_modes() {
+    let mut s = s();
+    // Empty params: the active cell, which holds no Python code.
+    let e = s.execute("python.output", json!({})).unwrap_err();
+    assert!(e.to_string().contains("isn't a Python cell"), "{e}");
+    s.execute("cell.set", json!({"cell": "B1", "input": "=1+1"})).unwrap();
+    assert!(s.execute("python.output", json!({"cell": "B1"})).is_err());
+    set(&mut s, "A1", "1");
+    for p in [json!({"cell": "A1", "mode": "sideways"}), json!({"cell": "A1", "mode": 1}), json!({"cell": "nowhere"})] {
+        assert!(s.execute("python.output", p.clone()).is_err(), "{p}");
+    }
+    assert_eq!(s.execute("cell.get", json!({"cell": "A1"})).unwrap()["python"]["output"], "value");
+    // python.set refuses a return type that isn't text too, rather than ignoring it.
+    assert!(s.execute("python.set", json!({"cell": "A1", "code": "1", "output": true})).is_err());
+}
+
+#[test]
+fn a_recorded_switch_keeps_the_return_type_it_set() {
+    // python.output without `mode` switches, so a recording keeps the return type the switch set
+    // (as Bold records `on`): played back on a cell that already has it, nothing switches back.
+    let mut s = s();
+    set(&mut s, "A1", "1");
+    s.execute("macro.record", json!({"relative": false})).unwrap();
+    s.execute("python.output", json!({"cell": "A1"})).unwrap();
+    s.execute("macro.record", json!({"stop": true, "name": "Rec"})).unwrap();
+    assert_eq!(output_of(&mut s, "A1"), "object");
+    let m = s.execute("macro.get", json!({"name": "Rec"})).unwrap();
+    let m = crate::macros::parse_macro(&m).unwrap();
+    let steps = crate::macros::command_steps(&m.steps);
+    let step = steps.iter().find(|(_, id, _)| *id == "python.output").map(|(_, _, p)| (*p).clone());
+    assert_eq!(step.and_then(|p| p.get("mode").cloned()), Some(json!("object")));
+    for _ in 0..2 {
+        s.execute("macro.run", json!({"name": "Rec", "onConflict": "overwrite"})).unwrap();
+        assert_eq!(output_of(&mut s, "A1"), "object");
+    }
+    // A run inside a script is recorded the same way.
+    s.execute("automate.newScript", json!({"name": "Switch", "commands": [{"command": "python.output", "params": {"cell": "A1"}}]})).unwrap();
+    s.execute("macro.record", json!({"relative": false})).unwrap();
+    s.execute("automate.runScript", json!({"name": "Switch"})).unwrap();
+    s.execute("macro.record", json!({"stop": true, "name": "Rec2"})).unwrap();
+    assert_eq!(output_of(&mut s, "A1"), "value");
+    for _ in 0..2 {
+        s.execute("macro.run", json!({"name": "Rec2", "onConflict": "overwrite"})).unwrap();
+        assert_eq!(output_of(&mut s, "A1"), "value");
+    }
+}

crates/engine/tests/it/command_sequence_fuzz.rs

@@ -793,6 +793,7 @@ const CURATED: &[(&str, usize)] = &[
     ("formulas.deleteName", 1),
     ("edit.replace", 1),
     ("python.set", 3),
+    ("python.output", 2),
 ];
 
 fn generate(rng: &mut Rng, s: &Session) -> Step {
@@ -845,6 +846,14 @@ fn curated_params(rng: &mut Rng, wb: &Workbook, id: &str) -> Json {
             }
             p
         }
+        "python.output" => {
+            // Mostly the cells python.set writes to; a missing mode switches.
+            let mut p = json!({"cell": near_cell(rng).a1()});
+            if rng.chance(50) {
+                p["mode"] = json!(if rng.chance(50) { "object" } else { "value" });
+            }
+            p
+        }
         "range.fill" => json!({"range": near_range(rng).a1(), "input": input(rng, wb)}),
         "selection.set" => {
             let mut p = json!({"range": range(rng)});