From 13691e95e5148db55e370b195dc7b9e7ed5414ed Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Wed, 14 Oct 2026 12:51:05 +0200 Subject: [PATCH] Remove Duplicates: a Sheet! prefix on the range works on that sheet, not the active one (refs #1283) --- crates/engine/src/cmd/data.rs | 22 +++++++++++++++++++--- crates/engine/src/tests.rs | 23 +++++++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/data.rs b/crates/engine/src/cmd/data.rs index 383624b70..2e7b379d0 100644 --- a/crates/engine/src/cmd/data.rs +++ b/crates/engine/src/cmd/data.rs @@ -147,6 +147,23 @@ fn data_range(s: &Session, p: &Json) -> Result { Ok(current_region(sh, d.selection.active)) } +/// The sheet and range a data command works on: the sheet `sheet` or a `Sheet!` prefix on `range` +/// names, else the active one. The selection belongs to the active sheet, so another sheet needs `range`. +fn data_target(s: &Session, p: &Json, cmd: &str) -> Result<(usize, RangeRef)> { + let sheet = target_sheet(s, p)?; + let d = s.doc()?; + if let Some(t) = str_param(p, "range") + && let (Some(name), _) = split_sheet(t) + && d.wb.sheet_index(&name) != Some(sheet) + { + return Err(bad(cmd, format!("`{t}` names a sheet other than `sheet`"))); + } + if sheet != d.wb.active_sheet && p.get("range").is_none() { + return Err(bad(cmd, "missing `range`: the selection is on the active sheet")); + } + Ok((sheet, data_range(s, p)?)) +} + /// Excel's header guess: the first row is text over a column of numbers, or is bold. fn guess_header(wb: &Workbook, sh: &Sheet, r: RangeRef) -> bool { if let Some(t) = sh.table_at(r.start) { @@ -736,9 +753,9 @@ fn sort_key(v: &Value) -> (u8, i64, String) { // ---------------------------------------------------------------- tools fn remove_duplicates(s: &mut Session, p: &Json) -> Result { - let r = data_range(s, p)?; + let (sheet, r) = data_target(s, p, "data.removeDuplicates")?; let d = s.doc()?; - let sh = d.wb.active().ok_or(EngineError::NoDocument)?; + let sh = d.wb.sheet(sheet).ok_or(EngineError::NoDocument)?; // Rows and columns past the data are blank: only the part with data is compared (whole // columns compared a million blank rows and reported them as removed duplicates). let r = sh.used_range().and_then(|u| r.intersection(&u)).unwrap_or(RangeRef::cell(r.start)); @@ -748,7 +765,6 @@ fn remove_duplicates(s: &mut Session, p: &Json) -> Result { .and_then(Json::as_array) .map(|a| a.iter().filter_map(|c| col_param(Some(c), r.start.col)).collect()) .unwrap_or_else(|| (r.start.col..=r.end.col).collect()); - let sheet = d.wb.active_sheet; let start = r.start.row + header as u32; let mut removed = 0; edit(s, |cx| { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index ae499a189..19a512d73 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1039,6 +1039,29 @@ fn remove_duplicates_compares_shown_text() { assert_eq!(r["removed"], 1); } +#[test] +fn remove_duplicates_with_a_sheet_prefix_works_on_the_named_sheet() { + let col = |s: &Session, sheet: &str| -> Vec { + let wb = &s.doc().unwrap().wb; + let sh = wb.sheet(wb.sheet_index(sheet).unwrap()).unwrap(); + (1..=3).map(|r| sh.value(CellRef::new(r, 0))).collect() + }; + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [["Name"], ["a"], ["a"], ["a"]]})).unwrap(); + s.execute("home.insertSheet", json!({"name": "Prices"})).unwrap(); + s.execute("range.setValues", json!({"range": "Prices!A1", "values": [["Item"], ["p1"], ["p1"], ["p2"]]})).unwrap(); + s.execute("sheet.activate", json!({"sheet": "Sheet1"})).unwrap(); + let r = s.execute("data.removeDuplicates", json!({"range": "Prices!A1:A4", "header": true})).unwrap(); + assert_eq!(r["removed"], 1); + assert_eq!(col(&s, "Prices"), [Value::from("p1"), Value::from("p2"), Value::Empty]); + assert_eq!(col(&s, "Sheet1"), ["a", "a", "a"].map(Value::from)); + // The selection belongs to the active sheet, so another sheet needs its range. + assert!(s.execute("data.removeDuplicates", json!({"sheet": "Prices", "header": true})).is_err()); + assert!(s.execute("data.removeDuplicates", json!({"sheet": "Sheet1", "range": "Prices!A1:A4", "header": true})).is_err()); + assert!(s.execute("data.removeDuplicates", json!({"range": "Nowhere!A1:A4", "header": true})).is_err()); + assert_eq!(col(&s, "Sheet1"), ["a", "a", "a"].map(Value::from)); +} + #[test] fn formatting_and_styles() { let mut s = s();