From f18ec0407fa1715812281a3c696f44815492463c Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Mon, 2 Nov 2026 12:06:25 -0500 Subject: [PATCH] Form controls: a fixed input range is refused for a macro that takes no inputs A button could be given a fixed input range for any macro name; pressing it then passed the range to macro.run as `inputs`, which check_params refuses for a macro without inputs ("it takes no inputs"), so such a button failed on every press. Now insert.button and control.assign refuse a fixed input when the named macro exists and declares no inputs, naming the macro; a macro the workbook does not have yet is not checked, and the assignment is kept as given. On open, a stored fixed input for a macro without inputs becomes the selection: the file itself assigns the macro on the selection, and the stored range would fail every press. Tests: a_fixed_input_for_a_macro_without_inputs_is_refused_when_assigned (insert and assign are refused with the reason in any letter case; a macro with an input and a macro not yet defined take the range; a stored fixed input comes back as the selection). format_and_assign and a_kept_button_action_comes_back_only_on_the_same_button give their macro an input. --- crates/engine/src/cmd/controls.rs | 22 ++++++++++++++++- crates/engine/src/tests/controls.rs | 38 +++++++++++++++++++++++++++-- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/controls.rs b/crates/engine/src/cmd/controls.rs index a7c0afbf8..5acfb75b6 100644 --- a/crates/engine/src/cmd/controls.rs +++ b/crates/engine/src/cmd/controls.rs @@ -404,6 +404,14 @@ pub(crate) fn take_requests(s: &mut Session, queued: usize, name: &str) -> Optio blocked } +/// The stored name of the workbook's macro `name` when it exists and declares no inputs: a fixed +/// input range would make every press fail (`macro.run` refuses inputs it doesn't take), so such +/// an assignment is refused when it is made and dropped to the selection when a file brings one +/// in. A macro the workbook doesn't have (yet) is not checked: the assignment is kept as given. +fn takes_no_inputs(wb: &Workbook, name: &str) -> Option { + crate::macros::find(wb, name).and_then(|i| wb.macros.get(i)).filter(|m| m.inputs.is_empty()).map(|m| m.name.clone()) +} + /// A button action from `macro` (with `input` and `output`) or `command` (with `params`). fn action_param(s: &Session, cmd: &str, p: &Json) -> Result> { if let Some(name) = str_param(p, "macro").map(str::trim).filter(|n| !n.is_empty()) { @@ -418,6 +426,9 @@ fn action_param(s: &Session, cmd: &str, p: &Json) -> Result Result> { /// button's name and anchor cell, the button at the stored place must have that name and anchor /// cell (another application may have added, removed or reordered controls), and it must not /// have been given another macro since (a stored macro action comes back only on a button that -/// still runs that macro, a stored command only on one the file gives no macro). +/// still runs that macro, a stored command only on one the file gives no macro). A stored fixed +/// input for a macro that takes no inputs becomes the selection (see [`takes_no_inputs`]). pub(crate) fn load_action_names(wb: &mut Workbook) { let Some(items) = stash::get_json(wb, ACTIONS_KEY) else { return }; stash::remove(wb, ACTIONS_KEY); @@ -681,6 +693,14 @@ pub(crate) fn load_action_names(wb: &mut Workbook) { if !valid || !wb.sheet(si).is_some_and(|sh| sh.controls.get(index).is_some_and(same)) { continue; } + let action = match action { + // The macro takes no inputs now: the stored range would fail every press, so the + // button runs the macro on the selection, as the file itself assigns it. + ControlAction::Macro { name, input: MacroInput::Fixed(_), output } if takes_no_inputs(wb, &name).is_some() => { + ControlAction::Macro { name, input: MacroInput::Selection, output } + } + other => other, + }; if let Some(c) = wb.sheet_mut(si).and_then(|sh| sh.controls.get_mut(index)) { c.action = Some(action); } diff --git a/crates/engine/src/tests/controls.rs b/crates/engine/src/tests/controls.rs index 27c0a685a..2bcf5d671 100644 --- a/crates/engine/src/tests/controls.rs +++ b/crates/engine/src/tests/controls.rs @@ -148,7 +148,8 @@ fn format_and_assign() { s.execute("control.format", json!({"id": c, "value": 25})).unwrap(); assert_eq!(v(&s, "B1"), Value::Number(50.0), "without a link nothing is written"); // Buttons: text and action. - s.execute("macro.save", json!({"name": "Fill", "steps": [{"command": "cell.set", "params": {"cell": "A1", "input": "x"}}]})).unwrap(); + // A fixed input needs a macro that takes one. + s.execute("macro.save", json!({"name": "Fill", "steps": [{"command": "cell.set", "params": {"cell": "Input.cell(1,1)", "input": "x"}}], "inputs": [{"name": "Input", "kind": "range"}]})).unwrap(); let b = id(&s.execute("insert.button", json!({"text": "Go", "macro": "Fill", "input": "C2:D4", "output": "F2"})).unwrap()); let action = control(&s, b).action; assert_eq!(action, Some(ControlAction::Macro { name: "Fill".into(), input: MacroInput::Fixed("C2:D4".into()), output: Some("F2".into()) })); @@ -380,7 +381,7 @@ fn reopen(s: &mut Session) { fn a_kept_button_action_comes_back_only_on_the_same_button() { use crate::cmd::controls::{load_action_names, with_action_names}; let mut s = s(); - s.execute("macro.save", json!({"name": "Mark", "steps": [{"command": "cell.set", "params": {"cell": "A1", "input": "m"}}]})).unwrap(); + s.execute("macro.save", json!({"name": "Mark", "steps": [{"command": "cell.set", "params": {"cell": "Input.cell(1,1)", "input": "m"}}], "inputs": [{"name": "Input", "kind": "range"}]})).unwrap(); s.execute("insert.button", json!({"at": "D2", "text": "Run", "command": "cell.set", "params": {"cell": "A9", "input": "cmd"}})).unwrap(); s.execute("insert.button", json!({"at": "D6", "text": "Other"})).unwrap(); s.execute("insert.button", json!({"at": "D9", "text": "Fixed", "macro": "Mark", "input": "F1:G2"})).unwrap(); @@ -467,6 +468,39 @@ fn a_kept_action_without_the_buttons_name_or_anchor_is_dropped() { assert!(matches!(kept(&items), Some(ControlAction::Command { .. })), "the writer's own entry still comes back"); } +/// A fixed input range for a macro that declares no inputs would fail every press (`macro.run` +/// refuses inputs the macro doesn't take), so the assignment is refused when it is made, naming +/// the macro; a macro the workbook doesn't have yet is not checked, and a macro with an input +/// takes the range. A file that brings such a stored input in gets the macro on the selection. +#[test] +fn a_fixed_input_for_a_macro_without_inputs_is_refused_when_assigned() { + use crate::cmd::controls::{load_action_names, with_action_names}; + let mut s = s(); + s.execute("macro.save", json!({"name": "Fill", "steps": [{"command": "cell.set", "params": {"cell": "A1", "input": "x"}}]})).unwrap(); + s.execute("macro.save", json!({"name": "Mark", "steps": [{"command": "cell.set", "params": {"cell": "Input.cell(1,1)", "input": "m"}}], "inputs": [{"name": "Input", "kind": "range"}]})).unwrap(); + let want = "macro 'Fill' takes no inputs"; + assert!(err(s.execute("insert.button", json!({"at": "B2", "macro": "Fill", "input": "C2:D4"}))).contains(want)); + assert_eq!(s.doc().unwrap().wb.sheet(0).unwrap().controls.len(), 0, "no button was placed"); + let b = id(&s.execute("insert.button", json!({"at": "B2", "macro": "Fill", "output": "F2"})).unwrap()); + let e = err(s.execute("control.assign", json!({"id": b, "macro": "fill", "input": "C2:D4"}))); + assert!(e.contains(want), "any letter case: {e}"); + assert_eq!(control(&s, b).action, Some(ControlAction::Macro { name: "Fill".into(), input: MacroInput::Selection, output: Some("F2".into()) })); + s.execute("control.assign", json!({"id": b, "macro": "Mark", "input": "C2:D4"})).unwrap(); + s.execute("control.assign", json!({"id": b, "macro": "Later", "input": "C2:D4"})).unwrap(); + assert_eq!(control(&s, b).action, Some(ControlAction::Macro { name: "Later".into(), input: MacroInput::Fixed("C2:D4".into()), output: None })); + // A file's stored fixed input for Fill (another version assigned it) comes back as the selection. + s.execute("control.assign", json!({"id": b, "macro": "Fill", "output": "F2"})).unwrap(); + let mut wb = (*s.doc().unwrap().wb).clone(); + wb.sheet_mut(0).unwrap().controls[0].action = + Some(ControlAction::Macro { name: "Fill".into(), input: MacroInput::Fixed("C2:D4".into()), output: Some("F2".into()) }); + let mut stored = with_action_names(&wb).unwrap().unwrap(); + load_action_names(&mut stored); + assert_eq!( + stored.sheets[0].controls[0].action, + Some(ControlAction::Macro { name: "Fill".into(), input: MacroInput::Selection, output: Some("F2".into()) }) + ); +} + /// The saved xlsx bytes of `s`'s workbook. fn saved_xlsx(s: &mut Session) -> Vec { let r = s.execute("file.saveBytes", json!({"format": "xlsx"})).unwrap();