Skip to content

refactor(html)!: cut translate to its four inputs - #834

Merged
andiwand merged 3 commits into
mainfrom
refactor/translate-four-inputs
Sep 6, 2026
Merged

refactor(html)!: cut translate to its four inputs#834
andiwand merged 3 commits into
mainfrom
refactor/translate-four-inputs

Conversation

@andiwand

@andiwand andiwand commented Sep 6, 2026

Copy link
Copy Markdown
Member

🤖 Generated with Claude Code

PR 4 of the v7 API plan.

html::translate had twenty overloads answering four questions.

The ten cache_path ones

Removed. The header already documented the parameter as ignored, and the
implementation was ten one-line forwards under a comment saying so. Nothing on
the render path writes to disk.

The interesting part is what the callers were doing with it. cli/server.cpp
built a directory per prefix and remove_all'd a cache root on startup;
test/html_test.cpp and the media/image tests each built a path to hand over.
All of it fed a parameter that was discarded — so this PR deletes a fair amount
of create_directories that made directories nobody wrote into.

The six narrowed-handle ones

translate(TextFile), (ImageFile), (ArchiveFile), (DocumentFile),
(PdfFile), (FontFile) are gone from the header. They are not lost work —
translate(DecodedFile) dispatches to every one of them, so they stay exactly
where they were, as file-local helpers behind the dispatcher:

namespace {
HtmlService translate_text_file(const TextFile &, const HtmlConfig &, const Logger &);
HtmlService translate_image_file(const ImageFile &, ...);
...
}  // namespace

Two of them delegate back out to html::translate and now say so with a
qualified name — an anonymous namespace in odr does not see odr::html, which
the previous html::translate(...) definition context did.

What remains

Four, for the four inputs that genuinely differ:

HtmlService translate(const DecodedFile &, const HtmlConfig &, const Logger & = …);
HtmlService translate(const Document &,   const HtmlConfig &, const Logger & = …);
HtmlService translate(const Filesystem &, const HtmlConfig &, const Logger & = …);
HtmlService translate(const Archive &,    const HtmlConfig &, const Logger & = …);

Bindings

The cache argument goes from all of them, so this is breaking for every
consumer:

before after
Java Html.translate(file, cachePath, config) Html.translate(file, config)
Python pyodr.html.translate(file, cache_path, config) pyodr.html.translate(file, config)
Swift HtmlTranslator.translate(file:cachePath:config:) HtmlTranslator.translate(file:config:)

wasm already called the cache-free overloads and is unchanged.

Net −344/+162, most of it deletion.

Verified

Full build (library, CLI, JNI + jar, python module, tests) clean. 195
html*/Html*/*Image*/*Media*/*Font*/*Csv*/*Markdown* gtests, 69
python tests, and the JNI junit suite via ctest --test-dir jni all pass.
Apple and its Swift tests are signature edits with no behaviour change.

Migration

Drop the cache path argument. If you built a directory to pass to it, drop the
directory too — nothing was ever written there.

Twenty overloads answered four questions. Ten took a cache_path that nothing
has read since the output became a set of streams, and the header said so.
Six more took a narrowed file handle - TextFile, ImageFile, ArchiveFile,
DocumentFile, PdfFile, FontFile - that translate(DecodedFile) already
dispatched to, so they were a second way to reach the same call.

Those six stay as file-local helpers behind the dispatcher, which is all they
ever were. What the header offers is the four inputs that genuinely differ: a
DecodedFile, a Document, a Filesystem, an Archive.

Every binding loses the cache argument with them, and the callers that built a
directory to pass lose the directory too.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016hxDa2rev11eLUEJZJ5nmz
@andiwand
andiwand force-pushed the refactor/translate-four-inputs branch from 180db2f to 1991e1e Compare September 6, 2026 13:12
It is the only way to the numbered line list for a file that
translate(DecodedFile) renders as something else - an xml as its source view,
a csv as a table - which the dispatcher's own comment says and
XmlHtml.translating_it_as_a_text_file_still_writes_the_line_list asserts. The
other five narrowed-handle overloads really did produce identical output and
stay gone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016hxDa2rev11eLUEJZJ5nmz
@andiwand

andiwand commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

🤖 Correction to this PR, found by building the whole v7 series together and running the full suite.

translate(TextFile) is back. I claimed all seven narrowed-handle overloads were reachable through translate(DecodedFile). Six were. TextFile is not:

// html.cpp, in the dispatcher, before the text branch
if (file.file_type() == FileType::xml) {
  return internal::html::create_xml_service(...);   // source view, not lines
}

translate(DecodedFile) deliberately renders an xml as its source view and a csv as a table. Narrowing to TextFile by hand was the only way to ask for the numbered line list instead — the dispatcher's own comment says so ("Translating it as a text file by hand still writes the line list"), and XmlHtml.translating_it_as_a_text_file_still_writes_the_line_list asserts it.

So this cuts twenty overloads to five, not four: DecodedFile, TextFile, Document, Filesystem, Archive. The header now says why TextFile is there.

How I missed it: the verification note above says "195 gtests" — that was a filtered run (html*, Html*, *Image*, *Media*, *Font*, *Csv*, *Markdown*) which does not include XmlHtml.*. I did not run the full suite on this branch, and the claim was broader than the check. Now run: 1460 passed, plus 68 python tests and the JNI junit suite.

The other five removals are unaffected — those really do produce byte-identical output through the dispatcher.

@andiwand
andiwand force-pushed the refactor/translate-four-inputs branch from d840623 to ac76e05 Compare September 6, 2026 13:18
Opening the bytes as FileType::text_file lands them in the dispatcher's text
branch, which writes the numbered line list - so choosing the rendering is the
open's job, not a second overload's. open_strategy handles `as ==
text_file` and an xml already reports [text_file, xml, ...] among its types.
DecodedFile::file() hands the bytes back, so a caller holding a decoded handle
can reopen without keeping the original around.

Reverts the overload restored in d840623 and points its test at the open.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012QiYawTVZyuMLQBQngdTeu
@andiwand
andiwand force-pushed the refactor/translate-four-inputs branch from ac76e05 to 0b0d00b Compare September 6, 2026 13:31
@andiwand
andiwand merged commit 6277bdb into main Sep 6, 2026
25 checks passed
@andiwand
andiwand deleted the refactor/translate-four-inputs branch September 6, 2026 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant