diff --git a/crates/engine/src/cmd/data.rs b/crates/engine/src/cmd/data.rs index 383624b70..508ceb465 100644 --- a/crates/engine/src/cmd/data.rs +++ b/crates/engine/src/cmd/data.rs @@ -182,19 +182,36 @@ struct SortKey { list: Option>, } +/// The sheet and range a sort 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 sort_target(s: &Session, p: &Json) -> 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("data.sort", format!("`{t}` names a sheet other than `sheet`"))); + } + if sheet != d.wb.active_sheet && p.get("range").is_none() { + return Err(bad("data.sort", "missing `range`: the selection is on the active sheet")); + } + Ok((sheet, data_range(s, p)?)) +} + fn quick_sort(s: &mut Session, p: &Json, asc: bool) -> Result { - let r = data_range(s, p)?; + let (sheet, r) = sort_target(s, p)?; let col = col_param(p.get("column"), r.start.col).unwrap_or(s.doc()?.selection.active.col).clamp(r.start.col, r.end.col); let d = s.doc()?; - let sh = d.wb.active().ok_or(EngineError::NoDocument)?; + let sh = d.wb.sheet(sheet).ok_or(EngineError::NoDocument)?; let header = guess_header(&d.wb, sh, r); - do_sort(s, r, header, vec![SortKey { col, desc: !asc, by: "values".into(), list: None }], false, false) + do_sort(s, sheet, r, header, vec![SortKey { col, desc: !asc, by: "values".into(), list: None }], false, false) } fn sort(s: &mut Session, p: &Json) -> Result { - let r = data_range(s, p)?; + let (sheet, r) = sort_target(s, p)?; let d = s.doc()?; - let sh = d.wb.active().ok_or(EngineError::NoDocument)?; + let sh = d.wb.sheet(sheet).ok_or(EngineError::NoDocument)?; let header = bool_param(p, "header").unwrap_or_else(|| guess_header(&d.wb, sh, r)); let by_cols = str_param(p, "orientation") == Some("columns"); let case = bool_param(p, "matchCase").unwrap_or(false); @@ -228,10 +245,10 @@ fn sort(s: &mut Session, p: &Json) -> Result { if keys.len() > 64 { return Err(bad("data.sort", "too many sort levels (64 max)")); } - do_sort(s, r, header, keys, by_cols, case) + do_sort(s, sheet, r, header, keys, by_cols, case) } -fn do_sort(s: &mut Session, r: RangeRef, header: bool, keys: Vec, by_cols: bool, case: bool) -> Result { +fn do_sort(s: &mut Session, sheet: usize, r: RangeRef, header: bool, keys: Vec, by_cols: bool, case: bool) -> Result { let body = if by_cols { if header && r.width() > 1 { RangeRef::new(CellRef::new(r.start.row, r.start.col + 1), r.end) } else { r } } else if header && r.height() > 1 { @@ -242,7 +259,6 @@ fn do_sort(s: &mut Session, r: RangeRef, header: bool, keys: Vec, by_co if body.count() > 20_000_000 { return Err(bad("data.sort", "range too large")); } - let sheet = s.doc()?.wb.active_sheet; edit(s, |cx| { let Some(sh) = cx.wb.sheet(sheet) else { return Ok(Json::Null) }; if sh.merges.iter().any(|m| m.intersects(&body) && !m.is_single()) { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index ae499a189..7d7a53157 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1027,6 +1027,31 @@ fn case_sensitive_sort_puts_lowercase_first() { assert_eq!(col, ["a", "A", "aB", "Ab", "b", "B"].map(Value::from)); } +#[test] +fn sort_with_a_sheet_prefix_sorts_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", "Qty"], ["a", 3], ["b", 1], ["c", 2]]})).unwrap(); + s.execute("home.insertSheet", json!({"name": "Prices"})).unwrap(); + s.execute("range.setValues", json!({"range": "Prices!A1", "values": [["Item", "Price"], ["p1", 10], ["p2", 30], ["p3", 20]]})).unwrap(); + s.execute("sheet.activate", json!({"sheet": "Sheet1"})).unwrap(); + let r = s.execute("data.sort", json!({"range": "Prices!A1:B4", "header": true, "keys": [{"column": "B", "order": "desc"}]})).unwrap(); + assert_eq!(r["sorted"], "A2:B4"); + assert_eq!(col(&s, "Prices"), ["p2", "p3", "p1"].map(Value::from)); + assert_eq!(col(&s, "Sheet1"), ["a", "b", "c"].map(Value::from)); + s.execute("data.sortAscending", json!({"range": "Prices!A2:B4", "column": "B"})).unwrap(); + assert_eq!(col(&s, "Prices"), ["p1", "p3", "p2"].map(Value::from)); + assert_eq!(col(&s, "Sheet1"), ["a", "b", "c"].map(Value::from)); + // The selection belongs to the active sheet, so another sheet needs its range. + assert!(s.execute("data.sort", json!({"sheet": "Prices", "keys": [{"column": "B"}]})).is_err()); + assert!(s.execute("data.sort", json!({"range": "Nowhere!A1:B4", "keys": [{"column": "B"}]})).is_err()); + assert_eq!(col(&s, "Sheet1"), ["a", "b", "c"].map(Value::from)); +} + #[test] fn remove_duplicates_compares_shown_text() { let mut s = s();