aleleba/gridcraft
1
0
Fork 0
mirror of https://github.com/storytold/gridcraft.git synced 2026-10-26 11:46:49 -06:00
Code Issues Packages Projects Releases Wiki Activity

engine: sort and remove duplicates move the rows' notes and hyperlinks

Browse Source 
do_sort moved cells, formulas, cell pictures, row heights and floating
object anchors through the sort permutation, and remove_duplicates moved
cells and pictures through the compaction, but neither touched
Sheet::comments or Sheet::hyperlinks (keyed by CellRef): a note that
described one record stayed behind and annotated whatever landed on that
row — 'late payment' on a paid invoice — and a note on a row removed by
Remove Duplicates annotated blank cells. Sheet::comments/hyperlinks are
exactly the range-scoped side data that row and column inserts already
remap and edit.clear already clears, so the omission looks unintended.

Both maps now travel through the same permutation the cells use (the
sort collects before re-inserting, so a swap cannot resurrect an entry
from a destination that is also a source; Remove Duplicates compacts the
kept rows and drops the rest).

Regression test: sort_and_remove_duplicates_move_the_rows_notes_and_links
— on main the note and link stay on their old cells; with the fix they
travel with their record.
This commit is contained in:
midasmatterhorn
2026-10-15 17:40:18 +03:00
parent e6b6b34970
commit f8cbfb7cb6
2 changed files with 102 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/engine/src/cmd/data.rs
+58
View File
@@ -295,6 +295,21 @@ fn do_sort(s: &mut Session, r: RangeRef, header: bool, keys: Vec<SortKey>, by_co
}
// Move whole lines (cells with formulas adjusted for relative references).
let lanes: Vec<u32> = if by_cols { (body.start.row..=body.end.row).collect() } else { (body.start.col..=body.end.col).collect() };
// Notes and hyperlinks hang off a cell, not a row, so they travel with the row they
// describe — the way cell pictures and row heights do. Collect them before inserting,
// so a swap can't resurrect one from a destination that is also a source.
let side: Vec<(CellRef, Comment)> = order
.iter()
.enumerate()
.flat_map(|(dst_i, &src_i)| lanes.iter().map(move |&k| (src_i, dst_i as u32, k)))
.filter_map(|(src_i, dst_i, k)| sh.comments.get(&line(src_i, k)).map(|cm| (line(dst_i, k), cm.clone())))
.collect();
let links: Vec<(CellRef, Hyperlink)> = order
.iter()
.enumerate()
.flat_map(|(dst_i, &src_i)| lanes.iter().map(move |&k| (src_i, dst_i as u32, k)))
.filter_map(|(src_i, dst_i, k)| sh.hyperlinks.get(&line(src_i, k)).map(|h| (line(dst_i, k), h.clone())))
.collect();
let mut moved: Vec<(CellRef, Option<Cell>)> = Vec::with_capacity(body.count() as usize);
let mut pictures = Vec::new();
for (dst_i, &src_i) in order.iter().enumerate() {
@@ -354,6 +369,18 @@ fn do_sort(s: &mut Session, r: RangeRef, header: bool, keys: Vec<SortKey>, by_co
}
}
}
// Every cell of the body is the destination of exactly one source cell, so clearing the
// body and inserting the permuted entries is the same permutation the cells went through.
for c in body.iter() {
shm.comments.remove(&c);
shm.hyperlinks.remove(&c);
}
for (at, cm) in side {
shm.comments.insert(at, cm);
}
for (at, h) in links {
shm.hyperlinks.insert(at, h);
}
shm.cell_pictures.extend(pictures);
for (i, h) in heights.into_iter().enumerate() {
let row = body.start.row + i as u32;
@@ -774,6 +801,24 @@ fn remove_duplicates(s: &mut Session, p: &Json) -> Result<Json> {
.filter_map(move |col| sh.cell_pictures.get(&CellRef::new(*row, col)).map(|p| (CellRef::new(start + i as u32, col), p.clone())))
})
.collect();
// Rows a filter keeps are compacted to `start + i`, so the notes and hyperlinks that
// describe them move with them; the ones on removed rows are dropped with the row.
let notes: Vec<_> = keep
.iter()
.enumerate()
.flat_map(|(i, row)| {
(r.start.col..=r.end.col)
.filter_map(move |col| sh.comments.get(&CellRef::new(*row, col)).map(|cm| (CellRef::new(start + i as u32, col), cm.clone())))
})
.collect();
let links: Vec<_> = keep
.iter()
.enumerate()
.flat_map(|(i, row)| {
(r.start.col..=r.end.col)
.filter_map(move |col| sh.hyperlinks.get(&CellRef::new(*row, col)).map(|h| (CellRef::new(start + i as u32, col), h.clone())))
})
.collect();
let rows: Vec<Vec<Option<Cell>>> =
keep.iter().map(|row| (r.start.col..=r.end.col).map(|c| sh.cell(CellRef::new(*row, c)).cloned()).collect()).collect();
let shm = cx.sheet_mut(sheet)?;
@@ -782,6 +827,13 @@ fn remove_duplicates(s: &mut Session, p: &Json) -> Result<Json> {
shm.remove_cell(CellRef::new(row, c));
}
}
for row in start..=r.end.row {
for c in r.start.col..=r.end.col {
let at = CellRef::new(row, c);
shm.comments.remove(&at);
shm.hyperlinks.remove(&at);
}
}
for (i, cells) in rows.into_iter().enumerate() {
for (j, cell) in cells.into_iter().enumerate() {
if let Some(cell) = cell {
@@ -790,6 +842,12 @@ fn remove_duplicates(s: &mut Session, p: &Json) -> Result<Json> {
}
}
shm.cell_pictures.extend(pictures);
for (at, cm) in notes {
shm.comments.insert(at, cm);
}
for (at, h) in links {
shm.hyperlinks.insert(at, h);
}
for c in RangeRef::new(CellRef::new(start, r.start.col), r.end).iter() {
cx.changed.push((sheet, c));
}
crates/engine/src/tests.rs
+44
View File
@@ -2925,6 +2925,50 @@ fn notes_comments_and_links_survive_a_json_save() {
assert_eq!(after.cells, before.cells);
}
/// Sorting and removing duplicates rewrite the rows of a range. The cells, pictures, row heights
/// and anchors travel with them, but the notes and hyperlinks hung off their old cells: a note
/// describing one record ended up annotating whatever landed on that row, and on a removed row it
/// annotated blank cells.
#[test]
fn sort_and_remove_duplicates_move_the_rows_notes_and_links() {
fn setup(values: [&str; 5]) -> Session {
let mut s = s();
for (row, text) in values.iter().enumerate() {
s.execute("cell.set", json!({"cell": format!("A{}", row + 1), "input": text})).unwrap();
}
s.execute("review.newNote", json!({"cell": "B5", "text": "late", "author": "Example"})).unwrap();
s.execute("insert.link", json!({"cell": "B4", "target": "https://example.com/4"})).unwrap();
s
}
let notes_and_links = |s: &Session| {
let sh = s.doc().unwrap().wb.active().unwrap().clone();
let cells = ["B2", "B3", "B4", "B5"].map(|c| CellRef::parse(c).unwrap());
(
cells.map(|c| sh.comments.get(&c).map(|m| m.text.clone())),
cells.map(|c| sh.hyperlinks.get(&c).map(|h| h.target.clone())),
)
};
// Sort ascending with unique keys d,a,e,b,c: the order becomes a(2), b(4), c(5), d(1), e(3).
// The note on B5 describes row 5's record ("c"), which lands on row 3; the link on B4
// describes row 4's record ("b"), which lands on row 2. Both must travel with their record.
let mut s = setup(["d", "a", "e", "b", "c"]);
s.execute("data.sort", json!({"range": "A1:B5", "keys": [{"column": "A"}]})).unwrap();
assert_eq!(v(&s, "A3"), Value::text("c"));
assert_eq!(v(&s, "A5"), Value::text("e"));
let (notes, links) = notes_and_links(&s);
assert_eq!(notes[1], Some("late".to_string()), "the note moved with its record: {notes:?}");
assert_eq!(links[0], Some("https://example.com/4".to_string()), "the link moved with its record: {links:?}");
// Remove duplicates keeps the first of each value: rows 3 and 5 go, and row 4's record moves
// to row 3. The note on B5 described row 5 and goes with it.
let mut s = setup(["a", "b", "a", "c", "a"]);
s.execute("data.removeDuplicates", json!({"range": "A1:B5"})).unwrap();
assert_eq!(v(&s, "A3"), Value::text("c"));
let (notes, _) = notes_and_links(&s);
assert!(notes.iter().all(Option::is_none), "a note on a removed row goes with it: {notes:?}");
}
/// Insert › Link sorts a target as Excel's dialog does: a sheet reference, a defined name or `#…` is
/// a place in this document, anything else an existing file or web page. A file saved as a
/// `location` opens nothing in Excel (#800).