feat(spreadsheet): write into an empty ods cell

CHANGELOG.md

@@ -16,6 +16,11 @@ The release run heads these entries with the version and opens a fresh
 
 ## Unreleased
 
+- An empty `.ods` cell can be written: the write cuts the run of empty cells
+  the position belongs to and states the `text:p` the file spells none of. A
+  merged cell that holds no paragraph takes one the same way. A position past
+  the last cell the file states still refuses.
+
 - A sheet cell can be typed into: an overlay opens on a double click or a key,
   Enter and Tab commit, and `odr.editing.getOperations()` hands the host the
   envelope `Document::edit` takes.

docs/design/spreadsheet-editing.md

@@ -1,6 +1,6 @@
 # Spreadsheet editing design
 
-Status: **steps 0 and 1 landed, and 2.1 with them; step 2 is next.** This
+Status: **steps 0 and 1 landed, and 2.1 with them; step 2 is under way.** This
 records why spreadsheet editing is staged the way it is, what the code already
 gives us, and the order the steps go in. It is a plan, not a record — update it
 as steps land.
@@ -40,7 +40,7 @@ results go stale the moment an input changes.
 | ODS string-cell edit | `odf_document.cpp::text_set_content` | Works: `Document.edit_ods_diff` edits five cells in memory. Only the run's text changes; `office:value` on a number cell is not touched |
 | ODS save | `odf_document.cpp::save` | Re-serialises `content.xml`, byte-copies the rest — the same shape a sheet needs |
 | ODS cell index | `odf_element_registry.cpp::Sheet::register_cell` | Per row a run of `(end, element_id, node)` entries; repeats collapse onto one entry. Written once at parse; nothing inserts |
-| ODS repeated cells | `odf_document.cpp::split_repeat` | A write cuts the run and `reindex_sheet` rebuilds the index (step 2.1, landed) |
+| ODS repeated and empty cells | `odf_document.cpp::claim_cell` | A write cuts the run and states the `text:p` an empty cell has none of; `reindex_sheet` rebuilds the index (step 2.1, landed) |
 | XLSX edit | `sheet_set_cell` | Writes a cell value (step 0.2, landed); `text_set_content` is still a no-op |
 | XLSX save | `ooxml_spreadsheet_document.cpp::save` | Writes back the worksheets and `workbook.xml`, copies the rest (step 0.2, landed) |
 | XLSX cells | `Sheet.cells` `(col,row) → {node, id}` map | Off-tree; an empty position has no `<c>` node |
@@ -389,10 +389,15 @@ Each step ships on its own. "Both" means `.ods` and `.xlsx`.
    rebuilds it off the dom, a cell node keeping the element it carries — which
    avoids a second copy of the parser's row loop. The `repeated` lock is gone.
 
-   **Open:** the same primitive for a position the file states no element for.
-   A run with a node but no element (`<table:table-cell number-columns-repeated=
-   "1000"/>`) only needs the split plus a `text:p`; a position past the row's
-   last cell or the sheet's last row needs appending and growing the extent.
+   **Landed for empty cells.** A run with a node but no element
+   (`<table:table-cell table:number-columns-repeated="1000"/>`) is cut the same
+   way, and the write states the `text:p`, because the reindex gives an element
+   to a node that is not empty. A cell a merge spans and that holds no
+   paragraph takes one too: the page reads it as editable, so the engine has to
+   agree.
+
+   **Open:** a position past the row's last cell or the sheet's last row needs
+   appending and growing the extent.
 2. XLSX: insert `<c r="…">` in column order into its `<row>`, create the
    `<row>` in row order, grow `<dimension ref>`.
 3. Rich cells: replace with one plain paragraph, keeping the cell style. The
@@ -473,8 +478,9 @@ Ordered by value over cost; all in step 0 or 1.
   edited row. Without it an edit into a blank cell shows the left neighbour's
   overflow painting across the new text.
 - **A position the engine cannot write yet** — an `.xlsx` cell with no `<c>`,
-  an `.ods` one with no element — carries no lock, so the page takes the edit
-  and `Document::edit` throws it back at the host. Until step 2, it says so.
+  an `.ods` one past the last the file states — carries no lock, so the page
+  takes the edit and `Document::edit` throws it back at the host. Until step 2,
+  it says so.
 - **Sheets past the cut** (`spreadsheet_limit`, `spreadsheet_cell_limit`) are
   not in the page and cannot be edited; the mode should say so where a view
   reports a `sheet_cut`.

src/odr/internal/odf/AGENTS.md

@@ -188,14 +188,21 @@ The structural/foundational gaps, roughly by value:
    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 holding a formula or richer markup than one plain
-   paragraph refuses, as does one the file states no element for.
+   paragraph refuses, as does a position past the last cell the file states.
 
-   A **repeated** cell is written by cutting the run: `split_repeat` copies the
+   A **repeated** cell is written by cutting the run: `claim_cell` 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:
+   before any of that, so a refused write leaves the run uncut.
+
+   An **empty** cell carries no element, because `index_sheet_rows` builds one
+   only for a node with content or a span. `claim_cell` therefore appends the
+   `text:p` *before* the reindex, which then sees a node that is not empty and
+   builds the element for it. A spanned cell that holds no paragraph takes one
+   from `text_run_of` instead, because the page already reads it as editable.
+   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.

