From d280e6ff005beb9c45c440094951a36e1e2139c7 Mon Sep 17 00:00:00 2001 From: Gert Pellin Date: Thu, 15 Oct 2026 04:44:22 +0200 Subject: [PATCH] Clear: Delete and the Clear commands skip the rows a filter hides (#1419) As in Excel, clearing a filtered selection leaves the rows the filter hides as they are; rows hidden by hand are still cleared. Checked boxes in hidden rows no longer hold back Delete on the boxes showing. --- crates/engine/src/cmd/edit.rs | 27 ++++++++++++---- crates/engine/src/tests.rs | 60 +++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 82dbe0be7..d31ebe08b 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -1220,6 +1220,13 @@ pub(crate) fn rewrite_all_formulas(wb: &mut gridcraft_model::Workbook, target: & // ---------------------------------------------------------------- clear +/// The rows of `range` an applied filter hides (rows hidden by hand are not among them). +fn filtered_rows(sh: &gridcraft_model::Sheet, range: RangeRef) -> std::collections::BTreeSet { + sh.rows.range(range.start.row..=range.end.row).filter(|(r, info)| info.hidden && sh.is_row_filtered(**r)).map(|(r, _)| *r).collect() +} + +/// The Clear commands (Delete is Clear Contents). As in Excel, the rows an applied filter hides +/// are left as they are; rows hidden by hand are cleared. fn clear(s: &mut Session, p: &Json, what: &str) -> Result { let sheet = target_sheet(s, p)?; let ranges = target_ranges(s, p)?; @@ -1230,7 +1237,10 @@ fn clear(s: &mut Session, p: &Json, what: &str) -> Result { d.wb.sheet(sheet).is_some_and(|sh| { ranges.iter().any(|r| { r.intersection(&sh.used_range().unwrap_or(RangeRef::cell(r.start))).is_some_and(|r| { - sh.cells.iter_range(r).any(|(c, x)| x.value == Value::Bool(true) && super::extra::is_checkbox_cell(&d.wb, sh, c)) + let skip = filtered_rows(sh, r); + sh.cells + .iter_range(r) + .any(|(c, x)| !skip.contains(&c.row) && x.value == Value::Bool(true) && super::extra::is_checkbox_cell(&d.wb, sh, c)) }) }) }) @@ -1244,8 +1254,12 @@ fn clear(s: &mut Session, p: &Json, what: &str) -> Result { }, None => continue, }; - let cells: Vec<(CellRef, Cell)> = - cx.wb.sheet(sheet).map(|sh| sh.cells.iter_range(r).map(|(c, x)| (c, x.clone())).collect()).unwrap_or_default(); + let skip = cx.wb.sheet(sheet).map(|sh| filtered_rows(sh, r)).unwrap_or_default(); + let cells: Vec<(CellRef, Cell)> = cx + .wb + .sheet(sheet) + .map(|sh| sh.cells.iter_range(r).filter(|(c, _)| !skip.contains(&c.row)).map(|(c, x)| (c, x.clone())).collect()) + .unwrap_or_default(); let mut cells = cells; if what == "contents" { for (_, cell) in cells.iter_mut() { @@ -1277,14 +1291,15 @@ fn clear(s: &mut Session, p: &Json, what: &str) -> Result { _ => {} } } + let cleared = |c: CellRef| r.contains(c) && !skip.contains(&c.row); if matches!(what, "all" | "comments") { - sh.comments.retain(|c, _| !r.contains(*c)); + sh.comments.retain(|c, _| !cleared(*c)); } if matches!(what, "all" | "hyperlinks") { - sh.hyperlinks.retain(|c, _| !r.contains(*c)); + sh.hyperlinks.retain(|c, _| !cleared(*c)); } if matches!(what, "all" | "formats") { - sh.merges.retain(|m| !r.contains_range(m)); + sh.merges.retain(|m| !r.contains_range(m) || skip.range(m.start.row..=m.end.row).next().is_some()); } for c in r.iter().take(1_000_000) { cx.changed.push((sheet, c)); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index ae499a189..3074ed670 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -827,6 +827,48 @@ fn sort_and_filter() { assert_eq!(r["hiddenRows"], 1); } +/// Delete and the Clear commands on a filtered selection clear only the rows showing, as in +/// Excel (#1419); rows hidden by hand are cleared with the rest. +#[test] +fn clearing_a_filtered_selection_leaves_the_rows_the_filter_hides() { + let mut s = s(); + let rows = [["Name", "Status"], ["John", "Active"], ["Mary", "Inactive"], ["David", "Active"], ["Sarah", "Inactive"], ["Michael", "Active"]]; + s.execute("range.setValues", json!({"range": "A1", "values": rows})).unwrap(); + s.execute("home.bold", json!({"range": "A2:A6", "on": true})).unwrap(); + s.execute("review.newNote", json!({"cell": "A3", "text": "note", "author": "Example"})).unwrap(); + s.execute("selection.set", json!({"cell": "A1"})).unwrap(); + s.execute("data.filter", json!({})).unwrap(); + s.execute("data.filterBy", json!({"column": "B", "values": ["Active"]})).unwrap(); + let bold = |s: &Session, a: &str| { + let d = s.doc().unwrap(); + d.wb.styles.get(d.wb.active().unwrap().style_id(CellRef::parse(a).unwrap())).font.bold + }; + + s.execute("edit.clearContents", json!({"range": "A2:A6"})).unwrap(); + for a in ["A2", "A4", "A6"] { + assert_eq!(v(&s, a), Value::Empty, "{a} shows and is cleared"); + } + assert_eq!(v(&s, "A3"), Value::from("Mary")); + assert_eq!(v(&s, "A5"), Value::from("Sarah")); + + s.execute("edit.clearFormats", json!({"range": "A2:A6"})).unwrap(); + assert!(!bold(&s, "A2")); + assert!(bold(&s, "A3"), "a filtered-out row keeps its format"); + + s.execute("edit.clearAll", json!({"range": "A2:B6"})).unwrap(); + assert_eq!(v(&s, "B2"), Value::Empty); + assert_eq!(v(&s, "B3"), Value::from("Inactive")); + assert_eq!(v(&s, "A5"), Value::from("Sarah")); + assert!(s.doc().unwrap().wb.active().unwrap().comments.contains_key(&CellRef::parse("A3").unwrap())); + + // Without the filter, a row hidden by hand is cleared like the rows around it. + s.execute("data.clearFilter", json!({})).unwrap(); + s.execute("home.hideRows", json!({"rows": "5:5"})).unwrap(); + s.execute("edit.clearContents", json!({"range": "A3:A5"})).unwrap(); + assert_eq!(v(&s, "A3"), Value::Empty); + assert_eq!(v(&s, "A5"), Value::Empty); +} + /// Top 10 keeps the top or bottom N items or percent, ties at the threshold included. #[test] fn filter_top10_items_and_percent() { @@ -2101,6 +2143,24 @@ fn checkboxes_keep_values_toggle_and_delete_in_two_passes() { assert_ne!(fmt_of(&s, "B2"), "checkbox"); } +/// A checked box in a row the filter hides doesn't hold back Delete on the boxes showing, and +/// stays as it is (#1419). +#[test] +fn delete_on_filtered_checkboxes_skips_the_hidden_rows() { + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [["Status"], ["on"], ["off"], ["on"]]})).unwrap(); + s.execute("insert.checkbox", json!({"range": "B2:B4"})).unwrap(); + s.execute("cell.toggleCheckbox", json!({"cell": "B3"})).unwrap(); + s.execute("selection.set", json!({"cell": "A1"})).unwrap(); + s.execute("data.filter", json!({})).unwrap(); + s.execute("data.filterBy", json!({"column": "A", "values": ["on"]})).unwrap(); + s.execute("edit.clearContents", json!({"range": "B2:B4"})).unwrap(); + assert_ne!(fmt_of(&s, "B2"), "checkbox", "the unchecked boxes showing are removed at once"); + assert_ne!(fmt_of(&s, "B4"), "checkbox"); + assert_eq!(fmt_of(&s, "B3"), "checkbox"); + assert_eq!(v(&s, "B3"), Value::Bool(true)); +} + #[test] fn checkboxes_survive_xlsx_as_cell_controls() { let mut s = s();