From 5ca6a474b93c35c1a42ba2e9755501144d782b1f Mon Sep 17 00:00:00 2001 From: sanyamsingh001 <314051937+sanyamsingh001@users.noreply.github.com> Date: Tue, 13 Oct 2026 10:59:32 +1000 Subject: [PATCH] PARITY-TABLE-SPLIT: split each original selected cell once Co-Authored-By: Codex --- crates/doc/src/table.rs | 165 ++++++++++++++++++++++++++++++- crates/engine/src/cmd/table.rs | 6 +- crates/engine/src/tests_table.rs | 122 +++++++++++++++++++++++ 3 files changed, 287 insertions(+), 6 deletions(-) diff --git a/crates/doc/src/table.rs b/crates/doc/src/table.rs index 7fe385e98..9449be93c 100644 --- a/crates/doc/src/table.rs +++ b/crates/doc/src/table.rs @@ -695,6 +695,79 @@ impl Table { self.set_regions(®ions); } + /// Split each original owner touched by `range` once. Remember merged-cell cuts in the + /// original grid: a neighboring split may refine that grid before we reach the owner. + pub fn split_cells(&mut self, range: CellRange, horizontal: bool) -> Result<(), String> { + if range.r0 > range.r1 || range.c0 > range.c1 || range.r1 >= self.nrows() || range.c1 >= self.ncols() { + return Err("cell range out of bounds".into()); + } + let owners = self.owners(); + let mut selected = std::collections::BTreeSet::new(); + for r in range.r0..=range.r1 { + for c in range.c0..=range.c1 { + let owner = owners.get(r * self.ncols() + c).ok_or("missing cell owner")?; + selected.insert(*owner); + } + } + let mut targets = Vec::new(); + for (r, c) in selected { + let cell = self.cell(r, c).ok_or("missing split cell")?; + let span = if horizontal { cell.row_span } else { cell.col_span } as usize; + let start = if horizontal { r } else { c }; + let cut = if span > 1 { Some(start.checked_add(span.div_ceil(2)).ok_or("invalid split boundary")?) } else { None }; + targets.push((r, c, cut)); + } + for i in 0..targets.len() { + let (r, c, cut) = *targets.get(i).ok_or("missing split target")?; + let cell = self.cell(r, c).ok_or("missing split cell")?; + let (rs, cs) = (cell.row_span as usize, cell.col_span as usize); + let mut blank = cell.blank_like(); + // Splitting creates an empty half, not a duplicate of graphic-cell content. + blank.graphic = None; + let before = if horizontal { self.nrows() } else { self.ncols() }; + if let Some(cut) = cut { + let (start, span) = if horizontal { (r, rs) } else { (c, cs) }; + let first = cut.checked_sub(start).filter(|n| *n > 0 && *n < span).ok_or("invalid split boundary")?; + let mut regions: Vec<_> = self.regions().into_iter().filter(|&(rr, cc, _, _)| (rr, cc) != (r, c)).collect(); + if horizontal { + regions.push((r, c, first, cs)); + regions.push((cut, c, rs - first, cs)); + } else { + regions.push((r, c, rs, first)); + regions.push((r, cut, rs, cs - first)); + } + self.set_regions(®ions); + } else { + // An originally single-track cell may now span two tracks inserted for its + // neighbor. split_cell divides that span instead of inserting another track. + self.split_cell(r, c, horizontal); + } + let first = self.cell(r, c).ok_or("missing split half")?; + let second = if horizontal { (r + first.row_span as usize, c) } else { (r, c + first.col_span as usize) }; + let second = self.cell_mut(second.0, second.1).ok_or("missing split half")?; + blank.row_span = second.row_span; + blank.col_span = second.col_span; + *second = blank; + let after = if horizontal { self.nrows() } else { self.ncols() }; + if after > before { + let at = if horizontal { r + 1 } else { c + 1 }; + let added = after - before; + for (row, col, cut) in targets.iter_mut().skip(i + 1) { + let axis = if horizontal { row } else { col }; + if *axis >= at { + *axis += added; + } + if let Some(cut) = cut + && *cut >= at + { + *cut += added; + } + } + } + } + Ok(()) + } + /// Split the cell covering (r, c) in two: `horizontal` stacks the halves (Split Cell /// Horizontally), otherwise side by side. A merged cell splits its span; a single cell gets a /// new row (column) that the cells beside it span, the row height (column width) shared. @@ -719,10 +792,11 @@ impl Table { return; } if horizontal { - let h = self.rows[or].height; + let row = self.rows[or].clone(); self.insert_rows(or + 1, 1); - self.rows[or].height = h / 2.0; - self.rows[or + 1].height = h / 2.0; + self.rows[or].height = row.height / 2.0; + // Both halves keep the split row's height mode and header/body/footer role. + self.rows[or + 1] = Row { height: row.height / 2.0, ..row }; // insert_rows doesn't grow regions that end at the split row; everything else on it // spans the new row now. let mut regions: Vec<_> = self.regions(); @@ -1255,4 +1329,89 @@ mod split_tests { assert_eq!(m.nrows(), 4); assert_eq!((m.cell(0, 0).unwrap().row_span, m.cell(2, 0).unwrap().row_span), (2, 2)); } + + #[test] + fn split_range_keeps_original_merged_cut_when_neighbor_tracks_are_refined() { + for horizontal in [true, false] { + let mut t = Table::new(1, 4, 4, 0, 0, 280.0); + for (i, size) in [40.0, 60.0, 80.0, 100.0].into_iter().enumerate() { + t.rows[i].mode = RowHeightMode::Exactly; + t.rows[i].height = if horizontal { size } else { 70.0 }; + t.columns[i].width = if horizontal { 70.0 } else { size }; + } + for r in 0..4 { + for c in 0..4 { + let cell = t.cell_mut(r, c).unwrap(); + cell.text.insert(0, &format!("{r},{c}")); + cell.style = format!("style-{r}-{c}"); + cell.insets = [r as f64 + c as f64 + 1.0; 4]; + } + } + let (merged, selected, owner, lower, neighbor, moved_neighbor) = if horizontal { + (CellRange::new(0, 1, 2, 1), CellRange::new(0, 0, 2, 1), (0, 1), (4, 1), (3, 3), (6, 3)) + } else { + (CellRange::new(1, 0, 1, 2), CellRange::new(0, 0, 1, 2), (1, 0), (1, 4), (3, 3), (3, 6)) + }; + t.merge(merged).unwrap(); + let original = t.clone(); + t.split_cells(selected, horizontal).unwrap(); + t.check().unwrap(); + assert_eq!((t.nrows(), t.ncols()), if horizontal { (7, 4) } else { (4, 7) }); + let sizes: Vec<_> = if horizontal { t.rows.iter().map(|r| r.height).collect() } else { t.columns.iter().map(|c| c.width).collect() }; + assert_eq!(sizes, [20.0, 20.0, 30.0, 30.0, 40.0, 40.0, 100.0]); + let first = t.cell(owner.0, owner.1).unwrap(); + let second = t.cell(lower.0, lower.1).unwrap(); + // The original three-track merge divides after its second original track, + // regardless of which of those tracks a neighboring selected cell subdivides. + assert_eq!(if horizontal { first.row_span } else { first.col_span }, 4); + assert_eq!(if horizontal { second.row_span } else { second.col_span }, 2); + assert_eq!(first.text, original.cell(owner.0, owner.1).unwrap().text); + assert!(second.text.is_empty()); + assert_eq!(second.style, first.style); + assert_eq!(second.insets, first.insets); + assert_eq!(t.cell(moved_neighbor.0, moved_neighbor.1), original.cell(neighbor.0, neighbor.1)); + let owners = t.owners(); + assert_eq!(owners.iter().copied().collect::>().len(), 18, "four original selected owners split once each"); + let mut before_text: Vec<_> = original.cells.iter().filter(|c| !c.text.is_empty()).map(|c| c.text.text.clone()).collect(); + let mut after_text: Vec<_> = t.cells.iter().filter(|c| !c.text.is_empty()).map(|c| c.text.text.clone()).collect(); + before_text.sort(); + after_text.sort(); + assert_eq!(after_text, before_text, "no original text is copied or lost"); + } + } + + #[test] + fn split_range_resolves_covered_positions_without_expanding_selection() { + for horizontal in [true, false] { + let mut t = Table::new(1, 3, 3, 0, 0, 180.0); + t.merge(CellRange::new(0, 0, 1, 1)).unwrap(); + let selected = if horizontal { CellRange::new(1, 1, 1, 2) } else { CellRange::new(1, 1, 2, 1) }; + t.split_cells(selected, horizontal).unwrap(); + t.check().unwrap(); + let owners = t.owners(); + assert_eq!(owners.iter().copied().collect::>().len(), 8, "only the two touched owners split"); + assert_eq!((t.nrows(), t.ncols()), if horizontal { (4, 3) } else { (3, 4) }); + assert_eq!(t.owner(0, 0), (0, 0)); + assert_eq!(if horizontal { t.owner(1, 0) } else { t.owner(0, 1) }, if horizontal { (1, 0) } else { (0, 1) }); + } + } + + #[test] + fn split_header_row_keeps_its_height_mode_and_role() { + let mut t = Table::new(1, 1, 2, 1, 1, 180.0); + t.rows[0].height = 60.0; + t.rows[0].mode = RowHeightMode::Exactly; + let body = t.rows[1].clone(); + let footer = t.rows[2].clone(); + t.split_cells(CellRange::new(0, 0, 0, 1), true).unwrap(); + t.check().unwrap(); + assert_eq!(t.header_rows(), 2); + for row in &t.rows[..2] { + assert_eq!(row.height, 30.0); + assert_eq!(row.mode, RowHeightMode::Exactly); + assert_eq!(row.kind, RowKind::Header); + } + assert_eq!(t.rows[2], body); + assert_eq!(t.rows[3], footer); + } } diff --git a/crates/engine/src/cmd/table.rs b/crates/engine/src/cmd/table.rs index c2313170a..6c6b1ff9d 100644 --- a/crates/engine/src/cmd/table.rs +++ b/crates/engine/src/cmd/table.rs @@ -178,7 +178,7 @@ fn table_specs() -> Vec { "Split Cell Horizontally", ["Table"], None, - "{} — the target cell becomes two, one above the other", + "{} — each target cell becomes two, one above the other", in_table, |s, p| split(s, p, true) ), @@ -187,7 +187,7 @@ fn table_specs() -> Vec { "Split Cell Vertically", ["Table"], None, - "{} — the target cell becomes two, side by side", + "{} — each target cell becomes two, side by side", in_table, |s, p| split(s, p, false) ), @@ -589,7 +589,7 @@ fn merge(s: &mut Session, p: &Value) -> Result { fn split(s: &mut Session, p: &Value, horizontal: bool) -> Result { let g = target(s, p, "table.split")?; edit_table(s, &g, "table.split", |t| { - t.split_cell(g.range.r0, g.range.c0, horizontal); + t.split_cells(g.range, horizontal).map_err(|e| bad("table.split", e))?; Ok(json!({"rows": t.nrows(), "cols": t.ncols()})) }) } diff --git a/crates/engine/src/tests_table.rs b/crates/engine/src/tests_table.rs index 45861347d..3934aabec 100644 --- a/crates/engine/src/tests_table.rs +++ b/crates/engine/src/tests_table.rs @@ -399,3 +399,125 @@ fn editing_beside_a_merged_cell_only_changes_the_selected_edge_segment() { } } } + +#[test] +fn split_selected_row_or_column_splits_both_cells() { + for horizontal in [true, false] { + let (mut s, sid, _) = session_with_frame(); + let tid = s.execute("table.insert", &json!({"rows": 2, "cols": 2})).unwrap()["table"].as_u64().unwrap(); + let params = if horizontal { json!({"rows": [0, 0], "cols": [0, 1]}) } else { json!({"rows": [0, 1], "cols": [0, 0]}) }; + s.execute("table.select", ¶ms).unwrap(); + let command = if horizontal { "table.splitHorizontally" } else { "table.splitVertically" }; + s.execute(command, &json!({})).unwrap(); + let table = table_for(&s, sid, tid); + assert_eq!((table.nrows(), table.ncols()), if horizontal { (3, 2) } else { (2, 3) }); + for r in 0..2 { + for c in 0..2 { + assert_eq!(table.owner(r, c), (r, c), "both selected cells have independent halves"); + } + } + check(&s); + } +} + +#[test] +fn split_selected_block_and_merged_cells_preserves_geometry_and_history() { + for horizontal in [true, false] { + for merged in [false, true] { + let (mut s, sid, _) = session_with_frame(); + let tid = s.execute("table.insert", &json!({"rows": 4, "cols": 4, "width": 240})).unwrap()["table"].as_u64().unwrap(); + s.execute("table.setRowHeight", &json!({"table": tid, "height": 60, "mode": "exactly"})).unwrap(); + s.execute("style.cell.create", &json!({"name": "Neighbor", "fill": "[Paper]", "insets": 3})).unwrap(); + s.execute("style.cell.create", &json!({"name": "Chosen", "fill": "[Black]", "tint": 0.15, "insets": 2})).unwrap(); + s.execute("style.cell.apply", &json!({"table": tid, "name": "Neighbor"})).unwrap(); + for r in 0..4 { + for c in 0..4 { + s.execute("table.setCell", &json!({"row": r, "col": c, "text": format!("{r},{c}")})).unwrap(); + } + } + let range = if !merged { + doc::CellRange::new(1, 1, 2, 2) + } else if horizontal { + doc::CellRange::new(0, 0, 2, 1) + } else { + doc::CellRange::new(0, 0, 1, 2) + }; + let selection = json!({"rows": [range.r0, range.r1], "cols": [range.c0, range.c1]}); + s.execute("table.select", &selection).unwrap(); + s.execute("style.cell.apply", &json!({"name": "Chosen"})).unwrap(); + if merged { + let merge = if horizontal { json!({"rows": [0, 2], "cols": [1, 1]}) } else { json!({"rows": [1, 1], "cols": [0, 2]}) }; + s.execute("table.merge", &merge).unwrap(); + s.execute("table.select", &selection).unwrap(); + } + let before_doc = s.doc().unwrap().doc.clone(); + let before_selection = s.doc().unwrap().selection.clone(); + let before = table_for(&s, sid, tid).clone(); + let before_layout = s.cache.get(&before_doc, doc::StoryId(sid), None); + let before_fragment = before_layout.frames.iter().flat_map(|f| &f.tables).find(|t| t.table == tid).unwrap(); + let undo_len = s.doc().unwrap().history.undo.len(); + let command = if horizontal { "table.splitHorizontally" } else { "table.splitVertically" }; + s.execute(command, &json!({})).unwrap(); + assert_eq!(s.doc().unwrap().history.undo.len(), undo_len + 1, "one split command creates one undo entry"); + let after_doc = s.doc().unwrap().doc.clone(); + let after_selection = s.doc().unwrap().selection.clone(); + let after = table_for(&s, sid, tid).clone(); + let after_layout = s.cache.get(&after_doc, doc::StoryId(sid), None); + let after_fragment = after_layout.frames.iter().flat_map(|f| &f.tables).find(|t| t.table == tid).unwrap(); + assert_eq!(after_fragment.rect, before_fragment.rect, "splitting does not resize the table"); + assert_eq!(after_fragment.cells.len(), before_fragment.cells.len() + 4, "four original owners split once"); + for placed in &before_fragment.cells { + let original = before.cell(placed.row, placed.col).unwrap(); + let matches: Vec<_> = after_fragment.cells.iter().filter(|c| c.source == placed.source).collect(); + assert_eq!(matches.len(), 1, "original cell text survives once: {}", placed.source); + let surviving = matches[0]; + let cell = after.cell(surviving.row, surviving.col).unwrap(); + let mut normalized = cell.clone(); + normalized.row_span = original.row_span; + normalized.col_span = original.col_span; + assert_eq!(&normalized, original, "existing cell text and formatting are preserved"); + if !range.contains(placed.row, placed.col) { + assert_eq!(surviving.rect, placed.rect, "unselected neighbor geometry is unchanged"); + continue; + } + let second_addr = if horizontal { + (surviving.row + cell.row_span as usize, surviving.col) + } else { + (surviving.row, surviving.col + cell.col_span as usize) + }; + let second = after.cell(second_addr.0, second_addr.1).unwrap(); + assert_eq!(after.owner(second_addr.0, second_addr.1), second_addr); + assert!(second.text.is_empty()); + assert_eq!(second.style, original.style); + assert_eq!(second.insets, original.insets); + assert_eq!(second.fill, original.fill); + assert_eq!(second.fill_tint, original.fill_tint); + let second_rect = after_fragment.cell(second_addr.0, second_addr.1).unwrap().rect; + assert_eq!(surviving.rect.union(second_rect), placed.rect, "the halves exactly cover the original cell"); + if horizontal { + assert_eq!(surviving.rect.y1, second_rect.y0); + } else { + assert_eq!(surviving.rect.x1, second_rect.x0); + } + } + let text = s.doc().unwrap().selection.text.unwrap(); + let address = text.cell.unwrap(); + assert_eq!(after.owner(address.row, address.col), (address.row, address.col)); + assert!(text.focus <= after.cell(address.row, address.col).unwrap().text.len()); + let cells = s.doc().unwrap().selection.cells.unwrap(); + assert!(cells.range.r1 < after.nrows() && cells.range.c1 < after.ncols()); + check(&s); + s.execute("edit.undo", &json!({})).unwrap(); + assert!(std::sync::Arc::ptr_eq(&s.doc().unwrap().doc, &before_doc)); + assert_eq!(s.doc().unwrap().selection, before_selection); + assert_eq!(s.doc().unwrap().history.undo.len(), undo_len); + s.execute("edit.redo", &json!({})).unwrap(); + assert!(std::sync::Arc::ptr_eq(&s.doc().unwrap().doc, &after_doc)); + assert_eq!(s.doc().unwrap().selection, after_selection); + let bytes = crate::cmd::file_bytes(&s.doc().unwrap().doc); + let reopened = crate::cmd::file_from(&bytes).unwrap(); + reopened.check().unwrap(); + assert_eq!(reopened.story(doc::StoryId(sid)).unwrap().tables.get(&tid).unwrap().as_ref(), &after); + } + } +}