src/odr/internal/odf/odf_document.cpp

@@ -370,21 +370,22 @@ class ElementAdapter final : public AdapterBase {
                       const CellValue &value) const override {
     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
+    if (cell == nullptr) {
+      throw UnsupportedOperation(); // the sheet states no node here
     }
     ElementIdentifier cell_id = cell->element_id;
 
-    // both refusals are decided on the run, before the split writes anything
-    if (get_node(cell_id).attribute("table:formula")) {
+    // both refusals are decided before the split writes anything
+    if (cell->node.attribute("table:formula")) {
       throw UnsupportedOperation(); // its dependants would go stale
     }
-    if (!holds_one_run(cell_id)) {
+    if (cell_id != null_element_id && !holds_one_run(cell_id)) {
       throw UnsupportedOperation();
     }
 
-    if (m_registry->sheet_cell_element_at(cell_id).is_repeated) {
-      cell_id = split_repeat(element_id, column, row); // `cell` is stale after
+    if (cell_id == null_element_id ||
+        m_registry->sheet_cell_element_at(cell_id).is_repeated) {
+      cell_id = claim_cell(element_id, column, row); // `cell` is stale after
     }
 
     pugi::xml_node node = get_node(cell_id);
@@ -911,12 +912,12 @@ class ElementAdapter final : public AdapterBase {
     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 {
+  /// Gives (@p column, @p row) an element of its own: cuts the row and the
+  /// cell run it is one position of, and states the `text:p` an empty cell
+  /// has none of. Reindexes: every pointer read before is stale.
+  [[nodiscard]] ElementIdentifier claim_cell(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);
 
@@ -931,13 +932,18 @@ class ElementAdapter final : public AdapterBase {
     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;
+    pugi::xml_node cell_node = cell_entry->node;
 
     // 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,
+    split_run(cell_node, "table:number-columns-repeated", cell_begin,
               cell_entry->end, column);
 
+    if (!cell_node.first_child()) {
+      cell_node.append_child("text:p"); // a node with no content gets none
+    }
+
     reindex_sheet(*m_registry, sheet_id);
 
     return m_registry->sheet_element_at(sheet_id).cell(column, row)->element_id;
@@ -947,8 +953,10 @@ class ElementAdapter final : public AdapterBase {
   /// 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 ||
+    if (paragraph_id == null_element_id) {
+      return true; // a spanned cell states no paragraph; the write states one
+    }
+    if (element_next_sibling(paragraph_id) != null_element_id ||
         element_type(paragraph_id) != ElementType::paragraph) {
       return false;
     }
@@ -958,11 +966,18 @@ class ElementAdapter final : public AdapterBase {
             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.
+  /// That run, and the paragraph around it, created where the cell states
+  /// neither. @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);
+    ElementIdentifier paragraph_id = element_first_child(cell_id);
+    if (paragraph_id == null_element_id) {
+      pugi::xml_node cell_node = get_node(cell_id);
+      const auto &[new_id, unused] = m_registry->create_element(
+          ElementType::paragraph, cell_node.append_child("text:p"));
+      m_registry->append_child(cell_id, new_id);
+      paragraph_id = new_id;
+    }
     if (const ElementIdentifier text_id = element_first_child(paragraph_id);
         text_id != null_element_id) {
       return text_id;

test/src/internal/odf/odf_sheet_write_test.cpp

@@ -208,14 +208,125 @@ TEST(OdfSheetWrite, a_formula_cell_refuses_to_be_written) {
   EXPECT_THROW(sheet.set_cell(0, 0, CellValue("y")), UnsupportedOperation);
 }
 
-/// An empty cell is written as no element at all.
-TEST(OdfSheetWrite, an_absent_cell_refuses_to_be_written) {
+/// Past the last cell the file states there is no node to write into.
+TEST(OdfSheetWrite, a_cell_past_the_sheet_refuses_to_be_written) {
   const Document document = document_of(flat_sheet(string_cell("a")));
   const Sheet sheet = first_sheet(document);
 
   EXPECT_THROW(sheet.set_cell(4, 4, CellValue("y")), UnsupportedOperation);
 }
 
+/// An empty cell is parsed as no element, so a write has to make one.
+TEST(OdfSheetWrite, an_empty_cell_is_written) {
+  const Document document =
+      document_of(flat_sheet(string_cell("a") + R"(<table:table-cell/>)"));
+  const Sheet sheet = first_sheet(document);
+
+  sheet.set_cell(1, 0, CellValue("y"));
+
+  EXPECT_EQ(sheet.cell(1, 0).value().text(), "y");
+  EXPECT_EQ(sheet.cell(0, 0).value().text(), "a");
+}
+
+TEST(OdfSheetWrite, an_empty_cell_takes_a_number) {
+  const Document document = document_of(flat_sheet(R"(<table:table-cell/>)"));
+  const Sheet sheet = first_sheet(document);
+
+  sheet.set_cell(0, 0, CellValue(12.5, "12.5"));
+
+  const CellValue value = sheet.cell(0, 0).value();
+  EXPECT_EQ(value.type(), ValueType::float_number);
+  ASSERT_TRUE(value.has_number());
+  EXPECT_DOUBLE_EQ(value.number(), 12.5);
+  EXPECT_EQ(value.text(), "12.5");
+}
+
+/// The style is the cell's, not the paragraph's, so writing keeps it.
+TEST(OdfSheetWrite, an_empty_cell_keeps_its_style) {
+  const Document document =
+      document_of(flat_sheet(R"(<table:table-cell table:style-name="ce1"/>)"));
+  first_sheet(document).set_cell(0, 0, CellValue("y"));
+
+  std::ostringstream saved;
+  document.save(saved);
+  EXPECT_NE(saved.str().find(R"(table:style-name="ce1")"), std::string::npos);
+}
+
+TEST(OdfSheetWrite, an_empty_repeated_cell_is_split_by_a_write) {
+  const Document document = document_of(
+      flat_sheet(R"(<table:table-cell table:number-columns-repeated="4"/>)"));
+  const Sheet sheet = first_sheet(document);
+
+  sheet.set_cell(2, 0, CellValue("y"));
+
+  EXPECT_EQ(sheet.cell(2, 0).value().text(), "y");
+  // the cells around it are still no element, so they state nothing at all
+  for (const std::uint32_t column : {0u, 1u, 3u}) {
+    EXPECT_FALSE(sheet.cell(column, 0).value().has_text()) << column;
+  }
+  EXPECT_EQ(sheet.dimensions().rows, 1);
+}
+
+TEST(OdfSheetWrite, an_empty_cell_of_a_repeated_row_is_written) {
+  const Document document =
+      document_of(R"(<?xml version="1.0" encoding="UTF-8"?>)"
+                  R"(<office:document office:mimetype=")"
+                  R"(application/vnd.oasis.opendocument.spreadsheet">)"
+                  R"(<office:body><office:spreadsheet>)"
+                  R"(<table:table table:name="s">)"
+                  R"(<table:table-row table:number-rows-repeated="3">)"
+                  R"(<table:table-cell table:number-columns-repeated="2"/>)"
+                  R"(</table:table-row></table:table>)"
+                  R"(</office:spreadsheet></office:body></office:document>)");
+  const Sheet sheet = first_sheet(document);
+
+  sheet.set_cell(1, 1, CellValue("y"));
+
+  EXPECT_EQ(sheet.cell(1, 1).value().text(), "y");
+  EXPECT_FALSE(sheet.cell(0, 1).value().has_text());
+  EXPECT_FALSE(sheet.cell(1, 0).value().has_text());
+  EXPECT_FALSE(sheet.cell(1, 2).value().has_text());
+  EXPECT_EQ(sheet.dimensions().rows, 3);
+}
+
+/// A formula cell can hold no cached value, and the formula refuses either
+/// way.
+TEST(OdfSheetWrite, an_empty_formula_cell_refuses_to_be_written) {
+  const Document document = document_of(flat_sheet(
+      R"xml(<table:table-cell table:formula="of:=SUM([.B1:.C1])"/>)xml"));
+  const Sheet sheet = first_sheet(document);
+
+  EXPECT_THROW(sheet.set_cell(0, 0, CellValue("y")), UnsupportedOperation);
+}
+
+/// A merged cell holds no paragraph until something is written into it.
+TEST(OdfSheetWrite, an_empty_spanned_cell_is_written) {
+  const Document document = document_of(
+      flat_sheet(R"(<table:table-cell table:number-columns-spanned="2"/>)"));
+  const Sheet sheet = first_sheet(document);
+
+  sheet.set_cell(0, 0, CellValue("y"));
+
+  EXPECT_EQ(sheet.cell(0, 0).value().text(), "y");
+  EXPECT_EQ(sheet.cell(0, 0).span().columns, 2);
+}
+
+TEST(OdfSheetWrite, an_empty_cell_written_into_saves_and_reopens) {
+  const Document document = document_of(
+      flat_sheet(R"(<table:table-cell table:number-columns-repeated="4"/>)"));
+  first_sheet(document).set_cell(2, 0, CellValue(41.5, "41.5"));
+
+  std::ostringstream saved;
+  document.save(saved);
+  const Document reopened = document_of(saved.str());
+  const Sheet sheet = first_sheet(reopened);
+
+  ASSERT_TRUE(sheet.cell(2, 0).value().has_number());
+  EXPECT_DOUBLE_EQ(sheet.cell(2, 0).value().number(), 41.5);
+  EXPECT_FALSE(sheet.cell(1, 0).value().has_text());
+  EXPECT_FALSE(sheet.cell(3, 0).value().has_text());
+}
+
 TEST(OdfSheetWrite, a_cell_of_several_paragraphs_refuses_to_be_written) {
   const Document document = document_of(
       flat_sheet(R"(<table:table-cell office:value-type="string">)"