Merge pull request #1422 from switch87/fix/1419-clear-skips-filtered-rows

Clear: Delete and the Clear commands skip the rows a filter hides (#1419)

crates/engine/src/cmd/edit.rs

@@ -213,6 +213,13 @@ fn redo(s: &mut Session, p: &Json) -> Result<Json> {
 
 // ---------------------------------------------------------------- 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<u32> {
+    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<Json> {
     let sheet = target_sheet(s, p)?;
     let ranges = target_ranges(s, p)?;
@@ -223,7 +230,10 @@ fn clear(s: &mut Session, p: &Json, what: &str) -> Result<Json> {
         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))
                 })
             })
         })
@@ -237,8 +247,12 @@ fn clear(s: &mut Session, p: &Json, what: &str) -> Result<Json> {
                 },
                 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();
             // Report every stored cell we touch, even beyond a million coordinates in a sparse
             // selection. Removed formula anchors must also reach calc so their spills disappear.
             cx.changed.extend(cells.iter().map(|(c, _)| (sheet, *c)));
@@ -273,16 +287,17 @@ fn clear(s: &mut Session, p: &Json, what: &str) -> Result<Json> {
                     _ => {}
                 }
             }
+            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") {
                 let mut unmerged = false;
                 sh.merges.retain(|m| {
-                    let keep = !r.contains_range(m);
+                    let keep = !r.contains_range(m) || skip.range(m.start.row..=m.end.row).next().is_some();
                     unmerged |= !keep;
                     keep
                 });

crates/engine/src/tests.rs

@@ -1081,6 +1081,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() {
@@ -2836,6 +2878,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();