From 1991e1ed3e894d4e650632057b5f31d76b3b072c Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 6 Sep 2026 13:56:26 +0200 Subject: [PATCH 1/3] refactor(html)!: cut translate to its four inputs 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) Claude-Session: https://claude.ai/code/session_016hxDa2rev11eLUEJZJ5nmz --- CHANGELOG.md | 4 + .../app/opendocument/core/DocumentTest.kt | 6 +- .../app/opendocument/core/HttpServerTest.kt | 4 +- apple/README.md | 4 +- apple/include/OdrCoreObjC/ODRHtml.h | 17 +- apple/src/ODRHtml.mm | 14 +- apple/tests/OdrCoreTests.swift | 9 +- cli/src/server.cpp | 15 +- cli/src/translate.cpp | 2 +- jni/README.md | 2 +- jni/java/app/opendocument/core/Html.java | 21 +-- jni/src/jni_html.cpp | 11 +- jni/tests/app/opendocument/core/HtmlTest.java | 23 +-- .../app/opendocument/core/HttpServerTest.java | 3 +- python/pyodr/__init__.py | 2 +- python/pyodr/cli.py | 6 +- python/src/bind_html.cpp | 24 +-- python/tests/test_html.py | 51 +++--- python/tests/test_http_server.py | 6 +- src/odr/html.cpp | 157 ++++++------------ src/odr/html.hpp | 67 +------- test/src/html_output_test.cpp | 3 +- test/src/html_test.cpp | 21 +-- test/src/internal/csv/csv_file_test.cpp | 3 +- test/src/internal/font/font_file.cpp | 4 +- test/src/internal/html/image_file_test.cpp | 10 +- test/src/internal/html/media_file_test.cpp | 24 +-- test/src/internal/xml/xml_file_test.cpp | 9 +- 28 files changed, 165 insertions(+), 357 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d687487fc..29a5dfeb7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- **Breaking**: `html::translate` is four overloads instead of twenty, taking a + `DecodedFile`, `Document`, `Filesystem` or `Archive`. The `cache_path` and + narrowed-handle ones are gone, in every binding too: drop the cache path. + - **Breaking**: the inert `HtmlConfig` fields `background_image_format`, `background_image_dpi`, `no_drm` and `embed_outline` are gone, with their java, python, objc and wasm mirrors. Drop them; nothing replaces them. diff --git a/android/src/androidTest/java/app/opendocument/core/DocumentTest.kt b/android/src/androidTest/java/app/opendocument/core/DocumentTest.kt index 230132485..f3a74c66f 100644 --- a/android/src/androidTest/java/app/opendocument/core/DocumentTest.kt +++ b/android/src/androidTest/java/app/opendocument/core/DocumentTest.kt @@ -85,11 +85,10 @@ class DocumentTest { @Test fun translateToHtml() { - val cache = Files.createDirectories(tempDir.resolve("cache")) val output = Files.createDirectories(tempDir.resolve("output")) val file = Odr.open(TestFiles.odtFile(tempDir).toString()) - val service = Html.translate(file, cache.toString(), HtmlConfig()) + val service = Html.translate(file, HtmlConfig()) val html = service.bringOffline(output.toString()) val pages = html.pages() @@ -112,9 +111,8 @@ class DocumentTest { @Test fun translateCsv() { - val cache = Files.createDirectories(tempDir.resolve("csv-cache")) val file = Odr.open(TestFiles.csvFile(tempDir).toString()) - val service = Html.translate(file, cache.toString(), HtmlConfig()) + val service = Html.translate(file, HtmlConfig()) // a spreadsheet: a document view plus one per sheet val views = service.listViews() diff --git a/android/src/androidTest/java/app/opendocument/core/HttpServerTest.kt b/android/src/androidTest/java/app/opendocument/core/HttpServerTest.kt index c6307a14a..16ecada87 100644 --- a/android/src/androidTest/java/app/opendocument/core/HttpServerTest.kt +++ b/android/src/androidTest/java/app/opendocument/core/HttpServerTest.kt @@ -4,7 +4,6 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 import java.io.IOException import java.net.HttpURLConnection import java.net.URL -import java.nio.file.Files import java.nio.file.Path import java.util.concurrent.atomic.AtomicReference import org.junit.Assert.assertEquals @@ -60,12 +59,11 @@ class HttpServerTest { val server = HttpServer() - val cachePath = Files.createDirectories(tempDir.resolve("doc-cache")).toString() val file = Odr.open(TestFiles.odtFile(tempDir).toString()) val htmlConfig = HtmlConfig() htmlConfig.embedImages = false htmlConfig.relativeResourcePaths = false - val service = Html.translate(file, cachePath, htmlConfig) + val service = Html.translate(file, htmlConfig) server.connectService(service, "doc") val views = service.listViews() assertEquals(1, views.size) diff --git a/apple/README.md b/apple/README.md index ff54e6657..5755bd265 100644 --- a/apple/README.md +++ b/apple/README.md @@ -24,7 +24,7 @@ Requires iOS 15 or macOS 12. let file = try DecodedFile.decode(path: path) let config = HtmlConfig() let service = try HtmlTranslator.translate( - file: file, cachePath: cacheDirectory, config: config) + file: file, config: config) for view in service.views { var resources: NSArray? @@ -50,7 +50,7 @@ it beats writing every page to disk up front. ```swift let service = try HtmlTranslator.translate( - file: file, cachePath: cacheDirectory, config: HtmlConfig()) + file: file, config: HtmlConfig()) let server = HttpServer() try server.connect(service, prefix: "doc") diff --git a/apple/include/OdrCoreObjC/ODRHtml.h b/apple/include/OdrCoreObjC/ODRHtml.h index e482b6d73..e12cd2efc 100644 --- a/apple/include/OdrCoreObjC/ODRHtml.h +++ b/apple/include/OdrCoreObjC/ODRHtml.h @@ -257,39 +257,34 @@ NS_SWIFT_NAME(HtmlService) NS_SWIFT_NAME(HtmlTranslator) @interface ODRHtmlTranslator : NSObject -/// Translates a decoded file. `cachePath` is a directory for temporary output. +/// Translates a decoded file, dispatching on what it decoded to. + (nullable ODRHtmlService *)translateFile:(ODRDecodedFile *)file - cachePath:(NSString *)cachePath config:(ODRHtmlConfig *)config error:(NSError **)error - NS_SWIFT_NAME(translate(file:cachePath:config:)); + NS_SWIFT_NAME(translate(file:config:)); + (nullable ODRHtmlService *)translateFile:(ODRDecodedFile *)file - cachePath:(NSString *)cachePath config:(ODRHtmlConfig *)config logger:(ODRLogger *)logger error:(NSError **)error - NS_SWIFT_NAME(translate(file:cachePath:config:logger:)); + NS_SWIFT_NAME(translate(file:config:logger:)); /// Translates an already-decoded document. + (nullable ODRHtmlService *)translateDocument:(ODRDocument *)document - cachePath:(NSString *)cachePath config:(ODRHtmlConfig *)config error:(NSError **)error - NS_SWIFT_NAME(translate(document:cachePath:config:)); + NS_SWIFT_NAME(translate(document:config:)); /// Translates a filesystem — a document's parts, or an archive's contents. + (nullable ODRHtmlService *)translateFilesystem:(ODRFilesystem *)filesystem - cachePath:(NSString *)cachePath config:(ODRHtmlConfig *)config error:(NSError **)error - NS_SWIFT_NAME(translate(filesystem:cachePath:config:)); + NS_SWIFT_NAME(translate(filesystem:config:)); /// Translates an archive. + (nullable ODRHtmlService *)translateArchive:(ODRArchive *)archive - cachePath:(NSString *)cachePath config:(ODRHtmlConfig *)config error:(NSError **)error - NS_SWIFT_NAME(translate(archive:cachePath:config:)); + NS_SWIFT_NAME(translate(archive:config:)); /// Applies a diff produced by the browser-side JavaScript back to `document`. + (BOOL)editDocument:(ODRDocument *)document diff --git a/apple/src/ODRHtml.mm b/apple/src/ODRHtml.mm index 4f3c70b98..6d47171ca 100644 --- a/apple/src/ODRHtml.mm +++ b/apple/src/ODRHtml.mm @@ -504,61 +504,51 @@ - (nullable ODRHtml *)bringOfflineTo:(NSString *)outputPath @implementation ODRHtmlTranslator + (nullable ODRHtmlService *)translateFile:(ODRDecodedFile *)file - cachePath:(NSString *)cachePath config:(ODRHtmlConfig *)config error:(NSError **)error { return [ODRHtmlTranslator translateFile:file - cachePath:cachePath config:config logger:ODRLogger.null error:error]; } + (nullable ODRHtmlService *)translateFile:(ODRDecodedFile *)file - cachePath:(NSString *)cachePath config:(ODRHtmlConfig *)config logger:(ODRLogger *)logger error:(NSError **)error { return guarded(error, [&]() -> ODRHtmlService * { return [ODRHtmlService - serviceWithHandle:odr::html::translate( - file.handle, to_string(cachePath), - config.nativeConfig, logger.handle)]; + serviceWithHandle:odr::html::translate(file.handle, config.nativeConfig, + logger.handle)]; }); } + (nullable ODRHtmlService *)translateDocument:(ODRDocument *)document - cachePath:(NSString *)cachePath config:(ODRHtmlConfig *)config error:(NSError **)error { return guarded(error, [&]() -> ODRHtmlService * { return [ODRHtmlService serviceWithHandle:odr::html::translate(document.handle, - to_string(cachePath), config.nativeConfig)]; }); } + (nullable ODRHtmlService *)translateFilesystem:(ODRFilesystem *)filesystem - cachePath:(NSString *)cachePath config:(ODRHtmlConfig *)config error:(NSError **)error { return guarded(error, [&]() -> ODRHtmlService * { return [ODRHtmlService serviceWithHandle:odr::html::translate(filesystem.handle, - to_string(cachePath), config.nativeConfig)]; }); } + (nullable ODRHtmlService *)translateArchive:(ODRArchive *)archive - cachePath:(NSString *)cachePath config:(ODRHtmlConfig *)config error:(NSError **)error { return guarded(error, [&]() -> ODRHtmlService * { return [ODRHtmlService serviceWithHandle:odr::html::translate(archive.handle, - to_string(cachePath), config.nativeConfig)]; }); } diff --git a/apple/tests/OdrCoreTests.swift b/apple/tests/OdrCoreTests.swift index da7530494..e5f503fff 100644 --- a/apple/tests/OdrCoreTests.swift +++ b/apple/tests/OdrCoreTests.swift @@ -121,7 +121,7 @@ final class HtmlTests: XCTestCase { private func service() throws -> HtmlService { let file = try DecodedFile.decode(path: try Fixture.odt()) return try HtmlTranslator.translate( - file: file, cachePath: try temporaryDirectory(), config: HtmlConfig()) + file: file, config: HtmlConfig()) } func testRendersHtml() throws { @@ -152,7 +152,7 @@ final class HtmlTests: XCTestCase { let file = try DecodedFile.decode(path: try Fixture.odt()) let service = try HtmlTranslator.translate( - file: file, cachePath: try temporaryDirectory(), config: config) + file: file, config: config) var resources: NSArray? let html = try XCTUnwrap(service.views.first).writeHtml(resources: &resources) @@ -188,7 +188,7 @@ final class HtmlTests: XCTestCase { "alpha,beta\ngamma,delta\nepsilon,zeta\n", as: "table.csv") let file = try DecodedFile.decode(path: path) let service = try HtmlTranslator.translate( - file: file, cachePath: try temporaryDirectory(), config: config) + file: file, config: config) var resources: NSArray? let html = try XCTUnwrap(service.views.first).writeHtml(resources: &resources) @@ -242,8 +242,7 @@ final class HttpServerTests: XCTestCase { let config = HtmlConfig() config.relativeResourcePaths = false let service = try HtmlTranslator.translate( - file: try DecodedFile.decode(path: try Fixture.odt()), - cachePath: try temporaryDirectory(), config: config) + file: try DecodedFile.decode(path: try Fixture.odt()), config: config) let server = HttpServer() try server.connect(service, prefix: "doc") diff --git a/cli/src/server.cpp b/cli/src/server.cpp index 034765cf5..93fc89f1a 100644 --- a/cli/src/server.cpp +++ b/cli/src/server.cpp @@ -6,7 +6,6 @@ #include #include -#include #include #include @@ -48,12 +47,6 @@ int main(const int argc, char **argv) { const HttpServer server{{}, logger}; - // the server does not own a cache any more, so the translation goes - // somewhere of our choosing - const std::filesystem::path cache_path = - std::filesystem::temp_directory_path() / "odr-server"; - std::filesystem::remove_all(cache_path); - // bind before anything is printed: the port is only known once the socket // is, and it is not necessarily the one that was asked for const std::uint32_t port = server.bind("localhost", 8080); @@ -66,11 +59,9 @@ int main(const int argc, char **argv) { { const std::string prefix = "file"; - const std::string prefix_cache_path = (cache_path / prefix).string(); - std::filesystem::create_directories(prefix_cache_path); const HtmlService service = - html::translate(decoded_file, prefix_cache_path, html_config, logger); + html::translate(decoded_file, html_config, logger); server.connect_service(service, prefix); const HtmlViews views = service.list_views(); ODR_INFO(logger, "hosted decoded file with id: " << prefix); @@ -86,11 +77,9 @@ int main(const int argc, char **argv) { : decoded_file.as_archive_file().archive().as_filesystem(); const std::string prefix = "filesystem"; - const std::string prefix_cache_path = (cache_path / prefix).string(); - std::filesystem::create_directories(prefix_cache_path); const HtmlService filesystem_service = - html::translate(filesystem, prefix_cache_path, html_config, logger); + html::translate(filesystem, html_config, logger); server.connect_service(filesystem_service, prefix); ODR_INFO(logger, "hosted filesystem with id: " << prefix); for (const auto &view : filesystem_service.list_views()) { diff --git a/cli/src/translate.cpp b/cli/src/translate.cpp index fbb28b7d3..32e2695fc 100644 --- a/cli/src/translate.cpp +++ b/cli/src/translate.cpp @@ -46,7 +46,7 @@ int main(const int argc, char **argv) { config.format_html = true; std::filesystem::create_directories(output); - const HtmlService service = html::translate(decoded_file, output, config); + const HtmlService service = html::translate(decoded_file, config); const Html html = service.bring_offline(output); return 0; diff --git a/jni/README.md b/jni/README.md index 451e62381..e3f0e0a1f 100644 --- a/jni/README.md +++ b/jni/README.md @@ -11,7 +11,7 @@ import app.opendocument.core.HtmlService; import app.opendocument.core.Odr; DecodedFile file = Odr.open("document.odt"); -HtmlService service = Html.translate(file, "cache-dir", new HtmlConfig()); +HtmlService service = Html.translate(file, new HtmlConfig()); app.opendocument.core.Html html = service.bringOffline("output-dir"); for (var page : html.pages()) { System.out.println(page.name + " " + page.path); diff --git a/jni/java/app/opendocument/core/Html.java b/jni/java/app/opendocument/core/Html.java index b3e021bee..c6a7701f1 100644 --- a/jni/java/app/opendocument/core/Html.java +++ b/jni/java/app/opendocument/core/Html.java @@ -55,28 +55,27 @@ public static final class LocatedResource { // the wrapper for the duration - keepAlive() does. /** Translates a decoded file to HTML. */ - public static HtmlService translate(DecodedFile file, String cachePath, HtmlConfig config) { + public static HtmlService translate(DecodedFile file, HtmlConfig config) { try { - return new HtmlService(translateFile(file.handle(), cachePath, config), file); + return new HtmlService(translateFile(file.handle(), config), file); } finally { file.keepAlive(); } } /** Translates a document to HTML. */ - public static HtmlService translate(Document document, String cachePath, HtmlConfig config) { + public static HtmlService translate(Document document, HtmlConfig config) { try { - return new HtmlService(translateDocument(document.handle(), cachePath, config), document); + return new HtmlService(translateDocument(document.handle(), config), document); } finally { document.keepAlive(); } } /** Translates a filesystem to HTML. */ - public static HtmlService translate(Filesystem filesystem, String cachePath, HtmlConfig config) { + public static HtmlService translate(Filesystem filesystem, HtmlConfig config) { try { - return new HtmlService( - translateFilesystem(filesystem.handle(), cachePath, config), filesystem); + return new HtmlService(translateFilesystem(filesystem.handle(), config), filesystem); } finally { filesystem.keepAlive(); } @@ -87,11 +86,9 @@ public static void edit(Document document, String diff) { document.edit(diff); } - private static native long translateFile(long fileHandle, String cachePath, HtmlConfig config); + private static native long translateFile(long fileHandle, HtmlConfig config); - private static native long translateDocument( - long documentHandle, String cachePath, HtmlConfig config); + private static native long translateDocument(long documentHandle, HtmlConfig config); - private static native long translateFilesystem( - long filesystemHandle, String cachePath, HtmlConfig config); + private static native long translateFilesystem(long filesystemHandle, HtmlConfig config); } diff --git a/jni/src/jni_html.cpp b/jni/src/jni_html.cpp index 2de882b60..5e5b18a1e 100644 --- a/jni/src/jni_html.cpp +++ b/jni/src/jni_html.cpp @@ -177,24 +177,21 @@ odr::HtmlResource &resource(jlong handle) { extern "C" JNIEXPORT jlong JNICALL Java_app_opendocument_core_Html_translateFile(JNIEnv *env, jclass, jlong file_handle, - jstring cache_path, jobject config) { return guarded(env, [&] { - return make_handle(odr::html::translate( - *from_handle(file_handle), to_string(env, cache_path), - odr_jni::html_config_from_java(env, config))); + return make_handle( + odr::html::translate(*from_handle(file_handle), + odr_jni::html_config_from_java(env, config))); }); } extern "C" JNIEXPORT jlong JNICALL Java_app_opendocument_core_Html_translateDocument(JNIEnv *env, jclass, jlong document_handle, - jstring cache_path, jobject config) { return guarded(env, [&] { return make_handle( odr::html::translate(*from_handle(document_handle), - to_string(env, cache_path), odr_jni::html_config_from_java(env, config))); }); } @@ -202,12 +199,10 @@ Java_app_opendocument_core_Html_translateDocument(JNIEnv *env, jclass, extern "C" JNIEXPORT jlong JNICALL Java_app_opendocument_core_Html_translateFilesystem(JNIEnv *env, jclass, jlong filesystem_handle, - jstring cache_path, jobject config) { return guarded(env, [&] { return make_handle( odr::html::translate(*from_handle(filesystem_handle), - to_string(env, cache_path), odr_jni::html_config_from_java(env, config))); }); } diff --git a/jni/tests/app/opendocument/core/HtmlTest.java b/jni/tests/app/opendocument/core/HtmlTest.java index abb898442..f23e9d622 100644 --- a/jni/tests/app/opendocument/core/HtmlTest.java +++ b/jni/tests/app/opendocument/core/HtmlTest.java @@ -16,17 +16,15 @@ class HtmlTest { @TempDir Path tempDir; private Html translateOffline(Path input) throws IOException { - Path cache = Files.createDirectories(tempDir.resolve("cache")); Path output = Files.createDirectories(tempDir.resolve("output")); DecodedFile file = Odr.open(input.toString()); - HtmlService service = Html.translate(file, cache.toString(), new HtmlConfig()); + HtmlService service = Html.translate(file, new HtmlConfig()); return service.bringOffline(output.toString()); } private String renderOdt(HtmlConfig config) throws IOException { - Path cache = Files.createTempDirectory(tempDir, "render"); DecodedFile file = Odr.open(TestFiles.odtFile(tempDir).toString()); - HtmlService service = Html.translate(file, cache.toString(), config); + HtmlService service = Html.translate(file, config); return service.listViews().get(0).writeHtml().html; } @@ -52,9 +50,8 @@ void viewportConfigRoundTrips() throws IOException { config.viewportWidth = 420; config.initialZoom = 1.5; - Path cache = Files.createDirectories(tempDir.resolve("cache")); DecodedFile file = Odr.open(TestFiles.odtFile(tempDir).toString()); - HtmlConfig readBack = Html.translate(file, cache.toString(), config).config(); + HtmlConfig readBack = Html.translate(file, config).config(); assertEquals(HtmlViewportMode.FIT_WIDTH, readBack.viewportMode); assertEquals(HtmlViewportMode.ACTUAL_SIZE, readBack.spreadsheetViewportMode); @@ -80,9 +77,8 @@ void minContentMarginReachesTheHtml() throws IOException { assertTrue( renderOdt(config).contains(":root{--odr-min-margin-top:12px;--odr-min-margin-left:1cm;}")); - Path cache = Files.createDirectories(tempDir.resolve("margin")); DecodedFile file = Odr.open(TestFiles.odtFile(tempDir).toString()); - HtmlConfig readBack = Html.translate(file, cache.toString(), config).config(); + HtmlConfig readBack = Html.translate(file, config).config(); assertEquals(new Measure(12, "px"), readBack.minContentMargin.top); assertEquals(new Measure(1, "cm"), readBack.minContentMargin.left); assertNull(readBack.minContentMargin.right); @@ -128,9 +124,8 @@ void colorSchemeReachesTheHtml() throws IOException { system.colorScheme = HtmlColorScheme.SYSTEM; assertTrue(renderOdt(system).contains("media=\"(prefers-color-scheme: dark)\"")); - Path cache = Files.createDirectories(tempDir.resolve("scheme")); DecodedFile file = Odr.open(TestFiles.odtFile(tempDir).toString()); - HtmlConfig readBack = Html.translate(file, cache.toString(), system).config(); + HtmlConfig readBack = Html.translate(file, system).config(); assertEquals(HtmlColorScheme.SYSTEM, readBack.colorScheme); } @@ -166,18 +161,17 @@ void translateDocument() throws IOException { void spreadsheetCutReachesTheView() throws IOException { assertEquals(Long.valueOf(500000L), new HtmlConfig().spreadsheetCellLimit); - Path cache = Files.createDirectories(tempDir.resolve("cache")); DecodedFile file = Odr.open(TestFiles.csvFile(tempDir).toString()); HtmlConfig full = new HtmlConfig(); - for (HtmlView view : Html.translate(file, cache.toString(), full).listViews()) { + for (HtmlView view : Html.translate(file, full).listViews()) { assertNull(view.sheetCut()); } HtmlConfig cut = new HtmlConfig(); cut.spreadsheetLimit = new TableDimensions(2, 1); cut.spreadsheetCellLimit = null; - HtmlService service = Html.translate(file, cache.toString(), cut); + HtmlService service = Html.translate(file, cut); assertNull(service.config().spreadsheetCellLimit); HtmlSheetCut sheetCut = service.listViews().get(1).sheetCut(); @@ -190,9 +184,8 @@ void spreadsheetCutReachesTheView() throws IOException { @Test void htmlServiceViews() throws IOException { - Path cache = Files.createDirectories(tempDir.resolve("cache")); DecodedFile file = Odr.open(TestFiles.odtFile(tempDir).toString()); - HtmlService service = Html.translate(file, cache.toString(), new HtmlConfig()); + HtmlService service = Html.translate(file, new HtmlConfig()); List views = service.listViews(); assertEquals(1, views.size()); diff --git a/jni/tests/app/opendocument/core/HttpServerTest.java b/jni/tests/app/opendocument/core/HttpServerTest.java index 6387fe301..bae3ceca0 100644 --- a/jni/tests/app/opendocument/core/HttpServerTest.java +++ b/jni/tests/app/opendocument/core/HttpServerTest.java @@ -61,11 +61,10 @@ void serveFile() throws Exception { HttpServer server = new HttpServer(); // the server hosts what it is given; translating is the caller's business - String cachePath = Files.createDirectories(tempDir.resolve("doc-cache")).toString(); DecodedFile file = Odr.open(TestFiles.odtFile(tempDir).toString()); HtmlConfig htmlConfig = new HtmlConfig(); htmlConfig.embedImages = false; - HtmlService service = Html.translate(file, cachePath, htmlConfig); + HtmlService service = Html.translate(file, htmlConfig); server.connectService(service, "doc"); List views = service.listViews(); assertEquals(1, views.size()); diff --git a/python/pyodr/__init__.py b/python/pyodr/__init__.py index 63ec87030..9aac6de33 100644 --- a/python/pyodr/__init__.py +++ b/python/pyodr/__init__.py @@ -6,7 +6,7 @@ Example: >>> import pyodr >>> file = pyodr.open("document.odt") - >>> service = pyodr.html.translate(file, cache_path, pyodr.HtmlConfig()) + >>> service = pyodr.html.translate(file, pyodr.HtmlConfig()) >>> html = service.bring_offline(output_path) >>> [page.path for page in html.pages()] """ diff --git a/python/pyodr/cli.py b/python/pyodr/cli.py index 6428f8ec8..e74e37739 100644 --- a/python/pyodr/cli.py +++ b/python/pyodr/cli.py @@ -42,9 +42,8 @@ def _translate(args, file) -> int: output.mkdir(parents=True, exist_ok=True) else: output = Path(tempfile.mkdtemp(prefix="pyodr-")) - cache = tempfile.mkdtemp(prefix="pyodr-cache-") - service = pyodr.html.translate(file, cache, pyodr.HtmlConfig()) + service = pyodr.html.translate(file, pyodr.HtmlConfig()) html = service.bring_offline(str(output)) for page in html.pages(): @@ -62,8 +61,7 @@ def _serve(args, file) -> int: html_config = pyodr.HtmlConfig() html_config.embed_images = False - cache = tempfile.mkdtemp(prefix="pyodr-server-") - service = pyodr.html.translate(file, cache, html_config) + service = pyodr.html.translate(file, html_config) prefix = "file" server = pyodr.HttpServer() diff --git a/python/src/bind_html.cpp b/python/src/bind_html.cpp index a1a5ae2e9..cd1f2bdcb 100644 --- a/python/src/bind_html.cpp +++ b/python/src/bind_html.cpp @@ -211,31 +211,31 @@ void odr_python::bind_html(py::module_ &m) { // GIL themselves. html.def( "translate", - [](const odr::DecodedFile &file, const std::string &cache_path, - const odr::HtmlConfig &config, const odr::Logger &logger) { - return odr::html::translate(file, cache_path, config, logger); + [](const odr::DecodedFile &file, const odr::HtmlConfig &config, + const odr::Logger &logger) { + return odr::html::translate(file, config, logger); }, - py::arg("file"), py::arg("cache_path"), py::arg("config"), + py::arg("file"), py::arg("config"), py::arg("logger") = odr::Logger::null(), py::call_guard(), "Translate a decoded file to HTML."); html.def( "translate", - [](const odr::Document &document, const std::string &cache_path, - const odr::HtmlConfig &config, const odr::Logger &logger) { - return odr::html::translate(document, cache_path, config, logger); + [](const odr::Document &document, const odr::HtmlConfig &config, + const odr::Logger &logger) { + return odr::html::translate(document, config, logger); }, - py::arg("document"), py::arg("cache_path"), py::arg("config"), + py::arg("document"), py::arg("config"), py::arg("logger") = odr::Logger::null(), py::call_guard(), "Translate a document to HTML."); html.def( "translate", - [](const odr::Filesystem &filesystem, const std::string &cache_path, - const odr::HtmlConfig &config, const odr::Logger &logger) { - return odr::html::translate(filesystem, cache_path, config, logger); + [](const odr::Filesystem &filesystem, const odr::HtmlConfig &config, + const odr::Logger &logger) { + return odr::html::translate(filesystem, config, logger); }, - py::arg("filesystem"), py::arg("cache_path"), py::arg("config"), + py::arg("filesystem"), py::arg("config"), py::arg("logger") = odr::Logger::null(), py::call_guard(), "Translate a filesystem to HTML."); diff --git a/python/tests/test_html.py b/python/tests/test_html.py index 047a24afe..343350f4e 100644 --- a/python/tests/test_html.py +++ b/python/tests/test_html.py @@ -5,11 +5,9 @@ def translate_offline(path, tmp_path): file = pyodr.open(str(path)) - cache = tmp_path / "cache" output = tmp_path / "output" - cache.mkdir() output.mkdir() - service = pyodr.html.translate(file, str(cache), pyodr.HtmlConfig()) + service = pyodr.html.translate(file, pyodr.HtmlConfig()) return service.bring_offline(str(output)) @@ -30,17 +28,16 @@ def test_html_config_defaults(): assert config.spreadsheet_cell_limit is None -def test_html_view_sheet_cut(csv_path, tmp_path): - cache = tmp_path / "cache" +def test_html_view_sheet_cut(csv_path): file = pyodr.open(str(csv_path)) config = pyodr.HtmlConfig() - service = pyodr.html.translate(file, str(cache), config) + service = pyodr.html.translate(file, config) assert all(view.sheet_cut() is None for view in service.list_views()) config.spreadsheet_limit = pyodr.TableDimensions(2, 1) config.spreadsheet_cell_limit = None - service = pyodr.html.translate(file, str(cache), config) + service = pyodr.html.translate(file, config) cut = service.list_views()[1].sheet_cut() assert cut is not None @@ -76,61 +73,57 @@ def test_html_config_viewport_defaults(): assert config.initial_zoom == 1.5 -def test_viewport_mode_reaches_the_html(odt_path, tmp_path): +def test_viewport_mode_reaches_the_html(odt_path): # The C++ suite covers the mode matrix; this only proves the config crosses # the binding. A text document without margins is reflowing content, so # `automatic` resolves to `actual_size`. - def render(name, config): - cache = tmp_path / name - cache.mkdir() + def render(config): file = pyodr.open(str(odt_path)) - service = pyodr.html.translate(file, str(cache), config) + service = pyodr.html.translate(file, config) content, _ = service.list_views()[0].write_html() return content assert ( '' - in render("automatic", pyodr.HtmlConfig()) + in render(pyodr.HtmlConfig()) ) fit_width = pyodr.HtmlConfig() fit_width.viewport_mode = pyodr.HtmlViewportMode.fit_width assert ( '' - in render("fit_width", fit_width) + in render(fit_width) ) # only paged content has a width to fit, hence the margins by_view = pyodr.HtmlConfig() by_view.viewport_mode = pyodr.HtmlViewportMode.fit_width_by_view by_view.text_document_margin = True - assert "--odr-fit:view" in render("by_view", by_view) + assert "--odr-fit:view" in render(by_view) raw = pyodr.HtmlConfig() raw.viewport_content = "width=420" - assert '' in render("raw", raw) + assert '' in render(raw) -def test_min_content_margin_reaches_the_html(odt_path, tmp_path): +def test_min_content_margin_reaches_the_html(odt_path): # The C++ suite covers where the floor lands; this only proves it crosses # the binding, unset sides and all. - def render(name, config): - cache = tmp_path / name - cache.mkdir() + def render(config): file = pyodr.open(str(odt_path)) - service = pyodr.html.translate(file, str(cache), config) + service = pyodr.html.translate(file, config) content, _ = service.list_views()[0].write_html() return content default = pyodr.HtmlConfig() assert default.min_content_margin.top is None - assert ":root{--odr-min-margin" not in render("default", default) + assert ":root{--odr-min-margin" not in render(default) config = pyodr.HtmlConfig() config.min_content_margin.top = pyodr.Measure("12px") config.min_content_margin.left = pyodr.Measure("1cm") - html = render("margin", config) + html = render(config) assert ":root{--odr-min-margin-top:12px;--odr-min-margin-left:1cm;}" in html assert "--odr-min-margin-right:" not in html @@ -161,11 +154,9 @@ def test_translate_document(odt_path, tmp_path): assert "Hello from pyodr!" in content -def test_html_service_views(odt_path, tmp_path): +def test_html_service_views(odt_path): file = pyodr.open(str(odt_path)) - cache = tmp_path / "cache" - cache.mkdir() - service = pyodr.html.translate(file, str(cache), pyodr.HtmlConfig()) + service = pyodr.html.translate(file, pyodr.HtmlConfig()) views = service.list_views() assert len(views) == 1 @@ -175,14 +166,12 @@ def test_html_service_views(odt_path, tmp_path): assert isinstance(resources, list) -def test_html_view_outlives_service(odt_path, tmp_path): +def test_html_view_outlives_service(odt_path): file = pyodr.open(str(odt_path)) - cache = tmp_path / "cache" - cache.mkdir() # The service temporary is dropped immediately; the view must keep it # alive. - view = pyodr.html.translate(file, str(cache), pyodr.HtmlConfig()).list_views()[0] + view = pyodr.html.translate(file, pyodr.HtmlConfig()).list_views()[0] content, _ = view.write_html() assert "Hello from pyodr!" in content diff --git a/python/tests/test_http_server.py b/python/tests/test_http_server.py index 554a90940..7f3acfe5b 100644 --- a/python/tests/test_http_server.py +++ b/python/tests/test_http_server.py @@ -20,16 +20,14 @@ def fetch(url, timeout=5.0): @pytest.mark.skipif(not pyodr.has_http_server, reason="built without the HTTP server") -def test_serve_file(odt_path, tmp_path): +def test_serve_file(odt_path): server = pyodr.HttpServer() # the server hosts what it is given; translating is the caller's business - cache_path = tmp_path / "doc-cache" - cache_path.mkdir() file = pyodr.open(str(odt_path)) html_config = pyodr.HtmlConfig() html_config.embed_images = False - service = pyodr.html.translate(file, str(cache_path), html_config) + service = pyodr.html.translate(file, html_config) server.connect_service(service, "doc") views = service.list_views() assert len(views) == 1 diff --git a/src/odr/html.cpp b/src/odr/html.cpp index 4c4d66644..12e54260b 100644 --- a/src/odr/html.cpp +++ b/src/odr/html.cpp @@ -214,6 +214,49 @@ void HtmlResource::write_resource(std::ostream &os) const { m_impl->write_resource(os); } +namespace { + +HtmlService translate_text_file(const TextFile &text_file, + const HtmlConfig &config, + const Logger &logger) { + return internal::html::create_text_service(text_file, config, logger); +} + +HtmlService translate_image_file(const ImageFile &image_file, + const HtmlConfig &config, + const Logger &logger) { + // refusing says what a blank `` cannot + if (!capabilities_by_file_type(image_file.file_type()).translate_html) { + throw UnsupportedFileType(image_file.file_type()); + } + return internal::html::create_image_service(image_file, config, logger); +} + +HtmlService translate_archive_file(const ArchiveFile &archive_file, + const HtmlConfig &config, + const Logger &logger) { + return html::translate(archive_file.archive(), config, logger); +} + +HtmlService translate_document_file(const DocumentFile &document_file, + const HtmlConfig &config, + const Logger &logger) { + return html::translate(document_file.document(), config, logger); +} + +HtmlService translate_pdf_file(const PdfFile &pdf_file, + const HtmlConfig &config, const Logger &logger) { + return internal::html::create_pdf_service(pdf_file, config, logger); +} + +HtmlService translate_font_file(const FontFile &font_file, + const HtmlConfig &config, + const Logger &logger) { + return internal::html::create_font_service(font_file, config, logger); +} + +} // namespace + HtmlService html::translate(const DecodedFile &file, const HtmlConfig &config, const Logger &logger) { // before the text branch: a csv is a text file, and rendering one as a line @@ -225,29 +268,29 @@ HtmlService html::translate(const DecodedFile &file, const HtmlConfig &config, if (file.is_markdown_file()) { return translate(file.as_markdown_file().document(), config, logger); } - // and before it for the same reason. Translating it as a text file by hand - // still writes the line list. + // and before it for the same reason. Opening the bytes as `text_file` is how + // to ask for the line list. if (file.file_type() == FileType::xml) { return internal::html::create_xml_service(file.as_text_file(), config, logger); } if (file.is_text_file()) { - return translate(file.as_text_file(), config, logger); + return translate_text_file(file.as_text_file(), config, logger); } if (file.is_image_file()) { - return translate(file.as_image_file(), config, logger); + return translate_image_file(file.as_image_file(), config, logger); } if (file.is_archive_file()) { - return translate(file.as_archive_file(), config, logger); + return translate_archive_file(file.as_archive_file(), config, logger); } if (file.is_document_file()) { - return translate(file.as_document_file(), config, logger); + return translate_document_file(file.as_document_file(), config, logger); } if (file.is_pdf_file()) { - return translate(file.as_pdf_file(), config, logger); + return translate_pdf_file(file.as_pdf_file(), config, logger); } if (file.is_font_file()) { - return translate(file.as_font_file(), config, logger); + return translate_font_file(file.as_font_file(), config, logger); } // No wrapper type to go through: nothing is decoded, so the plain // `DecodedFile` already carries the bytes and the type that names them. @@ -288,40 +331,6 @@ HtmlResourceLocator html::standard_resource_locator() { }; } -HtmlService html::translate(const TextFile &text_file, const HtmlConfig &config, - const Logger &logger) { - return internal::html::create_text_service(text_file, config, logger); -} - -HtmlService html::translate(const ImageFile &image_file, - const HtmlConfig &config, const Logger &logger) { - // refusing says what a blank `` cannot - if (!capabilities_by_file_type(image_file.file_type()).translate_html) { - throw UnsupportedFileType(image_file.file_type()); - } - return internal::html::create_image_service(image_file, config, logger); -} - -HtmlService html::translate(const ArchiveFile &archive_file, - const HtmlConfig &config, const Logger &logger) { - return translate(archive_file.archive(), config, logger); -} - -HtmlService html::translate(const DocumentFile &document_file, - const HtmlConfig &config, const Logger &logger) { - return translate(document_file.document(), config, logger); -} - -HtmlService html::translate(const PdfFile &pdf_file, const HtmlConfig &config, - const Logger &logger) { - return internal::html::create_pdf_service(pdf_file, config, logger); -} - -HtmlService html::translate(const FontFile &font_file, const HtmlConfig &config, - const Logger &logger) { - return internal::html::create_font_service(font_file, config, logger); -} - HtmlService html::translate(const Filesystem &filesystem, const HtmlConfig &config, const Logger &logger) { return internal::html::create_filesystem_service(filesystem, config, logger); @@ -337,70 +346,6 @@ HtmlService html::translate(const Document &document, const HtmlConfig &config, return internal::html::create_document_service(document, config, logger); } -// The `cache_path` overloads. Nothing reads the path: no renderer has since the -// output became a set of streams, and the `create_directories` that used to sit -// here made a directory nobody wrote into. - -HtmlService html::translate(const DecodedFile &file, - const std::string & /*cache_path*/, - const HtmlConfig &config, const Logger &logger) { - return translate(file, config, logger); -} - -HtmlService html::translate(const TextFile &text_file, - const std::string & /*cache_path*/, - const HtmlConfig &config, const Logger &logger) { - return translate(text_file, config, logger); -} - -HtmlService html::translate(const ImageFile &image_file, - const std::string & /*cache_path*/, - const HtmlConfig &config, const Logger &logger) { - return translate(image_file, config, logger); -} - -HtmlService html::translate(const ArchiveFile &archive_file, - const std::string & /*cache_path*/, - const HtmlConfig &config, const Logger &logger) { - return translate(archive_file, config, logger); -} - -HtmlService html::translate(const DocumentFile &document_file, - const std::string & /*cache_path*/, - const HtmlConfig &config, const Logger &logger) { - return translate(document_file, config, logger); -} - -HtmlService html::translate(const PdfFile &pdf_file, - const std::string & /*cache_path*/, - const HtmlConfig &config, const Logger &logger) { - return translate(pdf_file, config, logger); -} - -HtmlService html::translate(const FontFile &font_file, - const std::string & /*cache_path*/, - const HtmlConfig &config, const Logger &logger) { - return translate(font_file, config, logger); -} - -HtmlService html::translate(const Filesystem &filesystem, - const std::string & /*cache_path*/, - const HtmlConfig &config, const Logger &logger) { - return translate(filesystem, config, logger); -} - -HtmlService html::translate(const Archive &archive, - const std::string & /*cache_path*/, - const HtmlConfig &config, const Logger &logger) { - return translate(archive, config, logger); -} - -HtmlService html::translate(const Document &document, - const std::string & /*cache_path*/, - const HtmlConfig &config, const Logger &logger) { - return translate(document, config, logger); -} - void html::edit(const Document &document, const std::string_view diff, const Logger & /*logger*/) { const nlohmann::json json = nlohmann::json::parse(diff); diff --git a/src/odr/html.hpp b/src/odr/html.hpp index d564d28de..4d6438175 100644 --- a/src/odr/html.hpp +++ b/src/odr/html.hpp @@ -294,77 +294,22 @@ namespace html { HtmlResourceLocator standard_resource_locator(); /// @brief Translates a decoded file to HTML. +/// +/// The one entry point for a file: it dispatches on the decoded type, so a +/// caller that already narrowed to a @ref DocumentFile or a @ref PdfFile +/// passes it here too. HtmlService translate(const DecodedFile &file, const HtmlConfig &config, const Logger &logger = Logger::null()); -/// @brief Translates a text file to HTML. -HtmlService translate(const TextFile &text_file, const HtmlConfig &config, - const Logger &logger = Logger::null()); -/// @brief Translates an image file to HTML. -HtmlService translate(const ImageFile &image_file, const HtmlConfig &config, - const Logger &logger = Logger::null()); -/// @brief Translates an archive file to HTML. -HtmlService translate(const ArchiveFile &archive_file, const HtmlConfig &config, - const Logger &logger = Logger::null()); -/// @brief Translates a document file to HTML. -HtmlService translate(const DocumentFile &document_file, - const HtmlConfig &config, - const Logger &logger = Logger::null()); -/// @brief Translates a PDF file to HTML. -HtmlService translate(const PdfFile &pdf_file, const HtmlConfig &config, - const Logger &logger = Logger::null()); - -/// @brief Translates a font file to HTML (a specimen page). -HtmlService translate(const FontFile &font_file, const HtmlConfig &config, +/// @brief Translates a document to HTML. +HtmlService translate(const Document &document, const HtmlConfig &config, const Logger &logger = Logger::null()); - /// @brief Translates a filesystem to HTML. HtmlService translate(const Filesystem &filesystem, const HtmlConfig &config, const Logger &logger = Logger::null()); /// @brief Translates an archive to HTML. HtmlService translate(const Archive &archive, const HtmlConfig &config, const Logger &logger = Logger::null()); -/// @brief Translates a document to HTML. -HtmlService translate(const Document &document, const HtmlConfig &config, - const Logger &logger = Logger::null()); - -/// @name Translation with a cache path -/// -/// `cache_path` is ignored — nothing on the render path writes to disk, and no -/// renderer has read it since the output became a set of streams. Kept so -/// existing callers keep compiling; prefer the overloads above. -/// @{ -HtmlService translate(const DecodedFile &file, const std::string &cache_path, - const HtmlConfig &config, - const Logger &logger = Logger::null()); -HtmlService translate(const TextFile &text_file, const std::string &cache_path, - const HtmlConfig &config, - const Logger &logger = Logger::null()); -HtmlService translate(const ImageFile &image_file, - const std::string &cache_path, const HtmlConfig &config, - const Logger &logger = Logger::null()); -HtmlService translate(const ArchiveFile &archive_file, - const std::string &cache_path, const HtmlConfig &config, - const Logger &logger = Logger::null()); -HtmlService translate(const DocumentFile &document_file, - const std::string &cache_path, const HtmlConfig &config, - const Logger &logger = Logger::null()); -HtmlService translate(const PdfFile &pdf_file, const std::string &cache_path, - const HtmlConfig &config, - const Logger &logger = Logger::null()); -HtmlService translate(const FontFile &font_file, const std::string &cache_path, - const HtmlConfig &config, - const Logger &logger = Logger::null()); -HtmlService translate(const Filesystem &filesystem, - const std::string &cache_path, const HtmlConfig &config, - const Logger &logger = Logger::null()); -HtmlService translate(const Archive &archive, const std::string &cache_path, - const HtmlConfig &config, - const Logger &logger = Logger::null()); -HtmlService translate(const Document &document, const std::string &cache_path, - const HtmlConfig &config, - const Logger &logger = Logger::null()); -/// @} /// @brief Applies a diff to a document. The diff is what our JavaScript /// produces in the browser. diff --git a/test/src/html_output_test.cpp b/test/src/html_output_test.cpp index 493543628..40be7b18f 100644 --- a/test/src/html_output_test.cpp +++ b/test/src/html_output_test.cpp @@ -198,8 +198,7 @@ TEST_P(HtmlOutputTests, html_meta) { // and would bury the corpus log. const Logger render_logger = Logger::create_stdio("odr-test", LogLevel::warning); - HtmlService service = - html::translate(file, output_path_tmp, config, render_logger); + HtmlService service = html::translate(file, config, render_logger); Html html = service.bring_offline(output_path); fs::remove_all(output_path_tmp); diff --git a/test/src/html_test.cpp b/test/src/html_test.cpp index 3fbe96baf..1565a6caa 100644 --- a/test/src/html_test.cpp +++ b/test/src/html_test.cpp @@ -31,14 +31,11 @@ using namespace odr::test; TEST(html, linked_resources_are_served) { const auto logger = Logger::create_stdio("odr-test", LogLevel::verbose); - const std::string cache_path = - (std::filesystem::current_path() / "cache").string(); - const auto check = [&](const DecodedFile &file, const std::string &view) { HtmlConfig config; config.embed_shipped_resources = false; - const HtmlService service = html::translate(file, cache_path, config); + const HtmlService service = html::translate(file, config); std::ostringstream out; const HtmlResources resources = service.list_views().at(0).write_html(out); @@ -79,16 +76,13 @@ TEST(html, linked_resources_are_served) { TEST(html, linked_images_are_served) { const auto logger = Logger::create_stdio("odr-test", LogLevel::verbose); - const std::string cache_path = - (std::filesystem::current_path() / "images").string(); - const auto check = [&](const std::string &path) { const DecodedFile file(TestData::test_file_path(path), logger); HtmlConfig config; config.embed_images = false; - const HtmlService service = html::translate(file, cache_path, config); + const HtmlService service = html::translate(file, config); std::ostringstream out; const HtmlResources resources = service.list_views().at(0).write_html(out); @@ -398,10 +392,8 @@ TEST(html, views) { const Document document = document_file.document(); - const std::string cache_path = - (std::filesystem::current_path() / "cache").string(); const HtmlConfig config; - const HtmlService service = html::translate(document, cache_path, config); + const HtmlService service = html::translate(document, config); const HtmlViews &views = service.list_views(); @@ -423,7 +415,7 @@ TEST(html, paged_output_fits_the_viewport) { const std::string cache = (std::filesystem::current_path() / "fit").string(); std::ostringstream out; - html::translate(file, cache, config).list_views().at(0).write_html(out); + html::translate(file, config).list_views().at(0).write_html(out); return std::move(out).str(); }; @@ -504,8 +496,7 @@ TEST(html, each_view_fits_the_page_it_renders) { config.viewport_width = 400; const DecodedFile file{path}; - const HtmlService service = html::translate( - file, (std::filesystem::current_path() / "rotate").string(), config); + const HtmlService service = html::translate(file, config); const auto factor_of = [&](const std::size_t view) { std::ostringstream out; @@ -542,7 +533,7 @@ TEST(html, an_image_fits_the_viewport) { const std::string cache = (std::filesystem::current_path() / "image_fit").string(); std::ostringstream out; - html::translate(file, cache, config).list_views().at(0).write_html(out); + html::translate(file, config).list_views().at(0).write_html(out); return std::move(out).str(); }; diff --git a/test/src/internal/csv/csv_file_test.cpp b/test/src/internal/csv/csv_file_test.cpp index 7438fafbd..0262efc51 100644 --- a/test/src/internal/csv/csv_file_test.cpp +++ b/test/src/internal/csv/csv_file_test.cpp @@ -342,8 +342,7 @@ TEST(CsvDocument, renders_as_a_table) { const CsvFile file = CsvFile::from_file(File::from_memory("a,b\n1,2\n"), CsvOptions{}); - const HtmlService service = - html::translate(file.document(), "", HtmlConfig()); + const HtmlService service = html::translate(file.document(), HtmlConfig()); std::ostringstream out; service.list_views().back().write_html(out); diff --git a/test/src/internal/font/font_file.cpp b/test/src/internal/font/font_file.cpp index 83fb10dba..8c6017682 100644 --- a/test/src/internal/font/font_file.cpp +++ b/test/src/internal/font/font_file.cpp @@ -145,10 +145,8 @@ TEST(FontFileTest, specimen_page_embeds_font_and_glyph_grid) { const odr::FontFile font_file(std::make_shared( std::make_shared(sample_ttf()), FileType::truetype_font)); - const std::string cache_path = - (std::filesystem::temp_directory_path() / "odr_font_test").string(); const HtmlConfig config; - const HtmlService service = html::translate(font_file, cache_path, config); + const HtmlService service = html::translate(font_file, config); const HtmlViews &views = service.list_views(); ASSERT_EQ(views.size(), 1); diff --git a/test/src/internal/html/image_file_test.cpp b/test/src/internal/html/image_file_test.cpp index 4c1101013..5fed622b6 100644 --- a/test/src/internal/html/image_file_test.cpp +++ b/test/src/internal/html/image_file_test.cpp @@ -36,10 +36,6 @@ File png_file() { return image_file(std::string("\x89PNG\r\n\x1a\n", 8) + "payload"); } -std::string cache_path(const std::string &name) { - return (std::filesystem::current_path() / name).string(); -} - std::string write_path(const HtmlService &service, const std::string &path) { std::ostringstream out; service.write(path, out); @@ -113,8 +109,7 @@ TEST(image_file, svg_is_detected_and_opens_as_an_image) { TEST(image_file, svg_translates_to_an_image_page) { const DecodedFile file{svg_file()}; - const HtmlService service = - html::translate(file, cache_path("image_svg"), HtmlConfig()); + const HtmlService service = html::translate(file, HtmlConfig()); ASSERT_EQ(service.list_views().size(), 1); EXPECT_EQ(service.list_views().front().name(), "image"); @@ -130,8 +125,7 @@ TEST(image_file, ico_is_named_by_its_own_mime_type) { EXPECT_EQ(file.file_type(), FileType::windows_icon); EXPECT_TRUE(file.is_image_file()); - const HtmlService service = - html::translate(file, cache_path("image_ico"), HtmlConfig()); + const HtmlService service = html::translate(file, HtmlConfig()); const std::string html = write_path(service, "image.html"); EXPECT_NE(html.find("data:image/vnd.microsoft.icon;base64,"), std::string::npos); diff --git a/test/src/internal/html/media_file_test.cpp b/test/src/internal/html/media_file_test.cpp index f4b66afd5..c0b3e5136 100644 --- a/test/src/internal/html/media_file_test.cpp +++ b/test/src/internal/html/media_file_test.cpp @@ -31,7 +31,7 @@ const std::string mp4_signature = File mp4_file() { return media_file(mp4_signature); } -std::string cache_path(const std::string &name) { +std::string temp_path(const std::string &name) { return (std::filesystem::current_path() / name).string(); } @@ -59,8 +59,7 @@ TEST(media_file, audio_is_decoded_without_a_wrapper_type) { TEST(media_file, audio_translates_to_a_player) { const DecodedFile file{mp3_file()}; - const HtmlService service = - html::translate(file, cache_path("media_audio"), HtmlConfig()); + const HtmlService service = html::translate(file, HtmlConfig()); ASSERT_EQ(service.list_views().size(), 1); EXPECT_EQ(service.list_views().front().name(), "audio"); @@ -74,8 +73,7 @@ TEST(media_file, audio_translates_to_a_player) { TEST(media_file, video_translates_to_a_player) { const DecodedFile file{mp4_file()}; - const HtmlService service = - html::translate(file, cache_path("media_video"), HtmlConfig()); + const HtmlService service = html::translate(file, HtmlConfig()); ASSERT_EQ(service.list_views().size(), 1); EXPECT_EQ(service.list_views().front().name(), "video"); @@ -90,8 +88,7 @@ TEST(media_file, video_translates_to_a_player) { /// base64'd into the markup the way an image is. TEST(media_file, the_media_is_served_as_a_resource) { const DecodedFile file{mp4_file()}; - const HtmlService service = - html::translate(file, cache_path("media_resource"), HtmlConfig()); + const HtmlService service = html::translate(file, HtmlConfig()); EXPECT_TRUE(service.exists("video.mp4")); EXPECT_EQ(service.mimetype("video.mp4"), "video/mp4"); @@ -104,12 +101,11 @@ TEST(media_file, the_media_is_served_as_a_resource) { } TEST(media_file, bring_offline_writes_the_media_next_to_the_page) { - const std::string output_path = cache_path("media_offline"); + const std::string output_path = temp_path("media_offline"); std::filesystem::remove_all(output_path); const DecodedFile file{mp4_file()}; - const HtmlService service = - html::translate(file, cache_path("media_offline_cache"), HtmlConfig()); + const HtmlService service = html::translate(file, HtmlConfig()); const Html html = service.bring_offline(output_path); ASSERT_EQ(html.pages().size(), 1); @@ -128,7 +124,7 @@ TEST(media_file, bring_offline_writes_the_media_next_to_the_page) { /// webm as `video/x-matroska` and no browser would play it. The name it came /// in under wins whenever the same type claims it. TEST(media_file, a_webm_keeps_its_own_name_and_mime) { - const std::filesystem::path directory = cache_path("media_webm"); + const std::filesystem::path directory = temp_path("media_webm"); std::filesystem::create_directories(directory); for (const auto &[name, path, mime_type] : @@ -142,8 +138,7 @@ TEST(media_file, a_webm_keeps_its_own_name_and_mime) { const DecodedFile file{File(file_path.string())}; ASSERT_EQ(file.file_type(), FileType::matroska_video) << path; - const HtmlService service = - html::translate(file, cache_path("media_webm_cache"), HtmlConfig()); + const HtmlService service = html::translate(file, HtmlConfig()); EXPECT_TRUE(service.exists(name)) << path; EXPECT_EQ(service.mimetype(name), mime_type) << path; @@ -164,8 +159,7 @@ TEST(media_file, webp_opens_as_an_image) { EXPECT_EQ(decoded.file_type(), FileType::webp); EXPECT_TRUE(decoded.is_image_file()); - const HtmlService service = - html::translate(decoded, cache_path("media_webp"), HtmlConfig()); + const HtmlService service = html::translate(decoded, HtmlConfig()); ASSERT_EQ(service.list_views().size(), 1); // named, not guessed: a browser that honours the data URL's type has to be diff --git a/test/src/internal/xml/xml_file_test.cpp b/test/src/internal/xml/xml_file_test.cpp index 2dab7cbce..131585218 100644 --- a/test/src/internal/xml/xml_file_test.cpp +++ b/test/src/internal/xml/xml_file_test.cpp @@ -110,11 +110,12 @@ TEST(XmlDeclaration, the_encoding_pseudo_attribute_is_read_off_the_bytes) { EXPECT_EQ(declared_encoding(R"(")), HtmlConfig(), Logger::null()); + DecodedFile(File::from_memory(""), FileType::text_file), + HtmlConfig(), Logger::null()); std::ostringstream out; service.write("text.html", out); From d840623ae0ec9ccab98c8dd3223f051dc8fc2e89 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 6 Sep 2026 15:14:14 +0200 Subject: [PATCH 2/3] fixup: translate(TextFile) is not one of the duplicates 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) Claude-Session: https://claude.ai/code/session_016hxDa2rev11eLUEJZJ5nmz --- CHANGELOG.md | 9 ++++++--- src/odr/html.cpp | 13 ++++++------- src/odr/html.hpp | 8 ++++++++ 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 29a5dfeb7..ec5ff75a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,9 +16,12 @@ The release run heads these entries with the version and opens a fresh ## Unreleased -- **Breaking**: `html::translate` is four overloads instead of twenty, taking a - `DecodedFile`, `Document`, `Filesystem` or `Archive`. The `cache_path` and - narrowed-handle ones are gone, in every binding too: drop the cache path. +- **Breaking**: `html::translate` is five overloads instead of twenty, taking a + `DecodedFile`, `TextFile`, `Document`, `Filesystem` or `Archive`. The + `cache_path` and the other narrowed-handle ones are gone, in every binding + too: drop the cache path. `TextFile` stays because it is not the same call — + it writes the numbered line list, where `translate(DecodedFile)` renders an + xml as its source view and a csv as a table. - **Breaking**: the inert `HtmlConfig` fields `background_image_format`, `background_image_dpi`, `no_drm` and `embed_outline` are gone, with their diff --git a/src/odr/html.cpp b/src/odr/html.cpp index 12e54260b..324159c01 100644 --- a/src/odr/html.cpp +++ b/src/odr/html.cpp @@ -216,12 +216,6 @@ void HtmlResource::write_resource(std::ostream &os) const { namespace { -HtmlService translate_text_file(const TextFile &text_file, - const HtmlConfig &config, - const Logger &logger) { - return internal::html::create_text_service(text_file, config, logger); -} - HtmlService translate_image_file(const ImageFile &image_file, const HtmlConfig &config, const Logger &logger) { @@ -275,7 +269,7 @@ HtmlService html::translate(const DecodedFile &file, const HtmlConfig &config, logger); } if (file.is_text_file()) { - return translate_text_file(file.as_text_file(), config, logger); + return translate(file.as_text_file(), config, logger); } if (file.is_image_file()) { return translate_image_file(file.as_image_file(), config, logger); @@ -341,6 +335,11 @@ HtmlService html::translate(const Archive &archive, const HtmlConfig &config, return translate(archive.as_filesystem(), config, logger); } +HtmlService html::translate(const TextFile &text_file, const HtmlConfig &config, + const Logger &logger) { + return internal::html::create_text_service(text_file, config, logger); +} + HtmlService html::translate(const Document &document, const HtmlConfig &config, const Logger &logger) { return internal::html::create_document_service(document, config, logger); diff --git a/src/odr/html.hpp b/src/odr/html.hpp index 4d6438175..590addfda 100644 --- a/src/odr/html.hpp +++ b/src/odr/html.hpp @@ -301,6 +301,14 @@ HtmlResourceLocator standard_resource_locator(); HtmlService translate(const DecodedFile &file, const HtmlConfig &config, const Logger &logger = Logger::null()); +/// @brief Translates a text file to HTML, as a numbered line list. +/// +/// Not what @ref translate(const DecodedFile &, const HtmlConfig &, const +/// Logger &) does with one: it renders an xml as its source view and a csv as +/// a table. Narrowing by hand is how a caller asks for the lines instead. +HtmlService translate(const TextFile &text_file, const HtmlConfig &config, + const Logger &logger = Logger::null()); + /// @brief Translates a document to HTML. HtmlService translate(const Document &document, const HtmlConfig &config, const Logger &logger = Logger::null()); From 0b0d00b49fc691857c7c7e4056d18a064e8100b1 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 6 Sep 2026 15:30:16 +0200 Subject: [PATCH 3/3] fixup: the open decides, so translate(TextFile) is redundant 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 d840623a and points its test at the open. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_012QiYawTVZyuMLQBQngdTeu --- CHANGELOG.md | 9 +++------ src/odr/html.cpp | 16 ++++++++-------- src/odr/html.hpp | 14 +------------- test/src/internal/xml/xml_file_test.cpp | 3 +-- 4 files changed, 13 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec5ff75a1..0544b5e8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,12 +16,9 @@ The release run heads these entries with the version and opens a fresh ## Unreleased -- **Breaking**: `html::translate` is five overloads instead of twenty, taking a - `DecodedFile`, `TextFile`, `Document`, `Filesystem` or `Archive`. The - `cache_path` and the other narrowed-handle ones are gone, in every binding - too: drop the cache path. `TextFile` stays because it is not the same call — - it writes the numbered line list, where `translate(DecodedFile)` renders an - xml as its source view and a csv as a table. +- **Breaking**: `html::translate` takes only a `DecodedFile`, `Document`, + `Filesystem` or `Archive` now; drop the cache path, in every binding. Open a + file as `FileType::text_file` to render it as a numbered line list. - **Breaking**: the inert `HtmlConfig` fields `background_image_format`, `background_image_dpi`, `no_drm` and `embed_outline` are gone, with their diff --git a/src/odr/html.cpp b/src/odr/html.cpp index 324159c01..c7349c7a6 100644 --- a/src/odr/html.cpp +++ b/src/odr/html.cpp @@ -216,6 +216,12 @@ void HtmlResource::write_resource(std::ostream &os) const { namespace { +HtmlService translate_text_file(const TextFile &text_file, + const HtmlConfig &config, + const Logger &logger) { + return internal::html::create_text_service(text_file, config, logger); +} + HtmlService translate_image_file(const ImageFile &image_file, const HtmlConfig &config, const Logger &logger) { @@ -262,14 +268,13 @@ HtmlService html::translate(const DecodedFile &file, const HtmlConfig &config, if (file.is_markdown_file()) { return translate(file.as_markdown_file().document(), config, logger); } - // and before it for the same reason. Opening the bytes as `text_file` is how - // to ask for the line list. + // and before it for the same reason; open as `text_file` for the line list if (file.file_type() == FileType::xml) { return internal::html::create_xml_service(file.as_text_file(), config, logger); } if (file.is_text_file()) { - return translate(file.as_text_file(), config, logger); + return translate_text_file(file.as_text_file(), config, logger); } if (file.is_image_file()) { return translate_image_file(file.as_image_file(), config, logger); @@ -335,11 +340,6 @@ HtmlService html::translate(const Archive &archive, const HtmlConfig &config, return translate(archive.as_filesystem(), config, logger); } -HtmlService html::translate(const TextFile &text_file, const HtmlConfig &config, - const Logger &logger) { - return internal::html::create_text_service(text_file, config, logger); -} - HtmlService html::translate(const Document &document, const HtmlConfig &config, const Logger &logger) { return internal::html::create_document_service(document, config, logger); diff --git a/src/odr/html.hpp b/src/odr/html.hpp index 590addfda..4408d4c83 100644 --- a/src/odr/html.hpp +++ b/src/odr/html.hpp @@ -293,22 +293,10 @@ namespace html { HtmlResourceLocator standard_resource_locator(); -/// @brief Translates a decoded file to HTML. -/// -/// The one entry point for a file: it dispatches on the decoded type, so a -/// caller that already narrowed to a @ref DocumentFile or a @ref PdfFile -/// passes it here too. +/// @brief Translates a decoded file to HTML, dispatching on the decoded type. HtmlService translate(const DecodedFile &file, const HtmlConfig &config, const Logger &logger = Logger::null()); -/// @brief Translates a text file to HTML, as a numbered line list. -/// -/// Not what @ref translate(const DecodedFile &, const HtmlConfig &, const -/// Logger &) does with one: it renders an xml as its source view and a csv as -/// a table. Narrowing by hand is how a caller asks for the lines instead. -HtmlService translate(const TextFile &text_file, const HtmlConfig &config, - const Logger &logger = Logger::null()); - /// @brief Translates a document to HTML. HtmlService translate(const Document &document, const HtmlConfig &config, const Logger &logger = Logger::null()); diff --git a/test/src/internal/xml/xml_file_test.cpp b/test/src/internal/xml/xml_file_test.cpp index 131585218..63bf343e8 100644 --- a/test/src/internal/xml/xml_file_test.cpp +++ b/test/src/internal/xml/xml_file_test.cpp @@ -110,8 +110,7 @@ TEST(XmlDeclaration, the_encoding_pseudo_attribute_is_read_off_the_bytes) { EXPECT_EQ(declared_encoding(R"("), FileType::text_file),