From d4e59687c696940bb64cb3103778e7bea8f2fdd9 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 6 Sep 2026 14:57:35 +0200 Subject: [PATCH] feat(api)!: fold the decode options into one struct DecodePreference answered "which type" and CsvOptions answered "how to read it", on different roads: CsvFile::from_file bypassed open_strategy entirely, so a caller who wanted to name a separator could not get there through open. DecodeOptions carries both, and with a default argument the six open overloads become two. Two things fell out of routing csv through the strategy. A named type is not a probe with one candidate, so its engine's own refusal now reaches the caller instead of UnknownFileType - which is what kept from_file's diagnostics when it went - and an incoherent dialect stays std::invalid_argument rather than becoming "not a csv", because it is a caller mistake, not a claim about the bytes. DecodeOptions::as and ::as_csv are named factories rather than designated initializers: partial designated initialisation warns under -Wextra on the emscripten toolchain, and CI builds with -Werror. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016hxDa2rev11eLUEJZJ5nmz --- CHANGELOG.md | 8 ++ apple/include/OdrCoreObjC/ODRFile.h | 27 +++- apple/src/ODRFile.mm | 41 ++++-- cli/src/server.cpp | 8 +- jni/CMakeLists.txt | 3 +- .../app/opendocument/core/CsvOptions.java | 29 ++++ ...codePreference.java => DecodeOptions.java} | 12 +- jni/java/app/opendocument/core/Odr.java | 22 +++- jni/src/jni_core.cpp | 28 ++-- jni/tests/app/opendocument/core/FileTest.java | 15 +++ python/src/bind_core.cpp | 48 ++----- python/src/bind_file.cpp | 43 +++++- python/tests/test_file.py | 52 +++++--- src/odr/file.cpp | 23 ++-- src/odr/file.hpp | 58 ++++---- src/odr/internal/open_strategy.cpp | 81 +++++++----- src/odr/internal/open_strategy.hpp | 9 +- src/odr/odr.cpp | 28 +--- src/odr/odr.hpp | 25 ++-- test/src/document_list_test.cpp | 2 +- test/src/document_test.cpp | 24 ++-- test/src/file_test.cpp | 29 ++-- test/src/html_output_test.cpp | 5 +- test/src/html_test.cpp | 80 ++++++----- test/src/internal/csv/csv_file_test.cpp | 124 +++++++++++------- test/src/internal/iwork/keynote_test.cpp | 16 ++- test/src/internal/iwork/numbers_test.cpp | 14 +- test/src/internal/iwork/pages_test.cpp | 8 +- .../internal/markdown/markdown_file_test.cpp | 8 +- test/src/internal/odf/odf_flat_file_test.cpp | 14 +- .../internal/odf/odf_sheet_repeat_test.cpp | 2 +- test/src/internal/oldms/ppt_test.cpp | 4 +- test/src/internal/oldms/xls_test.cpp | 6 +- .../ooxml/ooxml_spreadsheet_merge_test.cpp | 7 +- test/src/internal/rtf/rtf_document_test.cpp | 17 +-- test/src/internal/svg/svg_file_test.cpp | 5 +- test/src/internal/text/text_file_test.cpp | 6 +- test/src/internal/xml/xml_file_test.cpp | 7 +- test/src/odr_test.cpp | 24 ++-- wasm/src/wasm_file.cpp | 5 +- 40 files changed, 573 insertions(+), 394 deletions(-) create mode 100644 jni/java/app/opendocument/core/CsvOptions.java rename jni/java/app/opendocument/core/{DecodePreference.java => DecodeOptions.java} (58%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 37db6f222..9319faa5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,14 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- **Breaking**: `DecodePreference` becomes `DecodeOptions`, gains a `csv` field + and is all `open` takes besides the file and logger. `CsvFile::from_file` and + `::with_options` go — use `DecodeOptions::as(type)` / `::as_csv(options)`. + +- **Breaking**: naming a type reports the engine's own refusal, so + `open(f, DecodeOptions::as(FileType::rich_text_format))` on non-rtf bytes + throws `NoRtfFile`, not `UnknownFileType`. Detection alone is unchanged. + - `TextEncoding` and its five lookups now reach the java, python, objc and wasm bindings, and a text file reports `encoding()` beside the `charset()` it keeps. wasm gets it as `Odr.enums.TextEncoding`. Nothing is removed. diff --git a/apple/include/OdrCoreObjC/ODRFile.h b/apple/include/OdrCoreObjC/ODRFile.h index a521c637b..d958a289d 100644 --- a/apple/include/OdrCoreObjC/ODRFile.h +++ b/apple/include/OdrCoreObjC/ODRFile.h @@ -216,13 +216,28 @@ NS_SWIFT_NAME(FileMeta) + (instancetype)new NS_UNAVAILABLE; @end -/// How to decode a file, when the caller knows better than detection does. -NS_SWIFT_NAME(DecodePreference) -@interface ODRDecodePreference : NSObject +/// How to read a csv file. An unset field is detected from the file's opening +/// bytes; a set one is taken as given. +NS_SWIFT_NAME(CsvOptions) +@interface ODRCsvOptions : NSObject +/// `nil` to detect. +@property(nonatomic, strong, nullable) NSNumber *encoding; +/// A one-character string, `nil` to detect. +@property(nonatomic, copy, nullable) NSString *separator; +/// A one-character string, `nil` to detect. +@property(nonatomic, copy, nullable) NSString *quote; +@end + +/// How to decode a file. Every field is optional; the default detects +/// everything. +NS_SWIFT_NAME(DecodeOptions) +@interface ODRDecodeOptions : NSObject /// Decode as this type, whatever detection says. `nil` to let it decide. @property(nonatomic, strong, nullable) NSNumber *asFileType; /// Types to prefer, most preferred first. @property(nonatomic, copy) NSArray *fileTypePriority; +/// Format-specific overrides for a file decoded as csv. +@property(nonatomic, strong) ODRCsvOptions *csv; @end /// A file, decoded or not — `odr::File`. @@ -261,11 +276,11 @@ NS_SWIFT_NAME(DecodedFile) as:(ODRFileType)type error:(NSError **)error NS_SWIFT_NAME(decode(path:as:)); -/// Decodes the file at `path` following `preference`. +/// Decodes the file at `path` per `options`. + (nullable instancetype)decodePath:(NSString *)path - preference:(ODRDecodePreference *)preference + options:(ODRDecodeOptions *)options error:(NSError **)error - NS_SWIFT_NAME(decode(path:preference:)); + NS_SWIFT_NAME(decode(path:options:)); /// Decodes an already-open file. + (nullable instancetype)decodeFile:(ODRFile *)file error:(NSError **)error diff --git a/apple/src/ODRFile.mm b/apple/src/ODRFile.mm index 9f4c2c4f7..2acee5dbf 100644 --- a/apple/src/ODRFile.mm +++ b/apple/src/ODRFile.mm @@ -196,13 +196,19 @@ + (instancetype)metaWithHandle:(const odr::FileMeta &)handle { @end -#pragma mark - ODRDecodePreference +#pragma mark - ODRCsvOptions -@implementation ODRDecodePreference +@implementation ODRCsvOptions +@end + +#pragma mark - ODRDecodeOptions + +@implementation ODRDecodeOptions - (instancetype)init { if ((self = [super init]) != nil) { _fileTypePriority = @[]; + _csv = [[ODRCsvOptions alloc] init]; } return self; } @@ -323,20 +329,32 @@ + (nullable instancetype)decodePath:(NSString *)path return guarded(error, [&]() -> ODRDecodedFile * { return [ODRDecodedFile decodedFileWithHandle:odr::open(to_string(path), - static_cast(type))]; + odr::DecodeOptions::as( + static_cast(type)))]; }); } + (nullable instancetype)decodePath:(NSString *)path - preference:(ODRDecodePreference *)preference + options:(ODRDecodeOptions *)options error:(NSError **)error { return guarded(error, [&]() -> ODRDecodedFile * { - odr::DecodePreference native; - if (preference.asFileType != nil) { + odr::DecodeOptions native; + if (options.asFileType != nil) { native.as_file_type = - static_cast(preference.asFileType.integerValue); + static_cast(options.asFileType.integerValue); + } + native.file_type_priority = to_file_types(options.fileTypePriority); + if (options.csv.encoding != nil) { + native.csv.encoding = + static_cast(options.csv.encoding.integerValue); + } + // one character, and an empty string means unset rather than a NUL + if (options.csv.separator.length > 0) { + native.csv.separator = [options.csv.separator characterAtIndex:0]; + } + if (options.csv.quote.length > 0) { + native.csv.quote = [options.csv.quote characterAtIndex:0]; } - native.file_type_priority = to_file_types(preference.fileTypePriority); return [ODRDecodedFile decodedFileWithHandle:odr::open(to_string(path), native)]; }); @@ -353,7 +371,7 @@ + (nullable instancetype)decodePath:(NSString *)path error:(NSError **)error { return guarded(error, [&]() -> ODRDecodedFile * { return [ODRDecodedFile - decodedFileWithHandle:odr::open(to_string(path), logger.handle)]; + decodedFileWithHandle:odr::open(to_string(path), {}, logger.handle)]; }); } @@ -364,7 +382,8 @@ + (nullable instancetype)decodePath:(NSString *)path return guarded(error, [&]() -> ODRDecodedFile * { return [ODRDecodedFile decodedFileWithHandle:odr::open(to_string(path), - static_cast(type), + odr::DecodeOptions::as( + static_cast(type)), logger.handle)]; }); } @@ -374,7 +393,7 @@ + (nullable instancetype)decodeFile:(ODRFile *)file error:(NSError **)error { return guarded(error, [&]() -> ODRDecodedFile * { return [ODRDecodedFile - decodedFileWithHandle:odr::open(file.handle, logger.handle)]; + decodedFileWithHandle:odr::open(file.handle, {}, logger.handle)]; }); } diff --git a/cli/src/server.cpp b/cli/src/server.cpp index 2da1b5112..537aee183 100644 --- a/cli/src/server.cpp +++ b/cli/src/server.cpp @@ -28,10 +28,10 @@ int main(const int argc, char **argv) { password = argv[2]; } - DecodePreference decode_preference; - decode_preference.as_file_type = FileType::zip; - - DecodedFile decoded_file = open(input, decode_preference, logger); + // the server offers the container's own entries beside the render, so a + // package is opened as the zip it is + DecodedFile decoded_file = + open(input, DecodeOptions::as(FileType::zip), logger); if (decoded_file.password_encrypted()) { if (!password) { diff --git a/jni/CMakeLists.txt b/jni/CMakeLists.txt index 225f3148c..1a635d3a6 100644 --- a/jni/CMakeLists.txt +++ b/jni/CMakeLists.txt @@ -70,7 +70,8 @@ add_jar(odr_java "java/app/opendocument/core/Bookmark.java" "java/app/opendocument/core/BreakType.java" "java/app/opendocument/core/Color.java" - "java/app/opendocument/core/DecodePreference.java" + "java/app/opendocument/core/CsvOptions.java" + "java/app/opendocument/core/DecodeOptions.java" "java/app/opendocument/core/DecodedFile.java" "java/app/opendocument/core/DirectionalMeasure.java" "java/app/opendocument/core/DirectionalString.java" diff --git a/jni/java/app/opendocument/core/CsvOptions.java b/jni/java/app/opendocument/core/CsvOptions.java new file mode 100644 index 000000000..c29140f36 --- /dev/null +++ b/jni/java/app/opendocument/core/CsvOptions.java @@ -0,0 +1,29 @@ +package app.opendocument.core; + +/** + * How to read a csv file. Mirrors {@code odr::CsvOptions}. An unset field is detected from the + * file's opening bytes; a set one is taken as given. + */ +public final class CsvOptions { + /** {@code null} to detect. */ + public TextEncoding encoding; + + /** {@code null} to detect. */ + public Character separator; + + /** {@code null} to detect. */ + public Character quote; + + // Flattened for the native layer: -1 for an unset field. + int encodingNative() { + return encoding == null ? -1 : encoding.toNative(); + } + + int separatorNative() { + return separator == null ? -1 : separator; + } + + int quoteNative() { + return quote == null ? -1 : quote; + } +} diff --git a/jni/java/app/opendocument/core/DecodePreference.java b/jni/java/app/opendocument/core/DecodeOptions.java similarity index 58% rename from jni/java/app/opendocument/core/DecodePreference.java rename to jni/java/app/opendocument/core/DecodeOptions.java index 4186804fe..053711305 100644 --- a/jni/java/app/opendocument/core/DecodePreference.java +++ b/jni/java/app/opendocument/core/DecodeOptions.java @@ -3,12 +3,20 @@ import java.util.ArrayList; import java.util.List; -/** Preference for decoding files. Mirrors {@code odr::DecodePreference}. */ -public final class DecodePreference { +/** + * How to decode a file. Mirrors {@code odr::DecodeOptions}. Every field is optional; the default + * detects everything. + */ +public final class DecodeOptions { /** Decode as this file type; {@code null} to detect. */ public FileType asFileType; + + /** Preferred types, most preferred first, among those detected. */ public List fileTypePriority = new ArrayList<>(); + /** Format-specific overrides for a file decoded as csv. */ + public CsvOptions csv = new CsvOptions(); + // Flattened for the native layer. int asFileTypeNative() { return asFileType == null ? -1 : asFileType.toNative(); diff --git a/jni/java/app/opendocument/core/Odr.java b/jni/java/app/opendocument/core/Odr.java index 153e82341..6c5fec003 100644 --- a/jni/java/app/opendocument/core/Odr.java +++ b/jni/java/app/opendocument/core/Odr.java @@ -128,13 +128,16 @@ public static DecodedFile open(String path, FileType as) { return new DecodedFile(openAsNative(path, as.toNative())); } - /** Opens and decodes a file with a decode preference. */ - public static DecodedFile open(String path, DecodePreference preference) { + /** Opens and decodes a file, per {@code options}. */ + public static DecodedFile open(String path, DecodeOptions options) { return new DecodedFile( - openWithPreferenceNative( + openWithOptionsNative( path, - preference.asFileTypeNative(), - preference.fileTypePriorityNative())); + options.asFileTypeNative(), + options.fileTypePriorityNative(), + options.csv.encodingNative(), + options.csv.separatorNative(), + options.csv.quoteNative())); } private static native int[] allFileTypesNative(); @@ -171,8 +174,13 @@ public static DecodedFile open(String path, DecodePreference preference) { private static native long openAsNative(String path, int as); - private static native long openWithPreferenceNative( - String path, int asFileType, int[] fileTypePriority); + private static native long openWithOptionsNative( + String path, + int asFileType, + int[] fileTypePriority, + int csvEncoding, + int csvSeparator, + int csvQuote); private Odr() {} } diff --git a/jni/src/jni_core.cpp b/jni/src/jni_core.cpp index 30c06508d..a48d201f1 100644 --- a/jni/src/jni_core.cpp +++ b/jni/src/jni_core.cpp @@ -232,7 +232,7 @@ Java_app_opendocument_core_Odr_openWithLoggerNative(JNIEnv *env, jclass, jlong logger) { return guarded(env, [&] { return make_handle( - odr::open(to_string(env, path), *from_handle(logger))); + odr::open(to_string(env, path), {}, *from_handle(logger))); }); } @@ -240,29 +240,41 @@ extern "C" JNIEXPORT jlong JNICALL Java_app_opendocument_core_Odr_openAsNative( JNIEnv *env, jclass, jstring path, jint as) { return guarded(env, [&] { return make_handle( - odr::open(to_string(env, path), static_cast(as))); + odr::open(to_string(env, path), + odr::DecodeOptions::as(static_cast(as)))); }); } extern "C" JNIEXPORT jlong JNICALL -Java_app_opendocument_core_Odr_openWithPreferenceNative( +Java_app_opendocument_core_Odr_openWithOptionsNative( JNIEnv *env, jclass, jstring path, jint as_file_type, - jintArray file_type_priority) { + jintArray file_type_priority, jint csv_encoding, jint csv_separator, + jint csv_quote) { return guarded(env, [&] { - odr::DecodePreference preference; + odr::DecodeOptions options; if (as_file_type >= 0) { - preference.as_file_type = static_cast(as_file_type); + options.as_file_type = static_cast(as_file_type); } if (jint *codes = env->GetIntArrayElements(file_type_priority, nullptr); codes != nullptr) { const jsize length = env->GetArrayLength(file_type_priority); for (jsize i = 0; i < length; ++i) { - preference.file_type_priority.push_back( + options.file_type_priority.push_back( static_cast(codes[i])); } env->ReleaseIntArrayElements(file_type_priority, codes, JNI_ABORT); } - return make_handle(odr::open(to_string(env, path), preference)); + // -1 is how java spells an unset field across the boundary + if (csv_encoding >= 0) { + options.csv.encoding = static_cast(csv_encoding); + } + if (csv_separator >= 0) { + options.csv.separator = static_cast(csv_separator); + } + if (csv_quote >= 0) { + options.csv.quote = static_cast(csv_quote); + } + return make_handle(odr::open(to_string(env, path), options)); }); } diff --git a/jni/tests/app/opendocument/core/FileTest.java b/jni/tests/app/opendocument/core/FileTest.java index c399632d4..f30f0c4a4 100644 --- a/jni/tests/app/opendocument/core/FileTest.java +++ b/jni/tests/app/opendocument/core/FileTest.java @@ -7,6 +7,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; +import java.nio.file.Files; import java.nio.file.Path; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -118,6 +119,20 @@ void openMissingFileThrows() { () -> Odr.open(tempDir.resolve("missing.odt").toString())); } + @Test + void openCarriesCsvOptions() throws IOException { + Path path = Files.writeString(tempDir.resolve("semicolons.csv"), "a;b\n1;2\n"); + + // detection would find the semicolon; a pipe it would not, so the caller says + DecodeOptions options = new DecodeOptions(); + options.asFileType = FileType.COMMA_SEPARATED_VALUES; + options.csv.separator = '|'; + + try (DecodedFile file = Odr.open(path.toString(), options)) { + assertEquals(FileType.COMMA_SEPARATED_VALUES, file.fileType()); + } + } + @Test void listFileTypes() throws IOException { Path odt = TestFiles.odtFile(tempDir); diff --git a/python/src/bind_core.cpp b/python/src/bind_core.cpp index e9d369f59..07a5af955 100644 --- a/python/src/bind_core.cpp +++ b/python/src/bind_core.cpp @@ -167,52 +167,20 @@ void odr_python::bind_functions(py::module_ &m) { // Python `ILogger` re-acquires it in the trampoline. m.def( "open", - [](const odr::File &file, const odr::Logger &logger) { - return odr::open(file, logger); - }, - py::arg("file"), py::arg("logger") = odr::Logger::null(), - py::call_guard(), "Decode a file."); - m.def( - "open", - [](const odr::File &file, const odr::FileType as, - const odr::Logger &logger) { return odr::open(file, as, logger); }, - py::arg("file"), py::arg("as_type"), - py::arg("logger") = odr::Logger::null(), - py::call_guard(), - "Decode a file as a specific file type."); - m.def( - "open", - [](const odr::File &file, const odr::DecodePreference &preference, + [](const odr::File &file, const odr::DecodeOptions &options, const odr::Logger &logger) { - return odr::open(file, preference, logger); - }, - py::arg("file"), py::arg("preference"), - py::arg("logger") = odr::Logger::null(), - py::call_guard(), - "Decode a file with a decode preference."); - m.def( - "open", - [](const std::string &path, const odr::Logger &logger) { - return odr::open(path, logger); + return odr::open(file, options, logger); }, - py::arg("path"), py::arg("logger") = odr::Logger::null(), - py::call_guard(), "Open and decode a file."); - m.def( - "open", - [](const std::string &path, const odr::FileType as, - const odr::Logger &logger) { return odr::open(path, as, logger); }, - py::arg("path"), py::arg("as_type"), + py::arg("file"), py::arg("options") = odr::DecodeOptions{}, py::arg("logger") = odr::Logger::null(), - py::call_guard(), - "Open and decode a file as a specific file type."); + py::call_guard(), "Decode a file."); m.def( "open", - [](const std::string &path, const odr::DecodePreference &preference, + [](const std::string &path, const odr::DecodeOptions &options, const odr::Logger &logger) { - return odr::open(path, preference, logger); + return odr::open(path, options, logger); }, - py::arg("path"), py::arg("preference"), + py::arg("path"), py::arg("options") = odr::DecodeOptions{}, py::arg("logger") = odr::Logger::null(), - py::call_guard(), - "Open and decode a file with a decode preference."); + py::call_guard(), "Open and decode a file."); } diff --git a/python/src/bind_file.cpp b/python/src/bind_file.cpp index 199a18946..695fa0807 100644 --- a/python/src/bind_file.cpp +++ b/python/src/bind_file.cpp @@ -164,11 +164,36 @@ void odr_python::bind_file(py::module_ &m) { .value("spreadsheet", odr::DocumentType::spreadsheet) .value("drawing", odr::DocumentType::drawing); - py::class_(m, "DecodePreference") - .def(py::init<>()) - .def_readwrite("as_file_type", &odr::DecodePreference::as_file_type) + py::class_(m, "CsvOptions", + "How to read a csv file. An unset field is " + "detected from the file's opening bytes.") + .def(py::init([](std::optional encoding, + std::optional separator, + std::optional quote) { + return odr::CsvOptions{encoding, separator, quote}; + }), + py::arg("encoding") = py::none(), py::arg("separator") = py::none(), + py::arg("quote") = py::none()) + .def_readwrite("encoding", &odr::CsvOptions::encoding) + .def_readwrite("separator", &odr::CsvOptions::separator) + .def_readwrite("quote", &odr::CsvOptions::quote); + + py::class_(m, "DecodeOptions", + "How to decode a file. Every field is " + "optional; the default detects everything.") + .def(py::init([](std::optional as_file_type, + std::vector file_type_priority, + odr::CsvOptions csv) { + return odr::DecodeOptions{ + as_file_type, std::move(file_type_priority), std::move(csv)}; + }), + py::arg("as_file_type") = py::none(), + py::arg("file_type_priority") = std::vector{}, + py::arg("csv") = odr::CsvOptions{}) + .def_readwrite("as_file_type", &odr::DecodeOptions::as_file_type) .def_readwrite("file_type_priority", - &odr::DecodePreference::file_type_priority); + &odr::DecodeOptions::file_type_priority) + .def_readwrite("csv", &odr::DecodeOptions::csv); py::class_(m, "FileMeta") .def(py::init<>()) @@ -246,18 +271,28 @@ void odr_python::bind_file(py::module_ &m) { .def("is_decodable", &odr::DecodedFile::is_decodable) .def("capabilities", &odr::DecodedFile::capabilities) .def("is_text_file", &odr::DecodedFile::is_text_file) + .def("is_csv_file", &odr::DecodedFile::is_csv_file) .def("is_image_file", &odr::DecodedFile::is_image_file) .def("is_archive_file", &odr::DecodedFile::is_archive_file) .def("is_document_file", &odr::DecodedFile::is_document_file) .def("is_pdf_file", &odr::DecodedFile::is_pdf_file) .def("is_font_file", &odr::DecodedFile::is_font_file) .def("as_text_file", &odr::DecodedFile::as_text_file) + .def("as_csv_file", &odr::DecodedFile::as_csv_file) .def("as_image_file", &odr::DecodedFile::as_image_file) .def("as_archive_file", &odr::DecodedFile::as_archive_file) .def("as_document_file", &odr::DecodedFile::as_document_file) .def("as_pdf_file", &odr::DecodedFile::as_pdf_file) .def("as_font_file", &odr::DecodedFile::as_font_file); + // A csv is a text file too, so `CsvFile` derives from `TextFile` the way the + // C++ handle does - `text()` still reads the raw bytes. + py::class_(m, "CsvFile") + .def("options", &odr::CsvFile::options, + "The options in use, every field resolved.") + .def("document", &odr::CsvFile::document, + "The csv as a one-sheet spreadsheet."); + py::class_(m, "TextFile") .def("encoding", &odr::TextFile::encoding, "The encoding the bytes were detected as, or decoded with.") diff --git a/python/tests/test_file.py b/python/tests/test_file.py index 3dc7c59d2..44de5255e 100644 --- a/python/tests/test_file.py +++ b/python/tests/test_file.py @@ -46,7 +46,9 @@ def test_file_name(txt_path): def test_file_name_of_an_archive_entry(odt_path): - file = pyodr.open(str(odt_path), pyodr.FileType.zip) + file = pyodr.open( + str(odt_path), pyodr.DecodeOptions(as_file_type=pyodr.FileType.zip) + ) filesystem = file.as_archive_file().archive().as_filesystem() assert filesystem.open("/META-INF/manifest.xml").name() == "manifest.xml" @@ -90,17 +92,33 @@ def test_open_json_file(json_path): def test_open_as_type(txt_path): - file = pyodr.open(str(txt_path), pyodr.FileType.text_file) + file = pyodr.open( + str(txt_path), pyodr.DecodeOptions(as_file_type=pyodr.FileType.text_file) + ) assert file.file_type() == pyodr.FileType.text_file -def test_open_with_preference(txt_path): - preference = pyodr.DecodePreference() - preference.as_file_type = pyodr.FileType.text_file - file = pyodr.open(str(txt_path), preference) +def test_open_with_options(txt_path): + options = pyodr.DecodeOptions(as_file_type=pyodr.FileType.text_file) + file = pyodr.open(str(txt_path), options) assert file.file_type() == pyodr.FileType.text_file +def test_open_carries_csv_options(tmp_path): + path = tmp_path / "semicolons.csv" + path.write_text("a;b\n1;2\n", encoding="utf-8") + + # detection would find the semicolon; a pipe it would not, so the caller says + csv = pyodr.open( + str(path), + pyodr.DecodeOptions( + as_file_type=pyodr.FileType.comma_separated_values, + csv=pyodr.CsvOptions(separator="|"), + ), + ).as_csv_file() + assert csv.options().separator == "|" + + def test_file_meta(csv_path): file = pyodr.open(str(csv_path)) meta = file.file_meta() @@ -113,7 +131,9 @@ def test_file_meta(csv_path): def test_open_zip_archive(odt_path): - file = pyodr.open(str(odt_path), pyodr.FileType.zip) + file = pyodr.open( + str(odt_path), pyodr.DecodeOptions(as_file_type=pyodr.FileType.zip) + ) assert file.is_archive_file() filesystem = file.as_archive_file().archive().as_filesystem() @@ -142,22 +162,24 @@ def test_open_from_memory(odt_path): def test_open_from_memory_as_type(odt_path): file = pyodr.File.from_memory(odt_path.read_bytes()) - assert pyodr.open(file, pyodr.FileType.zip).is_archive_file() + assert pyodr.open( + file, pyodr.DecodeOptions(as_file_type=pyodr.FileType.zip) + ).is_archive_file() - preference = pyodr.DecodePreference() - preference.as_file_type = pyodr.FileType.zip - assert pyodr.open(file, preference).is_archive_file() + options = pyodr.DecodeOptions(as_file_type=pyodr.FileType.zip) + assert pyodr.open(file, options).is_archive_file() def test_decoded_file_from_file(odt_path): file = pyodr.File.from_memory(odt_path.read_bytes()) assert pyodr.open(file).file_type() == pyodr.FileType.opendocument_text - assert pyodr.open(file, pyodr.FileType.zip).is_archive_file() + assert pyodr.open( + file, pyodr.DecodeOptions(as_file_type=pyodr.FileType.zip) + ).is_archive_file() - preference = pyodr.DecodePreference() - preference.as_file_type = pyodr.FileType.zip - assert pyodr.open(file, preference).is_archive_file() + options = pyodr.DecodeOptions(as_file_type=pyodr.FileType.zip) + assert pyodr.open(file, options).is_archive_file() def test_document_file_from_file(odt_path): diff --git a/src/odr/file.cpp b/src/odr/file.cpp index a202d2539..c14d2d4ba 100644 --- a/src/odr/file.cpp +++ b/src/odr/file.cpp @@ -32,6 +32,18 @@ deref(const std::shared_ptr &impl) { } // namespace +DecodeOptions DecodeOptions::as(const FileType type) { + DecodeOptions result; + result.as_file_type = type; + return result; +} + +DecodeOptions DecodeOptions::as_csv(const CsvOptions &options) { + DecodeOptions result = as(FileType::comma_separated_values); + result.csv = options; + return result; +} + File File::from_disk(const std::string &path) { return File(std::make_shared(path)); } @@ -276,13 +288,6 @@ std::shared_ptr TextFile::impl() const { return m_impl; } -CsvFile CsvFile::from_file(const File &file, const CsvOptions &options, - const Logger &logger) { - ODR_VERBOSE(logger, "open as csv with options"); - return CsvFile( - std::make_shared(file.impl(), options)); -} - CsvFile::CsvFile(std::shared_ptr impl) : DecodedFile(impl), m_impl{std::move(impl)} {} @@ -290,10 +295,6 @@ Document CsvFile::document() const { return Document(m_impl->document()); } CsvOptions CsvFile::options() const { return m_impl->options(); } -CsvFile CsvFile::with_options(const CsvOptions &options) const { - return CsvFile(m_impl->with_options(options)); -} - std::shared_ptr CsvFile::impl() const { return m_impl; } diff --git a/src/odr/file.hpp b/src/odr/file.hpp index 53dda5f90..35fbcd1f2 100644 --- a/src/odr/file.hpp +++ b/src/odr/file.hpp @@ -206,13 +206,6 @@ struct FileTypeCapabilities final { bool encrypt{}; ///< @ref Document::save with a password is supported }; -/// @brief Preference for decoding files. -struct DecodePreference final { - std::optional as_file_type; - - std::vector file_type_priority; -}; - /// @brief Collection of encryption states. enum class EncryptionState { unknown, @@ -284,6 +277,36 @@ enum class TextEncoding { shift_jis, }; +/// @brief How to read a csv file. +/// +/// An unset field is detected from the file's opening bytes; a set one is taken +/// as given. @ref CsvFile::options returns these with every field resolved, so +/// a caller can show what was detected and offer to change it. +struct CsvOptions final { + std::optional encoding{}; + std::optional separator{}; + std::optional quote{}; +}; + +/// @brief How to decode a file. +/// +/// Every field is optional; the default detects everything. @ref as_file_type +/// skips detection outright, @ref file_type_priority only reorders what +/// detection found. +struct DecodeOptions final { + /// Decode as exactly this type, rather than detecting one. + std::optional as_file_type; + /// Preferred types, most preferred first, among those detected. + std::vector file_type_priority; + /// Format-specific overrides for a file decoded as csv. + CsvOptions csv; + + /// @brief Decode as exactly @p type, skipping detection. + [[nodiscard]] static DecodeOptions as(FileType type); + /// @brief Decode as csv, reading it with @p options. + [[nodiscard]] static DecodeOptions as_csv(const CsvOptions &options); +}; + /// @brief Meta information about a file. /// /// The document fields are only meaningful when @ref document_type is set; @@ -416,27 +439,9 @@ class TextFile final : public DecodedFile { std::shared_ptr m_impl; }; -/// @brief How to read a csv file. -/// -/// An unset field is detected from the file's opening bytes; a set one is taken -/// as given. @ref CsvFile::options returns these with every field resolved, so -/// a caller can show what was detected and offer to change it. -struct CsvOptions final { - std::optional encoding{}; - std::optional separator{}; - std::optional quote{}; -}; - /// @brief Represents a csv file. class CsvFile final : public DecodedFile { public: - /// @brief Decodes @p file as a csv, with @p options. - /// @throws NoCsvFile if no separator was given and the file does not look - /// like one. - [[nodiscard]] static CsvFile from_file(const File &file, - const CsvOptions &options, - const Logger &logger = Logger::null()); - explicit CsvFile(std::shared_ptr); /// @brief The csv as a one-sheet spreadsheet. The other view of the same @@ -447,9 +452,6 @@ class CsvFile final : public DecodedFile { /// @brief The options in use, every field resolved. [[nodiscard]] CsvOptions options() const; - /// @brief The same file read with @p options. - [[nodiscard]] CsvFile with_options(const CsvOptions &options) const; - [[nodiscard]] std::shared_ptr impl() const; private: diff --git a/src/odr/internal/open_strategy.cpp b/src/odr/internal/open_strategy.cpp index b5d6b7c62..afb71ccb6 100644 --- a/src/odr/internal/open_strategy.cpp +++ b/src/odr/internal/open_strategy.cpp @@ -77,7 +77,7 @@ bool is_the_requested_ooxml(const ooxml::OfficeOpenXmlFile &file, /// exception (@ref UnsupportedFileType for a type we cannot decode at all). std::unique_ptr open_file_as(const std::shared_ptr &file, const FileType as, - const Logger &logger) { + const DecodeOptions &options, const Logger &logger) { if (as == FileType::opendocument_text || as == FileType::opendocument_presentation || as == FileType::opendocument_spreadsheet || @@ -252,8 +252,11 @@ open_file_as(const std::shared_ptr &file, const FileType as, if (as == FileType::comma_separated_values) { ODR_VERBOSE(logger, "open as csv"); try { - auto text = std::make_shared(file); - return std::make_unique(text); + return std::make_unique(file, options.csv); + } catch (const std::invalid_argument &) { + // an incoherent dialect is a caller mistake, not "these bytes are not a + // csv" - saying the latter would send them looking at the file + throw; } catch (...) { ODR_VERBOSE(logger, "failed to open as csv"); } @@ -454,9 +457,14 @@ open_strategy::list_file_types(const std::shared_ptr &file, return result; } +namespace { + +/// Decodes by what magic says, engine by engine. What @ref +/// open_strategy::open_file does when @p options names no type and no +/// priority - the probe below is for when it does. std::unique_ptr -open_strategy::open_file(const std::shared_ptr &file, - const Logger &logger) { +open_by_cascade(const std::shared_ptr &file, + const DecodeOptions &options, const Logger &logger) { auto file_type = magic::file_type(*file); ODR_VERBOSE(logger, "magic determined file type " << file_type_to_string(file_type)); @@ -557,7 +565,7 @@ open_strategy::open_file(const std::shared_ptr &file, ODR_VERBOSE(logger, "name says " << file_type_to_string(by_name) << ", try it"); try { - return open_file_as(file, by_name, logger); + return open_file_as(file, by_name, options, logger); } catch (...) { ODR_VERBOSE(logger, "failed to open as what the name says"); } @@ -565,7 +573,7 @@ open_strategy::open_file(const std::shared_ptr &file, try { ODR_VERBOSE(logger, "try open as csv"); - return std::make_unique(text); + return std::make_unique(file, options.csv); } catch (...) { ODR_VERBOSE(logger, "failed to open as csv"); } @@ -616,41 +624,25 @@ open_strategy::open_file(const std::shared_ptr &file, throw UnsupportedFileType(file_type); } +/// Decodes by probing the detected types, reordered by @p options. std::unique_ptr -open_strategy::open_file(const std::shared_ptr &file, - FileType as, const Logger &logger) { - DecodePreference preference; - preference.as_file_type = as; - return open_file(file, preference, logger); -} - -std::unique_ptr -open_strategy::open_file(const std::shared_ptr &file, - const DecodePreference &preference, - const Logger &logger) { - std::vector probe_types; - if (preference.as_file_type.has_value()) { - ODR_VERBOSE(logger, "using preferred file type " - << file_type_to_string(*preference.as_file_type)); - probe_types.push_back(*preference.as_file_type); - } else { - ODR_VERBOSE(logger, "probe file types"); - std::vector detected_types = list_file_types(file, logger); - probe_types.insert(probe_types.end(), detected_types.begin(), - detected_types.end()); - auto probe_types_end = std::ranges::unique(probe_types).begin(); - probe_types.erase(probe_types_end, probe_types.end()); - // more specific file types are further down the list, so we bring them up - std::ranges::reverse(probe_types); - } +open_by_probe(const std::shared_ptr &file, + const DecodeOptions &options, const Logger &logger) { + ODR_VERBOSE(logger, "probe file types"); + std::vector probe_types = + open_strategy::list_file_types(file, logger); + auto probe_types_end = std::ranges::unique(probe_types).begin(); + probe_types.erase(probe_types_end, probe_types.end()); + // more specific file types are further down the list, so we bring them up + std::ranges::reverse(probe_types); std::ranges::stable_sort(probe_types, - priority_comparator(preference.file_type_priority)); + priority_comparator(options.file_type_priority)); for (FileType as : probe_types) { ODR_VERBOSE(logger, "try opening as file type " << file_type_to_string(as)); try { - return open_file_as(file, as, logger); + return open_file_as(file, as, options, logger); } catch (...) { ODR_VERBOSE(logger, "failed to open as file type " << file_type_to_string(as)); @@ -661,4 +653,23 @@ open_strategy::open_file(const std::shared_ptr &file, throw UnknownFileType(); } +} // namespace + +std::unique_ptr +open_strategy::open_file(const std::shared_ptr &file, + const DecodeOptions &options, const Logger &logger) { + // A named type is not a probe with one candidate: there is nothing to move + // on to, so the format's own "not a ..." reaches the caller rather than + // being collapsed into UnknownFileType. + if (options.as_file_type.has_value()) { + ODR_VERBOSE(logger, "open as the requested file type " + << file_type_to_string(*options.as_file_type)); + return open_file_as(file, *options.as_file_type, options, logger); + } + if (options.file_type_priority.empty()) { + return open_by_cascade(file, options, logger); + } + return open_by_probe(file, options, logger); +} + } // namespace odr::internal diff --git a/src/odr/internal/open_strategy.hpp b/src/odr/internal/open_strategy.hpp index b6d82f873..3c7f451cc 100644 --- a/src/odr/internal/open_strategy.hpp +++ b/src/odr/internal/open_strategy.hpp @@ -5,7 +5,7 @@ namespace odr { enum class FileType; -struct DecodePreference; +struct DecodeOptions; class Logger; } // namespace odr @@ -22,12 +22,7 @@ list_file_types(const std::shared_ptr &file, const Logger &logger); std::unique_ptr -open_file(const std::shared_ptr &file, const Logger &logger); -std::unique_ptr -open_file(const std::shared_ptr &file, FileType as, - const Logger &logger); -std::unique_ptr open_file(const std::shared_ptr &file, - const DecodePreference &preference, const Logger &logger); + const DecodeOptions &options, const Logger &logger); } // namespace odr::internal::open_strategy diff --git a/src/odr/odr.cpp b/src/odr/odr.cpp index 3a6a74f34..bcfab2a67 100644 --- a/src/odr/odr.cpp +++ b/src/odr/odr.cpp @@ -203,33 +203,13 @@ std::string_view odr::mimetype(const std::string &path, const Logger &logger) { return mimetype(File::from_disk(path), logger); } -odr::DecodedFile odr::open(const File &file, const Logger &logger) { - return DecodedFile(internal::open_strategy::open_file(file.impl(), logger)); -} - -odr::DecodedFile odr::open(const File &file, const FileType as, - const Logger &logger) { - return DecodedFile( - internal::open_strategy::open_file(file.impl(), as, logger)); -} - -odr::DecodedFile odr::open(const File &file, const DecodePreference &preference, +odr::DecodedFile odr::open(const File &file, const DecodeOptions &options, const Logger &logger) { return DecodedFile( - internal::open_strategy::open_file(file.impl(), preference, logger)); -} - -odr::DecodedFile odr::open(const std::string &path, const Logger &logger) { - return open(File::from_disk(path), logger); -} - -odr::DecodedFile odr::open(const std::string &path, const FileType as, - const Logger &logger) { - return open(File::from_disk(path), as, logger); + internal::open_strategy::open_file(file.impl(), options, logger)); } odr::DecodedFile odr::open(const std::string &path, - const DecodePreference &preference, - const Logger &logger) { - return open(File::from_disk(path), preference, logger); + const DecodeOptions &options, const Logger &logger) { + return open(File::from_disk(path), options, logger); } diff --git a/src/odr/odr.hpp b/src/odr/odr.hpp index 6291883df..9d81c9491 100644 --- a/src/odr/odr.hpp +++ b/src/odr/odr.hpp @@ -97,25 +97,18 @@ list_file_types(const std::string &path, const Logger &logger = Logger::null()); [[nodiscard]] std::string_view mimetype(const std::string &path, const Logger &logger = Logger::null()); -/// @brief Decodes @p file. -[[nodiscard]] DecodedFile open(const File &file, - const Logger &logger = Logger::null()); -/// @brief Decodes @p file as @p as. -[[nodiscard]] DecodedFile open(const File &file, FileType as, - const Logger &logger = Logger::null()); -/// @brief Decodes @p file by @p preference. +/// @brief Decodes @p file, per @p options. +/// +/// The default @ref DecodeOptions detects everything, so +/// `open(file)` is the common call and +/// `open(file, DecodeOptions::as(FileType::comma_separated_values))` the way +/// to insist on a type. [[nodiscard]] DecodedFile open(const File &file, - const DecodePreference &preference, - const Logger &logger = Logger::null()); -/// @brief Opens and decodes the file at @p path. -[[nodiscard]] DecodedFile open(const std::string &path, - const Logger &logger = Logger::null()); -/// @brief Opens the file at @p path, decoding it as @p as. -[[nodiscard]] DecodedFile open(const std::string &path, FileType as, + const DecodeOptions &options = {}, const Logger &logger = Logger::null()); -/// @brief Opens the file at @p path, decoding it by @p preference. +/// @brief Opens and decodes the file at @p path, per @p options. [[nodiscard]] DecodedFile open(const std::string &path, - const DecodePreference &preference, + const DecodeOptions &options = {}, const Logger &logger = Logger::null()); } // namespace odr diff --git a/test/src/document_list_test.cpp b/test/src/document_list_test.cpp index c71d5bf1d..e9f3605fe 100644 --- a/test/src/document_list_test.cpp +++ b/test/src/document_list_test.cpp @@ -43,7 +43,7 @@ void collect_markers(const Element element, const ListType type, std::vector markers_of(const std::string &short_path) { const Logger logger = Logger::create_stdio("odr-test", LogLevel::warning); const DocumentFile document_file = - open(TestData::test_file_path(short_path), logger).as_document_file(); + open(TestData::test_file_path(short_path), {}, logger).as_document_file(); std::vector result; collect_markers(document_file.document().root_element(), ListType::unordered, diff --git a/test/src/document_test.cpp b/test/src/document_test.cpp index 0425fd372..2cccd35ef 100644 --- a/test/src/document_test.cpp +++ b/test/src/document_test.cpp @@ -73,7 +73,7 @@ Document edit_and_reload(const std::string &path, const char *diff, const Logger logger = Logger::create_stdio("odr-test", LogLevel::verbose); const DocumentFile document_file = - open(TestData::test_file_path(path), logger).as_document_file(); + open(TestData::test_file_path(path), {}, logger).as_document_file(); const Document document = document_file.document(); html::edit(document, diff); @@ -101,7 +101,7 @@ TEST(Document, odt) { const Logger logger = Logger::create_stdio("odr-test", LogLevel::verbose); const DocumentFile document_file = - open(TestData::test_file_path("odr-public/odt/about.odt"), logger) + open(TestData::test_file_path("odr-public/odt/about.odt"), {}, logger) .as_document_file(); EXPECT_EQ(document_file.file_type(), FileType::opendocument_text); @@ -124,7 +124,7 @@ TEST(Document, docx_page_layout) { const Logger logger = Logger::create_stdio("odr-test", LogLevel::verbose); const DocumentFile document_file = - open(TestData::test_file_path("odr-public/docx/sample3.docx"), logger) + open(TestData::test_file_path("odr-public/docx/sample3.docx"), {}, logger) .as_document_file(); const Document document = document_file.document(); @@ -147,7 +147,7 @@ TEST(Document, ods_sheet_page_layout) { const DocumentFile document_file = open(TestData::test_file_path("odr-public/ods/file_example_ODS_100.ods"), - logger) + {}, logger) .as_document_file(); const Document document = document_file.document(); @@ -168,7 +168,7 @@ TEST(Document, ods_sheet_page_layout_without_a_paper_size) { const DocumentFile document_file = open(TestData::test_file_path("odr-public/ods/file_example_ODS_10.ods"), - logger) + {}, logger) .as_document_file(); const Document document = document_file.document(); @@ -185,7 +185,7 @@ TEST(Document, xlsx_sheet_names) { const DocumentFile document_file = open(TestData::test_file_path("odr-public/xlsx/sampledatainsurance.xlsx"), - logger) + {}, logger) .as_document_file(); const Document document = document_file.document(); @@ -202,7 +202,7 @@ TEST(Document, odt_element_path) { const Logger logger = Logger::create_stdio("odr-test", LogLevel::verbose); const DocumentFile document_file = - open(TestData::test_file_path("odr-public/odt/about.odt"), logger) + open(TestData::test_file_path("odr-public/odt/about.odt"), {}, logger) .as_document_file(); EXPECT_EQ(document_file.file_type(), FileType::opendocument_text); @@ -226,7 +226,7 @@ TEST(Document, odt_element_path2) { const Logger logger = Logger::create_stdio("odr-test", LogLevel::verbose); const DocumentFile document_file = - open(TestData::test_file_path("odr-public/odt/style-various-1.odt"), + open(TestData::test_file_path("odr-public/odt/style-various-1.odt"), {}, logger) .as_document_file(); @@ -248,7 +248,7 @@ TEST(Document, odt_text_position) { const Logger logger = Logger::create_stdio("odr-test", LogLevel::verbose); const DocumentFile document_file = - open(TestData::test_file_path("odr-public/odt/style-various-1.odt"), + open(TestData::test_file_path("odr-public/odt/style-various-1.odt"), {}, logger) .as_document_file(); const Document document = document_file.document(); @@ -277,7 +277,7 @@ TEST(Document, odt_line_height_and_text_indent) { const Logger logger = Logger::create_stdio("odr-test", LogLevel::verbose); const DocumentFile document_file = - open(TestData::test_file_path("odr-public/odt/file-sample_100kB.odt"), + open(TestData::test_file_path("odr-public/odt/file-sample_100kB.odt"), {}, logger) .as_document_file(); const Document document = document_file.document(); @@ -297,7 +297,7 @@ TEST(Document, odg) { const Logger logger = Logger::create_stdio("odr-test", LogLevel::verbose); const DocumentFile document_file = - open(TestData::test_file_path("odr-public/odg/sample.odg"), logger) + open(TestData::test_file_path("odr-public/odg/sample.odg"), {}, logger) .as_document_file(); EXPECT_EQ(document_file.file_type(), FileType::opendocument_graphics); @@ -324,7 +324,7 @@ void edit_every_text_and_reload(const std::string &path, const Logger logger = Logger::create_stdio("odr-test", LogLevel::verbose); const DocumentFile document_file = - open(TestData::test_file_path(path), logger).as_document_file(); + open(TestData::test_file_path(path), {}, logger).as_document_file(); const Document document = document_file.document(); set_every_text(document.root_element(), "hello world!"); diff --git a/test/src/file_test.cpp b/test/src/file_test.cpp index d213df435..d03137177 100644 --- a/test/src/file_test.cpp +++ b/test/src/file_test.cpp @@ -35,9 +35,10 @@ TEST(File, from_disk_matches_the_path_constructor) { EXPECT_EQ(file.size(), File(path).size()); } -/// `open(file, as)` decodes as exactly what it is asked for. A container -/// names its own document type, and `as` is a claim about what is inside it - -/// so a claim the container contradicts is no reading of the file at all. +/// `open(file, DecodeOptions::as(as))` decodes as exactly what it is asked for. +/// A container names its own document type, and `as` is a claim about what is +/// inside it - so a claim the container contradicts is no reading of the file +/// at all. TEST(File, opening_as_the_wrong_document_type_throws) { const struct { const char *path; @@ -62,8 +63,11 @@ TEST(File, opening_as_the_wrong_document_type_throws) { for (const auto &[path, is, is_not] : cases) { const std::string file_path = TestData::test_file_path(path); - EXPECT_EQ(open(file_path, is).file_type(), is) << path; - EXPECT_THROW(std::ignore = open(file_path, is_not), UnknownFileType) + EXPECT_EQ(open(file_path, DecodeOptions::as(is)).file_type(), is) << path; + // the engine that refuses says so in its own words, and which engine that + // is varies by row - `Exception` is the shared base + EXPECT_THROW(std::ignore = open(file_path, DecodeOptions::as(is_not)), + Exception) << path; } } @@ -73,7 +77,7 @@ TEST(File, opening_as_the_wrong_document_type_throws) { TEST(File, an_encrypted_ooxml_opens_as_the_type_asked_for) { const DecodedFile file = open(TestData::test_file_path("odr-public/docx/encrypted.docx"), - FileType::office_open_xml_document); + DecodeOptions::as(FileType::office_open_xml_document)); EXPECT_EQ(file.file_type(), FileType::office_open_xml_encrypted); EXPECT_TRUE(file.password_encrypted()); @@ -88,13 +92,14 @@ TEST(File, a_flat_document_and_a_package_answer_a_wrong_type_alike) { R"(application/vnd.oasis.opendocument.text">)" R"()"; - EXPECT_THROW(std::ignore = open(File::from_memory(flat), - FileType::opendocument_graphics), - UnknownFileType); + EXPECT_THROW(std::ignore = + open(File::from_memory(flat), + DecodeOptions::as(FileType::opendocument_graphics)), + NoOpenDocumentFile); EXPECT_THROW(std::ignore = open(TestData::test_file_path("odr-public/odt/about.odt"), - FileType::opendocument_graphics), - UnknownFileType); + DecodeOptions::as(FileType::opendocument_graphics)), + NoOpenDocumentFile); } TEST(File, name_is_the_file_name_on_disk) { @@ -282,7 +287,7 @@ TEST(DecodedFile, wpd) { const auto path = TestData::test_file_path("odr-public/wpd/Sync3 Sample Page.wpd"); try { - DecodedFile file = open(path, logger); + DecodedFile file = open(path, {}, logger); FAIL(); } catch (const UnsupportedFileType &e) { EXPECT_EQ(e.file_type, FileType::word_perfect); diff --git a/test/src/html_output_test.cpp b/test/src/html_output_test.cpp index 40be7b18f..f09d0092f 100644 --- a/test/src/html_output_test.cpp +++ b/test/src/html_output_test.cpp @@ -94,9 +94,8 @@ TEST_P(HtmlOutputTests, html_meta) { GTEST_SKIP(); } - DecodePreference decode_preference; - decode_preference.as_file_type = test_file.type; - DecodedFile file = open(test_file.absolute_path, decode_preference, logger); + DecodedFile file = + open(test_file.absolute_path, DecodeOptions::as(test_file.type), logger); FileMeta file_meta = file.file_meta(); diff --git a/test/src/html_test.cpp b/test/src/html_test.cpp index 5f3293ac3..517f2298b 100644 --- a/test/src/html_test.cpp +++ b/test/src/html_test.cpp @@ -60,12 +60,12 @@ TEST(html, linked_resources_are_served) { }; check(open(TestData::test_file_path("odr-public/odt/about.odt"), - FileType::zip, logger), + DecodeOptions::as(FileType::zip), logger), "files.html"); - check( - open(TestData::test_file_path("odr-public/txt/lorem ipsum.txt"), logger), - "text.html"); - check(open(TestData::test_file_path("odr-public/pdf/empty.pdf"), logger), + check(open(TestData::test_file_path("odr-public/txt/lorem ipsum.txt"), {}, + logger), + "text.html"); + check(open(TestData::test_file_path("odr-public/pdf/empty.pdf"), {}, logger), "document.html"); } @@ -76,7 +76,7 @@ TEST(html, linked_images_are_served) { const auto logger = Logger::create_stdio("odr-test", LogLevel::verbose); const auto check = [&](const std::string &path) { - const DecodedFile file = open(TestData::test_file_path(path), logger); + const DecodedFile file = open(TestData::test_file_path(path), {}, logger); HtmlConfig config; config.embed_images = false; @@ -119,8 +119,8 @@ TEST(html, archive_entry_yields_to_a_shipped_resource) { const auto logger = Logger::create_stdio("odr-test", LogLevel::verbose); const DecodedFile file = - open(TestData::test_file_path("odr-public/odt/about.odt"), FileType::zip, - logger); + open(TestData::test_file_path("odr-public/odt/about.odt"), + DecodeOptions::as(FileType::zip), logger); HtmlConfig config((std::filesystem::current_path() / "collision").string()); config.embed_shipped_resources = false; @@ -154,8 +154,8 @@ TEST(html, archive_listing) { const auto logger = Logger::create_stdio("odr-test", LogLevel::verbose); const DecodedFile file = - open(TestData::test_file_path("odr-public/odt/about.odt"), FileType::zip, - logger); + open(TestData::test_file_path("odr-public/odt/about.odt"), + DecodeOptions::as(FileType::zip), logger); ASSERT_TRUE(file.is_archive_file()); const std::string output_path = @@ -196,7 +196,8 @@ TEST(html, archive_listing) { namespace { std::string render(const std::string &path, const HtmlConfig &config) { - const DecodedFile file = open(TestData::test_file_path(path), Logger::null()); + const DecodedFile file = + open(TestData::test_file_path(path), {}, Logger::null()); std::ostringstream out; html::translate(file, config).list_views().at(0).write_html(out); @@ -226,16 +227,16 @@ TEST(html, min_content_margin_reaches_every_view) { HtmlConfig config; const auto render_as = [&](const std::string &path, const FileType as) { - const DecodedFile file = - open(TestData::test_file_path(path), as, Logger::null()); + const DecodedFile file = open(TestData::test_file_path(path), + DecodeOptions::as(as), Logger::null()); std::ostringstream out; html::translate(file, config).list_views().at(0).write_html(out); return std::move(out).str(); }; const auto xml = [&] { - const DecodedFile file = - open(File::from_memory("c"), FileType::xml); + const DecodedFile file = open(File::from_memory("c"), + DecodeOptions::as(FileType::xml)); std::ostringstream out; html::translate(file, config).list_views().at(0).write_html(out); return std::move(out).str(); @@ -341,7 +342,7 @@ TEST(html, linked_dark_style_is_served) { config.color_scheme = HtmlColorScheme::system; const DecodedFile file = open( - TestData::test_file_path("odr-public/odt/about.odt"), Logger::null()); + TestData::test_file_path("odr-public/odt/about.odt"), {}, Logger::null()); const HtmlService service = html::translate(file, config); std::ostringstream out; @@ -367,16 +368,16 @@ TEST(html, color_scheme_reaches_every_view) { EXPECT_NE(text.find("--odr-text-gutter:#161b22"), std::string::npos); // a source view - const DecodedFile xml_file = - open(File::from_memory("c"), FileType::xml); + const DecodedFile xml_file = open(File::from_memory("c"), + DecodeOptions::as(FileType::xml)); std::ostringstream xml; html::translate(xml_file, config).list_views().at(0).write_html(xml); EXPECT_NE(xml.str().find("--odr-xml-name:#7ee787"), std::string::npos); // a file listing: the archive view of a zip const DecodedFile archive = - open(TestData::test_file_path("odr-public/odt/about.odt"), FileType::zip, - Logger::null()); + open(TestData::test_file_path("odr-public/odt/about.odt"), + DecodeOptions::as(FileType::zip), Logger::null()); std::ostringstream listing; html::translate(archive, config).list_views().at(0).write_html(listing); EXPECT_NE(listing.str().find("--odr-files-link:#6cb6ff"), std::string::npos); @@ -391,7 +392,8 @@ TEST(html, views) { const auto logger = Logger::create_stdio("odr-test", LogLevel::verbose); const DocumentFile document_file = - open(TestData::test_file_path("odr-public/ods/Senza nome 1.ods"), logger) + open(TestData::test_file_path("odr-public/ods/Senza nome 1.ods"), {}, + logger) .as_document_file(); const Document document = document_file.document(); @@ -412,8 +414,9 @@ TEST(html, views) { TEST(html, paged_output_fits_the_viewport) { const auto logger = Logger::create_stdio("odr-test", LogLevel::verbose); - const DecodedFile file = open( - TestData::test_file_path("odr-public/odp/style-various-1.odp"), logger); + const DecodedFile file = + open(TestData::test_file_path("odr-public/odp/style-various-1.odp"), {}, + logger); const auto render = [&](const HtmlConfig &config) { const std::string cache = @@ -531,7 +534,7 @@ TEST(html, an_image_fits_the_viewport) { const DecodedFile file = open(TestData::test_file_path("odr-public/png/tango-example-icons.png"), - logger); + {}, logger); const auto render = [&](const HtmlConfig &config) { const std::string cache = @@ -563,7 +566,7 @@ TEST(html, an_image_no_browser_decodes_is_not_translated) { // nothing decodes the bytes, so any will do const DecodedFile file = - open(File::from_memory("image bytes"), type, logger); + open(File::from_memory("image bytes"), DecodeOptions::as(type), logger); ASSERT_TRUE(file.is_image_file()) << file_type_to_string(type); EXPECT_THROW(std::ignore = html::translate(file, HtmlConfig()), UnsupportedFileType) @@ -575,7 +578,7 @@ namespace { std::string render_markdown(const std::string &markdown) { const DecodedFile file = - open(File::from_memory(markdown), FileType::markdown); + open(File::from_memory(markdown), DecodeOptions::as(FileType::markdown)); std::ostringstream out; html::translate(file, HtmlConfig()).list_views().at(0).write_html(out); return std::move(out).str(); @@ -590,7 +593,8 @@ DecodedFile csv_file(const std::uint32_t rows, const std::uint32_t columns) { } csv += "\n"; } - return open(File::from_memory(csv), FileType::comma_separated_values); + return open(File::from_memory(csv), + DecodeOptions::as(FileType::comma_separated_values)); } /// A flat ODF sheet holding @p rows, each a `table:table-row`, under the @@ -623,7 +627,8 @@ DecodedFile fods_file(const std::string &rows, columns + rows + R"()" R"()"; - return open(File::from_memory(fods), FileType::opendocument_spreadsheet); + return open(File::from_memory(fods), + DecodeOptions::as(FileType::opendocument_spreadsheet)); } /// A cell holding @p text, styled by `ce1`, or by `ce2` where it wraps. @@ -731,11 +736,13 @@ TEST(html, no_view_declares_a_document_wide_link_target) { const std::array views{ render(open(File::from_memory("a,b\n1,2\n"), - FileType::comma_separated_values)), - render(open(File::from_memory("c"), FileType::xml)), - render(open(File::from_memory("plain text"), FileType::text_file)), + DecodeOptions::as(FileType::comma_separated_values))), + render(open(File::from_memory("c"), + DecodeOptions::as(FileType::xml))), + render(open(File::from_memory("plain text"), + DecodeOptions::as(FileType::text_file))), render(open(File::from_memory("[a](https://x.example)\n"), - FileType::markdown)), + DecodeOptions::as(FileType::markdown))), }; for (const std::string &view : views) { @@ -974,8 +981,9 @@ std::string flat_ods_sheet(const std::string &width) { } std::optional flat_ods_fit(const std::string &width) { - const DecodedFile file = open(File::from_memory(flat_ods_sheet(width)), - FileType::opendocument_spreadsheet); + const DecodedFile file = + open(File::from_memory(flat_ods_sheet(width)), + DecodeOptions::as(FileType::opendocument_spreadsheet)); std::ostringstream out; html::translate(file, HtmlConfig()).list_views().at(0).write_html(out); return print_fit_of(std::move(out).str()); @@ -994,8 +1002,8 @@ TEST(html, a_sheet_is_only_ever_fitted_down) { } TEST(html, a_view_that_renders_no_sheet_has_no_cut) { - const DecodedFile file = - open(File::from_memory("c"), FileType::xml); + const DecodedFile file = open(File::from_memory("c"), + DecodeOptions::as(FileType::xml)); const HtmlService service = html::translate(file, HtmlConfig()); EXPECT_FALSE(service.list_views().at(0).sheet_cut().has_value()); diff --git a/test/src/internal/csv/csv_file_test.cpp b/test/src/internal/csv/csv_file_test.cpp index 7fce41734..c6f5b60b9 100644 --- a/test/src/internal/csv/csv_file_test.cpp +++ b/test/src/internal/csv/csv_file_test.cpp @@ -210,8 +210,9 @@ TEST(RecordReader, an_unterminated_quote_still_yields_its_field) { } TEST(CsvOptions, detection_fills_in_what_was_not_given) { - const CsvFile file = - CsvFile::from_file(File::from_memory("a;b\n1;2\n"), CsvOptions{}); + const CsvFile file = open(File::from_memory("a;b\n1;2\n"), + DecodeOptions::as(FileType::comma_separated_values)) + .as_csv_file(); const CsvOptions options = file.options(); EXPECT_EQ(options.separator, ';'); @@ -224,10 +225,15 @@ TEST(CsvOptions, detection_fills_in_what_was_not_given) { TEST(CsvOptions, a_given_separator_is_taken_as_given) { const std::string content = "a|b\n1|2\n"; - EXPECT_EQ( - CsvFile::from_file(File::from_memory(content), {}).options().separator, - '|'); - EXPECT_EQ(CsvFile::from_file(File::from_memory(content), {.separator = ','}) + EXPECT_EQ(open(File::from_memory(content), + DecodeOptions::as(FileType::comma_separated_values)) + .as_csv_file() + .options() + .separator, + '|'); + EXPECT_EQ(open(File::from_memory(content), + DecodeOptions::as_csv({.separator = ','})) + .as_csv_file() .options() .separator, ','); @@ -236,40 +242,55 @@ TEST(CsvOptions, a_given_separator_is_taken_as_given) { /// One column is no evidence of a csv, but it is a perfectly good csv once /// someone says so. TEST(CsvOptions, a_declared_separator_makes_anything_readable) { - EXPECT_THROW((void)CsvFile::from_file(File::from_memory("a\nb\nc\n"), {}), + EXPECT_THROW((void)open(File::from_memory("a\nb\nc\n"), + DecodeOptions::as(FileType::comma_separated_values)) + .as_csv_file(), NoCsvFile); - EXPECT_NO_THROW((void)CsvFile::from_file(File::from_memory("a\nb\nc\n"), - {.separator = ','})); + EXPECT_NO_THROW((void)open(File::from_memory("a\nb\nc\n"), + DecodeOptions::as_csv({.separator = ','})) + .as_csv_file()); // and so is prose, and an empty file - EXPECT_NO_THROW((void)CsvFile::from_file( - File::from_memory("lorem ipsum dolor\nsit amet\n"), {.separator = ','})); - EXPECT_NO_THROW( - (void)CsvFile::from_file(File::from_memory(""), {.separator = ','})); + EXPECT_NO_THROW((void)open(File::from_memory("lorem ipsum dolor\nsit amet\n"), + DecodeOptions::as_csv({.separator = ','})) + .as_csv_file()); + EXPECT_NO_THROW((void)open(File::from_memory(""), + DecodeOptions::as_csv({.separator = ','})) + .as_csv_file()); } TEST(CsvOptions, an_incoherent_dialect_is_a_caller_mistake) { - EXPECT_THROW((void)CsvFile::from_file(File::from_memory("a,b\n"), - {.separator = '"', .quote = '"'}), - std::invalid_argument); EXPECT_THROW( - (void)CsvFile::from_file(File::from_memory("a,b\n"), {.separator = '\n'}), + (void)open(File::from_memory("a,b\n"), + DecodeOptions::as_csv({.separator = '"', .quote = '"'})) + .as_csv_file(), std::invalid_argument); + EXPECT_THROW((void)open(File::from_memory("a,b\n"), + DecodeOptions::as_csv({.separator = '\n'})) + .as_csv_file(), + std::invalid_argument); } TEST(CsvOptions, a_given_encoding_skips_detection) { // latin-1 bytes that are not valid utf-8; detection would not name them const File file = File::from_memory("caf\xe9,x\nb,y\n"); - EXPECT_EQ(CsvFile::from_file(file, {.encoding = TextEncoding::iso_8859_1}) - .options() - .encoding, - TextEncoding::iso_8859_1); -} - -TEST(CsvOptions, with_options_derives_another_handle) { - const CsvFile file = - CsvFile::from_file(File::from_memory("a;b\n1;2\n"), CsvOptions{}); - const CsvFile other = file.with_options({.separator = ','}); + EXPECT_EQ( + open(file, DecodeOptions::as_csv({.encoding = TextEncoding::iso_8859_1})) + .as_csv_file() + .options() + .encoding, + TextEncoding::iso_8859_1); +} + +/// Reading the same bytes with different options is another `open`, not a +/// method on the handle: `file()` gives the bytes back. +TEST(CsvOptions, the_same_bytes_read_twice) { + const CsvFile file = open(File::from_memory("a;b\n1;2\n"), + DecodeOptions::as(FileType::comma_separated_values)) + .as_csv_file(); + const CsvFile other = + open(file.file(), DecodeOptions::as_csv({.separator = ','})) + .as_csv_file(); EXPECT_EQ(file.options().separator, ';'); EXPECT_EQ(other.options().separator, ','); @@ -278,15 +299,17 @@ TEST(CsvOptions, with_options_derives_another_handle) { TEST(CsvOptions, a_decoded_csv_is_reachable_as_one) { const File file( TestData::test_file_path("odr-public/csv/file_example_ODS_5000.csv")); - const DecodedFile decoded = open(file, FileType::comma_separated_values); + const DecodedFile decoded = + open(file, DecodeOptions::as(FileType::comma_separated_values)); EXPECT_TRUE(decoded.is_csv_file()); EXPECT_EQ(decoded.as_csv_file().options().separator, ','); } TEST(CsvDocument, a_csv_is_a_one_sheet_spreadsheet) { - const CsvFile file = CsvFile::from_file( - File::from_memory("a,b,c\n1,2,3\n4,5,6\n"), CsvOptions{}); + const CsvFile file = open(File::from_memory("a,b,c\n1,2,3\n4,5,6\n"), + DecodeOptions::as(FileType::comma_separated_values)) + .as_csv_file(); const Document document = file.document(); EXPECT_EQ(document.document_type(), DocumentType::spreadsheet); @@ -301,8 +324,9 @@ TEST(CsvDocument, a_csv_is_a_one_sheet_spreadsheet) { /// The sheet is rectangular even where the file is not: a short row pads, a /// long one widens. TEST(CsvDocument, ragged_rows_become_a_rectangle) { - const CsvFile file = CsvFile::from_file(File::from_memory("a,b\n1,2,3\n4\n"), - CsvOptions{.separator = ','}); + const CsvFile file = open(File::from_memory("a,b\n1,2,3\n4\n"), + DecodeOptions::as_csv({.separator = ','})) + .as_csv_file(); const Document document = file.document(); const Sheet sheet = (*document.root_element().children().begin()).as_sheet(); @@ -314,8 +338,9 @@ TEST(CsvDocument, ragged_rows_become_a_rectangle) { } TEST(CsvDocument, the_separator_directive_is_not_data) { - const CsvFile file = - CsvFile::from_file(File::from_memory("sep=;\na;b\n1;2\n"), CsvOptions{}); + const CsvFile file = open(File::from_memory("sep=;\na;b\n1;2\n"), + DecodeOptions::as(FileType::comma_separated_values)) + .as_csv_file(); const Document document = file.document(); const Sheet sheet = (*document.root_element().children().begin()).as_sheet(); @@ -329,19 +354,23 @@ TEST(CsvDocument, the_separator_directive_is_not_data) { TEST(CsvDocument, an_undecodable_encoding_has_no_document) { const File bytes = File::from_memory("a,b\n1,2\n"); - EXPECT_THROW( - (void)CsvFile::from_file(bytes, {.encoding = TextEncoding::shift_jis}), - NoCsvFile); + EXPECT_THROW((void)open(bytes, DecodeOptions::as_csv( + {.encoding = TextEncoding::shift_jis})) + .as_csv_file(), + NoCsvFile); - const CsvFile file = CsvFile::from_file( - bytes, {.encoding = TextEncoding::shift_jis, .separator = ','}); + const CsvFile file = + open(bytes, DecodeOptions::as_csv( + {.encoding = TextEncoding::shift_jis, .separator = ','})) + .as_csv_file(); EXPECT_FALSE(file.is_decodable()); EXPECT_THROW((void)file.document(), UnsupportedTextEncoding); } TEST(CsvDocument, renders_as_a_table) { - const CsvFile file = - CsvFile::from_file(File::from_memory("a,b\n1,2\n"), CsvOptions{}); + const CsvFile file = open(File::from_memory("a,b\n1,2\n"), + DecodeOptions::as(FileType::comma_separated_values)) + .as_csv_file(); const HtmlService service = html::translate(file.document(), HtmlConfig()); std::ostringstream out; @@ -393,8 +422,9 @@ namespace { ValueType value_type_at(const std::string &content, const std::uint32_t column, const std::uint32_t row) { - const CsvFile file = CsvFile::from_file(File::from_memory(content), - CsvOptions{.separator = ','}); + const CsvFile file = open(File::from_memory(content), + DecodeOptions::as_csv({.separator = ','})) + .as_csv_file(); const Document document = file.document(); const Sheet sheet = (*document.root_element().children().begin()).as_sheet(); return sheet.cell(column, row).value_type(); @@ -443,8 +473,9 @@ TEST(CsvValueType, a_column_one_wide_record_opened_holds_one_value) { /// Cells are not reachable by walking, so the generic path machinery has to /// get at them the other way — through `sheet_cell`. TEST(CsvDocument, a_cell_path_round_trips) { - const CsvFile file = - CsvFile::from_file(File::from_memory("a,b\n1,2\n3,4\n"), CsvOptions{}); + const CsvFile file = open(File::from_memory("a,b\n1,2\n3,4\n"), + DecodeOptions::as(FileType::comma_separated_values)) + .as_csv_file(); const Document document = file.document(); const Sheet sheet = (*document.root_element().children().begin()).as_sheet(); @@ -461,7 +492,8 @@ TEST(CsvDocument, a_cell_path_round_trips) { /// line list. TEST(CsvDocument, translating_the_decoded_file_yields_a_table) { const File bytes = File::from_memory("a,b\n1,2\n"); - const DecodedFile decoded = open(bytes, FileType::comma_separated_values); + const DecodedFile decoded = + open(bytes, DecodeOptions::as(FileType::comma_separated_values)); // a csv stays a text file and is rendered as a table anyway EXPECT_TRUE(decoded.is_text_file()); diff --git a/test/src/internal/iwork/keynote_test.cpp b/test/src/internal/iwork/keynote_test.cpp index ee0fc89fa..ad6500b3b 100644 --- a/test/src/internal/iwork/keynote_test.cpp +++ b/test/src/internal/iwork/keynote_test.cpp @@ -76,7 +76,7 @@ TEST(IworkKeynote, is_detected_by_content) { EXPECT_THAT(list_file_types(path, logger), testing::Contains(FileType::iwork_keynote)); - const DecodedFile file = open(path, logger); + const DecodedFile file = open(path, {}, logger); EXPECT_EQ(file.file_type(), FileType::iwork_keynote); EXPECT_EQ(file.file_category(), FileCategory::document); EXPECT_EQ(file.as_document_file().document_type(), @@ -90,7 +90,7 @@ TEST(IworkKeynote, empty) { const Logger logger = Logger::create_stdio("odr-test", LogLevel::verbose); const DocumentFile document_file = - open(TestData::test_file_path("odr-public/key/empty.key"), logger) + open(TestData::test_file_path("odr-public/key/empty.key"), {}, logger) .as_document_file(); EXPECT_EQ(document_file.file_type(), FileType::iwork_keynote); @@ -107,7 +107,7 @@ TEST(IworkKeynote, slide_text) { const Logger logger = Logger::create_stdio("odr-test", LogLevel::verbose); const DocumentFile document_file = - open(TestData::test_file_path("odr-public/key/style-various-1.key"), + open(TestData::test_file_path("odr-public/key/style-various-1.key"), {}, logger) .as_document_file(); @@ -131,7 +131,7 @@ TEST(IworkKeynote, slide_text) { TEST(IworkKeynote, slides_are_named_in_presentation_order) { const DocumentFile document_file = - open(TestData::test_file_path("odr-public/key/style-various-1.key"), + open(TestData::test_file_path("odr-public/key/style-various-1.key"), {}, Logger::null()) .as_document_file(); @@ -150,7 +150,8 @@ TEST(IworkKeynote, slides_are_named_in_presentation_order) { // 1024x768 Keynote has defaulted to since the 13 era. TEST(IworkKeynote, slide_page_layout_comes_from_the_show) { const DocumentFile document_file = - open(TestData::test_file_path("odr-public/key/empty.key"), Logger::null()) + open(TestData::test_file_path("odr-public/key/empty.key"), {}, + Logger::null()) .as_document_file(); const Document document = document_file.document(); @@ -320,7 +321,8 @@ TEST(IworkKeynote, a_numbers_package_is_not_keynote) { EXPECT_THAT(list_file_types(path, Logger::null()), testing::Not(testing::Contains(FileType::iwork_keynote))); EXPECT_THROW(std::ignore = - open(path, FileType::iwork_keynote, Logger::null()), - UnknownFileType); + open(path, DecodeOptions::as(FileType::iwork_keynote), + Logger::null()), + NoIworkFile); } } diff --git a/test/src/internal/iwork/numbers_test.cpp b/test/src/internal/iwork/numbers_test.cpp index 3df6206c6..213aa5259 100644 --- a/test/src/internal/iwork/numbers_test.cpp +++ b/test/src/internal/iwork/numbers_test.cpp @@ -84,7 +84,7 @@ TEST(IworkNumbers, is_detected_by_content) { EXPECT_THAT(list_file_types(path, logger), testing::Contains(FileType::iwork_numbers)); - const DecodedFile file = open(path, logger); + const DecodedFile file = open(path, {}, logger); EXPECT_EQ(file.file_type(), FileType::iwork_numbers); EXPECT_EQ(file.file_category(), FileCategory::document); EXPECT_EQ(file.as_document_file().document_type(), DocumentType::spreadsheet); @@ -94,7 +94,7 @@ TEST(IworkNumbers, is_detected_by_content) { // must come back as a sheet of its declared extent and no content. TEST(IworkNumbers, empty) { const DocumentFile document_file = - open(TestData::test_file_path("odr-public/numbers/empty.numbers"), + open(TestData::test_file_path("odr-public/numbers/empty.numbers"), {}, Logger::null()) .as_document_file(); EXPECT_EQ(document_file.file_type(), FileType::iwork_numbers); @@ -119,7 +119,7 @@ TEST(IworkNumbers, one_sheet_per_table) { const DocumentFile document_file = open(TestData::test_file_path( "odr-public/numbers/style-various-1.numbers"), - Logger::null()) + {}, Logger::null()) .as_document_file(); const Document document = document_file.document(); @@ -132,7 +132,7 @@ TEST(IworkNumbers, cell_values) { const DocumentFile document_file = open(TestData::test_file_path( "odr-public/numbers/style-various-1.numbers"), - Logger::null()) + {}, Logger::null()) .as_document_file(); const Document document = document_file.document(); @@ -153,7 +153,7 @@ TEST(IworkNumbers, a_table_wider_than_it_is_tall) { const DocumentFile document_file = open(TestData::test_file_path( "odr-public/numbers/style-various-1.numbers"), - Logger::null()) + {}, Logger::null()) .as_document_file(); const Document document = document_file.document(); @@ -172,7 +172,7 @@ TEST(IworkNumbers, every_cell_type_the_fixtures_hold) { const DocumentFile document_file = open(TestData::test_file_path( "odr-public/numbers/style-various-1.numbers"), - Logger::null()) + {}, Logger::null()) .as_document_file(); const Document document = document_file.document(); @@ -197,7 +197,7 @@ TEST(IworkNumbers, only_a_number_reports_a_float_value_type) { const DocumentFile document_file = open(TestData::test_file_path( "odr-public/numbers/style-various-1.numbers"), - Logger::null()) + {}, Logger::null()) .as_document_file(); const Document document = document_file.document(); const Sheet sheet = sheet_at(document.root_element(), 2); diff --git a/test/src/internal/iwork/pages_test.cpp b/test/src/internal/iwork/pages_test.cpp index 79a1d8402..8124b32fd 100644 --- a/test/src/internal/iwork/pages_test.cpp +++ b/test/src/internal/iwork/pages_test.cpp @@ -77,7 +77,7 @@ TEST(Iwork, pages_is_detected_by_content) { EXPECT_THAT(list_file_types(path, logger), testing::Contains(FileType::iwork_pages)); - const DecodedFile file = open(path, logger); + const DecodedFile file = open(path, {}, logger); EXPECT_EQ(file.file_type(), FileType::iwork_pages); EXPECT_EQ(file.file_category(), FileCategory::document); EXPECT_EQ(file.as_document_file().document_type(), DocumentType::text); @@ -89,7 +89,7 @@ TEST(Iwork, pages_empty) { const Logger logger = Logger::create_stdio("odr-test", LogLevel::verbose); const DocumentFile document_file = - open(TestData::test_file_path("odr-public/pages/empty.pages"), logger) + open(TestData::test_file_path("odr-public/pages/empty.pages"), {}, logger) .as_document_file(); EXPECT_EQ(document_file.file_type(), FileType::iwork_pages); @@ -107,7 +107,7 @@ TEST(Iwork, pages_body_text) { const DocumentFile document_file = open(TestData::test_file_path("odr-public/pages/style-various-1.pages"), - logger) + {}, logger) .as_document_file(); const Document document = document_file.document(); @@ -135,7 +135,7 @@ TEST(Iwork, pages_body_text) { TEST(Iwork, pages_table) { const DocumentFile document_file = open(TestData::test_file_path("odr-public/pages/style-various-1.pages"), - Logger::null()) + {}, Logger::null()) .as_document_file(); const Document document = document_file.document(); diff --git a/test/src/internal/markdown/markdown_file_test.cpp b/test/src/internal/markdown/markdown_file_test.cpp index ed35443a8..52f3b6529 100644 --- a/test/src/internal/markdown/markdown_file_test.cpp +++ b/test/src/internal/markdown/markdown_file_test.cpp @@ -29,7 +29,7 @@ namespace { /// document to a local before walking it. Document document(const std::string &markdown) { const DecodedFile file = - open(File::from_memory(markdown), FileType::markdown); + open(File::from_memory(markdown), DecodeOptions::as(FileType::markdown)); return file.as_markdown_file().document(); } @@ -89,7 +89,7 @@ std::vector types_of(const std::vector &elements) { /// the same bytes. TEST(MarkdownFile, a_markdown_file_is_a_text_file_that_loads_as_a_document) { const DecodedFile file = - open(File::from_memory("# hello"), FileType::markdown); + open(File::from_memory("# hello"), DecodeOptions::as(FileType::markdown)); const Document md = document("# hello"); EXPECT_EQ(file.file_type(), FileType::markdown); @@ -106,8 +106,8 @@ TEST(MarkdownFile, a_markdown_file_is_a_text_file_that_loads_as_a_document) { /// The whole point: a markdown file handed to the renderer comes out as prose, /// not as the line list a text file renders to. TEST(MarkdownFile, translating_the_decoded_file_yields_the_document) { - const DecodedFile file = - open(File::from_memory("# hello\n\ntext\n"), FileType::markdown); + const DecodedFile file = open(File::from_memory("# hello\n\ntext\n"), + DecodeOptions::as(FileType::markdown)); const HtmlService service = html::translate(file, HtmlConfig()); std::ostringstream out; diff --git a/test/src/internal/odf/odf_flat_file_test.cpp b/test/src/internal/odf/odf_flat_file_test.cpp index b0ffe608b..bb73a80d8 100644 --- a/test/src/internal/odf/odf_flat_file_test.cpp +++ b/test/src/internal/odf/odf_flat_file_test.cpp @@ -143,12 +143,14 @@ TEST(FlatOpenDocumentFile, opening_it_as_a_document_file_works) { TEST(FlatOpenDocumentFile, opening_it_as_a_named_type_works) { const std::string source = flat_text("Hello"); - EXPECT_EQ( - open(File::from_memory(source), FileType::opendocument_text).file_type(), - FileType::opendocument_text); - EXPECT_THROW(std::ignore = open(File::from_memory(source), - FileType::opendocument_graphics), - UnknownFileType); + EXPECT_EQ(open(File::from_memory(source), + DecodeOptions::as(FileType::opendocument_text)) + .file_type(), + FileType::opendocument_text); + EXPECT_THROW(std::ignore = + open(File::from_memory(source), + DecodeOptions::as(FileType::opendocument_graphics)), + NoOpenDocumentFile); } TEST(FlatOpenDocumentFile, the_body_decodes_to_the_same_tree_as_a_package) { diff --git a/test/src/internal/odf/odf_sheet_repeat_test.cpp b/test/src/internal/odf/odf_sheet_repeat_test.cpp index ba6cf90b8..fb41d315b 100644 --- a/test/src/internal/odf/odf_sheet_repeat_test.cpp +++ b/test/src/internal/odf/odf_sheet_repeat_test.cpp @@ -49,7 +49,7 @@ std::string repeated_rows(const std::uint32_t rows_repeated, std::shared_ptr document_of(const std::string &source) { const DocumentFile file = DecodedFile(open_strategy::open_file(std::make_shared(source), - Logger::null())) + {}, Logger::null())) .as_document_file(); return file.impl()->document(); } diff --git a/test/src/internal/oldms/ppt_test.cpp b/test/src/internal/oldms/ppt_test.cpp index 41605bdae..f1016543d 100644 --- a/test/src/internal/oldms/ppt_test.cpp +++ b/test/src/internal/oldms/ppt_test.cpp @@ -69,7 +69,7 @@ TEST(OldMs, ppt_empty) { const Logger logger = Logger::create_stdio("odr-test", LogLevel::verbose); const DocumentFile document_file = - open(TestData::test_file_path("odr-public/ppt/empty.ppt"), logger) + open(TestData::test_file_path("odr-public/ppt/empty.ppt"), {}, logger) .as_document_file(); EXPECT_EQ(document_file.file_type(), @@ -90,7 +90,7 @@ TEST(OldMs, ppt_style_various) { const Logger logger = Logger::create_stdio("odr-test", LogLevel::verbose); const DocumentFile document_file = - open(TestData::test_file_path("odr-public/ppt/style-various-1.ppt"), + open(TestData::test_file_path("odr-public/ppt/style-various-1.ppt"), {}, logger) .as_document_file(); diff --git a/test/src/internal/oldms/xls_test.cpp b/test/src/internal/oldms/xls_test.cpp index 747b32f1f..935b891ae 100644 --- a/test/src/internal/oldms/xls_test.cpp +++ b/test/src/internal/oldms/xls_test.cpp @@ -300,7 +300,7 @@ TEST(OldMs, xls_empty) { const Logger logger = Logger::create_stdio("odr-test", LogLevel::verbose); const DocumentFile document_file = - open(TestData::test_file_path("odr-public/xls/empty.xls"), logger) + open(TestData::test_file_path("odr-public/xls/empty.xls"), {}, logger) .as_document_file(); EXPECT_EQ(document_file.file_type(), FileType::legacy_excel_worksheets); @@ -326,7 +326,7 @@ TEST(OldMs, xls_file_example_10) { const DocumentFile document_file = open(TestData::test_file_path("odr-public/xls/file_example_XLS_10.xls"), - logger) + {}, logger) .as_document_file(); EXPECT_EQ(document_file.file_type(), FileType::legacy_excel_worksheets); @@ -371,7 +371,7 @@ TEST(OldMs, xls_file_example_5000) { const DocumentFile document_file = open(TestData::test_file_path("odr-public/xls/file_example_XLS_5000.xls"), - logger) + {}, logger) .as_document_file(); const Document document = document_file.document(); diff --git a/test/src/internal/ooxml/ooxml_spreadsheet_merge_test.cpp b/test/src/internal/ooxml/ooxml_spreadsheet_merge_test.cpp index 3245d288e..00ec92115 100644 --- a/test/src/internal/ooxml/ooxml_spreadsheet_merge_test.cpp +++ b/test/src/internal/ooxml/ooxml_spreadsheet_merge_test.cpp @@ -73,9 +73,10 @@ Sheet first_sheet(const Document &document) { } Document decode(const std::shared_ptr &file) { - return Document(DecodedFile(open_strategy::open_file(file, Logger::null())) - .as_document_file() - .document()); + return Document( + DecodedFile(open_strategy::open_file(file, {}, Logger::null())) + .as_document_file() + .document()); } constexpr const char *two_cells = diff --git a/test/src/internal/rtf/rtf_document_test.cpp b/test/src/internal/rtf/rtf_document_test.cpp index 74015c897..eac93dbef 100644 --- a/test/src/internal/rtf/rtf_document_test.cpp +++ b/test/src/internal/rtf/rtf_document_test.cpp @@ -230,14 +230,15 @@ TEST(RtfDocument, the_open_strategy_opens_an_rtf) { EXPECT_TRUE(detected.is_document_file()); // as does asking for the type outright - EXPECT_EQ(open(file, FileType::rich_text_format).file_type(), - FileType::rich_text_format); - // the branch's `NoRtfFile` is what `open_file` catches to move on to the - // next candidate type, so a caller asking for an rtf that is not one sees - // the strategy's own answer - EXPECT_THROW(std::ignore = open(File(memory_file("Hello, World!")), - FileType::rich_text_format), - UnknownFileType); + EXPECT_EQ( + open(file, DecodeOptions::as(FileType::rich_text_format)).file_type(), + FileType::rich_text_format); + // a named type has nothing to move on to, so the branch's own `NoRtfFile` + // reaches the caller rather than the strategy's `UnknownFileType` + EXPECT_THROW(std::ignore = + open(File(memory_file("Hello, World!")), + DecodeOptions::as(FileType::rich_text_format)), + NoRtfFile); // and the document-file path, which a caller reaches through `DocumentFile` const DocumentFile document_file = open(file).as_document_file(); diff --git a/test/src/internal/svg/svg_file_test.cpp b/test/src/internal/svg/svg_file_test.cpp index 6a168fd33..c79e3aa56 100644 --- a/test/src/internal/svg/svg_file_test.cpp +++ b/test/src/internal/svg/svg_file_test.cpp @@ -52,8 +52,9 @@ TEST(SvgFile, only_an_svg_opens_as_one) { // `open` reports its own failure to find a reading, not the format's EXPECT_THROW(std::ignore = open(File::from_memory(""), - FileType::scalable_vector_graphics, Logger::null()), - UnknownFileType); + DecodeOptions::as(FileType::scalable_vector_graphics), + Logger::null()), + NoSvgFile); } /// pugixml does not process namespaces, so a prefixed root has to be seen for diff --git a/test/src/internal/text/text_file_test.cpp b/test/src/internal/text/text_file_test.cpp index 89667d865..9fb0da64e 100644 --- a/test/src/internal/text/text_file_test.cpp +++ b/test/src/internal/text/text_file_test.cpp @@ -64,8 +64,10 @@ TEST(TextFile, unrecognised_bytes_do_not_open) { EXPECT_THROW(std::ignore = mimetype(junk), UnknownFileType); EXPECT_THROW(std::ignore = open(junk), UnknownFileType); - // asking for text by name is no way around it - EXPECT_THROW(std::ignore = open(junk, FileType::text_file), UnknownFileType); + // asking for text by name is no way around it, and says so as the text + // engine rather than as the strategy + EXPECT_THROW(std::ignore = open(junk, DecodeOptions::as(FileType::text_file)), + NoTextFile); } TEST(TextFile, encoding_comes_from_the_byte_order_mark) { diff --git a/test/src/internal/xml/xml_file_test.cpp b/test/src/internal/xml/xml_file_test.cpp index 953595c90..257c8885f 100644 --- a/test/src/internal/xml/xml_file_test.cpp +++ b/test/src/internal/xml/xml_file_test.cpp @@ -111,9 +111,10 @@ TEST(XmlDeclaration, the_encoding_pseudo_attribute_is_read_off_the_bytes) { /// Opening the same bytes as a text file is how to ask for the line list. TEST(XmlHtml, opening_it_as_a_text_file_writes_the_line_list) { - const HtmlService service = html::translate( - open(File::from_memory(""), FileType::text_file), HtmlConfig(), - Logger::null()); + const HtmlService service = + html::translate(open(File::from_memory(""), + DecodeOptions::as(FileType::text_file)), + HtmlConfig(), Logger::null()); std::ostringstream out; service.write("text.html", out); diff --git a/test/src/odr_test.cpp b/test/src/odr_test.cpp index 6e369109c..4f9e48a57 100644 --- a/test/src/odr_test.cpp +++ b/test/src/odr_test.cpp @@ -80,7 +80,7 @@ TEST(odr, types_md) { EXPECT_EQ(types.back(), FileType::markdown); // the name only adds a candidate; opening by path takes it - EXPECT_EQ(open(path, logger).file_type(), FileType::markdown); + EXPECT_EQ(open(path, {}, logger).file_type(), FileType::markdown); EXPECT_EQ(mimetype(path, logger), "text/markdown"); } @@ -89,11 +89,11 @@ TEST(odr, a_misnamed_file_is_what_its_bytes_are) { const auto logger = Logger::create_stdio("odr-test", LogLevel::verbose); const auto path = TestData::test_file_path("odr-public/odt/about.odt"); - EXPECT_EQ(open(path, logger).file_type(), FileType::opendocument_text); + EXPECT_EQ(open(path, {}, logger).file_type(), FileType::opendocument_text); // no name at all, so no hint: the same bytes come back as plain text const DecodedFile from_memory = - open(File::from_memory("# heading\n"), logger); + open(File::from_memory("# heading\n"), {}, logger); EXPECT_EQ(from_memory.file_type(), FileType::text_file); } @@ -109,7 +109,7 @@ TEST(odr, a_named_file_in_memory_is_offered_its_type) { EXPECT_EQ(types.front(), FileType::text_file); EXPECT_EQ(types.back(), FileType::markdown); - EXPECT_EQ(open(file, logger).file_type(), FileType::markdown); + EXPECT_EQ(open(file, {}, logger).file_type(), FileType::markdown); EXPECT_EQ(mimetype(file, logger), "text/markdown"); } @@ -193,9 +193,10 @@ TEST(FileTypeTable, html_is_named_but_not_decoded) { EXPECT_FALSE(capabilities.translate_html); const std::string page = "

hi

"; - EXPECT_THROW(std::ignore = - open(File::from_memory(page), html, Logger::null()), - UnknownFileType); + // named but undecodable, and the refusal says which of the two it is + EXPECT_THROW(std::ignore = open(File::from_memory(page), + DecodeOptions::as(html), Logger::null()), + UnsupportedFileType); } /// `FileType::unknown` is the only type we refuse to name a MIME type for. @@ -249,7 +250,8 @@ TEST(FileTypeCapabilities, color_scheme_matches_the_html) { std::optional file; try { - file = open(test_files.front().absolute_path, type, logger); + file = open(test_files.front().absolute_path, DecodeOptions::as(type), + logger); } catch (...) { continue; } @@ -296,15 +298,15 @@ TEST(FileTypeCapabilities, declaration_matches_the_engines) { const TestFile &test_file = test_files[i]; if (!declared.open) { - EXPECT_ANY_THROW(std::ignore = - open(test_file.absolute_path, type, logger)) + EXPECT_ANY_THROW(std::ignore = open(test_file.absolute_path, + DecodeOptions::as(type), logger)) << test_file.short_path; continue; } std::optional file; try { - file = open(test_file.absolute_path, type, logger); + file = open(test_file.absolute_path, DecodeOptions::as(type), logger); } catch (...) { // declared support is an upper bound — a single file may still fail continue; diff --git a/wasm/src/wasm_file.cpp b/wasm/src/wasm_file.cpp index 0fe432fb4..eaaaf8ff4 100644 --- a/wasm/src/wasm_file.cpp +++ b/wasm/src/wasm_file.cpp @@ -52,7 +52,7 @@ emscripten::val open(const std::string &bytes, std::string name, const emscripten::val &config) { return guarded([&] { return opened( - odr::open(from_bytes(bytes, std::move(name)), default_logger()), + odr::open(from_bytes(bytes, std::move(name)), {}, default_logger()), config); }); } @@ -61,7 +61,8 @@ emscripten::val open_as(const std::string &bytes, std::string name, const int as, const emscripten::val &config) { return guarded([&] { return opened(odr::open(from_bytes(bytes, std::move(name)), - static_cast(as), default_logger()), + DecodeOptions::as(static_cast(as)), + default_logger()), config); }); }