feat(document)!: take an op envelope, not a map of modified text - #859
Merged
Conversation
`Document::edit` parsed one key, `modifiedText`, a map of `DocumentPath` to
string. A sheet cell cannot be named that way - the cell a user types into may
have no element at all - so the wire format becomes the envelope
`docs/design/spreadsheet-editing.md` decision 2 spells:
{"version": 1, "ops": [{"op": "setCell", "sheet": 0, "column": 1,
"row": 2, "value": {"type": "number", "number": 12.5, "text": "12.5"}}]}
`setCell` names a position and carries a `CellValue` typed `number`, `string`
or `empty`. `setText` carries what `modifiedText` carried, addressed by path;
it gains the id form with `editing.md` phase 1.
`edit` throws on the first op it cannot apply and leaves the ones before it
applied, which is what the host's replay onto a fresh decode expects.
`odr.generateDiff()` emits the envelope, so the browser editor keeps working
and only the resource file changes - no rendered page moves.
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.4 of
docs/design/spreadsheet-editing.md— the second-to-last item of stage 0.The wire format
Document::editunderstood one key,modifiedText: a map ofDocumentPath→ string. A sheet cell cannot be named that way — decision 1's whole point is that the cell a user types into may have no element at all. So it becomes the envelope decision 2 spells:{"version": 1, "ops": [ {"op": "setCell", "sheet": 0, "column": 1, "row": 2, "value": {"type": "number", "number": 12.5, "text": "12.5"}}, {"op": "setCell", "sheet": 0, "column": 1, "row": 4, "value": {"type": "empty"}} ]}setCellnames a position and carries aCellValuetypednumber,stringorempty.numbertakes an optionaltext— without it the number spells itself, which is whatCellValue(double)already does.setTextcarries whatmodifiedTextcarried, addressed bypath. The plan has it becoming{id, text}withediting.mdphase 1; keeping it path-addressed now means text editing does not break in the meantime.versionmust be 1; an unknown op, version, or sheet ordinal throws.editthrows on the first op it cannot apply and leaves the ones before it applied — a host replays onto a fresh decode by construction (DocumentFile::document()decodes anew), so that is the contract, and it is now stated in the header rather than implied.No rendered page moves
odr.generateDiff()emits the envelope instead of the map, so the browser editor keeps working. I diffed a full run against the reference tree:output/— byte-identical, both repos.resources/document.js— the three lines ofgenerateDiff. Not diffed by CI's compare, and the function is never called while a page renders, so it cannot move a pixel.(
resources/pdf-annotation.{css,js}already differ from the reference on main — pre-existing drift, not this branch.) The staledocument.jsis worth folding into the next PR's regen, which needs one anyway.Also updated
The plan had gone stale in four places, now corrected: repeated ODS cells are no longer in step 0.2's refusal list, step 2.1 is marked landed for repeats with the open half (a position the file states no element for) called out, the "what the code gives us" table points at
split_repeat/reindex_sheetand the envelope, and 0.4 notes that coalescing here is what would let a batch of ODS writes reindex once rather than per write — measured at ~0.18 µs per row node per write, so 100 writes on a 20000-row sheet is 0.36 s today.Bindings pass the string through unchanged; their tests, the wasm test, and the three core
_difftests now speak the envelope.Checks
document_edit_test.cpp).-Wall -Wextra -Werrorover four TUs.