feat(spreadsheet): write a value into a cell, and save an xlsx - #856
Merged
Conversation
andiwand
force-pushed
the
feat/sheet-cell-value
branch
from
September 7, 2026 05:32
a0c419e to
ebf4c90
Compare
andiwand
force-pushed
the
feat/sheet-cell-write
branch
3 times, most recently
from
September 7, 2026 06:36
2117e41 to
7127cb2
Compare
`Sheet::set_cell` and `::clear_cell` write one cell of an `.ods` or an `.xlsx`, and both documents now report themselves editable. The hook under them is position-addressed - `sheet_set_cell(sheet, column, row, CellValue)` - because the cell an edit names need not have an element. `CellValue` becomes one type for reading and writing, so what a cell reads as is what writing it back takes. It is immutable, gains the text a write has to state, and is built by explicit constructors from a text, a number or a bare type; the `with_*` withers compose the rest, which is what a decoder needs. Its getters throw `ValueNotStated` rather than hand back an empty optional. `SheetCell::value` collects the text off the cell's children, so no engine states it and `xls` and `numbers` answer with it too. The java and python binding tests pinned `ods` as not editable; they now pin `edit` and `save` on it, as the capability table states. odf writes `office:value-type`, `office:value` and the `text:p` together: the file states a 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 nothing structural moves. ooxml rewrites the `c` and hands the registry a fresh text element; the elements that read the old children keep their ids and stop being reachable. A shared string is never written back into `sharedStrings.xml` - every other cell indexing that entry would change with it - so a written cell becomes `t="inlineStr"`. Five cells refuse rather than be written badly: one the file spells no element for, a repeated one, a covered one, one holding a formula, and one holding richer markup than a single plain paragraph. A formula cell waits for an evaluator: overwriting one leaves every value computed from it stale. A cell whose paragraph holds no run - what a blank styled cell and a cleared one are written as - is given a run rather than refused, so a cell stays writable after it has been cleared, saved and reopened. Found while implementing it: an xlsx cell holding an inline string read as **empty**, because the walker never descends into `is`. Fixed here, since a written string relies on that path. `save` for xlsx mirrors docx - write back the worksheets and `workbook.xml` from their dom, byte-copy the rest - and puts back the xml declaration pugixml is never asked to parse. Every save sets `calcPr/@fullCalcOnLoad` (ECMA-376 18.2.2): we rewrote the file and compute no formula, so the reader is asked to. Verified against LibreOffice by hand on a real `.ods` and a real `.xlsx`: both open and show the written number where it was put. Second step of `docs/design/spreadsheet-editing.md`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F17g1P9PbwFVspTzMSBqiQ
andiwand
force-pushed
the
feat/sheet-cell-write
branch
from
September 7, 2026 06:49
7127cb2 to
789fdad
Compare
A `CellValue` typed a number but stating none cleared the cell's text and its value attributes and only then threw out of `CellValue::number`, leaving the document half written. `Sheet::set_cell` now refuses it up front, so every refusal leaves the cell as it was. ECMA-376 18.2.27 orders the `workbook` children, and `save` appended a missing `calcPr` last - after an `extLst` or a `pivotCaches`, which the schema puts after it. A new one now goes before the first child that must follow it. `only_text_run` moves to the adapter's private section, and the comments that recited what the code says or where the work is going are cut. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J325TWocZ4iXBjvKv2ZVZi
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 Generated with Claude Code
Step 0.2 of
docs/design/spreadsheet-editing.md: a cell can be written, and anxlsx can be saved.
The API
One type for reading and writing.
CellValueis whatSheetCell::valuehands out and what
set_celltakes, so what a cell reads as is what writing itback takes — there is one vocabulary for a cell's content, not one per
direction.
It is immutable, and built by explicit constructors — a text alone is a
string cell, a number alone a number cell that spells itself, and
CellValue(ValueType)a bare typed one, which is what a decoder starts fromsince a file types a cell before it states anything. The
with_*witherscompose the rest, and take concrete values, not optionals. The getters return
concrete types and throw
ValueNotStatedwhere a cell states none, rather thanhanding back an optional to unwrap.
CellValuegainstextbecause for a string cell the text is the value,and because odf stores a number's display string beside it — only the caller
knows how a number should read.
SheetCell::valuecollects that text off thecell's children, once, so no engine has to state it;
xlsandnumbers, whichused to answer with the type alone, now answer with the text too.
Underneath is one adapter hook,
sheet_set_cell(sheet, column, row, CellValue),addressed by position, not by id — the cell an edit names need not have an
element, which is the whole reason the plan chose positions for sheets and ids
for text. Writing a value that holds a formula refuses: that waits for step 4.
Per engine
odf writes
office:value-type,office:valueand thetext:ptogether. Awriter that set only one leaves the file stating one thing and showing another.
It writes through the cell's single text run, so nothing structural moves.
ooxml rewrites the
cand hands the registry a fresh text element. Theelements that read the old children keep their ids and simply stop being
reachable — the tombstoning
editing.mdasks for. A shared string is neverwritten back into
sharedStrings.xml, since every other cell indexing thatentry would change with it, so a written cell becomes
t="inlineStr". There isa test for exactly that: two cells share one
si, one is written, the other isstill
same.What refuses, and why
A cell whose paragraph holds no run is not one of these: that is what a
blank styled cell and a cleared one are written as, so it is given a run rather
than refused. Without it a cell could be cleared, saved and never written
again.
Refusing is
UnsupportedOperation, and each case has a test.A read bug found on the way
An xlsx cell holding an inline string read as empty. Its text sits under
is, one level below the cell, and the walker does not descend into elements ithas no parser for — the same reason a shared string needed its own branch. So
<c t="inlineStr"><is><t>hello</t></is></c>rendered as a blank cell. Fixedhere, because a written string now relies on that path. Two tests pin it.
savefor xlsxMirrors docx: write back the worksheets and
workbook.xmlfrom their dom,byte-copy everything else. Two details worth flagging:
one, so it cannot write one, and a workbook part is expected to carry it.
calcPr/@fullCalcOnLoadis set on every save, not only after an edit(ECMA-376 18.2.2). We rewrote the file and compute no formula, so any cached
result in it may be one we invalidated; asking the reader to recalculate is
the honest thing for a writer that is not the application that made the file.
odsgains theeditcapability,xlsxgainseditandsave, andodf::Document::is_editableloses its// TODO fix spreadsheet editability.Review pass
A second commit fixes two things found reviewing the first:
CellValuetyped a numberbut stating none cleared the text and the value attributes and only then threw
out of
CellValue::number.Sheet::set_cellrefuses it up front now, soevery refusal leaves the cell as it was. Test pins it.
calcPrwas appended last. ECMA-376 18.2.27 orders theworkbookchildren and putsoleSize,pivotCaches,extLstand the restafter
calcPr, so a workbook carrying one of those and nocalcPrcameback out of order. The new one goes before the first child that must follow
it. Test pins it.
Plus
only_text_runinto the adapter's private section, and the comments thatrecited the code or named where the work is going are cut.
Checks
cell_value_test.cppamong them..odsand a real.xlsxedited,saved, and converted with
soffice --convert-to csv. Both open and show thenumber at the position it was written to, with the rest of the sheet intact —
including the clear → save → reopen → write → save round trip and a value read
out of one cell and written into another. Not committed as a test —
sofficeis not in CI.
-Wall -Wextra -Werrorover all eight touched TUs; clang-tidy clean.ooxml_spreadsheet_test_util.hppin feat(document): read the number and the formula a sheet cell holds #853now also build a
sharedStrings.xml, which is what the shared-string testneeds.
Worth knowing for step 2
Looking for a writable cell in a real
.odsis often a miss: the first publicfile I tried had no cell that could be written, because empty cells have no
element and the rest were repeated runs. That is the clearest argument yet that
splitting repeats is the next step, not a later one.
CI
The java and python binding tests each pinned
odsas not editable — thesame stale claim as the comment in
file_type_table.cpp— which is what thebuild (…, true)andwheelsjobs were failing on. They now pineditandsave. Both suites run clean locally:odr_jni_junitpasses, and 75 pythontests pass.
The
aarjob, red on main when this branch started, is green again.