feat(api): open a file from memory, and take a File wherever a path is taken - #662
Merged
Conversation
This was referenced Aug 8, 2026
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6fa0505da
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…s taken Every input entry point took a `std::string` path, so a caller holding bytes and no path — a download, a browser upload, a decrypted payload — had no way in. `internal::MemoryFile` already did the right thing, but no binding could reach it: binding the internals is exactly what the bindings are told not to do. `File` gains two named factories, `from_disk` and `from_memory`, and `DocumentFile` gains the same pair so the common case — bytes straight to a document — needs no intermediate handle. Named rather than constructors because `File(const std::string &)` already means *path*, so a `File(std::string)` overload would silently change what every existing call site does. The rest is symmetry: a caller holding a `File` now reaches everything a caller holding a path reaches — `DecodedFile::list_file_types`/`mimetype`, `DecodedFile(File, DecodePreference)`, `DocumentFile::type`/`meta` and its constructor, and the `odr::` free functions. The path forms become one-line forwards to `from_disk`, and `magic::mimetype` gains a `File` overload; it was the last place that reached for `DiskFile` on its own. The python binding exposes all of it. The `File` forms of `open` carry `py::call_guard<py::gil_scoped_release>` like their path siblings — decoding is long-running either way — and `DocumentFile.from_memory` copies the bytes out under the GIL before releasing it for the decode. The path overloads are now all one-line forwards to `from_disk`, so they can be deprecated and removed once `cli/`, `python/`, `jni/`, `apple/` and `test/` are migrated off them — CI builds with `-Werror`, so that order is forced. `File::from_disk` stays: it is the one place a path belongs. Output paths are a separate question — `File::copy`, `Document::save` and `HtmlService::bring_offline` genuinely write to disk. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjFJ66sma1ye9ZnhM3tNeH
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
Split out of #661 — no wasm in here.
Every input entry point took a
std::stringpath, so a caller holding bytes andno path — a download, a browser upload, a decrypted payload — had no way in.
internal::MemoryFilealready did the right thing, but no binding could reachit.
File::from_disk(path)andFile::from_memory(data), and the same pair onDocumentFileso the common case — bytes straight to a document — needs nointermediate handle. Named factories rather than constructors, because
File(const std::string &)already means path.Fileoverload wherever a path is taken:DecodedFile::list_file_types/mimetype,DecodedFile(File, DecodePreference),DocumentFile::type/metaand its constructor, and theodr::free functions. The path formsbecome one-line forwards to
from_disk.magic::mimetypegains aFileoverload — it was the last place that reachedfor
DiskFileon its own. The internals keep takingconst std::shared_ptr<abstract::File> &:open_strategyhands the file on toZipFile/TextFile/PdfFileand friends, which store it, so that is theparameter type that says what actually happens.
Fileforms ofopencarrypy::call_guard<py::gil_scoped_release>like their path siblings — decoding islong-running either way — and
DocumentFile.from_memorycopies the bytes outunder the GIL before releasing it for the decode.
Follow-up, not in scope: the path overloads are now pure forwards, so they can
be deprecated and removed once
cli/,python/,jni/,apple/andtest/are migrated off them. CI builds with
-Werror, so that order is forced.File::from_diskstays.Tested: full C++ suite (764 passed, 9 skipped at the top of the stack) and
the pytest suite (59 passed), which gains coverage for both factories, the
Fileentry points, binary round-tripping throughfrom_memory, and that theFileand path forms agree.