From 2cae3a22b16b65ef0b1c4ad68d9c254ad9a737fe Mon Sep 17 00:00:00 2001 From: dev-exe <29832920+dev-exe@users.noreply.github.com> Date: Wed, 14 Oct 2026 20:28:46 +0530 Subject: [PATCH] Slicers: a table named in `table` is found on any sheet insert.slicer and table.insertSlicer looked for the table named by `table` only on the active sheet. With another sheet active, the call failed with "Select a cell inside a table first, or pass `table`" although `table` was passed. Other table commands find a table by name in the whole workbook (Workbook::table). The named table is now found on any sheet, and the slicer goes on that table's sheet. `at` is read on that sheet, so "Sheet1!G2" names the table's own sheet. A name that matches no table gives "no table named `X`". Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/slicer.rs | 18 ++++++++++++++---- crates/engine/src/tests.rs | 29 +++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/slicer.rs b/crates/engine/src/cmd/slicer.rs index 00342a5a9..cb72ded71 100644 --- a/crates/engine/src/cmd/slicer.rs +++ b/crates/engine/src/cmd/slicer.rs @@ -97,11 +97,21 @@ fn reapply_slicer(wb: &mut Workbook, sheet: usize, id: u32) { } fn insert_slicer(s: &mut Session, p: &Json) -> Result { - let sheet = target_sheet(s, p)?; + // A table named by `table` is found on any sheet, as the other table commands find it, and the slicer goes on that + // table's sheet. Looked up on the active sheet only, a named table on another sheet gave "Select a cell inside a + // table first, or pass `table`" although `table` was passed. + let named = match str_param(p, "table") { + Some(name) => Some(s.doc()?.wb.table(name).ok_or_else(|| bad("insert.slicer", format!("no table named `{name}`")))?), + None => None, + }; + let sheet = match named { + Some((si, _)) => si, + None => target_sheet(s, p)?, + }; let d = s.doc()?; let sh = d.wb.sheet(sheet).ok_or(EngineError::NoDocument)?; - let t = match str_param(p, "table") { - Some(name) => sh.tables.iter().find(|t| t.name.eq_ignore_ascii_case(name)), + let t = match named { + Some((_, ti)) => sh.tables.get(ti), None => sh.table_at(d.selection.active), } .ok_or_else(|| EngineError::Other("Select a cell inside a table first, or pass `table`.".into()))?; @@ -119,7 +129,7 @@ fn insert_slicer(s: &mut Session, p: &Json) -> Result { } }; let table = t.name.clone(); - let at = cell_param(s, p, "at")? + let at = cell_param_on(s, p, "at", sheet)? .unwrap_or_else(|| CellRef::new(t.range.start.row, t.range.end.col.saturating_add(2).min(gridcraft_core::MAX_COLS - 1))); let columns = u32_param(p, "columns").unwrap_or(1).clamp(1, 10); let width = f64_param(p, "width").unwrap_or(168.0).clamp(60.0, 10000.0) as f32; diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index ae499a189..2e46b56ff 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1206,6 +1206,35 @@ fn table_rename_keeps_a_defined_name_with_the_same_name() { assert_eq!((v(&s, "E1"), v(&s, "F1")), (Value::Number(100.0), Value::Number(7.0))); } +#[test] +fn slicer_finds_a_named_table_on_another_sheet() { + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [["Region", "Qty"], ["East", 2], ["West", 5]]})).unwrap(); + s.execute("insert.table", json!({"range": "A1:B3", "name": "Sales"})).unwrap(); + s.execute("home.insertSheet", json!({"name": "Report"})).unwrap(); + let report = s.doc().unwrap().wb.sheet_index("Report").unwrap(); + assert_eq!(s.doc().unwrap().wb.active_sheet, report); + + // With another sheet active, `table` still names the table, and the slicer goes on the table's sheet. + let r = s.execute("insert.slicer", json!({"table": "Sales", "column": "Region", "at": "D2"})).unwrap(); + let id = r["slicer"].as_u64().unwrap(); + let home = s.doc().unwrap().wb.sheet_index("Sheet1").unwrap(); + assert_eq!(s.doc().unwrap().wb.sheet(home).unwrap().slicers.len(), 1); + assert!(s.doc().unwrap().wb.sheet(report).unwrap().slicers.is_empty()); + // It filters the table: "East" leaves the East row. + s.execute("selection.set", json!({"range": "Sheet1!A1"})).unwrap(); + s.execute("slicer.toggle", json!({"slicer": id, "value": "East"})).unwrap(); + let sh = s.doc().unwrap().wb.sheet(home).unwrap(); + assert!(!sh.is_row_hidden(1) && sh.is_row_hidden(2)); + + // `at` may name the table's sheet, and a table that isn't there is named in the error. + s.execute("home.insertSheet", json!({"name": "Notes"})).unwrap(); + s.execute("table.insertSlicer", json!({"table": "sales", "column": "Qty", "at": "Sheet1!G2"})).unwrap(); + assert_eq!(s.doc().unwrap().wb.sheet(home).unwrap().slicers.len(), 2); + let e = s.execute("insert.slicer", json!({"table": "Missing", "column": "Region"})).unwrap_err(); + assert!(e.to_string().contains("Missing"), "{e}"); +} + #[test] fn slicer_filters_table() { let mut s = s();