From c9525ab5fbe4b59df46ad7f9e4c4a6c67f58d83e Mon Sep 17 00:00:00 2001 From: Mikkel Georgsen <7456949+mikl0s@users.noreply.github.com> Date: Sat, 10 Oct 2026 18:40:02 +0200 Subject: [PATCH] Cell references: an invalid reference is an error instead of the active cell cell_param returned None for any cell argument it could not parse, so every command that falls back to the selection (cell.set, view.freezePanes, formulas.trace*, insert.* and others) silently used the active cell instead: set_cell with cell "ZZZZ0" wrote A1 and reported success. It also dropped a sheet prefix, so "Other!B2" was applied to the active sheet. cell_param now returns Result>. An absent or null value is still None, so the selection fallback is unchanged; an unparsable or non-string value is an error; a sheet prefix naming a sheet other than the one the command works on is an error. cell_param_on is for the commands that resolve their own sheet (cell.get, links, comments, notes, checkboxes, chart placement, paste): they accept a prefix naming that sheet. cell.set resolves its own target sheet and still writes to another existing sheet. --- crates/engine/src/cmd/data.rs | 4 +- crates/engine/src/cmd/edit.rs | 11 +-- crates/engine/src/cmd/extra.rs | 6 +- crates/engine/src/cmd/formulas.rs | 8 +-- crates/engine/src/cmd/insert.rs | 26 +++---- crates/engine/src/cmd/inspect.rs | 2 +- crates/engine/src/cmd/mod.rs | 24 ++++++- crates/engine/src/cmd/spelling.rs | 2 +- crates/engine/src/cmd/view.rs | 4 +- crates/mcp/src/tests.rs | 113 ++++++++++++++++++++++++++++++ 10 files changed, 168 insertions(+), 32 deletions(-) diff --git a/crates/engine/src/cmd/data.rs b/crates/engine/src/cmd/data.rs index fd50afb..74cbe58 100644 --- a/crates/engine/src/cmd/data.rs +++ b/crates/engine/src/cmd/data.rs @@ -741,7 +741,7 @@ fn text_to_columns(s: &mut Session, p: &Json) -> Result { p.get("fixedWidths").and_then(Json::as_array).map(|a| a.iter().filter_map(|x| x.as_u64().map(|v| v as usize)).collect()); let consecutive = bool_param(p, "treatConsecutive").unwrap_or(false); let quote = str_param(p, "textQualifier").and_then(|q| q.chars().next()).unwrap_or('"'); - let dest = cell_param(p, "destination").unwrap_or(r.start); + let dest = cell_param_on(s, p, "destination", sheet)?.unwrap_or(r.start); edit(s, |cx| { let Some(sh) = cx.wb.sheet(sheet) else { return Ok(()) }; let mut out: Vec<(CellRef, String)> = Vec::new(); @@ -980,7 +980,7 @@ fn compare_op(op: CfOperator, n: f64, a: Option, b: Option) -> bool { fn validate_cmd(s: &mut Session, p: &Json) -> Result { let d = s.doc()?; - let at = cell_param(p, "cell").unwrap_or(d.selection.active); + let at = cell_param(s, p, "cell")?.unwrap_or(d.selection.active); let input = str_param(p, "input").unwrap_or(""); match check_validation(&d.wb, d.wb.active_sheet, at, input) { None => Ok(json!({"ok": true})), diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index bb54fe0..0a2c532 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -192,7 +192,7 @@ pub(crate) fn input_to_cell(input: &str, sheet: usize, at: CellRef, wb: &mut gri fn cell_set(s: &mut Session, p: &Json) -> Result { let sheet = target_sheet(s, p)?; - let at = match cell_param(p, "cell") { + let at = match cell_param_on(s, p, "cell", sheet)? { Some(c) => c, None => s.doc()?.selection.active, }; @@ -340,8 +340,9 @@ fn selection_set(s: &mut Session, p: &Json) -> Result { s.doc_mut()?.wb_switch_sheet(i); } let ranges = target_ranges(s, p)?; + let active = cell_param(s, p, "active")?; let d = s.doc_mut()?; - let active = cell_param(p, "active").or_else(|| ranges.last().map(|r| r.start)).unwrap_or_default(); + let active = active.or_else(|| ranges.last().map(|r| r.start)).unwrap_or_default(); let mut sel = crate::Selection { active, anchor: ranges.last().map(|r| r.start).unwrap_or(active), ranges }; if let Some(sh) = d.wb.active() { sel.expand_merges(sh); @@ -672,7 +673,7 @@ fn paste(s: &mut Session, p: &Json) -> Result { fn paste_text(s: &mut Session, p: &Json, text: &str) -> Result { let sheet = target_sheet(s, p)?; - let at = cell_param(p, "at").unwrap_or(s.doc()?.selection.current().start); + let at = cell_param_on(s, p, "at", sheet)?.unwrap_or(s.doc()?.selection.current().start); let rows: Vec> = parse_tsv(text); let h = rows.len(); let w = rows.iter().map(Vec::len).max().unwrap_or(0); @@ -771,14 +772,14 @@ fn paste_special(s: &mut Session, p: &Json) -> Result { let link = bool_param(p, "link").unwrap_or(false); let dest_sheet = target_sheet(s, p)?; let sel = s.doc()?.selection.current(); - let at = cell_param(p, "at").unwrap_or(sel.start); + let at = cell_param_on(s, p, "at", dest_sheet)?.unwrap_or(sel.start); let src = clip.range; let (h, w) = if transpose { (src.width(), src.height()) } else { (src.height(), src.width()) }; // Tile the copy over a larger destination that is a multiple of its size. let tile = sel.height() % h == 0 && sel.width() % w == 0 && !sel.is_single() - && cell_param(p, "at").is_none() + && cell_param_on(s, p, "at", dest_sheet)?.is_none() && !sel.is_full_cols() && !sel.is_full_rows() && sel.count() <= 4_000_000; diff --git a/crates/engine/src/cmd/extra.rs b/crates/engine/src/cmd/extra.rs index b6a362b..a9d3c6d 100644 --- a/crates/engine/src/cmd/extra.rs +++ b/crates/engine/src/cmd/extra.rs @@ -376,7 +376,7 @@ fn move_chart(s: &mut Session, p: &Json) -> Result { Some(n) => Some(s.doc()?.wb.sheet_index(n).ok_or_else(|| bad("chart.move", "no such sheet"))?), None => None, }; - let at = cell_param(p, "at"); + let at = cell_param_on(s, p, "at", target.unwrap_or(si))?; edit(s, |cx| { let mut chart = cx.sheet_mut(si)?.charts.remove(ci); if let Some(a) = at { @@ -856,7 +856,7 @@ fn checkbox(s: &mut Session, p: &Json) -> Result { fn toggle_checkbox(s: &mut Session, p: &Json) -> Result { let si = target_sheet(s, p)?; let d = s.doc()?; - let cells: Vec = match cell_param(p, "cell") { + let cells: Vec = match cell_param_on(s, p, "cell", si)? { Some(c) => vec![c], None => d.selection.ranges.iter().flat_map(|r| r.iter().take(100_000).collect::>()).collect(), }; @@ -922,7 +922,7 @@ fn new_cell_style(s: &mut Session, p: &Json) -> Result { let name = str_param(p, "name").ok_or_else(|| bad("home.newCellStyle", "missing `name`"))?.to_string(); let d = s.doc()?; let sh = d.wb.active().ok_or(EngineError::NoDocument)?; - let c = cell_param(p, "fromCell").unwrap_or(d.selection.active); + let c = cell_param(s, p, "fromCell")?.unwrap_or(d.selection.active); let style = d.wb.styles.get(sh.style_id(c)).clone(); edit(s, |cx| { cx.wb.cell_styles.retain(|(n, _)| !n.eq_ignore_ascii_case(&name)); diff --git a/crates/engine/src/cmd/formulas.rs b/crates/engine/src/cmd/formulas.rs index f3576e4..019dd8f 100644 --- a/crates/engine/src/cmd/formulas.rs +++ b/crates/engine/src/cmd/formulas.rs @@ -373,7 +373,7 @@ fn create_from_selection(s: &mut Session, p: &Json) -> Result { fn trace_precedents(s: &mut Session, p: &Json) -> Result { let d = s.doc()?; - let at = cell_param(p, "cell").unwrap_or(d.selection.active); + let at = cell_param(s, p, "cell")?.unwrap_or(d.selection.active); let areas = d.calc.graph.precedents_of((d.wb.active_sheet, at)); let v: Vec = areas.iter().map(|a| json!({"sheet": d.wb.sheet(a.sheet).map(|s| s.name.clone()), "range": a.range.a1()})).collect(); Ok(Json::Array(v)) @@ -381,7 +381,7 @@ fn trace_precedents(s: &mut Session, p: &Json) -> Result { fn trace_dependents(s: &mut Session, p: &Json) -> Result { let d = s.doc()?; - let at = cell_param(p, "cell").unwrap_or(d.selection.active); + let at = cell_param(s, p, "cell")?.unwrap_or(d.selection.active); let mut deps = Vec::new(); d.calc.graph.dependents(d.wb.active_sheet, at, &mut deps); deps.sort(); @@ -422,7 +422,7 @@ fn error_checking(s: &mut Session, _: &Json) -> Result { fn evaluate_formula(s: &mut Session, p: &Json) -> Result { let d = s.doc()?; - let at = cell_param(p, "cell").unwrap_or(d.selection.active); + let at = cell_param(s, p, "cell")?.unwrap_or(d.selection.active); let sheet = d.wb.active_sheet; let text = match str_param(p, "formula") { Some(f) => f.trim_start_matches('=').to_string(), @@ -454,7 +454,7 @@ fn evaluate_formula(s: &mut Session, p: &Json) -> Result { fn evaluate(s: &mut Session, p: &Json) -> Result { let d = s.doc()?; let f = str_param(p, "formula").ok_or_else(|| bad("formulas.evaluate", "missing `formula`"))?; - let at = cell_param(p, "cell").unwrap_or(d.selection.active); + let at = cell_param(s, p, "cell")?.unwrap_or(d.selection.active); let v = gridcraft_calc::evaluate(&d.wb, d.wb.active_sheet, at, f); Ok(crate::cmd::inspect::value_json(&v)) } diff --git a/crates/engine/src/cmd/insert.rs b/crates/engine/src/cmd/insert.rs index 4c96005..463df81 100644 --- a/crates/engine/src/cmd/insert.rs +++ b/crates/engine/src/cmd/insert.rs @@ -577,7 +577,7 @@ fn insert_chart(s: &mut Session, p: &Json) -> Result { return Err(EngineError::Other("Select data for the chart first.".into())); } let id = d.wb.next_object_id(); - let at = cell_param(p, "at").unwrap_or_else(|| CellRef::new(r.start.row, r.end.col + 2)); + let at = cell_param_on(s, p, "at", sheet)?.unwrap_or_else(|| CellRef::new(r.start.row, r.end.col + 2)); let title = str_param(p, "title").map(str::to_string).or_else(|| { if series.len() == 1 { series[0].name.as_ref().map(|n| gridcraft_calc::evaluate(&d.wb, sheet, at, n).display()) @@ -626,6 +626,7 @@ fn find_chart(s: &Session, p: &Json) -> Result<(usize, usize)> { fn chart_set(s: &mut Session, p: &Json) -> Result { let (si, ci) = find_chart(s, p)?; + let at = cell_param_on(s, p, "at", si)?; edit(s, |cx| { let sh = cx.sheet_mut(si)?; let Some(c) = sh.charts.get_mut(ci) else { return Ok(Json::Null) }; @@ -659,7 +660,7 @@ fn chart_set(s: &mut Session, p: &Json) -> Result { if let Some(t) = p.get("yTitle") { c.y_title = t.as_str().map(str::to_string); } - if let Some(at) = cell_param(p, "at") { + if let Some(at) = at { c.anchor.cell = at; } if let Some(w) = f64_param(p, "width") { @@ -742,7 +743,7 @@ fn chart_delete(s: &mut Session, p: &Json) -> Result { fn insert_sparkline(s: &mut Session, p: &Json) -> Result { let src = str_param(p, "range").ok_or_else(|| bad("insert.sparkline", "missing `range`"))?.to_string(); - let loc = cell_param(p, "location").ok_or_else(|| bad("insert.sparkline", "missing `location`"))?; + let loc = cell_param(s, p, "location")?.ok_or_else(|| bad("insert.sparkline", "missing `location`"))?; let kind = match str_param(p, "type") { Some("column") => SparklineKind::Column, Some("winLoss") => SparklineKind::WinLoss, @@ -801,7 +802,7 @@ fn insert_picture(s: &mut Session, p: &Json) -> Result { let (w, h) = crate::io::image_size(&data).unwrap_or((320, 240)); let d = s.doc()?; let id = d.wb.next_object_id(); - let at = cell_param(p, "at").unwrap_or(d.selection.active); + let at = cell_param(s, p, "at")?.unwrap_or(d.selection.active); let scale = (640.0 / w.max(1) as f32).min(1.0); let img = Image { id, @@ -864,7 +865,7 @@ fn insert_shape(s: &mut Session, p: &Json) -> Result { }; let d = s.doc()?; let id = d.wb.next_object_id(); - let at = cell_param(p, "at").unwrap_or(d.selection.active); + let at = cell_param(s, p, "at")?.unwrap_or(d.selection.active); let fill = super::format::color_param(p.get("fill")).unwrap_or(if kind == ShapeKind::TextBox { Color::rgb(255, 255, 255) } else { Color::Theme(4, 0) }); let line = super::format::color_param(p.get("line")).unwrap_or(if kind == ShapeKind::TextBox { @@ -944,6 +945,7 @@ fn move_object(s: &mut Session, p: &Json) -> Result { let id = u32_param(p, "id").ok_or_else(|| bad("object.move", "missing `id`"))?; let kind = str_param(p, "kind").unwrap_or("").to_string(); let sheet = s.doc()?.wb.active_sheet; + let at = cell_param(s, p, "at")?; edit(s, |cx| { let sh = cx.sheet_mut(sheet)?; let anchor = match kind.as_str() { @@ -953,7 +955,7 @@ fn move_object(s: &mut Session, p: &Json) -> Result { _ => None, }; let Some(a) = anchor else { return Err(bad("object.move", "no such object")) }; - if let Some(at) = cell_param(p, "at") { + if let Some(at) = at { a.cell = at; } if let Some(v) = f64_param(p, "dx") { @@ -1000,7 +1002,7 @@ fn set_anchor_mode(s: &mut Session, p: &Json) -> Result { fn insert_link(s: &mut Session, p: &Json) -> Result { let sheet = target_sheet(s, p)?; - let at = cell_param(p, "cell").unwrap_or(s.doc()?.selection.active); + let at = cell_param_on(s, p, "cell", sheet)?.unwrap_or(s.doc()?.selection.active); if bool_param(p, "remove").unwrap_or(false) { return edit(s, |cx| { cx.sheet_mut(sheet)?.hyperlinks.remove(&at); @@ -1032,7 +1034,7 @@ fn insert_link(s: &mut Session, p: &Json) -> Result { fn comment(s: &mut Session, p: &Json, threaded: bool) -> Result { let sheet = target_sheet(s, p)?; - let at = cell_param(p, "cell").unwrap_or(s.doc()?.selection.active); + let at = cell_param_on(s, p, "cell", sheet)?.unwrap_or(s.doc()?.selection.active); let text = str_param(p, "text").unwrap_or("").to_string(); let author = str_param(p, "author").map(str::to_string).unwrap_or_else(|| s.prefs.user_name.clone()); edit(s, |cx| { @@ -1045,7 +1047,7 @@ fn comment(s: &mut Session, p: &Json, threaded: bool) -> Result { fn reply_comment(s: &mut Session, p: &Json) -> Result { let sheet = target_sheet(s, p)?; - let at = cell_param(p, "cell").unwrap_or(s.doc()?.selection.active); + let at = cell_param_on(s, p, "cell", sheet)?.unwrap_or(s.doc()?.selection.active); let text = str_param(p, "text").unwrap_or("").to_string(); let author = str_param(p, "author").map(str::to_string).unwrap_or_else(|| s.prefs.user_name.clone()); edit(s, |cx| { @@ -1057,7 +1059,7 @@ fn reply_comment(s: &mut Session, p: &Json) -> Result { fn resolve_comment(s: &mut Session, p: &Json) -> Result { let sheet = target_sheet(s, p)?; - let at = cell_param(p, "cell").unwrap_or(s.doc()?.selection.active); + let at = cell_param_on(s, p, "cell", sheet)?.unwrap_or(s.doc()?.selection.active); let v = bool_param(p, "resolved").unwrap_or(true); edit(s, |cx| { if let Some(c) = cx.sheet_mut(sheet)?.comments.get_mut(&at) { @@ -1078,7 +1080,7 @@ fn delete_comment(s: &mut Session, p: &Json) -> Result { fn show_note(s: &mut Session, p: &Json) -> Result { let sheet = target_sheet(s, p)?; - let at = cell_param(p, "cell").unwrap_or(s.doc()?.selection.active); + let at = cell_param_on(s, p, "cell", sheet)?.unwrap_or(s.doc()?.selection.active); edit(s, |cx| { if let Some(c) = cx.sheet_mut(sheet)?.comments.get_mut(&at) { c.visible = !c.visible; @@ -1303,7 +1305,7 @@ fn insert_icon(s: &mut Session, p: &Json) -> Result { let size = f64_param(p, "size").unwrap_or(48.0).clamp(8.0, 1000.0) as f32; let d = s.doc()?; let id = d.wb.next_object_id(); - let at = cell_param(p, "at").unwrap_or(d.selection.active); + let at = cell_param(s, p, "at")?.unwrap_or(d.selection.active); let color = super::format::color_param(p.get("color")).unwrap_or(Color::Theme(1, 0)); let shape = Shape { id, diff --git a/crates/engine/src/cmd/inspect.rs b/crates/engine/src/cmd/inspect.rs index de670e9..414ab42 100644 --- a/crates/engine/src/cmd/inspect.rs +++ b/crates/engine/src/cmd/inspect.rs @@ -119,7 +119,7 @@ fn cell_get(s: &mut Session, p: &Json) -> Result { let sheet = target_sheet(s, p)?; let d = s.doc()?; let sh = d.wb.sheet(sheet).ok_or(EngineError::NoDocument)?; - let c = cell_param(p, "cell").unwrap_or(d.selection.active); + let c = cell_param_on(s, p, "cell", sheet)?.unwrap_or(d.selection.active); let cell = sh.cell(c); let style = d.wb.styles.get(sh.style_id(c)); Ok(json!({ diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index c4a290e..7be6faf 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -219,8 +219,28 @@ pub(crate) fn split_sheet(r: &str) -> (Option, &str) { } } -pub(crate) fn cell_param(p: &Json, key: &str) -> Option { - str_param(p, key).and_then(|t| CellRef::parse(split_sheet(t).1)) +/// A single-cell parameter. Absent or null is `Ok(None)` (callers default it); a value that is not +/// a cell reference, or that names a sheet other than the active one, is an error rather than a +/// silent fallback to some other cell. +pub(crate) fn cell_param(s: &Session, p: &Json, key: &str) -> Result> { + cell_param_on(s, p, key, s.doc()?.wb.active_sheet) +} + +/// Like [`cell_param`], for a command that operates on `sheet` (a `Sheet!` prefix must name it). +pub(crate) fn cell_param_on(s: &Session, p: &Json, key: &str, sheet: usize) -> Result> { + let t = match p.get(key) { + None | Some(Json::Null) => return Ok(None), + Some(Json::String(t)) => t, + Some(v) => return Err(bad("cell", format!("invalid cell reference `{v}` for `{key}`"))), + }; + let (sh, body) = split_sheet(t); + let c = CellRef::parse(body).ok_or_else(|| bad("cell", format!("invalid cell reference `{t}` for `{key}`")))?; + if let Some(name) = sh + && s.doc()?.wb.sheet_index(&name) != Some(sheet) + { + return Err(bad("cell", format!("`{t}` for `{key}` names a sheet other than the one this command works on"))); + } + Ok(Some(c)) } /// Applies an edit to a copy of the active workbook, then recalculates and commits it. diff --git a/crates/engine/src/cmd/spelling.rs b/crates/engine/src/cmd/spelling.rs index 10892bc..e78a6ea 100644 --- a/crates/engine/src/cmd/spelling.rs +++ b/crates/engine/src/cmd/spelling.rs @@ -155,7 +155,7 @@ fn change(s: &mut Session, p: &Json) -> Result { if bool_param(p, "all").unwrap_or(false) { return s.execute("edit.replace", json!({"what": word, "with": to, "matchCase": true, "all": true})); } - let c = cell_param(p, "cell").ok_or_else(|| bad("review.changeSpelling", "missing `cell`"))?; + let c = cell_param(s, p, "cell")?.ok_or_else(|| bad("review.changeSpelling", "missing `cell`"))?; let text = s.doc()?.wb.active().and_then(|sh| sh.cell(c)).map(|x| x.input_text()).unwrap_or_default(); s.execute("cell.set", json!({"cell": c.a1(), "input": text.replacen(&word, &to, 1)})) } diff --git a/crates/engine/src/cmd/view.rs b/crates/engine/src/cmd/view.rs index d8450cd..c9bb195 100644 --- a/crates/engine/src/cmd/view.rs +++ b/crates/engine/src/cmd/view.rs @@ -113,7 +113,7 @@ fn freeze_panes(s: &mut Session, p: &Json) -> Result { if sh.freeze.is_some() && p.get("cell").is_none() { return set_freeze(s, None); } - let at = cell_param(p, "cell").unwrap_or(d.selection.active); + let at = cell_param(s, p, "cell")?.unwrap_or(d.selection.active); let top = sh.view_top_left; let rows = at.row.saturating_sub(top.row); let cols = at.col.saturating_sub(top.col); @@ -254,7 +254,7 @@ fn scale_fit(s: &mut Session, p: &Json) -> Result { } fn breaks(s: &mut Session, p: &Json) -> Result { - let at = cell_param(p, "cell").unwrap_or(s.doc()?.selection.active); + let at = cell_param(s, p, "cell")?.unwrap_or(s.doc()?.selection.active); let insert = bool_param(p, "insert").unwrap_or(false); let remove = bool_param(p, "remove").unwrap_or(false); let reset = bool_param(p, "reset").unwrap_or(false); diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index cf590be..844b923 100644 --- a/crates/mcp/src/tests.rs +++ b/crates/mcp/src/tests.rs @@ -392,3 +392,116 @@ fn file_commands_without_a_file_fail() { fails(&mut s, "execute_command", json!({"command": command, "params": {}})); } } + +// ---------- cell references ---------- + +fn cmd_fails(s: &mut Server, command: &str, params: Value) -> String { + fails(s, "execute_command", json!({"command": command, "params": params})) +} + +fn a1_display(s: &mut Server) -> String { + ok(s, "get_cell", json!({"cell": "A1"}))["text"].as_str().unwrap_or("").to_string() +} + +#[test] +fn set_cell_invalid_ref_is_error() { + let mut s = server(); + for bad in ["ZZZZ0", "B0", "A", "1", "A1:B2", "not a cell", "", "XFE1", "A1048577"] { + fails(&mut s, "set_cell", json!({"cell": bad, "input": "x"})); + } + assert_eq!(a1_display(&mut s), ""); +} + +#[test] +fn cell_ref_boundaries_still_work() { + let mut s = server(); + ok(&mut s, "set_cell", json!({"cell": "A1", "input": "first"})); + ok(&mut s, "set_cell", json!({"cell": "$B$2", "input": "abs"})); + ok(&mut s, "set_cell", json!({"cell": "XFD1048576", "input": "last"})); + ok(&mut s, "set_cell", json!({"cell": "xfd1", "input": "lower"})); + assert_eq!(ok(&mut s, "get_cell", json!({"cell": "B2"}))["text"], "abs"); + assert_eq!(ok(&mut s, "get_cell", json!({"cell": "XFD1048576"}))["text"], "last"); + assert_eq!(ok(&mut s, "get_cell", json!({"cell": "XFD1"}))["text"], "lower"); + assert_eq!(a1_display(&mut s), "first"); + // Ranges are still ranges for the commands that take them. + ok(&mut s, "read_range", json!({"range": "A1:B2"})); + ok(&mut s, "read_range", json!({"range": "$A$1:$B$2"})); + // The active sheet's own name is an accepted prefix; an absent cell still means the selection. + ok(&mut s, "set_cell", json!({"cell": "Sheet1!C3", "input": "named"})); + assert_eq!(ok(&mut s, "get_cell", json!({"cell": "C3"}))["text"], "named"); + ok(&mut s, "execute_command", json!({"command": "formulas.tracePrecedents", "params": {}})); +} + +#[test] +fn bad_ref_in_command_is_error() { + let mut s = server(); + ok(&mut s, "set_cell", json!({"cell": "A1", "input": "7"})); + for (cmd, key) in [("view.freezePanes", "cell"), ("formulas.tracePrecedents", "cell"), ("formulas.traceDependents", "cell"), ("cell.get", "cell")] + { + cmd_fails(&mut s, cmd, json!({ key: "B0" })); + cmd_fails(&mut s, cmd, json!({ key: "nonsense" })); + } + assert_eq!(a1_display(&mut s), "7"); +} + +#[test] +fn other_sheet_prefix_is_error() { + let mut s = server(); + ok(&mut s, "execute_command", json!({"command": "home.insertSheet", "params": {"name": "Other"}})); + ok(&mut s, "execute_command", json!({"command": "sheet.activate", "params": {"sheet": 0}})); + fails(&mut s, "set_cell", json!({"cell": "Nope!B2", "input": "x"})); + // A command that cannot honour a sheet prefix must not drop it and use the active sheet. + cmd_fails(&mut s, "view.freezePanes", json!({"cell": "Other!B2"})); + cmd_fails(&mut s, "formulas.tracePrecedents", json!({"cell": "Other!B2"})); + ok(&mut s, "set_cell", json!({"cell": "Sheet1!B2", "input": "here"})); + assert_eq!(ok(&mut s, "get_cell", json!({"cell": "B2"}))["text"], "here"); + // cell.set resolves another existing sheet itself and writes there, not to the active sheet. + ok(&mut s, "set_cell", json!({"cell": "Other!B3", "input": "there"})); + assert_eq!(ok(&mut s, "get_cell", json!({"cell": "B3"}))["text"], ""); + let r = ok(&mut s, "read_range", json!({"range": "Other!B3"})); + assert!(r.to_string().contains("there"), "{r}"); +} + +#[test] +fn cross_sheet_cell_refs_resolve() { + let mut s = server(); + ok(&mut s, "execute_command", json!({"command": "home.insertSheet", "params": {"name": "Other"}})); + ok(&mut s, "execute_command", json!({"command": "home.insertSheet", "params": {"name": "My Sheet"}})); + ok(&mut s, "execute_command", json!({"command": "sheet.activate", "params": {"sheet": 0}})); + ok(&mut s, "set_cell", json!({"cell": "Other!B3", "input": "there"})); + ok(&mut s, "set_cell", json!({"cell": "'My Sheet'!A1", "input": "quoted"})); + // Reads honour the prefix. + assert_eq!(ok(&mut s, "get_cell", json!({"cell": "Other!B3"}))["text"], "there"); + assert_eq!(ok(&mut s, "get_cell", json!({"cell": "'My Sheet'!A1"}))["text"], "quoted"); + assert_eq!(ok(&mut s, "get_cell", json!({"cell": "B3"}))["text"], ""); + // Cell-level commands that resolve their own sheet land on it. + ok(&mut s, "execute_command", json!({"command": "review.newComment", "params": {"cell": "Other!B3", "text": "note"}})); + let c = ok(&mut s, "get_cell", json!({"cell": "Other!B3"})); + assert!(c.to_string().contains("note"), "{c}"); + let c = ok(&mut s, "get_cell", json!({"cell": "B3"})); + assert!(!c.to_string().contains("note"), "{c}"); + // The active sheet may be a quoted name with spaces. + ok(&mut s, "execute_command", json!({"command": "sheet.activate", "params": {"sheet": "My Sheet"}})); + ok(&mut s, "execute_command", json!({"command": "view.freezePanes", "params": {"cell": "'My Sheet'!B2"}})); + cmd_fails(&mut s, "view.freezePanes", json!({"cell": "Other!B2"})); + // A prefix naming a different sheet than the `sheet` argument is an error, not resolved in favour of either. + cmd_fails(&mut s, "cell.get", json!({"cell": "Other!B3", "sheet": "Sheet1"})); +} + +#[test] +fn text_to_columns_destination_follows_its_sheet() { + let mut s = server(); + ok(&mut s, "execute_command", json!({"command": "home.insertSheet", "params": {"name": "Other"}})); + ok(&mut s, "execute_command", json!({"command": "sheet.activate", "params": {"sheet": 0}})); + ok(&mut s, "set_cell", json!({"cell": "Other!A1", "input": "a,b"})); + ok( + &mut s, + "execute_command", + json!({"command": "data.textToColumns", "params": {"range": "Other!A1", "delimiters": [","], "destination": "Other!D1"}}), + ); + let r = ok(&mut s, "read_range", json!({"range": "Other!D1:E1"})); + assert!(r.to_string().contains('a') && r.to_string().contains('b'), "{r}"); + // An unparsable destination and a prefix that contradicts the command's sheet are errors, not A1. + cmd_fails(&mut s, "data.textToColumns", json!({"range": "Other!A1", "delimiters": [","], "destination": "D0"})); + cmd_fails(&mut s, "data.textToColumns", json!({"range": "Other!A1", "delimiters": [","], "destination": "Sheet1!D1"})); +}