feat(odf): write a repeated sheet cell by cutting the run

CHANGELOG.md

@@ -16,6 +16,10 @@ The release run heads these entries with the version and opens a fresh
 
 ## Unreleased
 
+- `Sheet::set_cell` writes a repeated `.ods` cell: the run is cut into the
+  position written and the parts around it, which keep the value they had.
+  Only a cell the file states no element for still refuses.
+
 - **Fix**: a repeated `.ods` cell answers for the position it was looked up
   at rather than the anchor of its range, so `SheetCell::position()`,
   `Sheet::cell()` and `DocumentPath` all name the cell that was asked for.

src/odr/internal/odf/AGENTS.md

@@ -187,10 +187,18 @@ The structural/foundational gaps, roughly by value:
    `office:value-type`/`office:value` *and* the `text:p` under the cell — the
    file states the value and shows a rendering of it, and setting one without
    the other leaves it contradicting itself. It writes through the cell's
-   single text run, so a cell that is absent, repeated, holding a formula, or
-   holding richer markup than one plain paragraph refuses instead. Splitting a
-   repeat, which is what would let an absent or repeated cell be written, is
-   the next step in [`spreadsheet-editing.md`](../../../../docs/design/spreadsheet-editing.md).
+   single text run, so a cell holding a formula or richer markup than one plain
+   paragraph refuses, as does one the file states no element for.
+
+   A **repeated** cell is written by cutting the run: `split_repeat` copies the
+   `table:table-row` and the `table:table-cell` around the position and leaves
+   the original node as the one written, so its element and children survive.
+   `reindex_sheet` then rebuilds the sheet's position index off the dom, a cell
+   node keeping the element it already carries. Both refusals are decided
+   before any of that, so a refused write leaves the run uncut. Two costs:
+   cutting a repeated row copies every cell in it, so the elements grow with
+   the row rather than with the repeat; and the reindex walks the row nodes,
+   which a repeat collapses, so it is bounded by the dom rather than the grid.
 3. **Save never re-encrypts**, and refuses rather than dropping the encryption:
    a document decrypted from a password-protected package reports
    `is_savable(false) == false` and every `save` overload throws

src/odr/internal/odf/README.md

@@ -119,7 +119,8 @@ Roughly ordered by importance.
     not evaluated)
 - [x] edit
   - [x] cell values (number, string, cleared)
-  - [ ] a cell that is absent or repeated (the run has to be split first)
+  - [x] a repeated cell, by cutting the run around the position written
+  - [ ] a cell the file states no element for
   - [ ] a formula cell, and a cell of richer markup than one plain paragraph
 
 ### Presentation documents (`.odp`)

src/odr/internal/odf/odf_document.cpp

@@ -23,6 +23,7 @@
 #include <cstring>
 #include <mutex>
 #include <ostream>
+#include <span>
 #include <sstream>
 #include <unordered_map>
 
