From 59f7652211def548af5f86b1edac9a472a235001 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 9 Aug 2026 13:33:52 +0200 Subject: [PATCH 1/2] docs: plan the csv, json, markdown, rtf, xml and iwork modules One PLAN.md per module, each written before its stage 1: what the repo does today, what the target shape is, the decisions taken up front and why, the stages in order, and what is deferred by decision. csv and json describe modules that already exist; markdown, rtf, xml and iwork describe ones that do not. Keep them honest as stages land. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011ipdjdyMJ6LMVsfAnShvbm --- src/odr/internal/csv/PLAN.md | 284 ++++++++++++++++++++++++++++ src/odr/internal/iwork/PLAN.md | 296 +++++++++++++++++++++++++++++ src/odr/internal/json/PLAN.md | 185 +++++++++++++++++++ src/odr/internal/markdown/PLAN.md | 243 ++++++++++++++++++++++++ src/odr/internal/rtf/PLAN.md | 271 +++++++++++++++++++++++++++ src/odr/internal/xml/PLAN.md | 298 ++++++++++++++++++++++++++++++ 6 files changed, 1577 insertions(+) create mode 100644 src/odr/internal/csv/PLAN.md create mode 100644 src/odr/internal/iwork/PLAN.md create mode 100644 src/odr/internal/json/PLAN.md create mode 100644 src/odr/internal/markdown/PLAN.md create mode 100644 src/odr/internal/rtf/PLAN.md create mode 100644 src/odr/internal/xml/PLAN.md diff --git a/src/odr/internal/csv/PLAN.md b/src/odr/internal/csv/PLAN.md new file mode 100644 index 000000000..b84aeb1d4 --- /dev/null +++ b/src/odr/internal/csv/PLAN.md @@ -0,0 +1,284 @@ +# CSV plan + +Where the csv module is going, and in what order. Written before stage 1; keep +it honest as stages land. + +## Today + +`CsvFile` is detection only — 60 lines, `is_decodable() == false`, rendered by +`html::create_text_service` as a line list. `check_csv_file` reads the **whole** +file, hard-codes `,` and `"`, and demands a globally uniform field count > 1. +`text::TextFile`'s constructor runs `guess_charset` over the **whole** stream, +and `open_strategy` does both speculatively while listing file types. + +The detected charset is currently used nowhere: `html/text_file.cpp` pipes raw +bytes into a document declaring ``, so any non-UTF-8 text +file already renders as mojibake (`// TODO charset`, `html/text_file.cpp:71`). + +## Target + +A csv opens as a spreadsheet document — one sheet, cells reachable through +`SheetAdapter` — so the generic HTML renderer and every binding get table +rendering without format-specific code. Separator, charset, quoting and header +are autodetected from a bounded probe, overridable by the caller, and the +resolved values are readable back. + +## Decisions taken up front + +**Cell strings are UTF-8.** `Text::content()` returns `std::string` and every +binding treats it as UTF-8 (JNI `NewStringUTF`, embind, pybind11, `NSString`). +Passing legacy bytes through and letting a browser sort it out only works for +the html path, not the api path, so it is not an option. This is what makes an +encoding we cannot decode fatal to the *sheet* path specifically, while the +text path stays open to it. + +**No element per cell.** `ElementIdentifier` is a `std::uint64_t` +(`definitions.hpp:7`): encode `kind | row | column` into it and decode in the +adapter. A flat `ElementRegistry` over cells does not survive a large file, and +the virtual-id seam is what keeps every later scaling decision an +implementation detail. This is a deliberate deviation from the pattern in the +root `AGENTS.md`; it belongs in a `csv/AGENTS.md` once stage 2 lands. + +**Detection rejects; the parser does not.** These are two jobs and +`check_csv_file` currently conflates them — `open_strategy.cpp:281` even +constructs a `CsvFile` *as* the probe, which is why detection pays for a full +scan. Split into a probe returning resolved options plus a verdict, and a parser +taking resolved options and judging nothing. + +Detection heuristics (sniffing an unknown text file): score candidate separators +over the probe, require ≥ 2 columns, reject an unterminated quote at EOF. Both +rules are discriminators — one column is every line of prose ever written, and a +dangling quote is good evidence of not-csv — not statements about what is valid. + +The parser, given a separator, is **total**: any bytes plus a transcodable +charset yield some sheet. A one-column csv is a legitimate csv; a truncated +quoted field closes at EOF; an empty file is an empty sheet; ragged rows pad +short and widen long. Nothing here is unrepresentable, so nothing throws. + +What is left to fail is an incoherent options struct (separator equal to the +quote char, a separator of `"` or `\n`) — an argument error — and a charset we +cannot transcode. `NoCsvFile` therefore becomes purely a detection exception, +which is what its name always claimed. + +**Quoting is not type information.** RFC 4180 quoting is lexical — Excel quotes +any field holding a delimiter, quote or newline, and many writers quote +everything. Infer nothing from it. + +**Encoding is a public enum with a table behind it.** Encodings carry aliases +the way mime types do (`UTF-8`/`utf8`, `windows-1252`/`cp1252`, +`ISO-8859-1`/`latin1`) and uchardet hands back a name that has to map home, so +unlike `FileType` this needs both directions. Mirror `file_type_table.cpp`: +canonical name first, aliases after, one row per encoding, and a test that +fails when an alias is claimed twice. + +```cpp +enum class TextEncoding { unknown, utf8, utf16le, utf16be, utf32le, utf32be, + windows_1252, iso_8859_1, iso_8859_15, shift_jis, … }; + +std::vector all_text_encodings(); +std::string text_encoding_to_string(TextEncoding); +TextEncoding text_encoding_by_name(std::string_view) noexcept; // unknown if none +std::span text_encoding_names(TextEncoding) noexcept; +bool text_encoding_is_decodable(TextEncoding) noexcept; +``` + +Named `TextEncoding`/`encoding()` rather than `Charset`/`charset()` so it does +not collide with the accessor it replaces. `TextFile::charset()` stays, +implemented as `text_encoding_to_string(encoding())` and marked both +`/// @deprecated` (the existing convention, `html.hpp:49`) and `[[deprecated]]`. +There is no `-Werror` (`CMakeLists.txt:38`), so the attribute is warnings only; +the three binding call sites (`jni_file.cpp:288`, `bind_file.cpp:218`, +`ODRFile.mm:516`) move to `text_encoding_to_string(encoding())` and keep their +own public surface unchanged. + +**Reuse from `pdf` means extraction first.** Nothing reaches across into +`internal/pdf`. Anything shared moves into `internal/encoding` with pdf as one +of its users. Stage 1 as designed needs no runtime reuse at all, so this binds +only on the multibyte tier below. + +**Encoding tiers**, expressed as that last predicate. + +- **UTF-8/16/32 (+BOM)** — `utfcpp`, already a dep. +- **Single-byte legacy** (windows-1252, ISO-8859-1/-15) — a generated 256-entry + byte→codepoint table per encoding. Generate from Python's built-in codecs + (`bytes([i]).decode('cp1252')`), which are the standard mappings; only the + *shape* comes from `tools/pdf/generate_encoding_data.py` → `pdf_encoding_data.cpp` + (auto-generated header, `clang-format off`, licence note). Do **not** reuse + WinAnsiEncoding as cp1252: they are close, not equal, and ISO-8859-x is not + in the pdf tables at all — the header on `pdf_doc_encoding_to_unicode` + documents exactly this class of near-miss. +- **Multibyte legacy** (Shift-JIS/cp932, GBK, Big5, EUC-KR) — *deferred, not + declined*, with two viable routes and no need to pick until someone wants CJK + csv. Measured mapping counts: cp932 22 736, gbk 21 791, shift_jis 18 912, + big5 13 710, euc_kr 8 225. + - *Generate our own*, again from Python's codecs: exact, no refactor, but + ~85 k pairs ≈ 340 KB of static data as sorted `(uint16, char16)`, less with + the run encoding `pdf_cid_data` uses. + - *Hoist and share*: `pdf_cid.hpp:19` documents `translate_predefined_cmap` + as decoding the legacy CJK CMaps, "whose codes are the legacy encoding's + bytes", to UTF-8, and `pdf_cid_data.cpp` carries 17 RKSJ CMaps — + `90ms-RKSJ-H` being the Microsoft variant, i.e. cp932. That data is already + compiled into every binary, so sharing it costs no size. The price is the + extraction into `internal/encoding` and the lossiness of routing through a + character collection's UCS2 map. + +Until then a non-decodable encoding is *known but not decoded*: reported by +name, rendered as text with `text_encoding_to_string(...)` in the html header so +the browser decodes it. Only the spreadsheet path, which must hand UTF-8 to the +bindings, is closed to it. + +--- + +## Stage 1 — encoding, in `internal/encoding` + +No csv in this stage at all. It is the substrate everything else stands on, it +ships alone, and it closes a live bug: the detected charset is used nowhere +today, so every non-UTF-8 text file renders as mojibake. A package of its own +rather than a corner of `internal/text`, because it is what `pdf` would later +share. + +- public `TextEncoding` enum + `text_encoding_table.cpp` (aliases, both + directions, decodable predicate) as above; `TextFile::encoding()` added, + `charset()` deprecated, bindings moved off it. +- `to_utf8(std::string_view, TextEncoding)`. BOM sniffing is deterministic and + runs before uchardet. Single-byte tables generated from Python's codecs, in + the `tools/pdf` output shape. Invalid bytes become U+FFFD, never a throw + mid-render — the override is the answer to a bad guess. +- `guess_charset(std::istream &, std::size_t max_bytes)` — honour the existing + TODO in `text_util.hpp`. 64 KiB is enough for uchardet. Accept the + consequence: a file that is ASCII for 64 KiB with a `é` at 2 MB detects as + UTF-8 and meets an invalid byte later. That is what U+FFFD and the override + are for. +- a binary test (NUL byte / share of C0 controls) as the actual "is this text" + oracle. uchardet names a charset, it does not gate. +- `html/text_file.cpp` transcodes, or declares the charset for a non- + transcodable one, and drops its `// TODO charset`. + +## Stage 2 — the csv probe + +Splits detection from parsing and stops detection paying for a full scan. + +- `probe` reads one bounded prefix, shared by charset detection and separator + scoring, and returns resolved options plus a verdict. Drop the trailing + partial record from the sample. +- separator scoring over `{',', ';', '\t', '|'}`: quote-aware split, modal field + count per candidate, pick by share-of-lines-matching then field count; a + winner yielding one column loses. Honour Excel's `sep=;` first line. +- `open_strategy` keeps constructing a `CsvFile` to probe, because that + constructor *is* the probe now and is bounded. Detection reads its own 64 KiB + rather than sharing `text::TextFile`'s: caching the probe would make every + text file carry 64 KiB for the life of the object, and re-reading it costs + nothing. +- the existing tests in `csv_file_test.cpp` move to the probe: ragged rows and + dangling quotes stay rejections *there*, and stop being rejections in the + parser. The comment on `a_quoted_field_must_be_terminated` records a real + misclassification — the rule survives, on the detection side only. + +## Stage 3 — options, shaped like decryption + +`nullopt` means autodetect; the resolved values are readable back so a caller +can show "detected `;`, UTF-16LE" and offer an override. The realistic flow is +open → wrong → adjust → reopen. + +```cpp +struct CsvOptions { + std::optional charset; + std::optional separator; + std::optional quote; + std::optional header_row; + std::size_t probe_bytes{1 << 16}; +}; +``` + +Follow `decrypt`: an immutable handle deriving another handle, narrowed on the +concrete type (`file.cpp:332`, `file.cpp:345`). `TextFile::charset()` is already +half of this pattern — stage 3 finishes it. + +```cpp +CsvOptions options() const; +CsvFile with_options(const CsvOptions &) const; +``` + +This keeps `odr::open` untouched — no new overload across six `open` signatures +and six `DecodedFile` constructors — and the bindings already have the call +pattern from decrypt (`wasm/src/wasm_file.cpp:86`, `jni/src/jni_file.cpp:162`). +Do **not** copy `decrypt`'s state machine: `EncryptionState` gates rendering +because an encrypted file cannot be rendered at all (`file.cpp:167`), whereas a +csv without options always has a guess. An `options_required()` state would +invent a gate the format does not have. Nor does the failure map: a wrong +password leaves nothing to hand back, whereas the parser is total, so +`with_options` fails only on an incoherent struct or an untranscodable charset. + +Options are per-format, so they need a concrete handle, but stage 2 routes csv +through `as_document_file()` which has nowhere to put them. Add a public +`CsvFile` handle alongside `TextFile`/`PdfFile` — `as_csv_file()`, `options()`, +`with_options()`, `document()`. `PdfFile` is the precedent for a format-specific +handle narrowing an inherited operation. The same shape gives `TextFile` a +`with_charset(std::string)` next to its existing `charset()`. + +`header_row` is not in the struct: it would be inert until a sheet exists to +mark a header on. It arrives with stage 4, or not at all. + +**Bindings come after stage 4, not here.** `CsvFile` is the handle stage 4 hangs +`document()` off, and stage 4 also decides the file category — binding it now +means binding it twice. One binding pass, once the shape has settled. + +## Stage 4 — the sheet document + +The visible win. Whole file in memory; the renderer caps at +`spreadsheet_limit{10000, 500}` (`html.hpp:124`) so that is sufficient for +output at any file size worth caring about today. + +- `CsvDocument` : `internal::Document`, `DocumentType::spreadsheet`, one sheet. +- virtual element ids as decided above; `cell`/`dimensions` are the only way in + to the data, so stage 6 can change what is behind them. +- the sheet is rectangular even where the file is not — a short row pads, a + long one widens. The counterpart to detection no longer rejecting ragged + files. +- csv **stays** `FileCategory::text` (`file_type_table.cpp:416`), carrying + `DocumentType::spreadsheet`. Moving it to `document` was the plan and is not + what landed: a csv is still text, `TextFile::text()` still reads it, and + `document()` is the second view rather than a replacement. So `is_text_file()` + keeps answering true and `html::translate` orders its csv branch first. The + capabilities test enforces whatever the row claims. +- `html_output_test` skips csv (`// TODO enable zip, csv, json`). Csv now + produces real output, so enabling it needs reference output committed to the + output repo and the pointer advanced — a separate repo, a separate change. + +## Stage 5 — value types + +`ValueType` is only `{unknown, string, float_number}` +(`document_element.hpp:135`) and drives one CSS class — right alignment — so it +stays small. Decided per *column*: a reader compares down a column, so one +number in a column of prose is not a quantity. The first row is left out of the +sample, because a header names its column rather than holding a value. + +The grammar is strict on purpose. A leading zero is what tells `007` from a +quantity, and a thousands separator does not say which side of the Atlantic +wrote it — `1,234` is a thousand or one point two three four depending. Dates +are left alone entirely: `03/04/2026` is two different days and guessing gets +it wrong confidently. The raw string is always what is displayed. + +## Stage 6 — large files + +Deferred: with the 10 000-row render cap this buys nothing for html output. It +matters for open cost and for api consumers walking the sheet. Build it when +something needs it; `CsvDocument::cell`/`dimensions` are the seam. + +- Row-start offsets are the only index needed — a row boundary is the one place + where "outside quotes" is unambiguous. +- Checkpoint every ~1024 rows (offset + row index), binary search then scan + forward. 10 M rows at 8 bytes each is 80 MB and unshippable on a phone; + checkpoints are ~80 KB per GB. +- `sheet_dimensions()` needs an exact count, i.e. one full quote-aware scan. + Make it lazy — first dimension query, not open — and build the checkpoints in + that pass. +- Window staging: unescape **into** the staged buffer. `""` means a cell's value + is not a contiguous range of the original bytes, but unescaping only shrinks, + so each field compacts within its own slot and a `string_view` into the + rewritten window is valid. The window also holds the transcoded UTF-8, so one + buffer solves both. LRU 2–4 windows; the renderer's row-major walk hits them + sequentially. +- `abstract::File` offers `memory_data()` (whole file as a `string_view`, no + staging needed) and `disk_path()` (mmap-able). A csv inside a zip has neither + — that path stays full in-memory, by decision not by accident. diff --git a/src/odr/internal/iwork/PLAN.md b/src/odr/internal/iwork/PLAN.md new file mode 100644 index 000000000..f57831fc3 --- /dev/null +++ b/src/odr/internal/iwork/PLAN.md @@ -0,0 +1,296 @@ +# iWork plan + +Where an iwork module would go, and in what order. Written before stage 1; keep +it honest as stages land. + +## Today + +Nothing decodes, and unlike rtf there is not even a `FileType` yet. A `.pages` +is a zip, so `magic.cpp:95` reports `FileType::zip`, `list_file_types` probes +odf then ooxml (`open_strategy.cpp:213-238`), both fail, and the caller gets +`[zip]`. `odr::open` hands back a `zip::ZipFile` — an archive, not a document. + +Two fixtures are already committed: +`test/data/input/odr-public/pages/{empty.pages,style-various-1.pages}`, both +written by iWork 13.2 (`Metadata/BuildVersionHistory.plist`). Neither is listed +in `index.csv` and neither has reference output, so nothing exercises them. +`style-various-1.pages` carries `Index/Tables/` and nine files under `Data/`, +which is most of the surface below. + +New `FileType` entries append at the end of the enum — `file.hpp:98` says so — +and **the bindings do need updating** here, unlike rtf: `python/src/bind_file.cpp`, +`jni/java/app/opendocument/core/FileType.java`, +`apple/include/OdrCoreObjC/ODRFile.h` + `apple/src/ODRFile.mm`. Wasm does not: +it derives its enums from `odr::all_file_types()` at runtime +(`wasm/src/wasm_core.cpp:35`, `:69`). + +## Spec + +There is none, and none is coming. Nothing is vendored under +`offline/documentation/`, and Apple has never published the `.proto` schemas. + +The evidence available is, in order of trust: + +1. **The fixtures.** Byte layout verified against a file in the repo is the only + claim this plan treats as fact; everything else below is marked as needing + confirmation. +2. **MIT-licensed reverse engineering** — `numbers-parser`, `keynote-parser`, + `obriensp/iWorkFileFormat`. +3. **`libetonyek`** (Document Liberation Project, MPL-2.0). + +Read all of them for facts; **copy code from none of them**. Where `oldms/` +cites `[MS-XLS] §2.4.1`, this module cites a fixture and an offset instead — +`empty.pages Index/Document.iwa +0` — because that is the strongest reference +that exists. + +## Target + +`.pages` opens as a text document, `.numbers` as a spreadsheet, `.key` as a +presentation, all through the abstract document model, so the generic HTML +renderer and every binding get them without format-specific code. +`is_editable()` false, `save` throws, exactly as `oldms/text`. + +**iWork '13 and later only.** The 2005–2009 XML era is a different format and is +deferred by decision — see below. + +## Decisions taken up front + +**The module is `iwork`, not `apple`.** `apple/` at the repo root is already the +Objective-C bindings and the Swift package; a second meaning would be a trap. +Namespace `odr::internal::iwork`. + +**Three file types, one engine.** `iwork_pages`, `iwork_numbers`, +`iwork_keynote` appended to `FileType`, each with its own +`file_type_table.cpp` row and `DocumentType`. One `IworkFile` switches on the +type, exactly as `odf::OpenDocumentFile` (`odf_file.hpp:21`) does for the four +opendocument types — same constructor over an +`abstract::ReadableFilesystem`, same `document()` dispatch. + +**No new dependencies.** Two pieces would normally be a conan line each, and +both are wrong here: + +- **Snappy.** The `.iwa` framing is Apple's own: a 4-byte header per block, + `0x00` then a little-endian 24-bit compressed length, repeated to EOF. Stock + Snappy *framing* (the `sNaPpY` stream identifier, per-chunk CRC-32C) is not + present, so a stream decoder does not apply — only the **block** decoder does, + and that is a varint length plus literal/copy tags in about 150 lines. + Verified: `empty.pages Index/Document.iwa` opens `00 fb 1d 00`, i.e. 7675 + bytes, and the file is 7679; the block then starts `82 6e`, a varint + uncompressed length of 14082. +- **Protobuf.** Only the wire format is needed — varint, 64-bit, + length-delimited, 32-bit, with groups skipped. There are no schemas to + generate from, so a code generator would have nothing to do, and linking + conan `protobuf` would drag one into the wasm, android and apple builds to + replace ~150 lines. Read fields by number against hand-written accessors. + +Both live inside `iwork/` for now. The csv plan's rule applies — anything +genuinely shared moves out to its own package *first* — and a wire reader with +one user has not earned a package. + +**An `.iwa` is an object graph, not a tree.** Each file is a sequence of +`(varint length, TSP.ArchiveInfo, payload bytes)`. `ArchiveInfo` is field 1 = +identifier, field 2 = repeated `MessageInfo{1: type, 2: version, 3: length, +4: field_infos}`. Verified on the same fixture: `08 01` (identifier 1), +`12 52` (an 82-byte `MessageInfo`), `08 90 4e` (type 10000), `12 03 01 00 05` +(version 1.0.5), `18 e0 0c` (payload length 1632). + +Objects reference each other by identifier, so the engine **indexes first and +resolves lazily**: build `identifier → (type, component, span)` across the +package, then walk the graph from the document archive. Reading files in +directory order and hoping the tree falls out is the mistake to avoid. + +**Components come from `Index/Metadata.iwa`, not from file names.** It carries +the package's component list (identifier → locator). The fixture's names carry +id suffixes — `CalculationEngine-1732585.iwa`, +`AnnotationAuthorStorage-1732584.iwa`, `Index/Tables/DataList-1732820-2.iwa` — +so globbing for `CalculationEngine.iwa` finds nothing in one fixture and finds +it in the other. (`style-various-1.pages` has the unsuffixed spelling.) + +**A pinned type table, and fail soft on what is not in it.** A `constexpr` +table maps message type IDs to the archive kinds we understand, annotated with +the app versions it was confirmed against, keyed off +`Metadata/BuildVersionHistory.plist`. An unknown type ID is **skipped**. + +This is not a hole in the root `AGENTS.md`'s fail-fast rule, for the same +reason the rtf plan's leniency is not: that rule says throw *where the spec +dictates what to expect*. Here there is no spec, and an unknown type ID means +Apple shipped a version we have not mapped — not that the input is corrupt. A +reader that throws on one cannot open next year's files. + +Do throw on: framing that overruns the file, a Snappy block that decompresses +past its declared length, a varint that does not terminate, a reference to an +identifier the index does not hold, and nesting past a depth bound. + +**`Data/` needs no decoder.** Media is stored as ordinary zip entries; open it +through the filesystem, hand it to `ImageAdapter::image_file` with +`image_is_internal() == true`. + +**Flat `ElementRegistry`, copied not shared.** An iWork document's element count +is bounded by its paragraph and drawable count, so the pattern in the root +`AGENTS.md` applies unchanged and `oldms/text/doc_element_registry.*` is the +template — not csv, whose id-is-the-coordinate scheme exists because a sheet has +an element per cell. + +**Pages has two document modes.** Word processing is a text flow; page layout is +a canvas of floating text boxes, structurally closer to `odg` than `odt`. One +`FileType` and `DocumentType::text` either way — word processing first, page +layout rendered as pages of frames once frames exist (stage 4). + +**A Numbers sheet holds many tables.** Our `Sheet` (`document_element.hpp:300`) +is one grid plus `shapes()`. Map **one odr sheet per Numbers table**, named +` – `; taking only the first table per sheet would drop data +silently. + +**No writer, no editing.** Out of scope in every stage. + +--- + +## Stage 1 — detection and the container + +Nothing renders yet. The point is that the bytes come apart correctly and the +type is reported, which is also the whole of what a file picker needs. + +- `iwork_snappy.{hpp,cpp}` — Apple framing plus block decompression, over the + `std::istream *` / `std::streambuf *` shape `pdf::ObjectParser` uses + (`pdf_object_parser.hpp`). +- `iwork_protobuf.{hpp,cpp}` — wire reader: varints, the four wire types, skip + for unknown fields, a `NestingGuard` for length-delimited recursion. +- `iwork_archive.{hpp,cpp}` — `.iwa` → objects; the package index built from + `Index/Metadata.iwa`; `identifier → object` lookup. +- `iwork_file.{hpp,cpp}` — `IworkFile : abstract::DocumentFile` over an + `abstract::ReadableFilesystem`, shaped like `odf::OpenDocumentFile`. + `password_encrypted()` reports true when `Index/Metadata.iwph` is present and + `decrypt` throws (see Deferred). +- detection: a third probe after odf and ooxml in `list_file_types` + (`open_strategy.cpp:213-238`), a branch in `open_file_as` beside odf's + (`:48-66`), and in `open_file` (`:328`) and `open_document_file` (`:498`). + `Index/Document.iwa` present ⇒ iWork; **which app comes from the root + archive's type ID**, read off a fixture per app and put in the pinned table — + not guessed, and not taken from the extension, which a caller may have lost. +- three rows in `file_type_table.cpp`: extensions `pages` / `numbers` / `key`, + mime types `application/vnd.apple.{pages,numbers,keynote}` plus the + `application/x-iwork-*-sff*` spellings, `FileCategory::document`, the matching + `DocumentType`, `{.detect_by_content = true, .open = true}`. `odr_test` fails + if declared capabilities exceed what the engine does, so `translate_html` + arrives per format in the stages below. +- `FileType` entries at the end of the enum; the three bindings above. +- `CMakeLists.txt` `ODR_SOURCE_FILES`. + +**Tests** are inline byte strings: a hand-built Snappy block (literal, copy with +1-byte and 2-byte offsets, a length that overruns), varint edge cases +(continuation past 10 bytes, a 64-bit field), an `ArchiveInfo` with an unknown +type ID that must be skipped rather than thrown on, and framing truncated +mid-block. Only the type-reporting test needs the fixtures — the data repos are +fetched and optional, so everything that can be inline is. + +## Stage 2 — Pages text + +- walk from the document archive to the body's text storage + (`TSWP.StorageArchive` in the reverse-engineering literature; confirm the type + ID against the fixture before pinning it). +- the storage holds the text as a small number of large strings plus **parallel + run tables** — index/value pairs for paragraph styles, character styles and + attachments. **Paragraph boundaries come from the paragraph run table**, not + from splitting on `\n`; `U+2028` is a line break inside a paragraph and maps + to `line_break`. The text is already UTF-8, so `internal/encoding` is not + involved. +- `root → paragraph → span → text` in a flat `ElementRegistry`. +- `empty.pages` is the regression that matters here: an empty document must + produce an empty body and not an exception. +- table row: `iwork_pages` gains `.translate_html = true`. + +## Stage 3 — Pages styles + +- `Index/DocumentStylesheet.iwa`. Style archives are sparse property sets with a + parent reference, so resolution is an inheritance walk terminating at the + theme's default — cache resolved styles by identifier, and let equal property + sets share one resolved style the way `.doc` does. +- character and paragraph properties → `TextStyle` / `ParagraphStyle`; adjacent + equal-style runs merge into one span. +- font names interned in the style registry and never mutated afterwards, so + `TextStyle::font_name` (`const char *`) stays valid — the `.doc`/`.xls` rule. +- page geometry → `TextRootAdapter::text_root_page_layout`, which stage 2 + returns empty from. + +## Stage 4 — drawables, images, frames + +- drawable archives carry a geometry (position, size, transform) and a content + reference → `Frame` plus `Image`, `Rect`, `Line`, `CustomShape` as they map. +- images resolve to a `Data/` entry by name; hand the zip entry through + unchanged. +- Pages **page-layout mode** falls out here: it is drawables on pages with no + body flow, so once frames exist it is a different root assembly, not new + parsing. + +## Stage 5 — Keynote + +Cheaper than it looks, and therefore before Numbers: slides are a container +above the *same* text storage stage 2 and 3 already read. + +- slide archives → `Slide`, the slide's master → `MasterPage`, text boxes and + drawables reuse stages 2–4 unchanged. +- notes, builds and transitions are out (see Deferred). +- `iwork_keynote` gains `.translate_html = true`; needs a `.key` fixture in the + public data repo first. + +## Stage 6 — the tile reader + +Tables in iWork are stored as **tiles** — row ranges holding packed cell +records — with strings, formats and formulas kept in side "data lists" that +cells reference by index. Several cell-storage layouts exist across app +versions; this is where the pinned table earns itself, and where a version we +have not mapped must degrade to an empty cell rather than a wrong one. + +Do this for **Pages tables first** (`Table`, `TableRow`, `TableCell`), because +`style-various-1.pages` already carries `Index/Tables/` and exercises the reader +without a Numbers fixture existing. + +## Stage 7 — Numbers + +- sheets → one odr `Sheet` per table (see decisions), on top of stage 6. +- cached values only. `CalculationEngine.iwa` holds the formula graph and is not + read; a cell shows what Numbers last computed, which is the position `.xls` + takes. +- `iwork_numbers` gains `.translate_html = true`. + +--- + +## Deferred, by decision + +- **iWork '05–'09** (Pages 1–4, Keynote 1–5, Numbers 1–2) — gzipped XML, + `index.xml.gz` for Pages/Numbers and `index.apxl(.gz)` for Keynote. A + different format needing a different engine and its own stage 1, for files no + application has written since 2013. If it ever happens it lives at + `iwork/legacy/` behind the same three `FileType`s, which is why those are + named for the app and not the era. +- **Password-protected files** — `Index/Metadata.iwph` and encrypted `.iwa`s. + `password_encrypted()` reports it from stage 1; `decrypt` throws. +- **The package form.** On macOS a `.pages` can be a directory rather than a + zip. The engine would not notice — `common::SystemFilesystem` + (`common/filesystem.hpp:16`) roots at a directory and satisfies the same + interface — but nothing in the public API opens a directory today, so this is + a seam, not a stage. +- **Templates and iBooks Author** (`.template`, `.kth`, `.nmbtemplate`, `.iba`) + — same container; add the extensions to the existing rows once documents + work. +- **Charts, comments, footnotes, change tracking, formulas, `ViewState.iwa`, + Keynote builds and presenter notes.** +- **Writing and editing.** +- **A preview fallback.** Every iWork file embeds a rendered preview + (`preview.jpg`, `preview-web.jpg`, `preview-micro.jpg` in the fixtures; the + '09 era wrote `QuickLook/Preview.pdf`). Deliberately not an iWork feature: + odf and ooxml carry thumbnails too, so rendering a preview in place of a + decode is one cross-cutting mechanism with one honest capability story, to be + decided library-wide — not invented here, where it would make + `translate_html = true` claim a fidelity this engine does not have. + +## Test data + +`empty.pages` and `style-various-1.pages` are already in +`test/data/input/odr-public/pages/` but absent from `index.csv` and from +reference output. Add them to the index in stage 1, and regenerate reference +output when stage 2 flips `translate_html` on. + +Stages 5 and 7 each need a fixture that does not exist yet — one `.key` and one +`.numbers` in the public repo. Everything at container level stays inline, per +stage 1. diff --git a/src/odr/internal/json/PLAN.md b/src/odr/internal/json/PLAN.md new file mode 100644 index 000000000..14e20b250 --- /dev/null +++ b/src/odr/internal/json/PLAN.md @@ -0,0 +1,185 @@ +# JSON plan + +Where the json module is going, and in what order. Written before stage 1; keep +it honest as stages land. Sibling of [`csv/PLAN.md`](../csv/PLAN.md), and it +leans on what stage 1 there already landed in `internal/encoding`. + +## Today + +`JsonFile` is detection only — 35 lines, `is_decodable() == false`, +`FileCategory::text` — so `html::translate` routes it through +`translate(file.as_text_file())` to `html::create_text_service` and a json +renders exactly like a `.txt`: numbered lines, no folding, nothing the raw bytes +did not already say. + +`check_json_file` runs `nlohmann::json::parse` over the **whole** stream and +throws the result away (`// TODO limit check size`, `// TODO check if that even +works`, `json_util.cpp:8`). `open_strategy.cpp:288` constructs a `JsonFile` +purely to *name* the type while listing candidates, so a 200 MB json is fully +parsed just to be called a json, and the dom is destroyed immediately. + +The parse reads the **undecoded** stream — `check_json_file(*m_file->file()-> +stream())`, `json_file.cpp:10` — even though `JsonFile` holds a +`text::TextFile` whose `text()` has decoded to UTF-8 since the csv plan's stage +1. nlohmann accepts UTF-8 and skips a UTF-8 BOM, so a UTF-16 json — blessed by +RFC 4627, still emitted by Windows tooling — fails detection today and falls +back to plain text, where `html/text_file.cpp` then renders it correctly. The +rendering path is the one that already got fixed; the parse is what is left. + +json is on the skip list in `html_output_test.cpp:115`, and there is no +`test/src/internal/json/` at all. + +`nlohmann_json` is already a direct link dependency (`CMakeLists.txt:272`). + +## Target + +A json opens on its value tree: indented, foldable, keys visually distinct from +values, and a collapsed subtree saying how much it hides. Read-only, no +round-trip, no schema. The text view stays reachable and stays the floor. + +## Decisions taken up front + +**Render the parsed tree, not the bytes.** Colouring the existing text service +with a lexer is what "syntax highlighting" would mean, and it is the wrong axis: +what makes a large json readable is *folding and structure*, not colour, and +anyone who wants the literal bytes already has the text view. We parse the file +anyway. + +That said, a little colour is nearly free here and worth taking. Walking a typed +dom, every scalar already knows whether it is a string, a number, a boolean or +null, so four CSS classes plus one for keys cost an `if` in the writer — no +lexer, no grammar, nothing that can disagree with the parser. That is the whole +of the "highlighting" this needs. + +**Not a document.** `ElementType` (`document_element.hpp:87`) has no key/value +pair. Mapping objects onto `list`/`list_item` either drops the keys or smuggles +them into `text`, and then every binding walking the tree sees prose where the +file had structure — worse than the honest answer, which is that the element +model does not describe json. So `is_decodable()` stays `false`, json stays +`FileCategory::text`, and the html service is hand-written like +`html/text_file.cpp` and `html/pdf_file.cpp` rather than derived from the +generic renderer. That is a deliberate deviation from step 3 of "Adding / +extending a document format" in the root `AGENTS.md`; it belongs in a +`json/AGENTS.md` once stage 3 lands. + +If a json-as-data api is ever wanted, the honest shape is a small value api in +the public headers, not an `ElementAdapter`. Nothing here forecloses it. + +**The service parses; the file detects.** The tempting move is to keep the dom +on `JsonFile` so it is parsed once. Resist it. It puts `nlohmann/json.hpp` — the +single heaviest header in the build — into `json_file.hpp` and from there into +`open_strategy.cpp`; the escape (`nlohmann/json_fwd.hpp` plus a +`std::unique_ptr` member and an out-of-line destructor) is a real technique but +it buys nothing, because after stage 2 detection no longer builds a dom to +share. So: detection reads a bounded prefix and keeps nothing, the html service +parses `TextFile::text()` once in its constructor and owns the dom for its own +lifetime, and nlohmann stays confined to `json_util.cpp` and +`html/json_file.cpp`. + +**The text view is the floor.** A bounded probe accepts a prefix; the full parse +can still fail on byte 900 000. So `html::translate(const TextFile &)` builds +the json service inside a `try` and falls back to `create_text_service` when the +parse throws — a broken json renders as text with its syntax error visible, +which is the useful outcome. This is the one place the fallback lives; no other +call site needs to know. + +**Dispatch by narrowing, not by a new handle.** `translate(const DecodedFile &)` +switches on category (`html.cpp:213`) and json is `text`, so json arrives at +`translate(const TextFile &)`. Branch there on +`file_type() == FileType::javascript_object_notation`. The alternative — a +public `JsonFile` handle with `as_json_file()`, the way csv just got one — costs +the handle plus its surface in four bindings, and csv needed it only because it +had `options()` and `document()` to hang there. json has neither. Revisit if +json ever grows per-file options; view-shaping knobs belong in `HtmlConfig` +anyway. + +**Detection is a discriminator, not a validity check.** RFC 8259 allows a bare +scalar at the top level, so `42` and `"hello"` are valid json documents — and +also valid txt, and valid one-column csv. Requiring the first token to be `{` or +`[` is the same kind of rule as csv's "at least two columns": it is about +telling formats apart, not about what json is. The by-type path +(`open_strategy.cpp:167`, reached when the caller says "this is json" or the +extension does) parses in full and accepts what RFC 8259 accepts. + +**Number spelling is not preserved.** nlohmann keeps `std::int64_t` / +`std::uint64_t` / `double`, not the source token, so re-serialising a scalar +gives the shortest round-trip form: `1.50` renders as `1.5`, `1e3` as `1000.0`. +The value is faithful, the spelling is not. Accepted — the text view is one +click away and shows the file. Emitting the raw token instead would mean a +second, token-preserving parse, which is a large cost for a cosmetic gain. + +**Strings out of the dom are valid UTF-8.** nlohmann rejects invalid UTF-8 at +parse time, so the writer never has to sanitise — it takes +`get_ref()` straight through `html::escape_text` +(`html/common.hpp:48`). Only the *probe* can hold a `U+FFFD`, from a multi-byte +sequence cut at the boundary, and a replacement character inside a string +literal is syntactically fine. + +**Folding is `
`/``.** Native, scriptless, static html, and it +survives the fragment-writing `HtmlService` shape without a frontend object. The +summary line carries what the fold hides — `"users": [ … 128 items ]` — which +the dom knows for free. Expand-all/collapse-all needs script; add it when +someone asks. + +**Truncation is loud.** The renderer emits one row per scalar, so the browser's +node budget is the real limit and it arrives long before ours does. Collapsing +does not help: a `
`-hidden subtree is still in the DOM. So cap the +number of *emitted* values and close the output with a visible +`… 4 213 891 more values, not rendered`, rather than silently stopping. +`HtmlConfig::spreadsheet_limit` (`html.hpp:124`) is the precedent for where the +knob goes. Streaming a json too large for that is out of scope, exactly as csv +stage 5 is. + +--- + +## Stage 1 — parse decoded text + +A bug fix that ships alone, and the smallest possible diff. + +- `check_json_file` takes a `std::string_view` of decoded UTF-8 instead of an + `std::istream` of raw bytes; `JsonFile` passes `m_file->text()`. +- a non-decodable encoding is not a json: RFC 8259 §8.1 makes json Unicode by + definition, so there is nothing to fall back to. `text()` already throws + `UnsupportedTextEncoding`, and `open_strategy`'s `catch (...)` already turns + that into "not a json". +- first `test/src/internal/json/json_util_test.cpp`, inline string literals: an + object, an array, a bare scalar, a truncated document, a UTF-16LE json with a + BOM. + +## Stage 2 — bounded detection + +Stops type listing paying for a full dom, and splits detection from parsing the +way the csv plan did — same reason, same shape. + +- probe with `encoding::read_probe` + `encoding::to_utf8` + (`encoding/detect.hpp`, `encoding/transcode.hpp`), 64 KiB, the same default + the rest of the codebase uses. +- verdict from `nlohmann::json::sax_parse` with a callback that keeps no dom and + stops at the cut: first non-whitespace byte is `{` or `[`, and no syntax error + before the end of the probe. A prefix is *supposed* to end mid-document, so + running out of input is not a rejection — only a real syntax error is. +- `open_strategy` keeps constructing a `JsonFile` to probe, because that + constructor *is* the probe now and is bounded — as with csv. +- the by-type path stays a full parse and keeps accepting top-level scalars. + +## Stage 3 — the tree view + +- `internal/html/json_file.{hpp,cpp}` with `create_json_service`, alongside + `create_text_service`; one view, `json.html`. +- `html::translate(const TextFile &)` narrows on file type and falls back to the + text service when the full parse throws. +- the writer: nested `
`s with a CSS indent (not `
` — we control every
+  byte of the output), `
`/`` per object and array with the + child count in the summary, `odr-json-key` / `-string` / `-number` / + `-literal` classes, `escape_text` on everything, node budget with the loud + marker. +- `write_json_style` in `html/frontend.cpp` next to `write_text_style`. +- drop `javascript_object_notation` from the skip list at + `html_output_test.cpp:115`. + +## Stage 4 — only if wanted + +Auto-expand depth and node budget as `HtmlConfig` fields; expand/collapse-all +and search (both need script); copy-a-json-pointer per row; NDJSON +(`application/x-ndjson`) as a separate file type rendering as a list of trees. +None of it is needed for the view to be worth having. diff --git a/src/odr/internal/markdown/PLAN.md b/src/odr/internal/markdown/PLAN.md new file mode 100644 index 000000000..0de11438b --- /dev/null +++ b/src/odr/internal/markdown/PLAN.md @@ -0,0 +1,243 @@ +# Markdown plan + +Where markdown support is going, and in what order. Written before stage 1; +keep it honest as stages land. + +## Today + +`FileType::markdown` exists (`file.hpp:73`) and has a table row with extensions +and mime types (`file_type_table.cpp:431`), declared classification-only: +capabilities `{}`, `FileCategory::text`, `DocumentType::unknown`. Nothing +constructs it. `magic` does not know it, `open_strategy` never names it, and +`open_file(file, FileType::markdown)` falls through to +`UnsupportedFileType` (`open_strategy.cpp:198`). A `.md` opened today comes back +as `text_file` and renders as a line list through +`html::create_text_service`. + +So there is no code to undo. The work is a decoder, plus flipping the row. + +## Target + +A markdown file opens as a **text document** — `TextRoot` with paragraphs, +spans, lists, tables, links and images — so the generic HTML renderer and every +binding get it without format-specific code. That is the whole argument for +doing it this way rather than writing a markdown→HTML renderer next to +`html/text_file.cpp`: the latter would produce html output only, with no +element api, nothing for JNI/embind/pybind/ObjC, and no path to +`back_translate`. + +CommonMark is the base; GFM's tables, strikethrough, task lists and permissive +autolinks are the extensions worth having. Everything beyond that is somebody's +dialect and stays out. + +## Decisions taken up front + +**A library, not a hand-rolled parser.** This is the opposite call from +`d8c8715` (dropping vincentlaucsb-csv-parser to scan csv in-tree), and +deliberately so. Csv is a hundred lines of quote-aware scanning. CommonMark has +~650 conformance cases, and the places hand-written parsers rot — lazy +continuation, link reference definitions, the emphasis left/right-flanking +rules, list-item indent arithmetic — are exactly the ones that look easy for a +weekend and are then wrong forever. + +**md4c** (`md4c/0.5.2` on conancenter, MIT, ~5k LOC C). Chosen over cmark-gfm: +cmark-gfm has no conan recipe at all, builds an allocating AST we would +immediately walk once and throw away, and is a GitHub fork with a lax release +cadence. md4c is a SAX parser — `enter_block` / `leave_block` / `enter_span` / +`leave_span` / `text` — which maps onto `create_element` / `append_child` with a +single id stack and no intermediate tree. Its `MD_DIALECT_GITHUB` flag is +exactly the extension set above. Add it to `conanfile.py:51` next to +`nlohmann_json` / `uchardet`, and `find_package` + link in `CMakeLists.txt:59` +/ `:271`. + +Two things to check when wiring it up, because they decide small amounts of +work in stage 2: +- whether the conan package exports the `md4c-html` component, whose + `entity.h`/`entity_lookup` resolves the ~2000 named html entities. If not, we + handle `&`-class and numeric references ourselves and pass the rest + through literally. +- md4c parses bytes and assumes UTF-8 (`MD4C_USE_UTF8`), so decoding happens + before it, not inside it. + +**Markdown is a `DocumentFile`, not a `TextFile`.** `abstract::TextFile` fixes +`file_category()` to `text` (`abstract/file.hpp`); a document has to be +`FileCategory::document` with `DocumentType::text`. The table row changes +category with it. This is api-visible for `FileType::markdown` — and free, +because the row declares no capabilities today, so nothing can be relying on +it. + +**Input is UTF-8, produced by `internal/encoding`.** `MarkdownFile` takes a +`std::shared_ptr` exactly as `CsvFile` and `JsonFile` do +(`csv_file.hpp`), and calls `text()` for the decoded bytes. An encoding we +cannot decode throws `UnsupportedTextEncoding` (`exceptions.hpp:42`) — the same +rule the csv plan sets for the sheet path, and for the same reason: +`Text::content()` is UTF-8 to every binding, so passing legacy bytes through +and letting a browser sort it out is not available to us. + +**Detection is by caller, not by content.** `open_strategy` is entirely +content-driven — magic plus speculative probes — and has no extension path +anywhere. Markdown has no signature, and a content probe for it is a probe for +"prose with occasional punctuation", which is every plain text file with a `#` +comment or an `*` bullet in it. Sniffing would steal `text_file` matches and be +confidently wrong. So: `detect_by_content` stays **false**, markdown never joins +the speculative chain in `list_file_types` (`open_strategy.cpp:272`), and the +only way in is `DecodedFile(file, FileType::markdown)` (`file.hpp:341`) via a +new branch in `open_file` next to the text/csv/json ones +(`open_strategy.cpp:146`). Callers route on the filename, which is what they +already have. + +The consequence to accept: `.md` still opens as `text_file` by default. That is +correct behaviour for a format that is, by construction, valid plain text. + +**There is no `NoMarkdownFile`.** Every other format's exception exists because +detection rejects. Nothing rejects here: md4c is total — any UTF-8 byte +sequence is some markdown document — and there is no detection to fail. The +only failure mode is the undecodable encoding above. + +**Headings ride on paragraphs, as ODF already does.** `ElementType` has no +heading (`document_element.hpp:87`), and `odf_parser.cpp:304` maps `text:h` to +`ElementType::paragraph` with the level carried in the style. Markdown does the +same: `MD_BLOCK_H` with `detail->level` becomes a paragraph plus a `TextStyle` +holding `font_size` and `font_weight` from a small `StyleRegistry`. The cost is +honest and shared with odt — the html output is `

` with inline styles, not +`

`, so heading semantics are lost to screen readers and to anything walking +the api for an outline. Stage 5 is where that gets fixed properly, once, for +both engines. + +**Raw html is dropped.** CommonMark passes html blocks and inline html through +verbatim. There is no passthrough element in the model, and inventing one means +deciding what `Text::content()` returns for it in four bindings — a real +question, not a markdown question. `MD_BLOCK_HTML` and `MD_SPAN`-level raw html +are skipped in stage 1. Inline `` in a markdown file therefore renders as +nothing, which is worse than the status quo for exactly one use case, and is +the price of not opening that seam early. + +**Images stay external.** `ImageAdapter` (`abstract/document.hpp:495`) offers +`image_is_internal()` plus `image_href()`, so `![](diagram.svg)` becomes a +`Frame` + `Image` with `is_internal() == false` and the href passed through +untouched. No filesystem, no relative-path resolution, no fetching. A viewer +resolves it against wherever it got the document, which is the only party that +knows. + +## Graphs and drawings + +Worth separating two things that get said in one breath. + +*Inline SVG* is the raw-html question above, and blocked behind the same +decision. + +*Mermaid / graphviz / plantuml fences* are not a markdown feature at all — they +are a fenced code block with an info string, which a downstream renderer picks +up. Rendering them in C++ means embedding a graph layout engine, which is a +larger project than markdown support entire, and one that duplicates what every +frontend already ships. The right move is to keep the fence as a code block +that *carries its language*, and let the frontend render it. There is nowhere +to put that language today — which, with the heading level, makes two entries +for the same stage 5. + +## Element mapping + +| md4c | model | +|---|---| +| `MD_BLOCK_DOC` | `root` (`TextRoot`), default `PageLayout` as `doc_document.cpp:144` | +| `MD_BLOCK_H` (level 1–6) | `paragraph` + heading `TextStyle` | +| `MD_BLOCK_P` | `paragraph` | +| `MD_BLOCK_UL` / `OL` | `list` | +| `MD_BLOCK_LI` | `list_item` | +| `MD_BLOCK_QUOTE` | `group` + left `margin` on the paragraphs inside | +| `MD_BLOCK_CODE` | `paragraph` + monospace `TextStyle`; info string dropped | +| `MD_BLOCK_HR` | — (nothing in the model) | +| `MD_BLOCK_TABLE` / `THEAD` / `TBODY` / `TR` / `TH` / `TD` | `table` / `table_row` / `table_cell` | +| `MD_SPAN_EM` / `STRONG` / `DEL` | `span` + `font_style` / `font_weight` / `font_line_through` | +| `MD_SPAN_CODE` | `span` + monospace `font_name` | +| `MD_SPAN_A` | `link` | +| `MD_SPAN_IMG` | `frame` + `image` | +| `MD_TEXT_NORMAL` / `ENTITY` / `CODE` | `text` | +| `MD_TEXT_BR` | `line_break` | +| `MD_TEXT_SOFTBR` | a space appended to the current text | +| `MD_TEXT_NULLCHAR` | U+FFFD | + +Two renderer gaps this exposes, both pre-existing: +`html::translate_list` hardcodes `
    ` (`html/document_element.cpp:342`), so an +ordered list renders as bullets; and `ElementType::group` renders as its +children with no wrapper (`:72`), so a blockquote's structure survives only in +the margins its paragraphs carry. + +## Module layout + +Mirrors `oldms/text`, which is the reference the root `AGENTS.md` points at. + +``` +src/odr/internal/markdown/ + markdown_file.hpp/.cpp abstract::DocumentFile over a text::TextFile + markdown_document.hpp/.cpp internal::Document + the ElementAdapter + markdown_element_registry.hpp/.cpp flat vector, side maps for text/link/image + markdown_parser.hpp/.cpp md4c callbacks → registry, one id stack + markdown_style.hpp/.cpp StyleRegistry: heading sizes, code font, quote margin +``` + +Every `.cpp` goes into `ODR_SOURCE_FILES` (`CMakeLists.txt:86`). + +--- + +## Stage 1 — blocks + +The skeleton, end to end, with the least that is worth rendering. + +- md4c as a conan dependency; `MarkdownFile` over `text::TextFile`; the + `open_file` branch; the table row flipped to `FileCategory::document`, + `DocumentType::text`, `{.open = true, .translate_html = true}`. +- registry, adapter and `Document` per the pattern; `TextRootAdapter`, + `ParagraphAdapter`, `TextAdapter`, `LineBreakAdapter`. +- the block half of the table above: doc, headings, paragraphs, code blocks. + Lists and quotes land here too — they are blocks and cost a stack push each. +- tests inline, string literal in, element tree out; no fixture files + (`text_file_test.cpp` is the shape). + +`FileTypeCapabilities.declaration_matches_the_engines` +(`odr_test.cpp:163`) opens two files per type from the test data and asserts the +engines do not exceed the row, so a handful of `.md` samples go into the +test-data repo alongside this stage. + +## Stage 2 — inlines and styles + +- `SpanAdapter`, `LinkAdapter`; emphasis, strong, inline code, links. +- `markdown_style`: one `StyleRegistry` handing out the heading scale, the + monospace face and the quote margin, indexed from the registry the way + `doc_style` is. +- entity and soft-break handling per the table; the `entity_lookup` question + above resolves here. + +## Stage 3 — GFM + +`MD_DIALECT_GITHUB`: tables, strikethrough, task lists, permissive autolinks. +Tables are the substantial one — `TableAdapter` plus row/column/cell — and the +reason to do it before images: it is what people actually put in readmes. + +Task list items have no checkbox in the model. Render the box as text (`☐`/`☑`) +in the item's first text element rather than inventing an element type for it. + +## Stage 4 — images and frontmatter + +- `FrameAdapter` + `ImageAdapter`, external hrefs per the decision above. +- YAML/TOML frontmatter: md4c does not know it, so strip a leading `---` fence + before parsing and expose what it holds through `FileMeta`. Parse only the + flat scalars we have somewhere to put (`title`, `author`, `date`); do not + take on a YAML dependency for the rest. + +## Stage 5 — the two model gaps + +Both are api changes shared with the other engines, which is why they come +last and together rather than being smuggled in with the parser. + +- **heading level** — the odf mapping loses it too. Either an + `ElementType::heading` with a level, or a level on `ParagraphStyle`. The + latter is smaller and lets `html::translate_paragraph` emit `

    `…`

    ` for + odt and markdown at once; the former is more honest about it being a + different kind of thing. Decide with odf in the room. +- **code-block language** — a `std::string` on the paragraph, or a dedicated + code element. This is what makes the mermaid story work without odr rendering + anything, and it is the only reason a frontend can syntax-highlight. + +Deferred beyond this: raw html passthrough, footnotes, definition lists, +`back_translate` to markdown, and any dialect that is not GFM. diff --git a/src/odr/internal/rtf/PLAN.md b/src/odr/internal/rtf/PLAN.md new file mode 100644 index 000000000..8489e900f --- /dev/null +++ b/src/odr/internal/rtf/PLAN.md @@ -0,0 +1,271 @@ +# RTF plan + +Where an rtf module would go, and in what order. Written before stage 1; keep +it honest as stages land. + +## Today + +Nothing decodes. `FileType::rich_text_format` exists (`file.hpp:62`, under the +"Detection only" comment), `magic.cpp:123` matches `7B 5C 72 74 66 31` +(`{\rtf1`), and `file_type_table.cpp:390` carries a row with `rtf` extensions, +three mime types, `FileCategory::document`, `DocumentType::unknown` and +`{.detect_by_content = true}`. `open_strategy::open_file` has no branch for it, +so `odr::open` on an rtf reaches the fallthrough and throws `UnknownFileType`. + +The public enum is already mirrored by the bindings (`bind_file.cpp:39`, +`ODRFile.mm:43`), so **no binding work is needed for any stage below** — the +ordinal is spent, and everything else arrives through the abstract document +model the generic renderer already walks. + +## Spec + +`offline/documentation/MSFT-RTF/MSFT-RTF-080320/` — the RTF Specification +version 1.9.1 (March 2008, the final one). Cite section names in code, the way +`oldms/` cites `[MS-DOC] §2.4.1`; RTF's spec is not numbered, so cite the +heading (e.g. *Conventions of an RTF Reader*, *Table Definitions*, *Pictures*) +and the control word. + +Not the same document as `[MS-OXRTFEX]` (Exchange's HTML-in-RTF encapsulation) +or `[MS-OXRTFCP]` (RTF compression) — neither is in scope. + +## Target + +An rtf opens as a text document — `root → paragraph → span → text`, tables and +frames where the file has them — so the generic HTML renderer and every binding +get it without format-specific code. `DocumentType::text`, `open` and +`translate_html` in the table row; `is_editable()` false, `save` throws, exactly +as `oldms/text`. + +## Decisions taken up front + +**Mirror `pdf`, not `oldms`.** RTF is a *text* format: `{`, `}`, +`\controlword`, `\'hh`, and literal bytes. None of `oldms/text`'s +machinery applies — no CFB container, no FIB, no piece table, no +`PlcBteChpx`→`ChpxFkp` walk, no packed structs, no host-endianness caveat. What +does apply is `internal/pdf`'s parser shape, and it applies unusually well: + +- **`RtfTokenizer` follows `pdf::ObjectParser`** (`pdf_object_parser.hpp`): hold + `std::istream *` + `std::streambuf *`, prepare the stream once with an + `std::istream::sentry` in the constructor, and expose + `getc`/`bumpc`/`bumpnc(n)`/`ungetc` that throw `std::runtime_error` on + unexpected exhaust. This is not decoration: `\binN` needs a raw n-byte read + mid-stream (`bumpnc`), and `\'hh` needs the same `hex_char_to_int` / + `two_hex_to_char` statics. Those two helpers are six lines — **duplicate + them**, do not reach into `internal/pdf`. (The csv plan's rule: anything + genuinely shared moves out to its own package first, and two hex helpers do + not earn a package.) +- **`read_token()` returns a variant, like `GraphicsOperatorParser::read_operator()`** + — `GroupOpen`, `GroupClose`, `ControlWord{name, optional parameter}`, + `ControlSymbol{char}`, `Text{bytes}`, `Binary{bytes}`, `End`. The spec's + Appendix A sample reader is a callback design over globals; a pull-based token + stream is what the rest of this codebase looks like and is what makes the + tokenizer testable on inline strings. +- **`RtfState` follows `pdf::GraphicsState`** (`pdf_graphics_state.hpp`): + `std::vector stack` that is never empty, `save()` on `{`, `restore()` + on `}`, and a `ContentScope`-style RAII guard for destination groups. The + spec's *Conventions of an RTF Reader* prescribes precisely this ("stores its + current state on the stack … retrieves the current state from the stack"), and + `pdf` already carries the leniency precedent — "an unmatched `Q` is ignored: + the state the running content stream started from always remains". + +**Leniency is the spec, and is therefore not a violation of fail-fast.** The +root `AGENTS.md` says throw where the spec dictates what to expect. Here the +spec dictates the opposite: an unknown control word "should be ignored", and +`{\*` opens an ignorable destination the reader "should discard all text up to +and including the closing brace" of. A reader that throws on an unknown control +word cannot read any file written by a newer writer, which is the stated design +goal of `\*`. So: + +- **Ignore** unknown control words and control symbols; **skip** `{\*` groups + whose destination we do not implement; ignore an unmatched `}`. +- **Throw** where the spec does dictate: a group left open at EOF, an invalid + hex digit after `\'`, `\binN` running past EOF, nesting past a depth bound + (borrow `ObjectParser::NestingGuard` — the group stack is heap, but the + destination skip is recursive). +- The one place the spec *invites* rejection is `\cellxN` outside a table + ("probably created maliciously"). Not worth a throw at our fidelity; note and + ignore. + +**Text is bytes until a run ends.** `\'hh` yields one *byte*, not one character: +in a Shift-JIS run two consecutive `\'hh` escapes are one character. So the +accumulator is a byte buffer plus the run's `TextEncoding`, flushed through +`encoding::to_utf8` when the encoding changes, the formatting changes, or the +paragraph ends. Decoding per escape would corrupt every multibyte run and is the +single easiest mistake to make here. + +`\uN` is different — it is already a code point, so it bypasses the byte buffer +via `util::string::append_c32` (flushing the buffer first, to keep order). Two +traps: N is a *signed 16-bit* value, so `U+F020` arrives as `\u-4064` and must be +folded back with `+ 65536`; and `\ucN` gives the number of following characters +to skip as the ANSI fallback, is scoped like a character property (so it lives +in `RtfState`, restored by `}`), defaults to 1, and counts *any* control word or +symbol as one character — with a `\binN` and its payload counting as one. + +**Encoding comes from `internal/encoding`.** `\ansi`/`\mac`/`\pc`/`\pca` set a +document default, `\ansicpgN` overrides it, the font table's `\fcharsetN` (or +`\cpgN`, which supersedes it) sets it per font, and `\fN` selects the font. That +resolution chain lands on a `TextEncoding` and the transcode is +`encoding::to_utf8`. The single-byte tier covers what real-world rtf uses; +`\fcharset128`/`129`/`134`/`136` (Shift-JIS, EUC-KR, GB2312, Big5) hit the +"named but not decoded" tier, so their `\'hh` runs degrade to U+FFFD — acceptable, +because a writer emitting CJK generally emits `\uN` alongside, which is decoded +regardless of the run's encoding. + +**Flat `ElementRegistry`, copied not shared.** Unlike csv, an rtf's element +count is bounded by its paragraph count, so the pattern from the root +`AGENTS.md` applies unchanged and `oldms/text/doc_element_registry.*` is the +template. Every engine has its own registry (`doc`, `ppt`, `xls` all do); copy +it rather than inventing a shared one. + +**No writer.** RTF is the one format here that would be pleasant to *emit*, and +`back_translate` exists. Out of scope, in every stage. + +--- + +## Stage 1 — plumbing, tokenizer, plain text + +The narrowest thing that renders. No formatting at all beyond paragraph +structure, so the tokenizer and the group machinery can be proven before +anything is layered on them. + +- `rtf_tokenizer.{hpp,cpp}` as above, with the delimiter rules from *Control + Word* exactly: a control word is `\` + ASCII letters, terminated by a space + (consumed), by a digit or `-` (a parameter of up to 10 digits follows, itself + terminated by any non-digit, which is **not** consumed), or by any other + character (not consumed). A control *symbol* is `\` + one non-letter and takes + no delimiter — a space after it is text. +- `rtf_state.{hpp,cpp}`: the group stack, the current destination, and the + character/paragraph property structs (empty but for `\uc` in this stage). +- destination handling: a table of known destinations; `{\*\unknown` skips to the + matching `}`. The skip must **still tokenize** — a `\binN` inside a skipped + group carries raw bytes that can contain braces, so a brace-counting scan over + raw bytes desyncs. Same reason `\'7b` must not be mistaken for a group open. +- text: `\par` ends a paragraph, `\line` a line break, `\tab` a tab, `\page` a + page break; `\\`, `\{`, `\}` are literal; a bare CR/LF is *not* text and is + dropped; `\~` non-breaking space, `\_` non-breaking hyphen, `\-` optional + hyphen dropped. (`oldms/text`'s `TextCleaner` is the same job with different + spellings.) +- fields need no code: `\fldinst` is `\*`-marked, so the ignorable-destination + rule hides the instruction and leaves `\fldrslt`'s text flowing — the same + "cached result only" position `.doc` takes. +- `rtf_file.{hpp,cpp}` — `RtfFile : abstract::DocumentFile` over a plain + `abstract::File`, shaped like `pdf::PdfFile` (`pdf_file.hpp:16`); a branch in + both `open_strategy::open_file` and `open_document_file`; `types_by_content` + needs nothing, magic already reports the type. +- `file_type_table.cpp:390` → `DocumentType::text`, `.open = true`, + `.translate_html = true`. `odr_test` fails if declared capabilities exceed + what the engine does, so this row moves stage by stage. +- `CMakeLists.txt` `ODR_SOURCE_FILES`. + +**Tests** are inline string literals against the tokenizer and against a small +whole document — the delimiter rules (space eaten, digit kept, `\b0` vs `\b`), +`\'hh` in a windows-1252 run, `\uN` negative folding, `\ucN` skipping across a +control word, an unmatched `}`, an unterminated group, and a `\bin` payload +containing `}`. + +## Stage 2 — character formatting + +- `{\fonttbl{\fN\fcharsetN Name;}…}` and `{\colortbl;\redN\greenN\blueN;…}` — + both are destinations, both parsed into a `StyleRegistry` alongside the + element registry, mirroring `oldms/text/doc_style.*`. Note the colour table's + leading `;`: entry 0 is the "auto" colour and is empty. +- `\b \i \ul \ulnone \strike \scaps \caps \sub \super \nosupersub`, `\fN`, + `\fsN` (half-points), `\cfN`/`\cbN`, `\highlightN`, `\plain` (reset) → + `TextStyle`, resolved to spans exactly as `.doc` does: equal property sets + share one resolved style, adjacent equal-style runs merge. +- font names interned in the `StyleRegistry` and never mutated afterwards, so + `TextStyle::font_name` (`const char *`) stays valid — the `.doc`/`.xls` rule. + +## Stage 3 — paragraph, section, page + +- `\pard` reset, `\ql \qc \qr \qj`, `\liN \riN \fiN`, `\sbN \saN \slN` → + `ParagraphStyle`. Twips throughout; `Measure` already carries units. +- `\paperwN \paperhN \marglN \margrN \margtN \margbN \lndscpsxn` → + `PageLayout` off `TextRootAdapter::text_root_page_layout`, which stage 1 + returns empty from. +- `\sect`/`\sectd` — with one page layout there is nowhere to put a second + section, so treat a section break as a page break and take the first section's + geometry. Honest and enough; revisit only if a reference document needs it. +- lists: `{\listtext …}` and `{\pntext …}` carry the rendered bullet or number. + Emitting them as literal text is right at this fidelity and costs nothing; + real `ListItem` elements need `{\*\listtable}` + `{\*\listoverridetable}` + + `\lsN\ilvlN` resolution and belong in a later stage, not here. + +## Stage 4 — tables + +The largest single piece, and the one place RTF is genuinely awkward. + +*Table Definitions*: there is no table group. A row is a run of paragraphs each +carrying `\intbl`, cells terminated by `\cell`, the row by `\row`, and the +geometry given by a `` — `\trowd` followed by one `\cellxN` per cell, +where N is that cell's cumulative **right edge** in twips. + +- **Buffer the row; do not stream it.** The spec is explicit that Word 97 wrote + `` before the cells, Word 2002–2007 write it *after* (and repeat it + before), and the grammar admits all three orders. So accumulate cells into a + pending row and apply whichever `` was seen when `\row` fires — and + make a second, identical `\trowd` idempotent rather than a second row. +- column widths are differences of successive `\cellxN`; `\trleftN` is the row's + left edge and the first difference is measured from it. +- `\clmgf`/`\clmrg` (horizontal) and `\clvmgf`/`\clvmrg` (vertical) merges map + onto `TableCellAdapter::table_cell_span` plus `table_cell_is_covered` — the + `f` variants start a merged region, the bare ones continue it, so the span is + only known once the run ends. Another reason the row is buffered. +- consecutive rows sharing a `` are one table; a row whose cell edges + differ starts a new one. `\itapN` gives the nesting depth, with + `\nestcell`/`\nestrow` and `{\*\nesttableprops}` for the inner levels — + implement depth 0 first and treat deeper rows as their own table, then nest. +- borders and shading (`\clbrdr*`, `\trbrdr*`, `\clcbpat`) into `TableStyle` / + `TableCellStyle` last; they are independent of the reconstruction. + +## Stage 5 — images + +- `{\pict …}` is a destination whose payload is hex `#SDATA` by default or + `\binN` + raw `#BDATA`. Decode to a `std::string`, wrap in + `common::MemoryFile` (`common/file.hpp:34`), hand back through + `ImageAdapter::image_file` with `image_is_internal() == true`. +- `\pngblip` and `\jpegblip` render. `\emfblip`, `\wmetafileN`, `\macpict`, + `\pmmetafileN`, `\dibitmapN`, `\wbitmapN` have no decoder here + (`windows_metafile` / `enhanced_metafile` are classification-only file types), + so **drop the frame** rather than emit a broken `` — "pass through what we + don't model". Word-97-era files lean on `\wmetafile8` heavily; anything modern + is png/jpeg. +- **`\nonshppict` must be skipped explicitly.** The pair is + `{\*\shppict{\pict …}}{\nonshppict{\pict …}}` — the second group is the same + image again, for readers that cannot do `\shppict`, and it is *not* `\*`-marked, + so the ignorable-destination rule will not hide it. Miss this and every image + appears twice. +- `\picwgoalN`/`\pichgoalN` are the display size in twips → `Frame` measures; + `\picwN`/`\pichN` are the intrinsic pixel (or metafile extent) size and are the + fallback. `\picscalexN`/`\picscaleyN` are percentages applied on top. +- shapes (`{\shp{\*\shpinst{\sp{\sn pib}{\sv …}}}}`) hold pictures too, and + `{\object\objemb}` holds embedded OLE. Both out of scope; both are skipped + cleanly by the destination rule if left unimplemented. + +--- + +## Deferred, by decision + +- **Headers, footers, footnotes, endnotes, comments** — each its own destination + (`\header*`, `\footer*`, `\footnote`, `\annotation`). Cheap to reach once the + destination table exists, but there is nowhere in the element model to put + them today; `.doc` drops them for the same reason. +- **Real list items** — see stage 3. +- **Style sheet** (`{\stylesheet}`) and `\sN`/`\csN` inheritance. Direct + formatting first; this is the same layer `.doc` still misses (§2.4.6.5 there). + Worth more here than in `.doc`, because rtf writers lean on styles for heading + fonts. +- **Math** (`{\mmath …}`), **drawing objects**, **bidi** (`\rtlch`/`\ltrch` and + the associated-font `\af*` chain), **revision marks**, **East Asian composite + fonts**. +- **Multibyte `\'hh` runs** — blocked on the multibyte tier in the encoding + package, and largely masked by `\uN` in practice. + +## Test data + +There is no `.rtf` anywhere under `test/data`, and that tree is fetched from the +pinned repos rather than vendored. Stage 1–4 parser tests are inline string +literals (which is the better test anyway — an rtf fragment is readable in a +`R"(...)"`), but a render test needs a real fixture committed to +`test/data/input` plus the reference-output regen. Stage 5 needs one either way, +since a picture cannot reasonably be written inline. diff --git a/src/odr/internal/xml/PLAN.md b/src/odr/internal/xml/PLAN.md new file mode 100644 index 000000000..2e9e3b408 --- /dev/null +++ b/src/odr/internal/xml/PLAN.md @@ -0,0 +1,298 @@ +# XML plan + +Where an xml renderer would go, and in what order. Written before stage 1; keep +it honest as stages land. + +## Today + +`FileType::xml` exists (`file.hpp:150`, under the "Classification only" +comment) and carries a table row — `xml` extension, `application/xml` and +`text/xml`, `FileCategory::text`, `DocumentType::unknown`, +`{.detect_by_content = true}` (`file_type_table.cpp:664`). + +Detection already works. `list_file_types` parses the file with +`util::xml::check_xml_file` and reports `[text_file, xml]`, plus +`scalable_vector_graphics` on top when the root element says so +(`open_strategy.cpp:296`). What is missing is everything after that: +`open_file_as` has no `FileType::xml` branch, and `open_file`'s unknown-type +path tries csv, json and svg before falling through to `text::TextFile` +(`open_strategy.cpp:432`). So a `.xml` decodes as `text_file` and renders +through `html::create_text_service` as a numbered line list. + +Which is the whole problem. The xml files anyone opens on purpose — +`content.xml`, `document.xml`, anything a writer emitted rather than a human — +have no newlines in them, so the line list is one line several megabytes wide. + +Already in place and reusable: `NoXmlFile` (`exceptions.hpp:146`), thrown by +`util::xml::parse` (`util/xml_util.cpp:18`); pugixml 1.15 as a dependency +(`conanfile.py:51`); and `internal/encoding` for transcoding. + +## Target + +An xml file opens as `XmlFile` and renders as a **source view**: indented, +syntax-highlighted, foldable, in one self-contained html document with no +JavaScript and no external resources. It stays a `TextFile`. The work is a +decoder shell plus one html service — no `Document`, no element adapters. + +## Why not leave it to the browser + +Every current browser ships an xml tree viewer, and none of them is reachable +from here. They fire on a *response* served as `text/xml`; odr serves nothing — +`HtmlService` hands the host an html document that a WebView displays, and +inside an html document the viewer never engages. + +It would be the wrong lever even if it could be pulled. An +`` PI makes the browser run the XSLT instead of showing the +tree — silently rendering something else entirely. The viewers differ from each +other in folding, attribute display and error reporting. And the two engines +that matter most for this library, Android's WebView and WKWebView, are not the +browsers whose behaviour anyone checked. + +## Decisions taken up front + +**A file-level html service, not a document.** Xml has no document semantics. +Routing it through `ElementAdapter` would mean picking a `DocumentType` that is +a lie, and the generic renderer would have nothing to contribute — there is no +paragraph, no page, no sheet, only nesting. This is the opposite call from the +csv plan, and for a reason that transfers: a csv *is* a sheet, so the model +earns its keep and every binding gets a table for free. An xml file is not a +document that happens to be serialised as xml; it is the serialisation. So it +renders the way an image or a media file does — one `HtmlService`, html output +only, no element api. See also *the archive seam* below, which is where the +api-free choice does eventually cost something. + +**`XmlFile` mirrors `JsonFile`.** `abstract::TextFile` over a +`std::shared_ptr`, constructed with the same probe-in-the- +constructor shape (`json/json_file.cpp:10`), `is_decodable()` false. The table +row flips to `{.detect_by_content = true, .open = true, .translate_html = +true}`. + +The api-visible consequence: a `.xml` that reports `text_file` today will +report `xml`. No binding work — the enumerator already exists and the bindings +mirror the enum by ordinal — but a caller switching on `file_type()` sees the +change. That is the point of the change, and it is the same step csv and json +already took. + +**The tree is not the bytes.** "We can already read it" is true only of the +parsed tree, and a pugixml tree is a normalisation of the file, not a view of +it. Lost, unavoidably: the original indentation, attribute quote style, whether +a character came in as `A` or `A`, and — with `parse_eol` and +`parse_wconv_attribute`, both on by default — line-ending and in-attribute +newline spelling. + +Accept that, deliberately. The alternative is a byte-faithful lexer over the +raw text, which keeps the author's formatting but does nothing for the minified +file that motivated the whole feature, and still has to reconstruct nesting +before it can fold anything. Pretty-printing is the feature; fidelity is the +price. If someone later wants a true "view source", it is a second mode over +the same css, not a redesign — noted under *Deferred*. + +**Parse with `parse_full`, and keep whitespace-only text.** `util::xml::parse` +uses pugixml's defaults, which drop comments, processing instructions, the +declaration and the doctype — invisible in a viewer whose job is to show what +is in the file. `parse_full` is exactly those four added to `parse_default`. + +`parse_ws_pcdata` is a separate flag and a separate question: keeping every +whitespace-only text node preserves fidelity but fills the tree with nodes we +are about to reindent anyway. Take `parse_ws_pcdata_single`, which keeps +whitespace-only text only where it is an element's sole child — so ` ` +survives as content while the newline-and-tab between two sibling elements does +not. Note that this is the flag that makes the mixed-content rule below +decidable at all. + +Do **not** reuse `util::xml::parse` for this; it hard-codes the default flags +and every existing caller wants them. Add the options at the xml module's own +call site. + +**Encoding is declared in band, and pugixml will not honour it.** An xml file +names its own encoding in the declaration, which is better information than +`encoding::detect`'s uchardet guess over a 64 KiB probe. pugixml's +`encoding_auto` resolves UTF-8/16/32 from a BOM or the `` document is read as UTF-8 +and yields invalid UTF-8 in the node strings, silently. (Verify against 1.15 +before relying on the negative — but the design below is right either way.) + +So: read the declaration's `encoding` pseudo-attribute from the head of the +file, map it through `text_encoding_by_name`, transcode with +`encoding::to_utf8`, and hand pugixml UTF-8. Precedence is declaration, then +BOM, then `text::TextFile::encoding()`'s guess. `XmlFile::encoding()` returns +the resolved value, so a caller can show it. An encoding we can name but not +decode throws `UnsupportedTextEncoding`, as the csv sheet path does and for the +same reason: the tree path has to produce UTF-8, and there is no "let the +browser sort it out" once the bytes are inside a parser. + +**Mixed content is not reindented.** `

    a x b

    ` carries significant +whitespace, and nothing short of a schema can tell it from the insignificant +kind. The rule: an element with any non-whitespace text child renders its +children inline, on one line, untouched; an element whose children are all +elements is indented and foldable. This is what every xml viewer does, it is +the one non-trivial rule in the writer, and it is the first thing a test should +pin. + +**Highlighting is server-side spans.** One `` per +token, emitted by the writer. Not a JavaScript highlighter: the output has been +self-contained with no external resources since the css and js moved into the +document (`HtmlResource::is_shipped`, `html.hpp:51`), and shipping a +highlighter would reverse that for a job the writer is already doing as it +walks the tree. + +Classes, following the `odr-text-*` naming in `frontend.cpp`: `odr-xml` on the +root, then `-tag`, `-name`, `-attr`, `-value`, `-text`, `-cdata`, `-comment`, +`-pi`, `-decl`, `-doctype`. Light palette only, as `text_css` is — a dark mode +is a question for every view at once, not for this one. + +**Folding is `
    `/``, with no script.** The disclosure element +gets keyboard access, screen-reader semantics and — the reason it wins — +find-in-page that expands a collapsed section natively, which a `display:none` +toggle does not. Verify the Safari behaviour before promising it; Chrome and +Firefox have done it for years. + +The layout objection is answerable: `details`/`summary` both `display:block`, +`summary::marker` removed via `list-style:none`, indentation carried inside the +summary so the `white-space:pre` flow stays intact. The start tag goes in the +``, the children in the body, the end tag on a line after it. + +The cost, recorded honestly: bulk expand-all/collapse-all needs JavaScript, so +stage 2 ships without it. + +**No line numbers.** The text view has a numbered gutter (`html/text_file.cpp:101`). +Reproducing it here would number *our* lines, not the file's, which for a +reindented minified document is actively misleading. The gutter column goes to +the fold handles instead. + +**Xml is the last resort in detection.** Insert the branch in `open_file`'s +unknown-type path *after* svg and before the text fallthrough +(`open_strategy.cpp:423`), so anything with a more specific reading keeps it. + +Two behaviour changes fall out, both worth naming before they surprise someone. +A flat-xml ODF (`.fods` and friends) has no detection today — the flat mimetypes +are only aliases on the zip-backed rows (`file_type_table.cpp:26`) — so it +currently decodes as text and would now decode as xml. That is an improvement +(a source tree beats a single line) but it is not what a flat ODF should +eventually do, and it must not be mistaken for support. Likewise `.xhtml`, +`.rels`, `.plist` and every rss feed become source views rather than line +lists — correct for a source viewer, and correct that we do not try to *render* +xhtml. + +**No DTD processing, and that is a feature.** pugixml does not resolve external +entities and does not expand internal entity declarations; it handles the five +predefined entities and numeric character references, and leaves `&foo;` as +literal text. For a viewer that opens files from the internet this closes XXE +and entity-expansion attacks by construction rather than by policy. The +fidelity note is the same sentence read the other way: an undefined entity is +shown as written, which for a source view is the right answer anyway. + +**A parse failure falls back to text.** `XmlFile`'s constructor throws +`NoXmlFile`, `open_file` catches it and reaches `text::TextFile`, and a +malformed file renders as the line list it renders as today. Automatic and +correct — with one thing conceded up front: "show me the broken xml" is exactly +when a viewer is most wanted, and the tree path structurally cannot serve it. +That is the strongest argument for the byte-faithful second mode, and it is not +strong enough to build both now. + +## Module layout + +``` +src/odr/internal/xml/ + xml_file.hpp/.cpp abstract::TextFile over a text::TextFile; parse probe, encoding resolution +src/odr/internal/html/ + xml_file.hpp/.cpp create_xml_service — the HtmlService and the writer +``` + +Both `.cpp` go into `ODR_SOURCE_FILES`: the html one next to +`html/text_file.cpp` (`CMakeLists.txt:148`), the module one after `util/` and +before `zip/` (`CMakeLists.txt:252`). + +No `xml_util.cpp` — `internal/util/xml_util` is the shared xml helper and stays +where it is. Anything this module needs that is genuinely general (the +declaration sniff) belongs there, not in a second utility with the same name. + +--- + +## Stage 1 — it opens, and it renders + +The skeleton end to end, flat: highlighted and indented, not yet foldable. + +- `XmlFile` per the `JsonFile` shape; declaration sniff, transcode, + `parse_full | parse_ws_pcdata_single`; `encoding()` resolved as above. +- `open_file_as` gains a `FileType::xml` branch throwing `NoXmlFile`; + `open_file` gains one after svg. Table row flipped to `{.open = true, + .translate_html = true}`. +- `html/xml_file.cpp` with `create_xml_service`, and a `file_type()` branch in + `html::translate(const TextFile &)` (`html.cpp:256`) so xml gets the tree and + everything else keeps the line list. One view, `xml.html`, mirroring + `html/text_file.cpp:26`. +- the writer: declaration, doctype, PI, comment, element, attribute, text and + CDATA, escaped through `html::escape_text` / `escape_attribute`, indented, + each token in its span. The mixed-content rule lands here, not later. +- `xml_css` in `frontend.cpp`, `write_xml_style` in `frontend.hpp`. +- `test/src/internal/xml/xml_file_test.cpp`, inline string literals in, html + out (`text_file_test.cpp` is the shape). Minimum set: minified input + reindents; mixed content does not; comments/PI/doctype/CDATA all survive; + a declared non-UTF-8 encoding decodes; malformed input throws `NoXmlFile`. + +`FileTypeCapabilities.declaration_matches_the_engines` (`odr_test.cpp`) opens +files per type from the test data against the row, so a handful of `.xml` +samples go into the test-data repo with this stage — see *Test data*. + +## Stage 2 — folding + +- `
    `/`` per element with element children, per the + markup and css above. Elements with no children stay a plain line. +- everything open by default. Collapsing by default hurts find-in-page and + hides the thing the user opened the file to see; the only case for it is + size, which stage 3 handles with a threshold rather than a habit. +- the fold handle in the gutter column the line numbers do not occupy. + +## Stage 3 — size + +A `content.xml` is routinely tens of megabytes, and this path multiplies it: +pugixml's dom is roughly 1.5–2× the file plus the in-memory buffer, and a span +per token can be 5–10× the input in emitted html. Both land in a WebView on a +phone. + +- a node budget in `HtmlConfig`, following `spreadsheet_limit` + (`html.hpp:124`) — `std::optional xml_node_limit`, `nullopt` + for unlimited — and, past it, stop and emit a visible truncation notice + rather than a silently short document. +- past a lower threshold, default the fold state to closed below some depth. + This is the only case where collapsed-by-default is right, and it is a + response to a measurement, not a preference. +- measure before choosing the numbers, on a real `content.xml`. + +## Stage 4 — the archive seam + +The filesystem view links every entry as an `application/octet-stream` data url +(`html/filesystem.cpp:110`), so browsing into a zip and looking at +`word/document.xml` downloads it. Routing entries through `html::translate` +instead is a separate feature with its own questions (which types, resource +paths, how deep), but it is the one that turns this from an xml-file viewer +into a way to inspect any office document's parts. Named here so the +dependency is on record; not scoped here. + +## Deferred, by decision + +- **Byte-faithful mode.** A lexer over the raw text, sharing the css, keeping + the author's formatting and — the real payoff — able to render a malformed + file up to the point where it breaks. Wanted; not wanted enough to build two + renderers before one exists. +- **XSLT.** An `` PI is shown as the processing instruction it + is. Applying it means an XSLT engine, which is larger than every format in + this repository put together. +- **Rendering xhtml as html.** Same class of decision, and the answer is no for + the same reason: this is a source viewer. +- **Namespace resolution.** pugixml does not process namespaces + (`svg/svg_util.cpp:15` works around exactly this), and a source view should + show the prefixes the file actually uses. Nothing to do. +- **Expand-all / collapse-all**, and **search within the tree** — both need + JavaScript, and neither is worth being the reason this view starts shipping a + script. +- **Dark mode**, which is a question for `frontend.cpp` as a whole. + +## Test data + +Content, not fixtures, for everything a string literal can express — the parser +and writer tests are inline. The test-data repo needs only what +`declaration_matches_the_engines` opens: a minified `content.xml` lifted from an +odt, a hand-formatted document with comments and a doctype, one non-UTF-8 +declared encoding, and one file that is xml-shaped but malformed. From bf0dc398b244d47e16276d3927e7a5d4beadc298 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 9 Aug 2026 13:42:49 +0200 Subject: [PATCH 2/2] docs(rtf): fix the control-word delimiter and \uN rules Two errors the review caught, both of which would have been written into the tokenizer as specified: - the space terminating a control word's numeric parameter is a delimiter and is consumed, so `\fs24 Text` starts at `T` and `\bin4 `'s payload starts after the space. The plan said no terminator was consumed, which injects a space into ordinary formatted text and shifts every binary read by one byte. - `\uN` is a UTF-16 code unit, not a code point. Anything above the BMP arrives as a surrogate pair of two `\uN`; appending each half turns an emoji into two replacement characters. Test list extended to cover both. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011ipdjdyMJ6LMVsfAnShvbm --- src/odr/internal/rtf/PLAN.md | 45 ++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/src/odr/internal/rtf/PLAN.md b/src/odr/internal/rtf/PLAN.md index 8489e900f..b82a859dd 100644 --- a/src/odr/internal/rtf/PLAN.md +++ b/src/odr/internal/rtf/PLAN.md @@ -93,13 +93,23 @@ accumulator is a byte buffer plus the run's `TextEncoding`, flushed through paragraph ends. Decoding per escape would corrupt every multibyte run and is the single easiest mistake to make here. -`\uN` is different — it is already a code point, so it bypasses the byte buffer -via `util::string::append_c32` (flushing the buffer first, to keep order). Two -traps: N is a *signed 16-bit* value, so `U+F020` arrives as `\u-4064` and must be -folded back with `+ 65536`; and `\ucN` gives the number of following characters -to skip as the ANSI fallback, is scoped like a character property (so it lives -in `RtfState`, restored by `}`), defaults to 1, and counts *any* control word or -symbol as one character — with a `\binN` and its payload counting as one. +`\uN` is different — it bypasses the byte buffer (flushing it first, to keep +order). Three traps: + +- N is a **UTF-16 code unit, not a code point**, and anything above the BMP + arrives as a *surrogate pair*: two consecutive `\uN`, each with its own `\ucN` + fallback in between. So a high surrogate is held pending and combined with the + low one that follows; only the combined code point goes through + `util::string::append_c32`. Appending each half on its own turns every emoji + into two replacement characters. An unpaired surrogate still standing at a + flush is U+FFFD. +- N is *signed 16-bit*, so `U+F020` arrives as `\u-4064` and must be folded back + with `+ 65536` — before the surrogate test, which is what the folded value is + for. +- `\ucN` gives the number of following characters to skip as the ANSI fallback, + is scoped like a character property (so it lives in `RtfState`, restored by + `}`), defaults to 1, and counts *any* control word or symbol as one + character — with a `\binN` and its payload counting as one. **Encoding comes from `internal/encoding`.** `\ansi`/`\mac`/`\pc`/`\pca` set a document default, `\ansicpgN` overrides it, the font table's `\fcharsetN` (or @@ -130,10 +140,14 @@ anything is layered on them. - `rtf_tokenizer.{hpp,cpp}` as above, with the delimiter rules from *Control Word* exactly: a control word is `\` + ASCII letters, terminated by a space - (consumed), by a digit or `-` (a parameter of up to 10 digits follows, itself - terminated by any non-digit, which is **not** consumed), or by any other - character (not consumed). A control *symbol* is `\` + one non-letter and takes - no delimiter — a space after it is text. + (consumed), by a digit or `-` (a parameter of up to 10 digits follows), or by + any other character (not consumed). **The parameter's terminator is a + delimiter under the same rule** — a space there is consumed, anything else is + left unread. So `\fs24 Text` starts its text at `T`, not at a space, and + `\bin4 ` puts the four raw bytes immediately after the consumed space; + swallowing that space is the difference between correct text and a payload + read shifted by one. A control *symbol* is `\` + one non-letter and takes no + delimiter — a space after it is text. - `rtf_state.{hpp,cpp}`: the group stack, the current destination, and the character/paragraph property structs (empty but for `\uc` in this stage). - destination handling: a table of known destinations; `{\*\unknown` skips to the @@ -158,10 +172,11 @@ anything is layered on them. - `CMakeLists.txt` `ODR_SOURCE_FILES`. **Tests** are inline string literals against the tokenizer and against a small -whole document — the delimiter rules (space eaten, digit kept, `\b0` vs `\b`), -`\'hh` in a windows-1252 run, `\uN` negative folding, `\ucN` skipping across a -control word, an unmatched `}`, an unterminated group, and a `\bin` payload -containing `}`. +whole document — the delimiter rules (space eaten after a control word *and* +after its parameter, digit kept, `\b0` vs `\b`), `\'hh` in a windows-1252 run, +`\uN` negative folding, a non-BMP character as a surrogate pair across two +`\uN`, an unpaired surrogate, `\ucN` skipping across a control word, an +unmatched `}`, an unterminated group, and a `\bin` payload containing `}`. ## Stage 2 — character formatting