refactor(api)!: one way to open a file - #836
Merged
Merged
Conversation
andiwand
force-pushed
the
refactor/one-way-to-detect
branch
from
September 6, 2026 13:36
3ba5f19 to
0e2a00c
Compare
odr::open was six one-line forwards to six DecodedFile constructors, and the split had already reached the bindings: jni exposed both, apple went through the constructors, python bound each. The bodies move to open and the constructors go; DecodedFile keeps only the one that wraps a decoded impl. DocumentFile loses its factories and its decoding constructors with them. It was never a second kind of open - open_document_file repeated open_file's cascade engine for engine for zip and cfb, and both ends threw NoDocumentFile, which as_document_file() throws too. open(...).as_document_file() is the same call, so 73 lines of open_strategy go as well. Java gains Odr.open(File), which is what the removed DecodedFile(File) constructor was. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016hxDa2rev11eLUEJZJ5nmz
Its open() and openAs() built the handle directly rather than going through odr::open. Qualified, because the file has an open() of its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016hxDa2rev11eLUEJZJ5nmz
andiwand
force-pushed
the
refactor/one-way-to-open
branch
from
September 6, 2026 13:46
ca771d0 to
25dd861
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 Generated with Claude Code
PR 6 of the v7 API plan.
Stacked on #835 — dropping
DocumentFile::type/::metathere is whatleaves these constructors with no callers. Review #835 first; base retargets to
mainwhen it merges.odr::openhad a twinodr.cpp:203-230was six one-linereturn DecodedFile(...). Not a wrapper witha purpose — the same call under a second name, and the split had already spread:
Odr.open(path)andnew DecodedFile(path)), withthe javadoc admitting it: "Obtain via
Odr#openor the constructors"DecodedFile.__init__overloads and sixopenfunctionsThe bodies move into
odr::open.DecodedFilekeeps only the constructor thatwraps an already-decoded impl.
DocumentFilewas never a second kind of openDocumentFile::from_disk,::from_memory,DocumentFile(File)andDocumentFile(path)all reachedopen_strategy::open_document_file— whichrepeats
open_file's cascade engine for engine for zip and cfb, differingonly in the fallback, and both ends throw
NoDocumentFile.as_document_file()throws it too (
file.cpp:278).So
open(f).as_document_file()is not an approximation ofDocumentFile(f)—same engines, same order, same exception. 73 lines of
open_strategy.cppgowith it (737 → 664).
Bindings
new DecodedFile(path),(path, as),(file),new DocumentFile(path)Odr.open(...),.asDocumentFile()DecodedFile(...)×5,DocumentFile(...)×2,DocumentFile.from_disk/.from_memorypyodr.open(...),.as_document_file()decode(...)now callsodr::openJava gains
Odr.open(File), which is exactly what the removedDecodedFile(File)constructor was — otherwise an openedFilewould have hadno public way to decode.
About the test churn
~85 call sites moved. I rewrote them with a balanced-paren pass rather than a
regex, because a first attempt with a naive regex silently mangled a function
declaration (
DecodedFile csv_file(const std::uint32_t rows, ...)becameDecodedFile csv_file = open(const std::uint32_t rows, ...)). The final passguards against parameter-declaration shapes and leaves impl-handle
constructions alone —
DecodedFile(xml_file(content))and the rtf/svg casespass a
shared_ptrto an internal impl, so they keep the impl constructor.Two internal tests called
open_strategy::open_document_filedirectly and nowgo through
open_file+as_document_file().Verified
Full build clean. Full gtest suite: 1456 passed, 6 skipped (the same six
that skip on
main). 69 python tests and the JNI junit suite pass.Migration
DecodedFile(path) → odr::open(path) DecodedFile(file, as) → odr::open(file, as) DocumentFile(path) → odr::open(path).as_document_file() DocumentFile::from_memory(bytes) → odr::open(File::from_memory(bytes)).as_document_file()