@@ -367,27 +368,28 @@ class ElementAdapter final : public AdapterBase {
   void sheet_set_cell(const ElementIdentifier element_id,
                       const std::uint32_t column, const std::uint32_t row,
                       const CellValue &value) const override {
-    const ElementRegistry::Sheet &sheet =
-        m_registry->sheet_element_at(element_id);
-    const ElementRegistry::Sheet::Cell *cell = sheet.cell(column, row);
+    const ElementRegistry::Sheet::Cell *cell =
+        m_registry->sheet_element_at(element_id).cell(column, row);
     if (cell == nullptr || cell->element_id == null_element_id) {
       throw UnsupportedOperation(); // an empty cell is written as no element
     }
-    const ElementIdentifier cell_id = cell->element_id;
-    if (m_registry->sheet_cell_element_at(cell_id).is_repeated) {
-      throw UnsupportedOperation();
-    }
+    ElementIdentifier cell_id = cell->element_id;
 
-    pugi::xml_node node = get_node(cell_id);
-    if (node.attribute("table:formula")) {
+    // both refusals are decided on the run, before the split writes anything
+    if (get_node(cell_id).attribute("table:formula")) {
       throw UnsupportedOperation(); // its dependants would go stale
     }
-
-    const ElementIdentifier text_id = only_text_run(cell_id);
-    if (text_id == null_element_id) {
+    if (!holds_one_run(cell_id)) {
       throw UnsupportedOperation();
     }
-    text_set_content(text_id, value.has_text() ? value.text() : "");
+
+    if (m_registry->sheet_cell_element_at(cell_id).is_repeated) {
+      cell_id = split_repeat(element_id, column, row); // `cell` is stale after
+    }
+
+    pugi::xml_node node = get_node(cell_id);
+    text_set_content(text_run_of(cell_id),
+                     value.has_text() ? value.text() : "");
 
     static constexpr std::array stated = {
         "office:value-type", "office:value",      "office:boolean-value",
@@ -884,31 +886,93 @@ class ElementAdapter final : public AdapterBase {
     return m_registry->element_at(element_id).node;
   }
 
-  /// The single text run under a cell of one plain paragraph, created where
-  /// that paragraph is empty. Null where the markup is richer than that, which
-  /// a write then keeps rather than throws away.
-  [[nodiscard]] ElementIdentifier
-  only_text_run(const ElementIdentifier cell_id) const {
+  /// The attribute where @p node repeats at all, none where it covers one.
+  static void set_repeat(pugi::xml_node node, const char *attribute,
+                         const std::uint32_t repeated) {
+    node.remove_attribute(attribute);
+    if (repeated > 1) {
+      node.append_attribute(attribute).set_value(repeated);
+    }
+  }
+
+  /// Cuts the run [@p begin, @p end) so @p at stands alone, copying the parts
+  /// around it. @p node stays as the one at @p at, so its element survives.
+  static void split_run(pugi::xml_node node, const char *attribute,
+                        const std::uint32_t begin, const std::uint32_t end,
+                        const std::uint32_t at) {
+    if (end > at + 1) {
+      set_repeat(node.parent().insert_copy_after(node, node), attribute,
+                 end - at - 1);
+    }
+    if (at > begin) {
+      set_repeat(node.parent().insert_copy_before(node, node), attribute,
+                 at - begin);
+    }
+    set_repeat(node, attribute, 1);
+  }
+
+  /// Gives (@p column, @p row) a cell of its own, splitting the row and the
+  /// cell run it is one position of. Reindexes: every pointer read before is
+  /// stale.
+  [[nodiscard]] ElementIdentifier split_repeat(const ElementIdentifier sheet_id,
+                                               const std::uint32_t column,
+                                               const std::uint32_t row) const {
+    const ElementRegistry::Sheet &sheet =
+        m_registry->sheet_element_at(sheet_id);
+
+    const ElementRegistry::Sheet::Row *row_entry = sheet.row(row);
+    const std::size_t row_index = row_entry - sheet.rows.data();
+    const std::uint32_t row_begin =
+        row_index == 0 ? 0 : sheet.rows[row_index - 1].end;
+
+    const std::span<const ElementRegistry::Sheet::Cell> cells =
+        sheet.row_cells(*row_entry);
+    const ElementRegistry::Sheet::Cell *cell_entry = sheet.cell(column, row);
+    const std::size_t cell_index = cell_entry - cells.data();
+    const std::uint32_t cell_begin =
+        cell_index == 0 ? 0 : cells[cell_index - 1].end;
+
+    // the row first: the cell keeps its node, so its own run is unmoved
+    split_run(row_entry->node, "table:number-rows-repeated", row_begin,
+              row_entry->end, row);
+    split_run(cell_entry->node, "table:number-columns-repeated", cell_begin,
+              cell_entry->end, column);
+
+    reindex_sheet(*m_registry, sheet_id);
+
+    return m_registry->sheet_element_at(sheet_id).cell(column, row)->element_id;
+  }
+
+  /// Whether a write can go through the cell: one plain paragraph of one run
+  /// at most. Richer markup is kept rather than overwritten.
+  [[nodiscard]] bool holds_one_run(const ElementIdentifier cell_id) const {
     const ElementIdentifier paragraph_id = element_first_child(cell_id);
     if (paragraph_id == null_element_id ||
         element_next_sibling(paragraph_id) != null_element_id ||
         element_type(paragraph_id) != ElementType::paragraph) {
-      return null_element_id;
+      return false;
     }
     const ElementIdentifier text_id = element_first_child(paragraph_id);
-    if (text_id == null_element_id) {
-      const pugi::xml_node text_node =
-          get_node(paragraph_id).append_child(pugi::xml_node_type::node_pcdata);
-      const auto &[new_id, unused1, unused2] =
-          m_registry->create_text_element(text_node, text_node);
-      m_registry->append_child(paragraph_id, new_id);
-      return new_id;
-    }
-    if (element_next_sibling(text_id) != null_element_id ||
-        element_type(text_id) != ElementType::text) {
-      return null_element_id;
-    }
-    return text_id;
+    return text_id == null_element_id ||
+           (element_next_sibling(text_id) == null_element_id &&
+            element_type(text_id) == ElementType::text);
+  }
+
+  /// That run, created where the paragraph is empty - what a cleared cell is.
+  /// @ref holds_one_run has to pass.
+  [[nodiscard]] ElementIdentifier
+  text_run_of(const ElementIdentifier cell_id) const {
+    const ElementIdentifier paragraph_id = element_first_child(cell_id);
+    if (const ElementIdentifier text_id = element_first_child(paragraph_id);
+        text_id != null_element_id) {
+      return text_id;
+    }
+    const pugi::xml_node text_node =
+        get_node(paragraph_id).append_child(pugi::xml_node_type::node_pcdata);
+    const auto &[new_id, unused1, unused2] =
+        m_registry->create_text_element(text_node, text_node);
+    m_registry->append_child(paragraph_id, new_id);
+    return new_id;
   }
 
   /// The image's base64 bytes where the markup carries them itself.

src/odr/internal/odf/odf_element_registry.hpp

@@ -184,9 +184,9 @@ class ElementRegistry final
     return self.m_sheets.at(self.resolve_id(id));
   }
 
-  [[nodiscard]] const SheetCell &
-  sheet_cell_element_at(const ElementIdentifier id) const {
-    return m_sheet_cells.at(resolve_id(id));
+  [[nodiscard]] auto &sheet_cell_element_at(this auto &self,
+                                            const ElementIdentifier id) {
+    return self.m_sheet_cells.at(self.resolve_id(id));
   }
 
   [[nodiscard]] const SheetCell *

src/odr/internal/odf/odf_parser.cpp

@@ -159,6 +159,64 @@ bool is_cell_empty(const pugi::xml_node cell_node) {
          cell_node.attribute("table:number-rows-spanned").as_uint(1) <= 1;
 }
 
+/// The element a cell node already carries, so a reindex keeps it.
+using SheetCellElements = std::unordered_map<const void *, ElementIdentifier>;
+
+/// Builds @p sheet's row and cell index off its dom. A cell node in
+/// @p existing keeps that element; one that is not gets a fresh one.
+void index_sheet_rows(ElementRegistry &registry,
+                      const ElementIdentifier sheet_id,
+                      ElementRegistry::Sheet &sheet, const pugi::xml_node node,
+                      const SheetCellElements &existing) {
+  TableCursor cursor;
+
+  for_each_table_row(node, [&](const pugi::xml_node row_node) {
+    const std::uint32_t rows_repeated =
+        row_node.attribute("table:number-rows-repeated").as_uint(1);
+
+    sheet.register_row(cursor.row(), rows_repeated, row_node);
+
+    // TODO covered cells
+    for (const pugi::xml_node cell_node :
+         row_node.children("table:table-cell")) {
+      const std::uint32_t columns_repeated =
+          cell_node.attribute("table:number-columns-repeated").as_uint(1);
+      const std::uint32_t colspan =
+          cell_node.attribute("table:number-columns-spanned").as_uint(1);
+      const std::uint32_t rowspan =
+          cell_node.attribute("table:number-rows-spanned").as_uint(1);
+      const bool is_repeated = columns_repeated > 1 || rows_repeated > 1;
+
+      ElementIdentifier cell_id = null_element_id;
+      const auto kept = existing.find(cell_node.internal_object());
+      if (kept != std::end(existing)) {
+        cell_id = kept->second;
+        ElementRegistry::SheetCell &cell =
+            registry.sheet_cell_element_at(cell_id);
+        cell.position = cursor.position();
+        cell.is_repeated = is_repeated;
+      } else if (!is_cell_empty(cell_node)) {
+        const auto &[id, unused1, unused2] = registry.create_sheet_cell_element(
+            cell_node, cursor.position(), is_repeated);
+        cell_id = id;
+        registry.append_sheet_cell(sheet_id, cell_id);
+        parse_any_element_children(registry, cell_id, cell_node);
+      }
+
+      sheet.register_cell(cursor.column(), cursor.row(), columns_repeated,
+                          rows_repeated, cell_node, cell_id);
+
+      cursor.add_cell(colspan, rowspan, columns_repeated);
+    }
+
+    // TODO a rowspan out of a repeated row is dropped - `add_row` clears the
+    // cursor's pending ranges for a repeat > 1
+    cursor.add_row(rows_repeated);
+  });
+
+  sheet.dimensions.rows = cursor.row();
+}
+
 /// One entry per row and per cell node at most - repeats collapse onto one.
 void reserve_sheet(ElementRegistry::Sheet &sheet, const pugi::xml_node node) {
   std::size_t rows = 0;
@@ -199,47 +257,7 @@ parse_sheet(ElementRegistry &registry, const pugi::xml_node node) {
   sheet.dimensions.columns = cursor.column();
   cursor = {};
 
-  for_each_table_row(node, [&](const pugi::xml_node row_node) {
-    const std::uint32_t rows_repeated =
-        row_node.attribute("table:number-rows-repeated").as_uint(1);
-
-    sheet.register_row(cursor.row(), rows_repeated, row_node);
-
-    // TODO covered cells
-    for (const pugi::xml_node cell_node :
-         row_node.children("table:table-cell")) {
-      const std::uint32_t columns_repeated =
-          cell_node.attribute("table:number-columns-repeated").as_uint(1);
-      const std::uint32_t colspan =
-          cell_node.attribute("table:number-columns-spanned").as_uint(1);
-      const std::uint32_t rowspan =
-          cell_node.attribute("table:number-rows-spanned").as_uint(1);
-      const bool is_repeated = columns_repeated > 1 || rows_repeated > 1;
-
-      ElementIdentifier cell_id = null_element_id;
-      if (!is_cell_empty(cell_node)) {
-        const auto &[id, unused1, unused2] = registry.create_sheet_cell_element(
-            cell_node, cursor.position(), is_repeated);
-        cell_id = id;
-        registry.append_sheet_cell(element_id, cell_id);
-      }
-
-      sheet.register_cell(cursor.column(), cursor.row(), columns_repeated,
-                          rows_repeated, cell_node, cell_id);
-
-      if (cell_id != null_element_id) {
-        parse_any_element_children(registry, cell_id, cell_node);
-      }
-
-      cursor.add_cell(colspan, rowspan, columns_repeated);
-    }
-
-    // TODO a rowspan out of a repeated row is dropped - `add_row` clears the
-    // cursor's pending ranges for a repeat > 1
-    cursor.add_row(rows_repeated);
-  });
-
-  sheet.dimensions.rows = cursor.row();
+  index_sheet_rows(registry, element_id, sheet, node, {});
 
   for (const pugi::xml_node shape_node :
        node.child("table:shapes").children()) {
@@ -413,4 +431,22 @@ ElementIdentifier odf::parse_tree(ElementRegistry &registry,
   return root;
 }
 
+void odf::reindex_sheet(ElementRegistry &registry,
+                        const ElementIdentifier sheet_id) {
+  ElementRegistry::Sheet &sheet = registry.sheet_element_at(sheet_id);
+
+  SheetCellElements existing;
+  existing.reserve(sheet.cells.size());
+  for (const ElementRegistry::Sheet::Cell &cell : sheet.cells) {
+    if (cell.element_id != null_element_id) {
+      existing.emplace(cell.node.internal_object(), cell.element_id);
+    }
+  }
+
+  sheet.rows.clear();
+  sheet.cells.clear();
+  index_sheet_rows(registry, sheet_id, sheet,
+                   registry.element_at(sheet_id).node, existing);
+}
+
 } // namespace odr::internal

src/odr/internal/odf/odf_parser.hpp

@@ -11,4 +11,8 @@ class ElementRegistry;
 
 ElementIdentifier parse_tree(ElementRegistry &registry, pugi::xml_node node);
 
+/// Rebuilds the row and cell index of @p sheet_id off its dom, for a write
+/// that split a repeat. A cell node keeps the element it already carries.
+void reindex_sheet(ElementRegistry &registry, ElementIdentifier sheet_id);
+
 } // namespace odr::internal::odf

test/src/internal/odf/odf_sheet_repeat_test.cpp

@@ -141,3 +141,22 @@ TEST(OdfSheetRepeat, the_children_of_a_repeated_cell_are_shared) {
   EXPECT_EQ(*(*sheet.cell(0, 0).children().begin()).children().begin(), text);
   EXPECT_EQ(document.root_element().navigate_path(text.document_path()), text);
 }
+
+/// A write cuts the run into three rather than expanding it, so what it costs
+/// follows the row it touched and not the grid the repeat claims.
+TEST(OdfSheetRepeat, a_write_into_a_repeat_does_not_expand_it) {
+  const std::string source = flat_sheet(repeated_rows(1048576, 1024));
+  const std::shared_ptr<abstract::Document> held = document_of(source);
+  const auto *document = dynamic_cast<const odf::Document *>(held.get());
+  ASSERT_NE(document, nullptr);
+
+  const odr::Document public_document(held);
+  const Sheet sheet =
+      (*public_document.root_element().children().begin()).as_sheet();
+  sheet.set_cell(512, 1024, CellValue("y"));
+
+  EXPECT_EQ(sheet.cell(512, 1024).value().text(), "y");
+  EXPECT_EQ(sheet.cell(511, 1024).value().text(), "x");
+  EXPECT_EQ(sheet.cell(512, 1023).value().text(), "x");
+  EXPECT_LT(document->element_registry().size(), 32);
+}