Skip to content

feat(document): read the number and the formula a sheet cell holds - #853

Open
andiwand wants to merge 1 commit into
mainfrom
feat/sheet-cell-value
Open

feat(document): read the number and the formula a sheet cell holds#853
andiwand wants to merge 1 commit into
mainfrom
feat/sheet-cell-value

Conversation

@andiwand

@andiwand andiwand commented Sep 6, 2026

Copy link
Copy Markdown
Member

🤖 Generated with Claude Code

Step 0.1 of spreadsheet editing, plus the plan it is the first step of.

The plan

docs/design/spreadsheet-editing.md stages the work from "type a number into a
cell" to a formula engine, and records why each decision went the way it did.
The load-bearing ones:

  • A cell is addressed by position, not by id. The cell a user types into
    often has no element — every empty cell in both formats, every repeated cell
    in ODS — so an id cannot name it. Ids stay right for text documents, where an
    insertion point inside a paragraph has no position of its own.
  • The type follows the content. Telling a user "this cell holds a number, so
    you may not type text" is a rule no spreadsheet has and neither file format
    has either. A strict grammar decides number vs. string, a leading ' forces
    string, and both the value and its string are written.
  • Editing is a browser mode, not markup. No re-translate to switch modes,
    and no contenteditable on a cell — a td holds wrappers and shapes, so the
    editor is an overlay. The page carries only lock classes on the cells that
    cannot be edited.
  • Refusal is an event. Clicking a locked cell calls odr.onEditRefused, so
    a mobile host can put it in a snackbar. The host owns the wording — nothing
    here is localised — and the page de-dupes its own repeats so four taps are one
    snackbar. The same channel carries the dirty flag a save button needs.
  • Formulas are recomputed in C++, once, through the host. Generating JS from
    the expression tree means writing the function library twice. Shipping the
    engine as wasm inside the page stays possible — it is the same C++ — but costs
    an emscripten build inside every platform build, and no current host needs it.

What this PR implements

struct CellValue final {
  ValueType type{ValueType::unknown};
  std::optional<double> number;
  std::optional<std::string> formula;
};

CellValue SheetCell::value() const;

The text is not repeated in it — it stays in the cell's children, where it
is already read from. value_type() is unchanged and stays what
translate_sheet asks of every cell, which is the cheaper question; this PR
adds nothing to the render path.

engine number formula
odf office:value table:formula
ooxml <v>, where c/@t types one <f>
csv the field it already types numeric
xls, numbers

xls and numbers read every cell into its display string at parse time, so
neither the number nor an expression survives to be handed out; they answer with
the type alone.

Two details worth a look:

  • A shared ooxml formula writes its expression on the group's master only, so
    a member's formula is set and empty rather than absent — unset would
    claim the cell computes nothing. odr-public/xlsx/sample.xlsx has both kinds,
    and reading it confirms the distinction is real, not theoretical.
  • number is wider than type == float_number. A percentage or a currency
    states an office:value and is still typed a string, because the type is read
    from office:value-type alone. Fixing that changes cell alignment and the
    reference output, so it belongs with number formats, later.

util::number::parse

The locale-independent decimal parse this needed. std::strtod reads
LC_NUMERIC and truncates 1234.5 to 1234 under a german host locale;
std::from_chars for double is not in libc++ 18, the oldest standard library
in the profile matrix. So: istringstream imbued with the classic locale, strict
about trailing input. Three existing strtod sites could move onto it later.

Checks

  • 1543 tests; the 10 new ones cover number/string/formula/shared-formula/boolean
    per engine, built from inline XML — no new fixtures.
  • The xlsx builder that ooxml_spreadsheet_merge_test.cpp had is now
    ooxml_spreadsheet_test_util.hpp, used by both, since the write-side steps
    will want it too.
  • gcc-15 -Wall -Wextra -Werror over every touched TU, validated with a negative
    control. It caught two {.type = …} designated initialisers that AppleClang is
    happy with and gcc is not — they are plain assignments now.
  • NDK 28.2 clang against its libc++ for the two headers-facing TUs.
  • clang-tidy clean; the five findings it reports are the known pre-existing
    derived-method-shadowing-base-method ones on untouched public headers.

Not in this PR

The rest of step 0: the write hook, xlsx save, the op envelope replacing
modifiedText, and the capability rows. The contenteditable gate is deliberately
separate — it changes the reference output — and the plan now says so, having
originally told itself to gate on is_editable, which the same step makes true.

`SheetCell::value()` answers what a cell holds past the text it shows: the
number the file states, and the formula behind a cached result. The text is
not repeated - it stays in the cell's children, where it is read from.

odf reads `office:value` and `table:formula`, ooxml `<v>` and `<f>`, csv the
field it already types. `xls` and `numbers` keep only display strings at parse
time, so they answer with the type alone. `value_type` is unchanged and stays
the question the renderer asks of every cell, which is the cheaper one.

A shared ooxml formula writes its expression on the group's master, so a
member's formula is set and empty rather than absent; `sample.xlsx` has both
kinds.

`util::number::parse` is the locale-independent decimal parse this wanted:
`std::strtod` reads `LC_NUMERIC` and would truncate `1234.5` to `1234` on a
german host, and `std::from_chars` for double is not in libc++ 18.

This is the first step of `docs/design/spreadsheet-editing.md`, the staged plan
for spreadsheet editing that lands with it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F17g1P9PbwFVspTzMSBqiQ
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant