From 7c6c841d68c45f70bd6473d633017c21a2dc8514 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 27 Jul 2026 23:33:25 +0200 Subject: [PATCH 1/4] feat!: clean up the public API for 6.0.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Breaking changes to `odr::` ahead of the 6.0.0 tag, plus two long-standing defects in the public element API. Fixed: * `ElementIterator::operator++(int)` was `const`, so it could not advance, and returned the successor rather than the pre-increment value — `it++` silently violated the forward-iterator contract the class advertises. * `Element` and `ElementIterator` equality compared only the element identifier, so element n of one document compared equal to element n of an unrelated one. Non-existent iterators still compare equal to each other so `ElementRange::end()` keeps working. Changed: * Shape geometry is typed: `Frame`, `Rect`, `Line`, `Circle` and `CustomShape` return `Measure` instead of `std::string`, and `Frame::z_index` returns `std::optional`. The OOXML and ppt engines already built a `Measure` internally and called `to_string()` only to satisfy the string interface, so for them this is a simplification. * `Measure::to_string` renders 7 significant digits in positional notation via the new `util::number::to_string_significant`. The iostream default rounded to 4 (a drawing coordinate of `-6734.61mm` came out `-6735mm`) and could fall back to scientific notation, which CSS and SVG lengths reject. 7 digits is also the ceiling a `float` carries, so an xlsx column width stored as float `68.55` does not come back as `68.550003`. * `TextStyle::font_name` is `std::optional` rather than `const char *`, matching its sibling fields and keeping the zero-copy contract the oldms style registries are built around. * Every exception derives from `odr::Exception`. Decoders still throw plain `std::runtime_error` for malformed input with no dedicated type, so that remains the widest net. Mirrored in Python as `pyodr.Error`. * `Color` uses named factories `from_rgb` / `from_argb`, replacing the `Color(std::uint32_t, bool dummy)` disambiguator. * `html::edit` takes `std::string_view`; `Html::config()` is `const`. * `Measure` moved to ``, where AGENTS.md already documented it; `` still provides it. Removed: * `DecoderEngine` and the whole decoder-selection dimension. Since the external backends went in #622 the enum had one value and could not select anything. Drops `DecodePreference::with_engine` / `engine_priority`, `list_decoder_engines`, `decoder_engine_by_name` / `_to_string`, `DecodedFile::decoder_engine`, both decoder-engine exceptions, the `abstract::DecodedFile` hook and its twelve engine overrides, and the bindings' equivalents including `DecoderEngine.java`. Also drops a duplicate `table_position.cpp` from `ODR_SOURCE_FILES`, documents `word_perfect` / `rich_text_format` as detection-only, corrects the README (json is supported), and adds CHANGELOG.md with an API stability policy. Reference outputs regenerated: 232 files, numeric formatting only, no structural change. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FoqZKnajAkjWg1tdX3EqBq --- CHANGELOG.md | 84 ++++ CMakeLists.txt | 2 +- README.md | 16 +- cli/src/back_translate.cpp | 2 +- jni/CMakeLists.txt | 1 - jni/java/app/opendocument/core/Circle.java | 16 +- .../app/opendocument/core/CustomShape.java | 16 +- .../opendocument/core/DecodePreference.java | 12 - .../app/opendocument/core/DecodedFile.java | 6 - .../app/opendocument/core/DecoderEngine.java | 14 - jni/java/app/opendocument/core/Frame.java | 20 +- jni/java/app/opendocument/core/Line.java | 16 +- jni/java/app/opendocument/core/Odr.java | 29 +- .../app/opendocument/core/OdrException.java | 16 - jni/java/app/opendocument/core/Rect.java | 16 +- jni/src/jni_convert.hpp | 5 + jni/src/jni_core.cpp | 40 +- jni/src/jni_document.cpp | 116 ++--- jni/src/jni_file.cpp | 7 - jni/src/jni_html.cpp | 2 +- jni/src/jni_style.cpp | 27 +- jni/src/odr_jni.cpp | 7 - jni/tests/app/opendocument/core/MetaTest.java | 6 - python/src/bind_core.cpp | 51 ++- python/src/bind_file.cpp | 9 +- python/src/bind_html.cpp | 2 +- python/src/bind_style.cpp | 7 +- python/tests/test_meta.py | 5 - src/odr/document_element.cpp | 97 ++-- src/odr/document_element.hpp | 66 ++- src/odr/exceptions.cpp | 116 ++--- src/odr/exceptions.hpp | 114 +++-- src/odr/file.cpp | 9 - src/odr/file.hpp | 13 +- src/odr/html.cpp | 4 +- src/odr/html.hpp | 5 +- src/odr/internal/abstract/document.hpp | 52 ++- src/odr/internal/abstract/file.hpp | 1 - src/odr/internal/cfb/cfb_file.cpp | 4 - src/odr/internal/cfb/cfb_file.hpp | 1 - src/odr/internal/common/image_file.cpp | 4 - src/odr/internal/common/image_file.hpp | 1 - src/odr/internal/csv/csv_file.cpp | 4 - src/odr/internal/csv/csv_file.hpp | 1 - src/odr/internal/font/font_file.cpp | 4 - src/odr/internal/font/font_file.hpp | 1 - src/odr/internal/html/document_element.cpp | 13 +- src/odr/internal/html/document_style.cpp | 82 ++-- src/odr/internal/json/json_file.cpp | 4 - src/odr/internal/json/json_file.hpp | 1 - src/odr/internal/odf/AGENTS.md | 2 +- src/odr/internal/odf/odf_document.cpp | 143 +++--- src/odr/internal/odf/odf_file.cpp | 4 - src/odr/internal/odf/odf_file.hpp | 1 - src/odr/internal/oldms/oldms_file.cpp | 4 - src/odr/internal/oldms/oldms_file.hpp | 1 - .../oldms/presentation/ppt_document.cpp | 19 +- .../internal/oldms/spreadsheet/xls_style.cpp | 6 +- .../internal/oldms/spreadsheet/xls_style.hpp | 6 +- src/odr/internal/oldms/text/doc_style.cpp | 4 +- src/odr/internal/oldms/text/doc_style.hpp | 6 +- src/odr/internal/ooxml/AGENTS.md | 2 +- src/odr/internal/ooxml/ooxml_file.cpp | 4 - src/odr/internal/ooxml/ooxml_file.hpp | 1 - src/odr/internal/ooxml/ooxml_util.cpp | 2 +- .../ooxml_presentation_document.cpp | 38 +- .../ooxml_spreadsheet_document.cpp | 82 ++-- .../spreadsheet/ooxml_spreadsheet_style.cpp | 4 +- .../ooxml/text/ooxml_text_document.cpp | 22 +- src/odr/internal/open_strategy.cpp | 422 +++++++----------- src/odr/internal/open_strategy.hpp | 6 - src/odr/internal/pdf/AGENTS.md | 2 - src/odr/internal/pdf/pdf_file.cpp | 4 - src/odr/internal/pdf/pdf_file.hpp | 1 - src/odr/internal/svm/svm_file.cpp | 4 - src/odr/internal/svm/svm_file.hpp | 1 - src/odr/internal/text/text_file.cpp | 4 - src/odr/internal/text/text_file.hpp | 1 - src/odr/internal/util/number_util.cpp | 41 ++ src/odr/internal/util/number_util.hpp | 19 + src/odr/internal/zip/zip_file.cpp | 4 - src/odr/internal/zip/zip_file.hpp | 1 - src/odr/odr.cpp | 18 - src/odr/odr.hpp | 14 - src/odr/quantity.cpp | 11 + src/odr/quantity.hpp | 31 +- src/odr/style.cpp | 24 +- src/odr/style.hpp | 20 +- test/CMakeLists.txt | 1 + test/data/reference-output/odr-private | 2 +- test/data/reference-output/odr-public | 2 +- test/src/html_output_test.cpp | 29 +- test/src/internal/oldms/doc_test.cpp | 10 +- test/src/internal/oldms/ppt_test.cpp | 6 +- test/src/internal/oldms/xls_test.cpp | 4 +- test/src/internal/util/number_util_test.cpp | 46 ++ 96 files changed, 1033 insertions(+), 1193 deletions(-) create mode 100644 CHANGELOG.md delete mode 100644 jni/java/app/opendocument/core/DecoderEngine.java create mode 100644 src/odr/internal/util/number_util.cpp create mode 100644 src/odr/internal/util/number_util.hpp create mode 100644 test/src/internal/util/number_util_test.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 000000000..d2831cef2 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,84 @@ +# Changelog + +Notable changes to `odrcore`. Releases before 6.0.0 are documented only in the +[GitHub releases](https://github.com/opendocument-app/OpenDocument.core/releases). + +This project follows [semantic versioning](#versioning-and-api-stability) as +described at the bottom of this file. + +## Unreleased — 6.0.0 + +A breaking release that cleans up the public API. Everything below is a +source-incompatible change to `odr::`; see the migration notes per entry. + +### Removed + +- **Deprecated API and the pdf2htmlEX / wvWare backends** (#622). +- **`DecoderEngine` and the whole decoder-selection dimension.** Since the + external backends were dropped in #622, the enum had exactly one value and + could not select anything. Gone: `DecoderEngine`, + `DecodePreference::with_engine`, `DecodePreference::engine_priority`, + `list_decoder_engines`, `decoder_engine_by_name`, `decoder_engine_to_string`, + `DecodedFile::decoder_engine`, `UnknownDecoderEngine` and + `UnsupportedDecoderEngine`. Also removed from the Python and Java bindings + (`DecoderEngine.java` is deleted). + *Migration:* drop the arguments; there is only one decoder. + +### Changed + +- **`FileMeta` collapsed into a single type** (#626). +- **`Logger` is a value type with a public `ILogger` sink** (#625). +- **`HttpServer::listen` split into `bind` and `listen`** (#624). +- **Shape geometry is typed.** `Frame`, `Rect`, `Line`, `Circle` and + `CustomShape` return `Measure` instead of `std::string` for + `x`/`y`/`width`/`height`/`x1`/`y1`/`x2`/`y2`, and `Frame::z_index` returns + `std::optional` instead of a string. + *Migration:* call `.to_string()` where you previously got a string. +- **`Measure::to_string` keeps 10 significant digits** instead of 4. The old + precision silently rounded document geometry (a drawing coordinate of + `-6734.61mm` was emitted as `-6735mm`). Emitted HTML now carries the more + accurate value; reference outputs change numerically but not structurally. +- **`TextStyle::font_name` is `std::optional`** instead of + `const char *`, matching its sibling fields. + *Migration:* `style.font_name != nullptr` becomes + `style.font_name.has_value()`. +- **Every exception derives from `odr::Exception`**, so `catch (const + odr::Exception &)` catches all typed library errors. Note the decoders still + throw plain `std::runtime_error` for malformed input without a dedicated + type. The Python bindings mirror this with a base `pyodr.Error`; the Java + bindings already had `OdrException`. +- **`Color` uses named factories.** `Color(std::uint32_t rgb)` and + `Color(std::uint32_t argb, bool dummy)` are replaced by `Color::from_rgb` and + `Color::from_argb`. +- **`html::edit` takes `std::string_view`** instead of `const char *`. +- **`Html::config()` is `const`**, matching `HtmlService::config()`. +- **`Measure` moved from `` to ``.** Both + headers still provide it; `style.hpp` includes `quantity.hpp`. + +### Fixed + +- **`ElementIterator::operator++(int)` advances the iterator.** It was `const`, + so it could not advance, and it returned the *successor* rather than the + pre-increment value — meaning `it++` silently violated the forward-iterator + contract the class advertises. +- **`Element` and `ElementIterator` equality compare the document.** They + previously compared only the element identifier, so element *n* of one + document compared equal to element *n* of an unrelated one. +- `FileType::word_perfect` and `FileType::rich_text_format` are documented as + detection-only: magic recognises them so a caller can report the type, but + there is no decoder and opening one throws. + +## Versioning and API stability + +`odrcore` uses semantic versioning. + +- The **public API** is everything under `src/odr/*.hpp` — the headers directly + in the `odr/` directory. Within a major version these stay + source-compatible; additions are minor releases. +- **`odr::internal`** (`src/odr/internal/**`) is *not* public. These headers are + installed because parts of the public API hand back `internal::abstract` + types, but anything in that namespace may change in any release. +- **ABI** is not guaranteed between releases, including patch releases. Link + against a matching build. +- The **bindings** (`pyodr`, `app.opendocument.core`) track the C++ API and + break with it. diff --git a/CMakeLists.txt b/CMakeLists.txt index ffd1ffcd7..a2735b8b9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -111,7 +111,6 @@ set(ODR_SOURCE_FILES "src/odr/internal/common/random.cpp" "src/odr/internal/common/style.cpp" "src/odr/internal/common/table_cursor.cpp" - "src/odr/table_position.cpp" "src/odr/internal/common/table_range.cpp" "src/odr/internal/common/temporary_file.cpp" @@ -228,6 +227,7 @@ set(ODR_SOURCE_FILES "src/odr/internal/util/document_util.cpp" "src/odr/internal/util/file_util.cpp" "src/odr/internal/util/hash_util.cpp" + "src/odr/internal/util/number_util.cpp" "src/odr/internal/util/odr_meta_util.cpp" "src/odr/internal/util/stream_util.cpp" "src/odr/internal/util/string_util.cpp" diff --git a/README.md b/README.md index e11e08392..8a8882ae0 100644 --- a/README.md +++ b/README.md @@ -12,14 +12,22 @@ C++ library to visualize files, especially documents, in HTML. - [doc](https://github.com/opendocument-app/OpenDocument.core/issues/104), [ppt](https://github.com/opendocument-app/OpenDocument.core/issues/106), [xls](https://github.com/opendocument-app/OpenDocument.core/issues/105) - [pdf](https://github.com/opendocument-app/OpenDocument.core/issues/108) - txt +- json - [zip](https://github.com/opendocument-app/OpenDocument.core/issues/109) - [cfb](https://github.com/opendocument-app/OpenDocument.core/issues/110) (Microsoft Compound File Binary File Format) +- ttf / otf (font specimen pages) -## Unsupported files +## Detected but not decoded + +These are recognised by `list_file_types` / `mimetype` so a caller can report +them, but there is no decoder — opening one throws: - rtf +- wpd (WordPerfect) + +## Unsupported files + - pages -- json - xml - yaml @@ -47,7 +55,9 @@ As an alternative to the Conan remote you can also export the package locally vi ## Version -Versions and history are tracked on [GitHub](https://github.com/opendocument-app/OpenDocument.core). +Notable changes are in [`CHANGELOG.md`](CHANGELOG.md), which also states what +counts as public API and what stability to expect. Release history is on +[GitHub](https://github.com/opendocument-app/OpenDocument.core/releases). ## Testing diff --git a/cli/src/back_translate.cpp b/cli/src/back_translate.cpp index 21cce3583..22e186ad3 100644 --- a/cli/src/back_translate.cpp +++ b/cli/src/back_translate.cpp @@ -28,7 +28,7 @@ int main(int, char **argv) { const Document document = document_file.document(); const std::string diff = internal::util::file::read(diff_path); - html::edit(document, diff.c_str()); + html::edit(document, diff); document.save(output); diff --git a/jni/CMakeLists.txt b/jni/CMakeLists.txt index 9ee37fe3d..ad3659695 100644 --- a/jni/CMakeLists.txt +++ b/jni/CMakeLists.txt @@ -53,7 +53,6 @@ add_jar(odr_java "java/app/opendocument/core/CustomShape.java" "java/app/opendocument/core/DecodePreference.java" "java/app/opendocument/core/DecodedFile.java" - "java/app/opendocument/core/DecoderEngine.java" "java/app/opendocument/core/DirectionalMeasure.java" "java/app/opendocument/core/DirectionalString.java" "java/app/opendocument/core/Document.java" diff --git a/jni/java/app/opendocument/core/Circle.java b/jni/java/app/opendocument/core/Circle.java index 3b401cbd9..222073d71 100644 --- a/jni/java/app/opendocument/core/Circle.java +++ b/jni/java/app/opendocument/core/Circle.java @@ -6,19 +6,19 @@ public final class Circle extends Element { super(handle, owner); } - public String x() { + public Measure x() { return xNative(handle()); } - public String y() { + public Measure y() { return yNative(handle()); } - public String width() { + public Measure width() { return widthNative(handle()); } - public String height() { + public Measure height() { return heightNative(handle()); } @@ -26,13 +26,13 @@ public GraphicStyle style() { return styleNative(handle()); } - private native String xNative(long handle); + private native Measure xNative(long handle); - private native String yNative(long handle); + private native Measure yNative(long handle); - private native String widthNative(long handle); + private native Measure widthNative(long handle); - private native String heightNative(long handle); + private native Measure heightNative(long handle); private native GraphicStyle styleNative(long handle); } diff --git a/jni/java/app/opendocument/core/CustomShape.java b/jni/java/app/opendocument/core/CustomShape.java index cce6368cc..de48d666e 100644 --- a/jni/java/app/opendocument/core/CustomShape.java +++ b/jni/java/app/opendocument/core/CustomShape.java @@ -6,19 +6,19 @@ public final class CustomShape extends Element { super(handle, owner); } - public String x() { + public Measure x() { return xNative(handle()); } - public String y() { + public Measure y() { return yNative(handle()); } - public String width() { + public Measure width() { return widthNative(handle()); } - public String height() { + public Measure height() { return heightNative(handle()); } @@ -26,13 +26,13 @@ public GraphicStyle style() { return styleNative(handle()); } - private native String xNative(long handle); + private native Measure xNative(long handle); - private native String yNative(long handle); + private native Measure yNative(long handle); - private native String widthNative(long handle); + private native Measure widthNative(long handle); - private native String heightNative(long handle); + private native Measure heightNative(long handle); private native GraphicStyle styleNative(long handle); } diff --git a/jni/java/app/opendocument/core/DecodePreference.java b/jni/java/app/opendocument/core/DecodePreference.java index f9b89c321..4186804fe 100644 --- a/jni/java/app/opendocument/core/DecodePreference.java +++ b/jni/java/app/opendocument/core/DecodePreference.java @@ -7,26 +7,14 @@ public final class DecodePreference { /** Decode as this file type; {@code null} to detect. */ public FileType asFileType; - /** Decode with this engine; {@code null} to choose automatically. */ - public DecoderEngine withEngine; - public List fileTypePriority = new ArrayList<>(); - public List enginePriority = new ArrayList<>(); // Flattened for the native layer. int asFileTypeNative() { return asFileType == null ? -1 : asFileType.toNative(); } - int withEngineNative() { - return withEngine == null ? -1 : withEngine.toNative(); - } - int[] fileTypePriorityNative() { return fileTypePriority.stream().mapToInt(FileType::toNative).toArray(); } - - int[] enginePriorityNative() { - return enginePriority.stream().mapToInt(DecoderEngine::toNative).toArray(); - } } diff --git a/jni/java/app/opendocument/core/DecodedFile.java b/jni/java/app/opendocument/core/DecodedFile.java index 1e2873700..e31725f4e 100644 --- a/jni/java/app/opendocument/core/DecodedFile.java +++ b/jni/java/app/opendocument/core/DecodedFile.java @@ -42,10 +42,6 @@ public FileMeta fileMeta() { return fileMetaNative(handle()); } - public DecoderEngine decoderEngine() { - return DecoderEngine.fromNative(decoderEngineNative(handle())); - } - public boolean passwordEncrypted() { return passwordEncryptedNative(handle()); } @@ -127,8 +123,6 @@ public FontFile asFontFile() { private native FileMeta fileMetaNative(long handle); - private native int decoderEngineNative(long handle); - private native boolean passwordEncryptedNative(long handle); private native int encryptionStateNative(long handle); diff --git a/jni/java/app/opendocument/core/DecoderEngine.java b/jni/java/app/opendocument/core/DecoderEngine.java deleted file mode 100644 index c5755d843..000000000 --- a/jni/java/app/opendocument/core/DecoderEngine.java +++ /dev/null @@ -1,14 +0,0 @@ -package app.opendocument.core; - -/** Mirrors {@code odr::DecoderEngine}; constant order must match the C++ declaration. */ -public enum DecoderEngine { - ODR; - - static DecoderEngine fromNative(int code) { - return code < 0 ? null : values()[code]; - } - - int toNative() { - return ordinal(); - } -} diff --git a/jni/java/app/opendocument/core/Frame.java b/jni/java/app/opendocument/core/Frame.java index b0ba77897..d7153f1fb 100644 --- a/jni/java/app/opendocument/core/Frame.java +++ b/jni/java/app/opendocument/core/Frame.java @@ -10,23 +10,23 @@ public AnchorType anchorType() { return AnchorType.fromNative(anchorTypeNative(handle())); } - public String x() { + public Measure x() { return xNative(handle()); } - public String y() { + public Measure y() { return yNative(handle()); } - public String width() { + public Measure width() { return widthNative(handle()); } - public String height() { + public Measure height() { return heightNative(handle()); } - public String zIndex() { + public Integer zIndex() { return zIndexNative(handle()); } @@ -36,15 +36,15 @@ public GraphicStyle style() { private native int anchorTypeNative(long handle); - private native String xNative(long handle); + private native Measure xNative(long handle); - private native String yNative(long handle); + private native Measure yNative(long handle); - private native String widthNative(long handle); + private native Measure widthNative(long handle); - private native String heightNative(long handle); + private native Measure heightNative(long handle); - private native String zIndexNative(long handle); + private native Integer zIndexNative(long handle); private native GraphicStyle styleNative(long handle); } diff --git a/jni/java/app/opendocument/core/Line.java b/jni/java/app/opendocument/core/Line.java index e8820c80a..113653563 100644 --- a/jni/java/app/opendocument/core/Line.java +++ b/jni/java/app/opendocument/core/Line.java @@ -6,19 +6,19 @@ public final class Line extends Element { super(handle, owner); } - public String x1() { + public Measure x1() { return x1Native(handle()); } - public String y1() { + public Measure y1() { return y1Native(handle()); } - public String x2() { + public Measure x2() { return x2Native(handle()); } - public String y2() { + public Measure y2() { return y2Native(handle()); } @@ -26,13 +26,13 @@ public GraphicStyle style() { return styleNative(handle()); } - private native String x1Native(long handle); + private native Measure x1Native(long handle); - private native String y1Native(long handle); + private native Measure y1Native(long handle); - private native String x2Native(long handle); + private native Measure x2Native(long handle); - private native String y2Native(long handle); + private native Measure y2Native(long handle); private native GraphicStyle styleNative(long handle); } diff --git a/jni/java/app/opendocument/core/Odr.java b/jni/java/app/opendocument/core/Odr.java index 8d374b9ba..e4a914208 100644 --- a/jni/java/app/opendocument/core/Odr.java +++ b/jni/java/app/opendocument/core/Odr.java @@ -57,14 +57,6 @@ public static String mimetypeByFileType(FileType type) { return mimetypeByFileTypeNative(type.toNative()); } - public static String decoderEngineToString(DecoderEngine engine) { - return decoderEngineToStringNative(engine.toNative()); - } - - public static DecoderEngine decoderEngineByName(String name) { - return DecoderEngine.fromNative(decoderEngineByNameNative(name)); - } - /** Determines the possible file types of a file. */ public static List listFileTypes(String path) { List result = new ArrayList<>(); @@ -74,15 +66,6 @@ public static List listFileTypes(String path) { return result; } - /** Determines the decoder engines for a file type. */ - public static List listDecoderEngines(FileType as) { - List result = new ArrayList<>(); - for (int code : listDecoderEnginesNative(as.toNative())) { - result.add(DecoderEngine.fromNative(code)); - } - return result; - } - /** Determines the MIME type of a file. */ public static native String mimetype(String path); @@ -107,9 +90,7 @@ public static DecodedFile open(String path, DecodePreference preference) { openWithPreferenceNative( path, preference.asFileTypeNative(), - preference.withEngineNative(), - preference.fileTypePriorityNative(), - preference.enginePriorityNative())); + preference.fileTypePriorityNative())); } private static native int fileTypeByFileExtensionNative(String extension); @@ -128,14 +109,8 @@ public static DecodedFile open(String path, DecodePreference preference) { private static native String mimetypeByFileTypeNative(int type); - private static native String decoderEngineToStringNative(int engine); - - private static native int decoderEngineByNameNative(String name); - private static native int[] listFileTypesNative(String path); - private static native int[] listDecoderEnginesNative(int as); - private static native long openNative(String path); private static native long openWithLoggerNative(String path, long logger); @@ -143,7 +118,7 @@ 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 withEngine, int[] fileTypePriority, int[] enginePriority); + String path, int asFileType, int[] fileTypePriority); private Odr() {} } diff --git a/jni/java/app/opendocument/core/OdrException.java b/jni/java/app/opendocument/core/OdrException.java index 54d2ed08a..582f2ca69 100644 --- a/jni/java/app/opendocument/core/OdrException.java +++ b/jni/java/app/opendocument/core/OdrException.java @@ -44,22 +44,6 @@ public UnsupportedFileType(String message) { } } - public static final class UnknownDecoderEngine extends OdrException { - private static final long serialVersionUID = 1L; - - public UnknownDecoderEngine(String message) { - super(message); - } - } - - public static final class UnsupportedDecoderEngine extends OdrException { - private static final long serialVersionUID = 1L; - - public UnsupportedDecoderEngine(String message) { - super(message); - } - } - public static final class FileReadError extends OdrException { private static final long serialVersionUID = 1L; diff --git a/jni/java/app/opendocument/core/Rect.java b/jni/java/app/opendocument/core/Rect.java index b11214f68..ae3b41238 100644 --- a/jni/java/app/opendocument/core/Rect.java +++ b/jni/java/app/opendocument/core/Rect.java @@ -6,19 +6,19 @@ public final class Rect extends Element { super(handle, owner); } - public String x() { + public Measure x() { return xNative(handle()); } - public String y() { + public Measure y() { return yNative(handle()); } - public String width() { + public Measure width() { return widthNative(handle()); } - public String height() { + public Measure height() { return heightNative(handle()); } @@ -26,13 +26,13 @@ public GraphicStyle style() { return styleNative(handle()); } - private native String xNative(long handle); + private native Measure xNative(long handle); - private native String yNative(long handle); + private native Measure yNative(long handle); - private native String widthNative(long handle); + private native Measure widthNative(long handle); - private native String heightNative(long handle); + private native Measure heightNative(long handle); private native GraphicStyle styleNative(long handle); } diff --git a/jni/src/jni_convert.hpp b/jni/src/jni_convert.hpp index b88379314..4fd48edc6 100644 --- a/jni/src/jni_convert.hpp +++ b/jni/src/jni_convert.hpp @@ -10,11 +10,16 @@ #include #include +#include namespace odr_jni { // C++ to Java; optionals map to null. jstring make_string_opt(JNIEnv *env, const std::optional &value); +jstring make_string_opt(JNIEnv *env, + const std::optional &value); +jobject make_integer_opt(JNIEnv *env, const std::optional &value); +jobject make_measure(JNIEnv *env, const odr::Measure &value); jobject make_measure(JNIEnv *env, const std::optional &value); jobject make_color(JNIEnv *env, const std::optional &value); jobject make_directional_measure(JNIEnv *env, diff --git a/jni/src/jni_core.cpp b/jni/src/jni_core.cpp index 488e0b2cf..ecfa5c437 100644 --- a/jni/src/jni_core.cpp +++ b/jni/src/jni_core.cpp @@ -128,23 +128,6 @@ Java_app_opendocument_core_Odr_mimetypeByFileTypeNative(JNIEnv *env, jclass, }); } -extern "C" JNIEXPORT jstring JNICALL -Java_app_opendocument_core_Odr_decoderEngineToStringNative(JNIEnv *env, jclass, - jint engine) { - return guarded(env, [&] { - return to_jstring(env, odr::decoder_engine_to_string( - static_cast(engine))); - }); -} - -extern "C" JNIEXPORT jint JNICALL -Java_app_opendocument_core_Odr_decoderEngineByNameNative(JNIEnv *env, jclass, - jstring name) { - return guarded(env, [&] { - return static_cast(odr::decoder_engine_by_name(to_string(env, name))); - }); -} - extern "C" JNIEXPORT jintArray JNICALL Java_app_opendocument_core_Odr_listFileTypesNative(JNIEnv *env, jclass, jstring path) { @@ -158,19 +141,6 @@ Java_app_opendocument_core_Odr_listFileTypesNative(JNIEnv *env, jclass, }); } -extern "C" JNIEXPORT jintArray JNICALL -Java_app_opendocument_core_Odr_listDecoderEnginesNative(JNIEnv *env, jclass, - jint as) { - return guarded(env, [&] { - std::vector codes; - for (const odr::DecoderEngine engine : - odr::list_decoder_engines(static_cast(as))) { - codes.push_back(static_cast(engine)); - } - return to_jint_array(env, codes); - }); -} - extern "C" JNIEXPORT jstring JNICALL Java_app_opendocument_core_Odr_mimetype(JNIEnv *env, jclass, jstring path) { return guarded(env, [&] { @@ -204,16 +174,13 @@ extern "C" JNIEXPORT jlong JNICALL Java_app_opendocument_core_Odr_openAsNative( extern "C" JNIEXPORT jlong JNICALL Java_app_opendocument_core_Odr_openWithPreferenceNative( - JNIEnv *env, jclass, jstring path, jint as_file_type, jint with_engine, - jintArray file_type_priority, jintArray engine_priority) { + JNIEnv *env, jclass, jstring path, jint as_file_type, + jintArray file_type_priority) { return guarded(env, [&] { odr::DecodePreference preference; if (as_file_type >= 0) { preference.as_file_type = static_cast(as_file_type); } - if (with_engine >= 0) { - preference.with_engine = static_cast(with_engine); - } const auto append_codes = [&](jintArray array, auto &target, auto transform) { const jsize length = env->GetArrayLength(array); @@ -225,9 +192,6 @@ Java_app_opendocument_core_Odr_openWithPreferenceNative( }; append_codes(file_type_priority, preference.file_type_priority, [](jint code) { return static_cast(code); }); - append_codes(engine_priority, preference.engine_priority, [](jint code) { - return static_cast(code); - }); return make_handle(odr::open(to_string(env, path), preference)); }); } diff --git a/jni/src/jni_document.cpp b/jni/src/jni_document.cpp index 6b81afb98..7046c3ec9 100644 --- a/jni/src/jni_document.cpp +++ b/jni/src/jni_document.cpp @@ -724,41 +724,41 @@ Java_app_opendocument_core_Frame_anchorTypeNative(JNIEnv *env, jobject, }); } -extern "C" JNIEXPORT jstring JNICALL +extern "C" JNIEXPORT jobject JNICALL Java_app_opendocument_core_Frame_xNative(JNIEnv *env, jobject, jlong handle) { return guarded(env, [&] { - return odr_jni::make_string_opt(env, element(handle).as_frame().x()); + return odr_jni::make_measure(env, element(handle).as_frame().x()); }); } -extern "C" JNIEXPORT jstring JNICALL +extern "C" JNIEXPORT jobject JNICALL Java_app_opendocument_core_Frame_yNative(JNIEnv *env, jobject, jlong handle) { return guarded(env, [&] { - return odr_jni::make_string_opt(env, element(handle).as_frame().y()); + return odr_jni::make_measure(env, element(handle).as_frame().y()); }); } -extern "C" JNIEXPORT jstring JNICALL +extern "C" JNIEXPORT jobject JNICALL Java_app_opendocument_core_Frame_widthNative(JNIEnv *env, jobject, jlong handle) { return guarded(env, [&] { - return odr_jni::make_string_opt(env, element(handle).as_frame().width()); + return odr_jni::make_measure(env, element(handle).as_frame().width()); }); } -extern "C" JNIEXPORT jstring JNICALL +extern "C" JNIEXPORT jobject JNICALL Java_app_opendocument_core_Frame_heightNative(JNIEnv *env, jobject, jlong handle) { return guarded(env, [&] { - return odr_jni::make_string_opt(env, element(handle).as_frame().height()); + return odr_jni::make_measure(env, element(handle).as_frame().height()); }); } -extern "C" JNIEXPORT jstring JNICALL +extern "C" JNIEXPORT jobject JNICALL Java_app_opendocument_core_Frame_zIndexNative(JNIEnv *env, jobject, jlong handle) { return guarded(env, [&] { - return odr_jni::make_string_opt(env, element(handle).as_frame().z_index()); + return odr_jni::make_integer_opt(env, element(handle).as_frame().z_index()); }); } @@ -772,30 +772,34 @@ Java_app_opendocument_core_Frame_styleNative(JNIEnv *env, jobject, // app.opendocument.core.Rect -extern "C" JNIEXPORT jstring JNICALL +extern "C" JNIEXPORT jobject JNICALL Java_app_opendocument_core_Rect_xNative(JNIEnv *env, jobject, jlong handle) { - return guarded( - env, [&] { return to_jstring(env, element(handle).as_rect().x()); }); + return guarded(env, [&] { + return odr_jni::make_measure(env, element(handle).as_rect().x()); + }); } -extern "C" JNIEXPORT jstring JNICALL +extern "C" JNIEXPORT jobject JNICALL Java_app_opendocument_core_Rect_yNative(JNIEnv *env, jobject, jlong handle) { - return guarded( - env, [&] { return to_jstring(env, element(handle).as_rect().y()); }); + return guarded(env, [&] { + return odr_jni::make_measure(env, element(handle).as_rect().y()); + }); } -extern "C" JNIEXPORT jstring JNICALL +extern "C" JNIEXPORT jobject JNICALL Java_app_opendocument_core_Rect_widthNative(JNIEnv *env, jobject, jlong handle) { - return guarded( - env, [&] { return to_jstring(env, element(handle).as_rect().width()); }); + return guarded(env, [&] { + return odr_jni::make_measure(env, element(handle).as_rect().width()); + }); } -extern "C" JNIEXPORT jstring JNICALL +extern "C" JNIEXPORT jobject JNICALL Java_app_opendocument_core_Rect_heightNative(JNIEnv *env, jobject, jlong handle) { - return guarded( - env, [&] { return to_jstring(env, element(handle).as_rect().height()); }); + return guarded(env, [&] { + return odr_jni::make_measure(env, element(handle).as_rect().height()); + }); } extern "C" JNIEXPORT jobject JNICALL @@ -808,28 +812,32 @@ Java_app_opendocument_core_Rect_styleNative(JNIEnv *env, jobject, // app.opendocument.core.Line -extern "C" JNIEXPORT jstring JNICALL +extern "C" JNIEXPORT jobject JNICALL Java_app_opendocument_core_Line_x1Native(JNIEnv *env, jobject, jlong handle) { - return guarded( - env, [&] { return to_jstring(env, element(handle).as_line().x1()); }); + return guarded(env, [&] { + return odr_jni::make_measure(env, element(handle).as_line().x1()); + }); } -extern "C" JNIEXPORT jstring JNICALL +extern "C" JNIEXPORT jobject JNICALL Java_app_opendocument_core_Line_y1Native(JNIEnv *env, jobject, jlong handle) { - return guarded( - env, [&] { return to_jstring(env, element(handle).as_line().y1()); }); + return guarded(env, [&] { + return odr_jni::make_measure(env, element(handle).as_line().y1()); + }); } -extern "C" JNIEXPORT jstring JNICALL +extern "C" JNIEXPORT jobject JNICALL Java_app_opendocument_core_Line_x2Native(JNIEnv *env, jobject, jlong handle) { - return guarded( - env, [&] { return to_jstring(env, element(handle).as_line().x2()); }); + return guarded(env, [&] { + return odr_jni::make_measure(env, element(handle).as_line().x2()); + }); } -extern "C" JNIEXPORT jstring JNICALL +extern "C" JNIEXPORT jobject JNICALL Java_app_opendocument_core_Line_y2Native(JNIEnv *env, jobject, jlong handle) { - return guarded( - env, [&] { return to_jstring(env, element(handle).as_line().y2()); }); + return guarded(env, [&] { + return odr_jni::make_measure(env, element(handle).as_line().y2()); + }); } extern "C" JNIEXPORT jobject JNICALL @@ -842,31 +850,33 @@ Java_app_opendocument_core_Line_styleNative(JNIEnv *env, jobject, // app.opendocument.core.Circle -extern "C" JNIEXPORT jstring JNICALL +extern "C" JNIEXPORT jobject JNICALL Java_app_opendocument_core_Circle_xNative(JNIEnv *env, jobject, jlong handle) { - return guarded( - env, [&] { return to_jstring(env, element(handle).as_circle().x()); }); + return guarded(env, [&] { + return odr_jni::make_measure(env, element(handle).as_circle().x()); + }); } -extern "C" JNIEXPORT jstring JNICALL +extern "C" JNIEXPORT jobject JNICALL Java_app_opendocument_core_Circle_yNative(JNIEnv *env, jobject, jlong handle) { - return guarded( - env, [&] { return to_jstring(env, element(handle).as_circle().y()); }); + return guarded(env, [&] { + return odr_jni::make_measure(env, element(handle).as_circle().y()); + }); } -extern "C" JNIEXPORT jstring JNICALL +extern "C" JNIEXPORT jobject JNICALL Java_app_opendocument_core_Circle_widthNative(JNIEnv *env, jobject, jlong handle) { return guarded(env, [&] { - return to_jstring(env, element(handle).as_circle().width()); + return odr_jni::make_measure(env, element(handle).as_circle().width()); }); } -extern "C" JNIEXPORT jstring JNICALL +extern "C" JNIEXPORT jobject JNICALL Java_app_opendocument_core_Circle_heightNative(JNIEnv *env, jobject, jlong handle) { return guarded(env, [&] { - return to_jstring(env, element(handle).as_circle().height()); + return odr_jni::make_measure(env, element(handle).as_circle().height()); }); } @@ -881,35 +891,37 @@ Java_app_opendocument_core_Circle_styleNative(JNIEnv *env, jobject, // app.opendocument.core.CustomShape -extern "C" JNIEXPORT jstring JNICALL +extern "C" JNIEXPORT jobject JNICALL Java_app_opendocument_core_CustomShape_xNative(JNIEnv *env, jobject, jlong handle) { return guarded(env, [&] { - return odr_jni::make_string_opt(env, element(handle).as_custom_shape().x()); + return odr_jni::make_measure(env, element(handle).as_custom_shape().x()); }); } -extern "C" JNIEXPORT jstring JNICALL +extern "C" JNIEXPORT jobject JNICALL Java_app_opendocument_core_CustomShape_yNative(JNIEnv *env, jobject, jlong handle) { return guarded(env, [&] { - return odr_jni::make_string_opt(env, element(handle).as_custom_shape().y()); + return odr_jni::make_measure(env, element(handle).as_custom_shape().y()); }); } -extern "C" JNIEXPORT jstring JNICALL +extern "C" JNIEXPORT jobject JNICALL Java_app_opendocument_core_CustomShape_widthNative(JNIEnv *env, jobject, jlong handle) { return guarded(env, [&] { - return to_jstring(env, element(handle).as_custom_shape().width()); + return odr_jni::make_measure(env, + element(handle).as_custom_shape().width()); }); } -extern "C" JNIEXPORT jstring JNICALL +extern "C" JNIEXPORT jobject JNICALL Java_app_opendocument_core_CustomShape_heightNative(JNIEnv *env, jobject, jlong handle) { return guarded(env, [&] { - return to_jstring(env, element(handle).as_custom_shape().height()); + return odr_jni::make_measure(env, + element(handle).as_custom_shape().height()); }); } diff --git a/jni/src/jni_file.cpp b/jni/src/jni_file.cpp index 0e7dd0466..9fc9b89cc 100644 --- a/jni/src/jni_file.cpp +++ b/jni/src/jni_file.cpp @@ -138,13 +138,6 @@ Java_app_opendocument_core_DecodedFile_fileMetaNative(JNIEnv *env, jobject, }); } -extern "C" JNIEXPORT jint JNICALL -Java_app_opendocument_core_DecodedFile_decoderEngineNative(JNIEnv *env, jobject, - jlong handle) { - return guarded( - env, [&] { return static_cast(decoded(handle).decoder_engine()); }); -} - extern "C" JNIEXPORT jboolean JNICALL Java_app_opendocument_core_DecodedFile_passwordEncryptedNative(JNIEnv *env, jobject, diff --git a/jni/src/jni_html.cpp b/jni/src/jni_html.cpp index f2d3dbf8e..288b66d43 100644 --- a/jni/src/jni_html.cpp +++ b/jni/src/jni_html.cpp @@ -175,7 +175,7 @@ extern "C" JNIEXPORT void JNICALL Java_app_opendocument_core_Html_editDocument( JNIEnv *env, jclass, jlong document_handle, jstring diff) { guarded(env, [&] { odr::html::edit(*from_handle(document_handle), - to_string(env, diff).c_str()); + to_string(env, diff)); }); } diff --git a/jni/src/jni_style.cpp b/jni/src/jni_style.cpp index ac28fe34d..9dce56a55 100644 --- a/jni/src/jni_style.cpp +++ b/jni/src/jni_style.cpp @@ -120,6 +120,27 @@ jstring make_string_opt(JNIEnv *env, const std::optional &value) { return value.has_value() ? to_jstring(env, *value) : nullptr; } +jstring make_string_opt(JNIEnv *env, + const std::optional &value) { + return value.has_value() ? to_jstring(env, *value) : nullptr; +} + +jobject make_integer_opt(JNIEnv *env, + const std::optional &value) { + if (!value.has_value()) { + return nullptr; + } + return call_static_object(env, "java/lang/Integer", "valueOf", + "(I)Ljava/lang/Integer;", + static_cast(*value)); +} + +jobject make_measure(JNIEnv *env, const odr::Measure &value) { + return new_object(env, "app/opendocument/core/Measure", + "(DLjava/lang/String;)V", value.magnitude(), + to_jstring(env, value.unit().to_string())); +} + jobject make_measure(JNIEnv *env, const std::optional &value) { if (!value.has_value()) { return nullptr; @@ -167,9 +188,9 @@ jobject make_text_style(JNIEnv *env, const odr::TextStyle &style) { "(Ljava/lang/String;Lapp/opendocument/core/Measure;II" "Ljava/lang/Boolean;Ljava/lang/Boolean;Ljava/lang/String;" "Lapp/opendocument/core/Color;Lapp/opendocument/core/Color;I)V", - style.font_name == nullptr ? nullptr : to_jstring(env, style.font_name), - make_measure(env, style.font_size), enum_code(style.font_weight), - enum_code(style.font_style), box_boolean(env, style.font_underline), + make_string_opt(env, style.font_name), make_measure(env, style.font_size), + enum_code(style.font_weight), enum_code(style.font_style), + box_boolean(env, style.font_underline), box_boolean(env, style.font_line_through), make_string_opt(env, style.font_shadow), make_color(env, style.font_color), diff --git a/jni/src/odr_jni.cpp b/jni/src/odr_jni.cpp index aa1b04ca7..7377b00a0 100644 --- a/jni/src/odr_jni.cpp +++ b/jni/src/odr_jni.cpp @@ -134,13 +134,6 @@ void throw_java(JNIEnv *env) { } catch (const odr::UnsupportedFileType &e) { throw_new(env, "app/opendocument/core/OdrException$UnsupportedFileType", e.what()); - } catch (const odr::UnknownDecoderEngine &e) { - throw_new(env, "app/opendocument/core/OdrException$UnknownDecoderEngine", - e.what()); - } catch (const odr::UnsupportedDecoderEngine &e) { - throw_new(env, - "app/opendocument/core/OdrException$UnsupportedDecoderEngine", - e.what()); } catch (const odr::FileReadError &e) { throw_new(env, "app/opendocument/core/OdrException$FileReadError", e.what()); diff --git a/jni/tests/app/opendocument/core/MetaTest.java b/jni/tests/app/opendocument/core/MetaTest.java index 7987bc70c..f8ea8640c 100644 --- a/jni/tests/app/opendocument/core/MetaTest.java +++ b/jni/tests/app/opendocument/core/MetaTest.java @@ -44,10 +44,4 @@ void tablePosition() { assertEquals("A", TablePosition.toColumnString(0)); assertEquals("A1", new TablePosition(0, 0).toString()); } - - @Test - void listDecoderEngines() { - assertTrue( - Odr.listDecoderEngines(FileType.OPENDOCUMENT_TEXT).contains(DecoderEngine.ODR)); - } } diff --git a/python/src/bind_core.cpp b/python/src/bind_core.cpp index c6f355e5d..16e4de347 100644 --- a/python/src/bind_core.cpp +++ b/python/src/bind_core.cpp @@ -32,29 +32,35 @@ void odr_python::bind_core(py::module_ &m) { &odr::GlobalParams::set_libmagic_database_path, py::arg("path")); - py::register_exception(m, "UnsupportedOperation"); + // Mirrors odr::Exception, so `except odr.Error` catches the whole library. + const py::exception error(m, "Error"); + + py::register_exception(m, "UnsupportedOperation", + error); + // The one deliberate exception: mapping onto Python's own FileNotFoundError + // is worth more here than sitting under `Error`. py::register_exception(m, "FileNotFound", PyExc_FileNotFoundError); - py::register_exception(m, "UnknownFileTypeError"); - py::register_exception(m, - "UnsupportedFileTypeError"); - py::register_exception( - m, "UnknownDecoderEngineError"); - py::register_exception( - m, "UnsupportedDecoderEngineError"); - py::register_exception(m, "FileReadError"); - py::register_exception(m, "FileWriteError"); - py::register_exception(m, "NoDocumentFileError"); - py::register_exception(m, - "UnknownDocumentTypeError"); + py::register_exception(m, "UnknownFileTypeError", + error); + py::register_exception( + m, "UnsupportedFileTypeError", error); + py::register_exception(m, "FileReadError", error); + py::register_exception(m, "FileWriteError", error); + py::register_exception(m, "NoDocumentFileError", error); + py::register_exception( + m, "UnknownDocumentTypeError", error); py::register_exception( - m, "UnsupportedCryptoAlgorithmError"); - py::register_exception(m, "WrongPasswordError"); - py::register_exception(m, "DecryptionFailedError"); - py::register_exception(m, "NotEncryptedError"); - py::register_exception(m, "FileEncryptedError"); + m, "UnsupportedCryptoAlgorithmError", error); + py::register_exception(m, "WrongPasswordError", + error); + py::register_exception(m, "DecryptionFailedError", + error); + py::register_exception(m, "NotEncryptedError", error); + py::register_exception(m, "FileEncryptedError", + error); py::register_exception( - m, "DocumentCopyProtectedError"); + m, "DocumentCopyProtectedError", error); } void odr_python::bind_functions(py::module_ &m) { @@ -85,12 +91,6 @@ void odr_python::bind_functions(py::module_ &m) { return std::string(odr::mimetype_by_file_type(type)); }, py::arg("type")); - m.def("decoder_engine_to_string", &odr::decoder_engine_to_string, - py::arg("engine")); - m.def( - "decoder_engine_by_name", - [](const std::string &name) { return odr::decoder_engine_by_name(name); }, - py::arg("name")); m.def( "list_file_types", @@ -99,7 +99,6 @@ void odr_python::bind_functions(py::module_ &m) { }, py::arg("path"), py::arg("logger") = odr::Logger::null(), "Determine the possible file types of a file."); - m.def("list_decoder_engines", &odr::list_decoder_engines, py::arg("as_type")); m.def( "mimetype", [](const std::string &path, const odr::Logger &logger) { diff --git a/python/src/bind_file.cpp b/python/src/bind_file.cpp index b5f6380c6..51102e98b 100644 --- a/python/src/bind_file.cpp +++ b/python/src/bind_file.cpp @@ -68,9 +68,6 @@ void odr_python::bind_file(py::module_ &m) { .value("memory", odr::FileLocation::memory) .value("disk", odr::FileLocation::disk); - py::enum_(m, "DecoderEngine") - .value("odr", odr::DecoderEngine::odr); - py::enum_(m, "EncryptionState") .value("unknown", odr::EncryptionState::unknown) .value("not_encrypted", odr::EncryptionState::not_encrypted) @@ -87,11 +84,8 @@ void odr_python::bind_file(py::module_ &m) { py::class_(m, "DecodePreference") .def(py::init<>()) .def_readwrite("as_file_type", &odr::DecodePreference::as_file_type) - .def_readwrite("with_engine", &odr::DecodePreference::with_engine) .def_readwrite("file_type_priority", - &odr::DecodePreference::file_type_priority) - .def_readwrite("engine_priority", - &odr::DecodePreference::engine_priority); + &odr::DecodePreference::file_type_priority); py::class_(m, "FileMeta") .def(py::init<>()) @@ -141,7 +135,6 @@ void odr_python::bind_file(py::module_ &m) { .def("file_type", &odr::DecodedFile::file_type) .def("file_category", &odr::DecodedFile::file_category) .def("file_meta", &odr::DecodedFile::file_meta) - .def("decoder_engine", &odr::DecodedFile::decoder_engine) .def("password_encrypted", &odr::DecodedFile::password_encrypted) .def("encryption_state", &odr::DecodedFile::encryption_state) .def("decrypt", &odr::DecodedFile::decrypt, py::arg("password")) diff --git a/python/src/bind_html.cpp b/python/src/bind_html.cpp index 7ecaeb169..76e7e34f8 100644 --- a/python/src/bind_html.cpp +++ b/python/src/bind_html.cpp @@ -198,7 +198,7 @@ void odr_python::bind_html(py::module_ &m) { html.def( "edit", [](const odr::Document &document, const std::string &diff) { - odr::html::edit(document, diff.c_str()); + odr::html::edit(document, diff); }, py::arg("document"), py::arg("diff"), "Apply a diff (produced by the browser-side editor) to a document."); diff --git a/python/src/bind_style.cpp b/python/src/bind_style.cpp index b2dc25469..78698d97a 100644 --- a/python/src/bind_style.cpp +++ b/python/src/bind_style.cpp @@ -105,7 +105,8 @@ void odr_python::bind_style(py::module_ &m) { py::class_(m, "Color") .def(py::init<>()) - .def(py::init(), py::arg("rgb")) + .def_static("from_rgb", &odr::Color::from_rgb, py::arg("rgb")) + .def_static("from_argb", &odr::Color::from_argb, py::arg("argb")) .def(py::init(), py::arg("red"), py::arg("green"), py::arg("blue")) .def(py::init(), @@ -125,10 +126,10 @@ void odr_python::bind_style(py::module_ &m) { .def_property_readonly( "font_name", [](const odr::TextStyle &style) -> std::optional { - if (style.font_name == nullptr) { + if (!style.font_name.has_value()) { return std::nullopt; } - return std::string(style.font_name); + return std::string(*style.font_name); }) .def_readwrite("font_size", &odr::TextStyle::font_size) .def_readwrite("font_weight", &odr::TextStyle::font_weight) diff --git a/python/tests/test_meta.py b/python/tests/test_meta.py index 081508a55..faae16ed3 100644 --- a/python/tests/test_meta.py +++ b/python/tests/test_meta.py @@ -70,11 +70,6 @@ def test_mimetype_roundtrip(): ) -def test_decoder_engine(): - assert pyodr.decoder_engine_by_name("odr") == pyodr.DecoderEngine.odr - assert pyodr.decoder_engine_to_string(pyodr.DecoderEngine.odr) == "odr" - - def test_global_params(): assert isinstance(pyodr.GlobalParams.odr_core_data_path(), str) assert isinstance(pyodr.GlobalParams.libmagic_database_path(), str) diff --git a/src/odr/document_element.cpp b/src/odr/document_element.cpp index 36e7093c3..4cb4d88d0 100644 --- a/src/odr/document_element.cpp +++ b/src/odr/document_element.cpp @@ -191,11 +191,10 @@ ElementIterator &ElementIterator::operator++() { return *this; } -ElementIterator ElementIterator::operator++(int) const { - if (!exists_()) { - return {}; - } - return {m_adapter, m_adapter->element_next_sibling(m_identifier)}; +ElementIterator ElementIterator::operator++(int) { + ElementIterator result = *this; + ++*this; + return result; } bool ElementIterator::exists_() const { @@ -464,109 +463,113 @@ AnchorType Frame::anchor_type() const { : AnchorType::as_char; // TODO default? } -std::optional Frame::x() const { +std::optional Frame::x() const { return exists_() ? m_adapter2->frame_x(m_identifier) - : std::optional(); + : std::optional(); } -std::optional Frame::y() const { +std::optional Frame::y() const { return exists_() ? m_adapter2->frame_y(m_identifier) - : std::optional(); + : std::optional(); } -std::optional Frame::width() const { +std::optional Frame::width() const { return exists_() ? m_adapter2->frame_width(m_identifier) - : std::optional(); + : std::optional(); } -std::optional Frame::height() const { +std::optional Frame::height() const { return exists_() ? m_adapter2->frame_height(m_identifier) - : std::optional(); + : std::optional(); } -std::optional Frame::z_index() const { +std::optional Frame::z_index() const { return exists_() ? m_adapter2->frame_z_index(m_identifier) - : std::optional(); + : std::optional(); } GraphicStyle Frame::style() const { return exists_() ? m_adapter2->frame_style(m_identifier) : GraphicStyle(); } -std::string Rect::x() const { - return exists_() ? m_adapter2->rect_x(m_identifier) : ""; +Measure Rect::x() const { + return exists_() ? m_adapter2->rect_x(m_identifier) : Measure(0, {}); } -std::string Rect::y() const { - return exists_() ? m_adapter2->rect_y(m_identifier) : ""; +Measure Rect::y() const { + return exists_() ? m_adapter2->rect_y(m_identifier) : Measure(0, {}); } -std::string Rect::width() const { - return exists_() ? m_adapter2->rect_width(m_identifier) : ""; +Measure Rect::width() const { + return exists_() ? m_adapter2->rect_width(m_identifier) : Measure(0, {}); } -std::string Rect::height() const { - return exists_() ? m_adapter2->rect_height(m_identifier) : ""; +Measure Rect::height() const { + return exists_() ? m_adapter2->rect_height(m_identifier) : Measure(0, {}); } GraphicStyle Rect::style() const { return exists_() ? m_adapter2->rect_style(m_identifier) : GraphicStyle(); } -std::string Line::x1() const { - return exists_() ? m_adapter2->line_x1(m_identifier) : ""; +Measure Line::x1() const { + return exists_() ? m_adapter2->line_x1(m_identifier) : Measure(0, {}); } -std::string Line::y1() const { - return exists_() ? m_adapter2->line_y1(m_identifier) : ""; +Measure Line::y1() const { + return exists_() ? m_adapter2->line_y1(m_identifier) : Measure(0, {}); } -std::string Line::x2() const { - return exists_() ? m_adapter2->line_x2(m_identifier) : ""; +Measure Line::x2() const { + return exists_() ? m_adapter2->line_x2(m_identifier) : Measure(0, {}); } -std::string Line::y2() const { - return exists_() ? m_adapter2->line_y2(m_identifier) : ""; +Measure Line::y2() const { + return exists_() ? m_adapter2->line_y2(m_identifier) : Measure(0, {}); } GraphicStyle Line::style() const { return exists_() ? m_adapter2->line_style(m_identifier) : GraphicStyle(); } -std::string Circle::x() const { - return exists_() ? m_adapter2->circle_x(m_identifier) : ""; +Measure Circle::x() const { + return exists_() ? m_adapter2->circle_x(m_identifier) : Measure(0, {}); } -std::string Circle::y() const { - return exists_() ? m_adapter2->circle_y(m_identifier) : ""; +Measure Circle::y() const { + return exists_() ? m_adapter2->circle_y(m_identifier) : Measure(0, {}); } -std::string Circle::width() const { - return exists_() ? m_adapter2->circle_width(m_identifier) : ""; +Measure Circle::width() const { + return exists_() ? m_adapter2->circle_width(m_identifier) : Measure(0, {}); } -std::string Circle::height() const { - return exists_() ? m_adapter2->circle_height(m_identifier) : ""; +Measure Circle::height() const { + return exists_() ? m_adapter2->circle_height(m_identifier) : Measure(0, {}); } GraphicStyle Circle::style() const { return exists_() ? m_adapter2->circle_style(m_identifier) : GraphicStyle(); } -std::optional CustomShape::x() const { - return exists_() ? m_adapter2->custom_shape_x(m_identifier) : ""; +std::optional CustomShape::x() const { + return exists_() ? m_adapter2->custom_shape_x(m_identifier) + : std::optional(); } -std::optional CustomShape::y() const { - return exists_() ? m_adapter2->custom_shape_y(m_identifier) : ""; +std::optional CustomShape::y() const { + return exists_() ? m_adapter2->custom_shape_y(m_identifier) + : std::optional(); } -std::string CustomShape::width() const { - return exists_() ? m_adapter2->custom_shape_width(m_identifier) : ""; +Measure CustomShape::width() const { + return exists_() ? m_adapter2->custom_shape_width(m_identifier) + : Measure(0, {}); } -std::string CustomShape::height() const { - return exists_() ? m_adapter2->custom_shape_height(m_identifier) : ""; +Measure CustomShape::height() const { + return exists_() ? m_adapter2->custom_shape_height(m_identifier) + : Measure(0, {}); } GraphicStyle CustomShape::style() const { diff --git a/src/odr/document_element.hpp b/src/odr/document_element.hpp index 8f8e4e5c4..f8a108ccd 100644 --- a/src/odr/document_element.hpp +++ b/src/odr/document_element.hpp @@ -1,10 +1,14 @@ #pragma once +#include +#include +#include #include #include #include #include +#include namespace odr { struct TablePosition; @@ -186,8 +190,15 @@ class Element { const internal::abstract::ElementAdapter *m_adapter{nullptr}; ElementIdentifier m_identifier{null_element_id}; + /// Identifiers are only unique within one document, so the adapter has to + /// take part. Elements that don't exist carry no adapter to compare and are + /// all equal to each other. friend bool operator==(const Element &lhs, const Element &rhs) { - return lhs.m_identifier == rhs.m_identifier; + if (!lhs.exists_() || !rhs.exists_()) { + return lhs.exists_() == rhs.exists_(); + } + return lhs.m_adapter == rhs.m_adapter && + lhs.m_identifier == rhs.m_identifier; } [[nodiscard]] bool exists_() const; @@ -209,15 +220,22 @@ class ElementIterator { Element operator*() const; ElementIterator &operator++(); - ElementIterator operator++(int) const; + ElementIterator operator++(int); private: const internal::abstract::ElementAdapter *m_adapter{nullptr}; ElementIdentifier m_identifier{null_element_id}; + /// Same rule as @ref Element: compare the adapter too, and treat every + /// exhausted iterator as the end iterator regardless of which range it came + /// from. friend bool operator==(const ElementIterator &lhs, const ElementIterator &rhs) { - return lhs.m_identifier == rhs.m_identifier; + if (!lhs.exists_() || !rhs.exists_()) { + return lhs.exists_() == rhs.exists_(); + } + return lhs.m_adapter == rhs.m_adapter && + lhs.m_identifier == rhs.m_identifier; } [[nodiscard]] bool exists_() const; @@ -446,11 +464,11 @@ class Frame final : public ElementBase { using ElementBase::ElementBase; [[nodiscard]] AnchorType anchor_type() const; - [[nodiscard]] std::optional x() const; - [[nodiscard]] std::optional y() const; - [[nodiscard]] std::optional width() const; - [[nodiscard]] std::optional height() const; - [[nodiscard]] std::optional z_index() const; + [[nodiscard]] std::optional x() const; + [[nodiscard]] std::optional y() const; + [[nodiscard]] std::optional width() const; + [[nodiscard]] std::optional height() const; + [[nodiscard]] std::optional z_index() const; [[nodiscard]] GraphicStyle style() const; }; @@ -460,10 +478,10 @@ class Rect final : public ElementBase { public: using ElementBase::ElementBase; - [[nodiscard]] std::string x() const; - [[nodiscard]] std::string y() const; - [[nodiscard]] std::string width() const; - [[nodiscard]] std::string height() const; + [[nodiscard]] Measure x() const; + [[nodiscard]] Measure y() const; + [[nodiscard]] Measure width() const; + [[nodiscard]] Measure height() const; [[nodiscard]] GraphicStyle style() const; }; @@ -473,10 +491,10 @@ class Line final : public ElementBase { public: using ElementBase::ElementBase; - [[nodiscard]] std::string x1() const; - [[nodiscard]] std::string y1() const; - [[nodiscard]] std::string x2() const; - [[nodiscard]] std::string y2() const; + [[nodiscard]] Measure x1() const; + [[nodiscard]] Measure y1() const; + [[nodiscard]] Measure x2() const; + [[nodiscard]] Measure y2() const; [[nodiscard]] GraphicStyle style() const; }; @@ -486,10 +504,10 @@ class Circle final : public ElementBase { public: using ElementBase::ElementBase; - [[nodiscard]] std::string x() const; - [[nodiscard]] std::string y() const; - [[nodiscard]] std::string width() const; - [[nodiscard]] std::string height() const; + [[nodiscard]] Measure x() const; + [[nodiscard]] Measure y() const; + [[nodiscard]] Measure width() const; + [[nodiscard]] Measure height() const; [[nodiscard]] GraphicStyle style() const; }; @@ -500,10 +518,10 @@ class CustomShape final public: using ElementBase::ElementBase; - [[nodiscard]] std::optional x() const; - [[nodiscard]] std::optional y() const; - [[nodiscard]] std::string width() const; - [[nodiscard]] std::string height() const; + [[nodiscard]] std::optional x() const; + [[nodiscard]] std::optional y() const; + [[nodiscard]] Measure width() const; + [[nodiscard]] Measure height() const; [[nodiscard]] GraphicStyle style() const; }; diff --git a/src/odr/exceptions.cpp b/src/odr/exceptions.cpp index 70aedc358..30165a769 100644 --- a/src/odr/exceptions.cpp +++ b/src/odr/exceptions.cpp @@ -6,149 +6,131 @@ namespace odr { UnsupportedOperation::UnsupportedOperation() - : std::runtime_error("unsupported operation") {} + : Exception("unsupported operation") {} UnsupportedOperation::UnsupportedOperation(const std::string &message) - : std::runtime_error("unsupported operation: " + message) {} + : Exception("unsupported operation: " + message) {} -FileNotFound::FileNotFound() : std::runtime_error("file not found") {} +FileNotFound::FileNotFound() : Exception("file not found") {} FileNotFound::FileNotFound(const std::string &path) - : std::runtime_error("file not found: " + path) {} + : Exception("file not found: " + path) {} -UnknownFileType::UnknownFileType() : std::runtime_error("unknown file type") {} +UnknownFileType::UnknownFileType() : Exception("unknown file type") {} UnsupportedFileType::UnsupportedFileType(const FileType file_type) - : std::runtime_error("unknown file type: " + - file_type_to_string(file_type)), + : Exception("unknown file type: " + file_type_to_string(file_type)), file_type{file_type} {} -UnknownDecoderEngine::UnknownDecoderEngine() - : std::runtime_error("unknown decoder engine") {} - -UnsupportedDecoderEngine::UnsupportedDecoderEngine( - const DecoderEngine decoder_engine) - : std::runtime_error("unsupported decoder engine: " + - decoder_engine_to_string(decoder_engine)), - decoder_engine{decoder_engine} {} - -FileReadError::FileReadError() : std::runtime_error("file read error") {} +FileReadError::FileReadError() : Exception("file read error") {} FileWriteError::FileWriteError(const std::string &path) - : std::runtime_error("file write error: " + path) {} + : Exception("file write error: " + path) {} -NoZipFile::NoZipFile() : std::runtime_error("not a zip file") {} +NoZipFile::NoZipFile() : Exception("not a zip file") {} -ZipSaveError::ZipSaveError() : std::runtime_error("zip save error") {} +ZipSaveError::ZipSaveError() : Exception("zip save error") {} -CfbError::CfbError(const char *desc) : std::runtime_error(desc) {} +CfbError::CfbError(const char *desc) : Exception(desc) {} NoCfbFile::NoCfbFile() : CfbError("no cfb file") {} CfbFileCorrupted::CfbFileCorrupted() : CfbError("cfb file corrupted") {} -NoTextFile::NoTextFile() : std::runtime_error("not a text file") {} +NoTextFile::NoTextFile() : Exception("not a text file") {} -NoCsvFile::NoCsvFile() : std::runtime_error("not a csv file") {} +NoCsvFile::NoCsvFile() : Exception("not a csv file") {} -NoJsonFile::NoJsonFile() : std::runtime_error("not a json file") {} +NoJsonFile::NoJsonFile() : Exception("not a json file") {} -UnknownCharset::UnknownCharset() : std::runtime_error("unknown charset") {} +UnknownCharset::UnknownCharset() : Exception("unknown charset") {} -NoImageFile::NoImageFile() : std::runtime_error("not an image file") {} +NoImageFile::NoImageFile() : Exception("not an image file") {} -NoArchiveFile::NoArchiveFile() : std::runtime_error("not an archive file") {} +NoArchiveFile::NoArchiveFile() : Exception("not an archive file") {} -NoDocumentFile::NoDocumentFile() : std::runtime_error("not a document file") {} +NoDocumentFile::NoDocumentFile() : Exception("not a document file") {} NoOpenDocumentFile::NoOpenDocumentFile() - : std::runtime_error("not an open document file") {} + : Exception("not an open document file") {} NoOfficeOpenXmlFile::NoOfficeOpenXmlFile() - : std::runtime_error("not an office open xml file") {} + : Exception("not an office open xml file") {} -NoPdfFile::NoPdfFile() : std::runtime_error("not a pdf file") {} +NoPdfFile::NoPdfFile() : Exception("not a pdf file") {} -NoFontFile::NoFontFile() : std::runtime_error("not a font file") {} +NoFontFile::NoFontFile() : Exception("not a font file") {} NoLegacyMicrosoftFile::NoLegacyMicrosoftFile() - : std::runtime_error("not a legacy microsoft office file") {} + : Exception("not a legacy microsoft office file") {} -NoXmlFile::NoXmlFile() : std::runtime_error("not an xml file") {} +NoXmlFile::NoXmlFile() : Exception("not an xml file") {} UnsupportedCryptoAlgorithm::UnsupportedCryptoAlgorithm() - : std::runtime_error("unsupported crypto algorithm") {} + : Exception("unsupported crypto algorithm") {} -NoSvmFile::NoSvmFile() : std::runtime_error("not a svm file") {} +NoSvmFile::NoSvmFile() : Exception("not a svm file") {} -MalformedSvmFile::MalformedSvmFile() - : std::runtime_error("malformed svm file") {} +MalformedSvmFile::MalformedSvmFile() : Exception("malformed svm file") {} -UnsupportedEndian::UnsupportedEndian() - : std::runtime_error("unsupported endian") {} +UnsupportedEndian::UnsupportedEndian() : Exception("unsupported endian") {} MsUnsupportedCryptoAlgorithm::MsUnsupportedCryptoAlgorithm() - : std::runtime_error("unsupported crypto algorithm") {} + : Exception("unsupported crypto algorithm") {} UnknownDocumentType::UnknownDocumentType() - : std::runtime_error("unknown document type") {} + : Exception("unknown document type") {} -InvalidPrefix::InvalidPrefix() : std::runtime_error("invalid prefix string") {} +InvalidPrefix::InvalidPrefix() : Exception("invalid prefix string") {} InvalidPrefix::InvalidPrefix(const std::string &prefix) - : std::runtime_error("invalid prefix string: " + prefix) {} + : Exception("invalid prefix string: " + prefix) {} DocumentCopyProtectedException::DocumentCopyProtectedException() - : std::runtime_error("document copy protection") {} + : Exception("document copy protection") {} ResourceNotAccessible::ResourceNotAccessible() - : std::runtime_error("resource not accessible") {} + : Exception("resource not accessible") {} ResourceNotAccessible::ResourceNotAccessible(const std::string &name, const std::string &path) - : std::runtime_error("resource not accessible: " + name + " at " + path) {} + : Exception("resource not accessible: " + name + " at " + path) {} -PrefixInUse::PrefixInUse() : std::runtime_error("prefix in use") {} +PrefixInUse::PrefixInUse() : Exception("prefix in use") {} PrefixInUse::PrefixInUse(const std::string &prefix) - : std::runtime_error("prefix in use: " + prefix) {} + : Exception("prefix in use: " + prefix) {} ServerBindFailed::ServerBindFailed(const std::string &host, const std::uint32_t port) - : std::runtime_error("server bind failed: " + host + ":" + - std::to_string(port)) {} + : Exception("server bind failed: " + host + ":" + std::to_string(port)) {} ServerAlreadyBound::ServerAlreadyBound() - : std::runtime_error("server is bound already") {} + : Exception("server is bound already") {} -ServerNotBound::ServerNotBound() : std::runtime_error("server is not bound") {} +ServerNotBound::ServerNotBound() : Exception("server is not bound") {} UnsupportedOption::UnsupportedOption(const std::string &message) - : std::runtime_error("unsupported option: " + message) {} + : Exception("unsupported option: " + message) {} NullPointerError::NullPointerError(const std::string &variable) - : std::runtime_error("null pointer error: " + variable) {} + : Exception("null pointer error: " + variable) {} -WrongPasswordError::WrongPasswordError() - : std::runtime_error("wrong password error") {} +WrongPasswordError::WrongPasswordError() : Exception("wrong password error") {} -DecryptionFailed::DecryptionFailed() - : std::runtime_error("decryption failed") {} +DecryptionFailed::DecryptionFailed() : Exception("decryption failed") {} -NotEncryptedError::NotEncryptedError() - : std::runtime_error("not encrypted error") {} +NotEncryptedError::NotEncryptedError() : Exception("not encrypted error") {} InvalidPath::InvalidPath(const std::string &message) - : std::runtime_error("invalid path: " + message) {} + : Exception("invalid path: " + message) {} UnsupportedFileEncoding::UnsupportedFileEncoding(const std::string &message) - : std::runtime_error("unsupported file encoding: " + message) {} + : Exception("unsupported file encoding: " + message) {} -FileEncryptedError::FileEncryptedError() - : std::runtime_error("file encrypted error") {} +FileEncryptedError::FileEncryptedError() : Exception("file encrypted error") {} UnauthenticatedReadError::UnauthenticatedReadError() - : std::runtime_error( - "cannot read encrypted object without authentication") {} + : Exception("cannot read encrypted object without authentication") {} } // namespace odr diff --git a/src/odr/exceptions.hpp b/src/odr/exceptions.hpp index cd0f528bc..3377688f3 100644 --- a/src/odr/exceptions.hpp +++ b/src/odr/exceptions.hpp @@ -5,66 +5,62 @@ namespace odr { enum class FileType; -enum class DecoderEngine; + +/// @brief Base of every exception type this library declares. +/// +/// Catching this catches every typed error below. Note that the decoders also +/// throw plain `std::runtime_error` for malformed input that has no dedicated +/// type, so `std::runtime_error` remains the widest net. +struct Exception : std::runtime_error { + using std::runtime_error::runtime_error; +}; /// @brief Unsupported operation exception -struct UnsupportedOperation final : std::runtime_error { +struct UnsupportedOperation final : Exception { UnsupportedOperation(); explicit UnsupportedOperation(const std::string &message); }; /// @brief File not found exception -struct FileNotFound final : std::runtime_error { +struct FileNotFound final : Exception { FileNotFound(); explicit FileNotFound(const std::string &path); }; /// @brief Unknown file type exception -struct UnknownFileType final : std::runtime_error { +struct UnknownFileType final : Exception { UnknownFileType(); }; /// @brief Unsupported file type exception -struct UnsupportedFileType final : std::runtime_error { +struct UnsupportedFileType final : Exception { FileType file_type; explicit UnsupportedFileType(FileType file_type); }; -/// @brief Unknown decoder engine exception -struct UnknownDecoderEngine final : std::runtime_error { - UnknownDecoderEngine(); -}; - -/// @brief Unsupported decoder engine exception -struct UnsupportedDecoderEngine final : std::runtime_error { - DecoderEngine decoder_engine; - - explicit UnsupportedDecoderEngine(DecoderEngine decoder_engine); -}; - /// @brief File read error -struct FileReadError final : std::runtime_error { +struct FileReadError final : Exception { FileReadError(); }; /// @brief File write error -struct FileWriteError final : std::runtime_error { +struct FileWriteError final : Exception { explicit FileWriteError(const std::string &path); }; /// @brief No ZIP file exception base -struct NoZipFile final : std::runtime_error { +struct NoZipFile final : Exception { NoZipFile(); }; -/// @brief ZIP save error base -struct ZipSaveError : std::runtime_error { +/// @brief ZIP save error base; `internal::zip::MinizSaveError` refines it. +struct ZipSaveError : Exception { ZipSaveError(); }; -/// @brief CFB error base -struct CfbError : std::runtime_error { +/// @brief CFB error base; NoCfbFile and CfbFileCorrupted refine it. +struct CfbError : Exception { explicit CfbError(const char *desc); }; @@ -79,180 +75,180 @@ struct CfbFileCorrupted final : CfbError { }; /// @brief No text file exception -struct NoTextFile final : std::runtime_error { +struct NoTextFile final : Exception { NoTextFile(); }; /// @brief No csv file exception -struct NoCsvFile final : std::runtime_error { +struct NoCsvFile final : Exception { NoCsvFile(); }; /// @brief No json file exception -struct NoJsonFile final : std::runtime_error { +struct NoJsonFile final : Exception { NoJsonFile(); }; /// @brief Unknown charset exception -struct UnknownCharset final : std::runtime_error { +struct UnknownCharset final : Exception { UnknownCharset(); }; /// @brief No image file exception -struct NoImageFile final : std::runtime_error { +struct NoImageFile final : Exception { NoImageFile(); }; /// @brief No archive file exception -struct NoArchiveFile final : std::runtime_error { +struct NoArchiveFile final : Exception { NoArchiveFile(); }; /// @brief No document file exception -struct NoDocumentFile final : std::runtime_error { +struct NoDocumentFile final : Exception { NoDocumentFile(); }; /// @brief No open document file exception -struct NoOpenDocumentFile final : std::runtime_error { +struct NoOpenDocumentFile final : Exception { NoOpenDocumentFile(); }; /// @brief No office open document file exception -struct NoOfficeOpenXmlFile final : std::runtime_error { +struct NoOfficeOpenXmlFile final : Exception { NoOfficeOpenXmlFile(); }; /// @brief No PDF file exception -struct NoPdfFile final : std::runtime_error { +struct NoPdfFile final : Exception { NoPdfFile(); }; /// @brief No font file exception -struct NoFontFile final : std::runtime_error { +struct NoFontFile final : Exception { NoFontFile(); }; /// @brief No legacy Microsoft Office file -struct NoLegacyMicrosoftFile final : std::runtime_error { +struct NoLegacyMicrosoftFile final : Exception { NoLegacyMicrosoftFile(); }; /// @brief No XML file exception -struct NoXmlFile final : std::runtime_error { +struct NoXmlFile final : Exception { NoXmlFile(); }; /// @brief Unsupported crypto algorithm exception -struct UnsupportedCryptoAlgorithm final : std::runtime_error { +struct UnsupportedCryptoAlgorithm final : Exception { UnsupportedCryptoAlgorithm(); }; /// @brief No SVM file exception base -struct NoSvmFile final : std::runtime_error { +struct NoSvmFile final : Exception { NoSvmFile(); }; /// @brief Malformed SVM file exception base -struct MalformedSvmFile final : std::runtime_error { +struct MalformedSvmFile final : Exception { MalformedSvmFile(); }; /// @brief Unsupported endian exception -struct UnsupportedEndian final : std::runtime_error { +struct UnsupportedEndian final : Exception { UnsupportedEndian(); }; /// @brief Unsupported MS crypto algorithm exception -struct MsUnsupportedCryptoAlgorithm final : std::runtime_error { +struct MsUnsupportedCryptoAlgorithm final : Exception { MsUnsupportedCryptoAlgorithm(); }; /// @brief Unknown document type exception -struct UnknownDocumentType final : std::runtime_error { +struct UnknownDocumentType final : Exception { UnknownDocumentType(); }; /// @brief Invalid prefix string -struct InvalidPrefix final : std::runtime_error { +struct InvalidPrefix final : Exception { InvalidPrefix(); explicit InvalidPrefix(const std::string &prefix); }; /// @brief Document copy protected exception -struct DocumentCopyProtectedException final : std::runtime_error { +struct DocumentCopyProtectedException final : Exception { DocumentCopyProtectedException(); }; /// @brief Resource is not accessible -struct ResourceNotAccessible final : std::runtime_error { +struct ResourceNotAccessible final : Exception { ResourceNotAccessible(); ResourceNotAccessible(const std::string &name, const std::string &path); }; /// @brief Prefix already in use -struct PrefixInUse final : std::runtime_error { +struct PrefixInUse final : Exception { PrefixInUse(); explicit PrefixInUse(const std::string &prefix); }; /// @brief HTTP server socket could not be bound -struct ServerBindFailed final : std::runtime_error { +struct ServerBindFailed final : Exception { ServerBindFailed(const std::string &host, std::uint32_t port); }; /// @brief HTTP server is bound already -struct ServerAlreadyBound final : std::runtime_error { +struct ServerAlreadyBound final : Exception { ServerAlreadyBound(); }; /// @brief HTTP server has not been bound -struct ServerNotBound final : std::runtime_error { +struct ServerNotBound final : Exception { ServerNotBound(); }; /// @brief Unsupported option -struct UnsupportedOption final : std::runtime_error { +struct UnsupportedOption final : Exception { explicit UnsupportedOption(const std::string &message); }; /// @brief Null pointer error -struct NullPointerError final : std::runtime_error { +struct NullPointerError final : Exception { explicit NullPointerError(const std::string &variable); }; /// @brief Wrong password error -struct WrongPasswordError final : std::runtime_error { +struct WrongPasswordError final : Exception { explicit WrongPasswordError(); }; /// @brief Decryption failed -struct DecryptionFailed final : std::runtime_error { +struct DecryptionFailed final : Exception { explicit DecryptionFailed(); }; /// @brief Not encrypted error -struct NotEncryptedError final : std::runtime_error { +struct NotEncryptedError final : Exception { explicit NotEncryptedError(); }; /// @brief Invalid path -struct InvalidPath final : std::runtime_error { +struct InvalidPath final : Exception { explicit InvalidPath(const std::string &message); }; /// @brief Unsupported file encoding -struct UnsupportedFileEncoding final : std::runtime_error { +struct UnsupportedFileEncoding final : Exception { explicit UnsupportedFileEncoding(const std::string &message); }; /// @brief File is encrypted -struct FileEncryptedError final : std::runtime_error { +struct FileEncryptedError final : Exception { explicit FileEncryptedError(); }; /// @brief Read attempted on an encrypted file that has not been authenticated -struct UnauthenticatedReadError final : std::runtime_error { +struct UnauthenticatedReadError final : Exception { explicit UnauthenticatedReadError(); }; diff --git a/src/odr/file.cpp b/src/odr/file.cpp index 6d7ecae46..9baea9b6b 100644 --- a/src/odr/file.cpp +++ b/src/odr/file.cpp @@ -59,11 +59,6 @@ std::string_view DecodedFile::mimetype(const std::string &path, return internal::magic::mimetype(path); } -std::vector -DecodedFile::list_decoder_engines(const FileType as) { - return internal::open_strategy::list_decoder_engines(as); -} - DecodedFile::DecodedFile(std::shared_ptr impl) : m_impl{std::move(impl)} { if (m_impl == nullptr) { @@ -104,10 +99,6 @@ FileCategory DecodedFile::file_category() const noexcept { FileMeta DecodedFile::file_meta() const noexcept { return m_impl->file_meta(); } -DecoderEngine DecodedFile::decoder_engine() const noexcept { - return m_impl->decoder_engine(); -} - bool DecodedFile::password_encrypted() const { return m_impl->password_encrypted(); } diff --git a/src/odr/file.hpp b/src/odr/file.hpp index 387755007..c8f703241 100644 --- a/src/odr/file.hpp +++ b/src/odr/file.hpp @@ -51,9 +51,10 @@ enum class FileType { legacy_powerpoint_presentation, legacy_excel_worksheets, + // Detection only — magic recognises these so a caller can report the type, + // but there is no decoder behind them and opening one throws. // https://en.wikipedia.org/wiki/WordPerfect word_perfect, - // https://en.wikipedia.org/wiki/Rich_Text_Format rich_text_format, @@ -103,18 +104,11 @@ enum class FileLocation { disk, }; -/// @brief Collection of decoder engines. -enum class DecoderEngine { - odr, -}; - /// @brief Preference for decoding files. struct DecodePreference final { std::optional as_file_type; - std::optional with_engine; std::vector file_type_priority; - std::vector engine_priority; }; /// @brief Collection of encryption states. @@ -188,8 +182,6 @@ class DecodedFile { [[nodiscard]] static std::vector list_file_types(const std::string &path, const Logger &logger = Logger::null()); - [[nodiscard]] static std::vector - list_decoder_engines(FileType as); [[nodiscard]] static std::string_view mimetype(const std::string &path, const Logger &logger = Logger::null()); @@ -209,7 +201,6 @@ class DecodedFile { [[nodiscard]] FileType file_type() const noexcept; [[nodiscard]] FileCategory file_category() const noexcept; [[nodiscard]] FileMeta file_meta() const noexcept; - [[nodiscard]] DecoderEngine decoder_engine() const noexcept; [[nodiscard]] bool password_encrypted() const; [[nodiscard]] EncryptionState encryption_state() const; diff --git a/src/odr/html.cpp b/src/odr/html.cpp index ec522968c..554539076 100644 --- a/src/odr/html.cpp +++ b/src/odr/html.cpp @@ -62,7 +62,7 @@ void HtmlConfig::init() { Html::Html(HtmlConfig config, std::vector pages) : m_config{std::move(config)}, m_pages{std::move(pages)} {} -const HtmlConfig &Html::config() { return m_config; } +const HtmlConfig &Html::config() const { return m_config; } const std::vector &Html::pages() const { return m_pages; } @@ -320,7 +320,7 @@ HtmlService html::translate(const Document &document, logger); } -void html::edit(const Document &document, const char *diff, +void html::edit(const Document &document, const std::string_view diff, const Logger & /*logger*/) { const nlohmann::json json = nlohmann::json::parse(diff); for (const auto &[key, value] : json["modifiedText"].items()) { diff --git a/src/odr/html.hpp b/src/odr/html.hpp index a8e5d7d01..0f6c06c94 100644 --- a/src/odr/html.hpp +++ b/src/odr/html.hpp @@ -9,6 +9,7 @@ #include #include #include +#include #include namespace odr::internal::abstract { @@ -193,7 +194,7 @@ class Html final { public: Html(HtmlConfig config, std::vector pages); - [[nodiscard]] const HtmlConfig &config(); + [[nodiscard]] const HtmlConfig &config() const; [[nodiscard]] const std::vector &pages() const; private: @@ -376,7 +377,7 @@ HtmlService translate(const Document &document, const std::string &cache_path, /// @param document Document to edit. /// @param diff Diff to apply. /// @param logger Logger to use for logging. -void edit(const Document &document, const char *diff, +void edit(const Document &document, std::string_view diff, const Logger &logger = Logger::null()); } // namespace html diff --git a/src/odr/internal/abstract/document.hpp b/src/odr/internal/abstract/document.hpp index 6ba013d0d..f11a4db77 100644 --- a/src/odr/internal/abstract/document.hpp +++ b/src/odr/internal/abstract/document.hpp @@ -1,5 +1,9 @@ #pragma once +#include +#include + +#include #include #include @@ -427,15 +431,15 @@ class FrameAdapter { [[nodiscard]] virtual AnchorType frame_anchor_type(ElementIdentifier element_id) const = 0; - [[nodiscard]] virtual std::optional + [[nodiscard]] virtual std::optional frame_x(ElementIdentifier element_id) const = 0; - [[nodiscard]] virtual std::optional + [[nodiscard]] virtual std::optional frame_y(ElementIdentifier element_id) const = 0; - [[nodiscard]] virtual std::optional + [[nodiscard]] virtual std::optional frame_width(ElementIdentifier element_id) const = 0; - [[nodiscard]] virtual std::optional + [[nodiscard]] virtual std::optional frame_height(ElementIdentifier element_id) const = 0; - [[nodiscard]] virtual std::optional + [[nodiscard]] virtual std::optional frame_z_index(ElementIdentifier element_id) const = 0; [[nodiscard]] virtual GraphicStyle @@ -446,13 +450,11 @@ class RectAdapter { public: virtual ~RectAdapter() = default; - [[nodiscard]] virtual std::string - rect_x(ElementIdentifier element_id) const = 0; - [[nodiscard]] virtual std::string - rect_y(ElementIdentifier element_id) const = 0; - [[nodiscard]] virtual std::string + [[nodiscard]] virtual Measure rect_x(ElementIdentifier element_id) const = 0; + [[nodiscard]] virtual Measure rect_y(ElementIdentifier element_id) const = 0; + [[nodiscard]] virtual Measure rect_width(ElementIdentifier element_id) const = 0; - [[nodiscard]] virtual std::string + [[nodiscard]] virtual Measure rect_height(ElementIdentifier element_id) const = 0; [[nodiscard]] virtual GraphicStyle @@ -463,14 +465,10 @@ class LineAdapter { public: virtual ~LineAdapter() = default; - [[nodiscard]] virtual std::string - line_x1(ElementIdentifier element_id) const = 0; - [[nodiscard]] virtual std::string - line_y1(ElementIdentifier element_id) const = 0; - [[nodiscard]] virtual std::string - line_x2(ElementIdentifier element_id) const = 0; - [[nodiscard]] virtual std::string - line_y2(ElementIdentifier element_id) const = 0; + [[nodiscard]] virtual Measure line_x1(ElementIdentifier element_id) const = 0; + [[nodiscard]] virtual Measure line_y1(ElementIdentifier element_id) const = 0; + [[nodiscard]] virtual Measure line_x2(ElementIdentifier element_id) const = 0; + [[nodiscard]] virtual Measure line_y2(ElementIdentifier element_id) const = 0; [[nodiscard]] virtual GraphicStyle line_style(ElementIdentifier element_id) const = 0; @@ -480,13 +478,13 @@ class CircleAdapter { public: virtual ~CircleAdapter() = default; - [[nodiscard]] virtual std::string + [[nodiscard]] virtual Measure circle_x(ElementIdentifier element_id) const = 0; - [[nodiscard]] virtual std::string + [[nodiscard]] virtual Measure circle_y(ElementIdentifier element_id) const = 0; - [[nodiscard]] virtual std::string + [[nodiscard]] virtual Measure circle_width(ElementIdentifier element_id) const = 0; - [[nodiscard]] virtual std::string + [[nodiscard]] virtual Measure circle_height(ElementIdentifier element_id) const = 0; [[nodiscard]] virtual GraphicStyle @@ -497,13 +495,13 @@ class CustomShapeAdapter { public: virtual ~CustomShapeAdapter() = default; - [[nodiscard]] virtual std::optional + [[nodiscard]] virtual std::optional custom_shape_x(ElementIdentifier element_id) const = 0; - [[nodiscard]] virtual std::optional + [[nodiscard]] virtual std::optional custom_shape_y(ElementIdentifier element_id) const = 0; - [[nodiscard]] virtual std::string + [[nodiscard]] virtual Measure custom_shape_width(ElementIdentifier element_id) const = 0; - [[nodiscard]] virtual std::string + [[nodiscard]] virtual Measure custom_shape_height(ElementIdentifier element_id) const = 0; [[nodiscard]] virtual GraphicStyle diff --git a/src/odr/internal/abstract/file.hpp b/src/odr/internal/abstract/file.hpp index 155cbe114..ca199949c 100644 --- a/src/odr/internal/abstract/file.hpp +++ b/src/odr/internal/abstract/file.hpp @@ -36,7 +36,6 @@ class DecodedFile { [[nodiscard]] virtual std::shared_ptr file() const noexcept = 0; - [[nodiscard]] virtual DecoderEngine decoder_engine() const noexcept = 0; [[nodiscard]] virtual FileType file_type() const noexcept = 0; [[nodiscard]] virtual FileCategory file_category() const noexcept = 0; [[nodiscard]] virtual std::string_view mimetype() const noexcept = 0; diff --git a/src/odr/internal/cfb/cfb_file.cpp b/src/odr/internal/cfb/cfb_file.cpp index baaceb69e..7f0e6ea2c 100644 --- a/src/odr/internal/cfb/cfb_file.cpp +++ b/src/odr/internal/cfb/cfb_file.cpp @@ -12,10 +12,6 @@ std::shared_ptr CfbFile::file() const noexcept { return m_cfb->file(); } -DecoderEngine CfbFile::decoder_engine() const noexcept { - return DecoderEngine::odr; -} - FileType CfbFile::file_type() const noexcept { return FileType::compound_file_binary_format; } diff --git a/src/odr/internal/cfb/cfb_file.hpp b/src/odr/internal/cfb/cfb_file.hpp index 24bbc0dd5..1f650a89d 100644 --- a/src/odr/internal/cfb/cfb_file.hpp +++ b/src/odr/internal/cfb/cfb_file.hpp @@ -19,7 +19,6 @@ class CfbFile final : public abstract::ArchiveFile { [[nodiscard]] std::shared_ptr file() const noexcept override; - [[nodiscard]] DecoderEngine decoder_engine() const noexcept override; [[nodiscard]] FileType file_type() const noexcept override; [[nodiscard]] std::string_view mimetype() const noexcept override; [[nodiscard]] FileMeta file_meta() const noexcept override; diff --git a/src/odr/internal/common/image_file.cpp b/src/odr/internal/common/image_file.cpp index f3f520cb4..d2a65b5d8 100644 --- a/src/odr/internal/common/image_file.cpp +++ b/src/odr/internal/common/image_file.cpp @@ -16,10 +16,6 @@ FileType ImageFile::file_type() const noexcept { return m_file_type; } FileMeta ImageFile::file_meta() const noexcept { return {}; } -DecoderEngine ImageFile::decoder_engine() const noexcept { - return DecoderEngine::odr; -} - std::string_view ImageFile::mimetype() const noexcept { return ""; } bool ImageFile::is_decodable() const noexcept { return false; } diff --git a/src/odr/internal/common/image_file.hpp b/src/odr/internal/common/image_file.hpp index 0c70163d6..6d18e1ea4 100644 --- a/src/odr/internal/common/image_file.hpp +++ b/src/odr/internal/common/image_file.hpp @@ -12,7 +12,6 @@ class ImageFile final : public abstract::ImageFile { [[nodiscard]] FileType file_type() const noexcept override; [[nodiscard]] FileMeta file_meta() const noexcept override; - [[nodiscard]] DecoderEngine decoder_engine() const noexcept override; [[nodiscard]] std::string_view mimetype() const noexcept override; [[nodiscard]] bool is_decodable() const noexcept override; diff --git a/src/odr/internal/csv/csv_file.cpp b/src/odr/internal/csv/csv_file.cpp index 110c5da84..a129c3924 100644 --- a/src/odr/internal/csv/csv_file.cpp +++ b/src/odr/internal/csv/csv_file.cpp @@ -14,10 +14,6 @@ std::shared_ptr CsvFile::file() const noexcept { return m_file->file(); } -DecoderEngine CsvFile::decoder_engine() const noexcept { - return DecoderEngine::odr; -} - FileType CsvFile::file_type() const noexcept { return FileType::comma_separated_values; } diff --git a/src/odr/internal/csv/csv_file.hpp b/src/odr/internal/csv/csv_file.hpp index 100c664a6..50a0c84fe 100644 --- a/src/odr/internal/csv/csv_file.hpp +++ b/src/odr/internal/csv/csv_file.hpp @@ -14,7 +14,6 @@ class CsvFile final : public abstract::TextFile { [[nodiscard]] std::shared_ptr file() const noexcept override; - [[nodiscard]] DecoderEngine decoder_engine() const noexcept override; [[nodiscard]] FileType file_type() const noexcept override; [[nodiscard]] std::string_view mimetype() const noexcept override; [[nodiscard]] FileMeta file_meta() const noexcept override; diff --git a/src/odr/internal/font/font_file.cpp b/src/odr/internal/font/font_file.cpp index 98e359875..2f4360c17 100644 --- a/src/odr/internal/font/font_file.cpp +++ b/src/odr/internal/font/font_file.cpp @@ -23,10 +23,6 @@ std::shared_ptr FontFile::file() const noexcept { return m_file; } -DecoderEngine FontFile::decoder_engine() const noexcept { - return DecoderEngine::odr; -} - FileType FontFile::file_type() const noexcept { return m_file_type; } std::string_view FontFile::mimetype() const noexcept { diff --git a/src/odr/internal/font/font_file.hpp b/src/odr/internal/font/font_file.hpp index 0cbd36030..d7e4e80fd 100644 --- a/src/odr/internal/font/font_file.hpp +++ b/src/odr/internal/font/font_file.hpp @@ -18,7 +18,6 @@ class FontFile final : public abstract::FontFile { [[nodiscard]] std::shared_ptr file() const noexcept override; - [[nodiscard]] DecoderEngine decoder_engine() const noexcept override; [[nodiscard]] FileType file_type() const noexcept override; [[nodiscard]] std::string_view mimetype() const noexcept override; [[nodiscard]] FileMeta file_meta() const noexcept override; diff --git a/src/odr/internal/html/document_element.cpp b/src/odr/internal/html/document_element.cpp index 3422667ea..3376f346e 100644 --- a/src/odr/internal/html/document_element.cpp +++ b/src/odr/internal/html/document_element.cpp @@ -476,12 +476,13 @@ void html::translate_line(const Element &element, const WritingState &state) { translate_drawing_style(style))); state.out().write_element_begin( - "line", HtmlElementOptions() - .set_close_type(HtmlCloseType::trailing) - .set_attributes(HtmlAttributesVector{{"x1", line.x1()}, - {"y1", line.y1()}, - {"x2", line.x2()}, - {"y2", line.y2()}})); + "line", + HtmlElementOptions() + .set_close_type(HtmlCloseType::trailing) + .set_attributes(HtmlAttributesVector{{"x1", line.x1().to_string()}, + {"y1", line.y1().to_string()}, + {"x2", line.x2().to_string()}, + {"y2", line.y2().to_string()}})); state.out().write_element_end("svg"); } diff --git a/src/odr/internal/html/document_style.cpp b/src/odr/internal/html/document_style.cpp index 2315c58b4..2ead0ef68 100644 --- a/src/odr/internal/html/document_style.cpp +++ b/src/odr/internal/html/document_style.cpp @@ -127,8 +127,9 @@ std::string html::translate_inner_page_style(const PageLayout &page_layout) { std::string html::translate_text_style(const TextStyle &text_style) { std::string result; - if (const char *font_name = text_style.font_name; font_name != nullptr) { - result.append("font-family:").append(font_name).append(";"); + if (const std::optional font_name = text_style.font_name; + font_name.has_value()) { + result.append("font-family:").append(*font_name).append(";"); } if (const std::optional font_size = text_style.font_size; font_size.has_value()) { @@ -381,56 +382,55 @@ std::string html::translate_frame_properties(const Frame &frame) { result += "display:block;"; result += "float:right;clear:both;"; result += "shape-outside:content-box;"; - if (const std::optional x = frame.x(); x.has_value()) { - result += "margin-left:" + *x + ";"; + if (const std::optional x = frame.x(); x.has_value()) { + result += "margin-left:" + x->to_string() + ";"; } - if (const std::optional y = frame.y(); y.has_value()) { - result += "margin-top:" + *y + ";"; + if (const std::optional y = frame.y(); y.has_value()) { + result += "margin-top:" + y->to_string() + ";"; } result += "margin-right:calc(100% - "; - result += frame.x().value_or("0in"); + result += frame.x().value_or(Measure(0, DynamicUnit("in"))).to_string(); result += " - "; - result += *frame.width(); + result += frame.width()->to_string(); result += ");"; } else if (text_wrap == TextWrap::after) { result += "display:block;"; result += "float:left;clear:both;"; result += "shape-outside:content-box;"; - if (const std::optional x = frame.x(); x.has_value()) { - result += "margin-left:" + *x + ";"; + if (const std::optional x = frame.x(); x.has_value()) { + result += "margin-left:" + x->to_string() + ";"; } - if (const std::optional y = frame.y(); y.has_value()) { - result += "margin-top:" + *y + ";"; + if (const std::optional y = frame.y(); y.has_value()) { + result += "margin-top:" + y->to_string() + ";"; } } else if (text_wrap == TextWrap::none) { result += "display:block;"; - if (const std::optional x = frame.x(); x.has_value()) { - result += "margin-left:" + *x + ";"; + if (const std::optional x = frame.x(); x.has_value()) { + result += "margin-left:" + x->to_string() + ";"; } - if (const std::optional y = frame.y(); y.has_value()) { - result += "margin-top:" + *y + ";"; + if (const std::optional y = frame.y(); y.has_value()) { + result += "margin-top:" + y->to_string() + ";"; } } else { result += "display:block;"; result += "position:absolute;"; - if (const std::optional x = frame.x(); x.has_value()) { - result += "left:" + *x + ";"; + if (const std::optional x = frame.x(); x.has_value()) { + result += "left:" + x->to_string() + ";"; } - if (const std::optional y = frame.y(); y.has_value()) { - result += "top:" + *y + ";"; + if (const std::optional y = frame.y(); y.has_value()) { + result += "top:" + y->to_string() + ";"; } } - if (const std::optional width = frame.width(); - width.has_value()) { - result += "width:" + *width + ";"; + if (const std::optional width = frame.width(); width.has_value()) { + result += "width:" + width->to_string() + ";"; } - if (const std::optional height = frame.height(); + if (const std::optional height = frame.height(); height.has_value()) { - result += "height:" + *height + ";"; + result += "height:" + height->to_string() + ";"; } - if (const std::optional z_index = frame.z_index(); + if (const std::optional z_index = frame.z_index(); z_index.has_value()) { - result += "z-index:" + *z_index + ";"; + result += "z-index:" + std::to_string(*z_index) + ";"; } return result; } @@ -438,20 +438,20 @@ std::string html::translate_frame_properties(const Frame &frame) { std::string html::translate_rect_properties(const Rect &rect) { std::string result; result += "position:absolute;"; - result += "left:" + rect.x() + ";"; - result += "top:" + rect.y() + ";"; - result += "width:" + rect.width() + ";"; - result += "height:" + rect.height() + ";"; + result += "left:" + rect.x().to_string() + ";"; + result += "top:" + rect.y().to_string() + ";"; + result += "width:" + rect.width().to_string() + ";"; + result += "height:" + rect.height().to_string() + ";"; return result; } std::string html::translate_circle_properties(const Circle &circle) { std::string result; result += "position:absolute;"; - result += "left:" + circle.x() + ";"; - result += "top:" + circle.y() + ";"; - result += "width:" + circle.width() + ";"; - result += "height:" + circle.height() + ";"; + result += "left:" + circle.x().to_string() + ";"; + result += "top:" + circle.y().to_string() + ";"; + result += "width:" + circle.width().to_string() + ";"; + result += "height:" + circle.height().to_string() + ";"; return result; } @@ -459,18 +459,18 @@ std::string html::translate_custom_shape_properties(const CustomShape &custom_shape) { std::string result; result += "position:absolute;"; - if (const std::optional x = custom_shape.x(); x.has_value()) { - result += "left:" + *x + ";"; + if (const std::optional x = custom_shape.x(); x.has_value()) { + result += "left:" + x->to_string() + ";"; } else { result += "left:0;"; } - if (const std::optional y = custom_shape.y(); y.has_value()) { - result += "top:" + *y + ";"; + if (const std::optional y = custom_shape.y(); y.has_value()) { + result += "top:" + y->to_string() + ";"; } else { result += "top:0;"; } - result += "width:" + custom_shape.width() + ";"; - result += "height:" + custom_shape.height() + ";"; + result += "width:" + custom_shape.width().to_string() + ";"; + result += "height:" + custom_shape.height().to_string() + ";"; return result; } diff --git a/src/odr/internal/json/json_file.cpp b/src/odr/internal/json/json_file.cpp index 3aab589e6..bfcbf4bd6 100644 --- a/src/odr/internal/json/json_file.cpp +++ b/src/odr/internal/json/json_file.cpp @@ -14,10 +14,6 @@ std::shared_ptr JsonFile::file() const noexcept { return m_file->file(); } -DecoderEngine JsonFile::decoder_engine() const noexcept { - return DecoderEngine::odr; -} - FileType JsonFile::file_type() const noexcept { return FileType::javascript_object_notation; } diff --git a/src/odr/internal/json/json_file.hpp b/src/odr/internal/json/json_file.hpp index 22fb5c9e2..1ef4889a8 100644 --- a/src/odr/internal/json/json_file.hpp +++ b/src/odr/internal/json/json_file.hpp @@ -14,7 +14,6 @@ class JsonFile final : public abstract::TextFile { [[nodiscard]] std::shared_ptr file() const noexcept override; - [[nodiscard]] DecoderEngine decoder_engine() const noexcept override; [[nodiscard]] FileType file_type() const noexcept override; [[nodiscard]] std::string_view mimetype() const noexcept override; [[nodiscard]] FileMeta file_meta() const noexcept override; diff --git a/src/odr/internal/odf/AGENTS.md b/src/odr/internal/odf/AGENTS.md index 1de85c0e5..f18df0c6c 100644 --- a/src/odr/internal/odf/AGENTS.md +++ b/src/odr/internal/odf/AGENTS.md @@ -5,7 +5,7 @@ per-feature checklist in [`README.md`](README.md). Shared architecture (the element-adapter pattern, build/test loop, conventions) is in the top-level [`AGENTS.md`](../../../../AGENTS.md); read it first. -**Scope.** One engine (`DecoderEngine::odr`) reading **all four ODF document +**Scope.** Reading **all four ODF document types** — text (`.odt`), presentation (`.odp`), spreadsheet (`.ods`), graphics (`.odg`) — plus their template and legacy StarOffice variants, through the abstract model. Reader + style resolver + a **partial editor** (text-content diff --git a/src/odr/internal/odf/odf_document.cpp b/src/odr/internal/odf/odf_document.cpp index 6f1c2d511..1f9dfc059 100644 --- a/src/odr/internal/odf/odf_document.cpp +++ b/src/odr/internal/odf/odf_document.cpp @@ -127,6 +127,25 @@ void Document::save(const Path & /*path*/, const char * /*password*/) const { namespace { +/// Parses a `svg:*` length attribute. An absent or unparsable value yields +/// nullopt rather than throwing — geometry is presentational. +std::optional read_measure(const pugi::xml_attribute attribute) { + if (!attribute) { + return std::nullopt; + } + try { + return Measure(attribute.value()); + } catch (...) { // NOLINT(bugprone-empty-catch) + return std::nullopt; + } +} + +/// Same, for the attributes the API reports unconditionally; a missing value +/// degrades to a bare zero. +Measure read_measure_or_zero(const pugi::xml_attribute attribute) { + return read_measure(attribute).value_or(Measure(0, DynamicUnit())); +} + class ElementAdapter final : public abstract::ElementAdapter, public abstract::TextRootAdapter, public abstract::SlideAdapter, @@ -722,146 +741,114 @@ class ElementAdapter final : public abstract::ElementAdapter, } return AnchorType::at_page; } - [[nodiscard]] std::optional + [[nodiscard]] std::optional frame_x(const ElementIdentifier element_id) const override { - const pugi::xml_node node = get_node(element_id); - if (const pugi::xml_attribute attribute = node.attribute("svg:x"); - attribute) { - return attribute.value(); - } - return std::nullopt; + return read_measure(get_node(element_id).attribute("svg:x")); } - [[nodiscard]] std::optional + [[nodiscard]] std::optional frame_y(const ElementIdentifier element_id) const override { - const pugi::xml_node node = get_node(element_id); - if (const pugi::xml_attribute attribute = node.attribute("svg:y"); - attribute) { - return attribute.value(); - } - return std::nullopt; + return read_measure(get_node(element_id).attribute("svg:y")); } - [[nodiscard]] std::optional + [[nodiscard]] std::optional frame_width(const ElementIdentifier element_id) const override { - const pugi::xml_node node = get_node(element_id); - if (const pugi::xml_attribute attribute = node.attribute("svg:width"); - attribute) { - return attribute.value(); - } - return std::nullopt; + return read_measure(get_node(element_id).attribute("svg:width")); } - [[nodiscard]] std::optional + [[nodiscard]] std::optional frame_height(const ElementIdentifier element_id) const override { - const pugi::xml_node node = get_node(element_id); - if (const pugi::xml_attribute attribute = node.attribute("svg:height"); - attribute) { - return attribute.value(); - } - return std::nullopt; + return read_measure(get_node(element_id).attribute("svg:height")); } - [[nodiscard]] std::optional + [[nodiscard]] std::optional frame_z_index(const ElementIdentifier element_id) const override { - const pugi::xml_node node = get_node(element_id); - if (const pugi::xml_attribute attribute = node.attribute("draw:z-index"); - attribute) { - return attribute.value(); + const pugi::xml_attribute attribute = + get_node(element_id).attribute("draw:z-index"); + if (!attribute) { + return std::nullopt; } - return std::nullopt; + return static_cast(attribute.as_int()); } [[nodiscard]] GraphicStyle frame_style(const ElementIdentifier element_id) const override { return get_intermediate_style(element_id).graphic_style; } - [[nodiscard]] std::string + [[nodiscard]] Measure rect_x(const ElementIdentifier element_id) const override { - return get_node(element_id).attribute("svg:x").value(); + return read_measure_or_zero(get_node(element_id).attribute("svg:x")); } - [[nodiscard]] std::string + [[nodiscard]] Measure rect_y(const ElementIdentifier element_id) const override { - return get_node(element_id).attribute("svg:y").value(); + return read_measure_or_zero(get_node(element_id).attribute("svg:y")); } - [[nodiscard]] std::string + [[nodiscard]] Measure rect_width(const ElementIdentifier element_id) const override { - return get_node(element_id).attribute("svg:width").value(); + return read_measure_or_zero(get_node(element_id).attribute("svg:width")); } - [[nodiscard]] std::string + [[nodiscard]] Measure rect_height(const ElementIdentifier element_id) const override { - return get_node(element_id).attribute("svg:height").value(); + return read_measure_or_zero(get_node(element_id).attribute("svg:height")); } [[nodiscard]] GraphicStyle rect_style(const ElementIdentifier element_id) const override { return get_intermediate_style(element_id).graphic_style; } - [[nodiscard]] std::string + [[nodiscard]] Measure line_x1(const ElementIdentifier element_id) const override { - return get_node(element_id).attribute("svg:x1").value(); + return read_measure_or_zero(get_node(element_id).attribute("svg:x1")); } - [[nodiscard]] std::string + [[nodiscard]] Measure line_y1(const ElementIdentifier element_id) const override { - return get_node(element_id).attribute("svg:y1").value(); + return read_measure_or_zero(get_node(element_id).attribute("svg:y1")); } - [[nodiscard]] std::string + [[nodiscard]] Measure line_x2(const ElementIdentifier element_id) const override { - return get_node(element_id).attribute("svg:x2").value(); + return read_measure_or_zero(get_node(element_id).attribute("svg:x2")); } - [[nodiscard]] std::string + [[nodiscard]] Measure line_y2(const ElementIdentifier element_id) const override { - return get_node(element_id).attribute("svg:y2").value(); + return read_measure_or_zero(get_node(element_id).attribute("svg:y2")); } [[nodiscard]] GraphicStyle line_style(const ElementIdentifier element_id) const override { return get_intermediate_style(element_id).graphic_style; } - [[nodiscard]] std::string + [[nodiscard]] Measure circle_x(const ElementIdentifier element_id) const override { - return get_node(element_id).attribute("svg:x").value(); + return read_measure_or_zero(get_node(element_id).attribute("svg:x")); } - [[nodiscard]] std::string + [[nodiscard]] Measure circle_y(const ElementIdentifier element_id) const override { - return get_node(element_id).attribute("svg:y").value(); + return read_measure_or_zero(get_node(element_id).attribute("svg:y")); } - [[nodiscard]] std::string + [[nodiscard]] Measure circle_width(const ElementIdentifier element_id) const override { - return get_node(element_id).attribute("svg:width").value(); + return read_measure_or_zero(get_node(element_id).attribute("svg:width")); } - [[nodiscard]] std::string + [[nodiscard]] Measure circle_height(const ElementIdentifier element_id) const override { - return get_node(element_id).attribute("svg:height").value(); + return read_measure_or_zero(get_node(element_id).attribute("svg:height")); } [[nodiscard]] GraphicStyle circle_style(const ElementIdentifier element_id) const override { return get_intermediate_style(element_id).graphic_style; } - [[nodiscard]] std::optional + [[nodiscard]] std::optional custom_shape_x(const ElementIdentifier element_id) const override { - const pugi::xml_node node = get_node(element_id); - if (const pugi::xml_attribute attribute = node.attribute("svg:x"); - attribute) { - return attribute.value(); - } - return std::nullopt; + return read_measure(get_node(element_id).attribute("svg:x")); } - [[nodiscard]] std::optional + [[nodiscard]] std::optional custom_shape_y(const ElementIdentifier element_id) const override { - const pugi::xml_node node = get_node(element_id); - if (const pugi::xml_attribute attribute = node.attribute("svg:y"); - attribute) { - return attribute.value(); - } - return std::nullopt; + return read_measure(get_node(element_id).attribute("svg:y")); } - [[nodiscard]] std::string + [[nodiscard]] Measure custom_shape_width(const ElementIdentifier element_id) const override { - const pugi::xml_node node = get_node(element_id); - return node.attribute("svg:width").value(); + return read_measure_or_zero(get_node(element_id).attribute("svg:width")); } - [[nodiscard]] std::string + [[nodiscard]] Measure custom_shape_height(const ElementIdentifier element_id) const override { - const pugi::xml_node node = get_node(element_id); - return node.attribute("svg:height").value(); + return read_measure_or_zero(get_node(element_id).attribute("svg:height")); } [[nodiscard]] GraphicStyle custom_shape_style(const ElementIdentifier element_id) const override { diff --git a/src/odr/internal/odf/odf_file.cpp b/src/odr/internal/odf/odf_file.cpp index 0893e60a3..1f7280b60 100644 --- a/src/odr/internal/odf/odf_file.cpp +++ b/src/odr/internal/odf/odf_file.cpp @@ -36,10 +36,6 @@ std::shared_ptr OpenDocumentFile::file() const noexcept { return {}; } -DecoderEngine OpenDocumentFile::decoder_engine() const noexcept { - return DecoderEngine::odr; -} - FileType OpenDocumentFile::file_type() const noexcept { return m_file_meta.type; } diff --git a/src/odr/internal/odf/odf_file.hpp b/src/odr/internal/odf/odf_file.hpp index aa48e3190..07ad105b1 100644 --- a/src/odr/internal/odf/odf_file.hpp +++ b/src/odr/internal/odf/odf_file.hpp @@ -25,7 +25,6 @@ class OpenDocumentFile final : public virtual abstract::DocumentFile { [[nodiscard]] std::shared_ptr file() const noexcept override; - [[nodiscard]] DecoderEngine decoder_engine() const noexcept override; [[nodiscard]] FileType file_type() const noexcept override; [[nodiscard]] std::string_view mimetype() const noexcept override; [[nodiscard]] FileMeta file_meta() const noexcept override; diff --git a/src/odr/internal/oldms/oldms_file.cpp b/src/odr/internal/oldms/oldms_file.cpp index 2c7464aaf..6b83d5126 100644 --- a/src/odr/internal/oldms/oldms_file.cpp +++ b/src/odr/internal/oldms/oldms_file.cpp @@ -67,10 +67,6 @@ std::shared_ptr LegacyMicrosoftFile::file() const noexcept { return {}; } -DecoderEngine LegacyMicrosoftFile::decoder_engine() const noexcept { - return DecoderEngine::odr; -} - FileType LegacyMicrosoftFile::file_type() const noexcept { return m_file_meta.type; } diff --git a/src/odr/internal/oldms/oldms_file.hpp b/src/odr/internal/oldms/oldms_file.hpp index 66e8f4f13..ac6870a7e 100644 --- a/src/odr/internal/oldms/oldms_file.hpp +++ b/src/odr/internal/oldms/oldms_file.hpp @@ -22,7 +22,6 @@ class LegacyMicrosoftFile final : public abstract::DocumentFile { [[nodiscard]] std::shared_ptr file() const noexcept override; - [[nodiscard]] DecoderEngine decoder_engine() const noexcept override; [[nodiscard]] FileType file_type() const noexcept override; [[nodiscard]] std::string_view mimetype() const noexcept override; [[nodiscard]] FileMeta file_meta() const noexcept override; diff --git a/src/odr/internal/oldms/presentation/ppt_document.cpp b/src/odr/internal/oldms/presentation/ppt_document.cpp index 8b4d99b8b..4597300f5 100644 --- a/src/odr/internal/oldms/presentation/ppt_document.cpp +++ b/src/odr/internal/oldms/presentation/ppt_document.cpp @@ -194,25 +194,25 @@ class ElementAdapter final : public abstract::ElementAdapter, [[maybe_unused]] const ElementIdentifier element_id) const override { return AnchorType::at_page; } - [[nodiscard]] std::optional + [[nodiscard]] std::optional frame_x(const ElementIdentifier element_id) const override { return anchor_measure(element_id, [](const Anchor &a) { return a.left; }); } - [[nodiscard]] std::optional + [[nodiscard]] std::optional frame_y(const ElementIdentifier element_id) const override { return anchor_measure(element_id, [](const Anchor &a) { return a.top; }); } - [[nodiscard]] std::optional + [[nodiscard]] std::optional frame_width(const ElementIdentifier element_id) const override { return anchor_measure(element_id, [](const Anchor &a) { return a.right - a.left; }); } - [[nodiscard]] std::optional + [[nodiscard]] std::optional frame_height(const ElementIdentifier element_id) const override { return anchor_measure(element_id, [](const Anchor &a) { return a.bottom - a.top; }); } - [[nodiscard]] std::optional + [[nodiscard]] std::optional frame_z_index(const ElementIdentifier /*element_id*/) const override { return std::nullopt; } @@ -277,10 +277,10 @@ class ElementAdapter final : public abstract::ElementAdapter, m_registry->element_style_index(element_id)); } - // Converts one field of a frame's anchor to a Measure string, or nullopt - // when the frame has no anchor. + // Converts one field of a frame's anchor to a Measure, or nullopt when the + // frame has no anchor. template - [[nodiscard]] std::optional + [[nodiscard]] std::optional anchor_measure(const ElementIdentifier element_id, const Selector &select) const { const std::optional &anchor = @@ -288,8 +288,7 @@ class ElementAdapter final : public abstract::ElementAdapter, if (!anchor.has_value()) { return std::nullopt; } - return Measure(select(*anchor) / master_units_per_inch, DynamicUnit("in")) - .to_string(); + return Measure(select(*anchor) / master_units_per_inch, DynamicUnit("in")); } [[maybe_unused]] diff --git a/src/odr/internal/oldms/spreadsheet/xls_style.cpp b/src/odr/internal/oldms/spreadsheet/xls_style.cpp index d28216547..c68b52e46 100644 --- a/src/odr/internal/oldms/spreadsheet/xls_style.cpp +++ b/src/odr/internal/oldms/spreadsheet/xls_style.cpp @@ -33,7 +33,7 @@ constexpr std::array default_palette = { std::optional icv_color(const std::uint16_t icv, const std::span palette) { if (icv < built_in_colors.size()) { - return Color(built_in_colors[icv]); + return Color::from_rgb(built_in_colors[icv]); } if (const std::size_t index = icv - built_in_colors.size(); index < palette_color_count) { @@ -41,7 +41,7 @@ std::optional icv_color(const std::uint16_t icv, const LongRgb &color = palette[index]; return Color(color.red, color.green, color.blue); } - return Color(default_palette[index]); + return Color::from_rgb(default_palette[index]); } return std::nullopt; } @@ -75,7 +75,7 @@ StyleRegistry::StyleRegistry(std::vector fonts, const Font &font = font_at(m_fonts, xf.ifnt); TextStyle &text = style.text_style; - text.font_name = font.name.c_str(); + text.font_name = font.name; if (font.fixed.dyHeight != 0) { text.font_size = Measure(font.fixed.dyHeight / 20.0, DynamicUnit("pt")); } diff --git a/src/odr/internal/oldms/spreadsheet/xls_style.hpp b/src/odr/internal/oldms/spreadsheet/xls_style.hpp index c8ab6fe8e..107af4342 100644 --- a/src/odr/internal/oldms/spreadsheet/xls_style.hpp +++ b/src/odr/internal/oldms/spreadsheet/xls_style.hpp @@ -32,9 +32,9 @@ class StyleRegistry final { [[nodiscard]] const ResolvedStyle &cell_style(std::uint16_t ixfe) const; private: - /// Owns the font names: `TextStyle::font_name` (`const char *`) points into - /// them. Never modified after construction (moving the registry is fine — - /// the strings themselves do not move). + /// Owns the font names: `TextStyle::font_name` (a `std::string_view`) points + /// into them. Never modified after construction (moving the registry is fine + /// — the strings themselves do not move). std::vector m_fonts; std::vector m_cell_styles; }; diff --git a/src/odr/internal/oldms/text/doc_style.cpp b/src/odr/internal/oldms/text/doc_style.cpp index 17799bdaa..9c1a8e5c7 100644 --- a/src/odr/internal/oldms/text/doc_style.cpp +++ b/src/odr/internal/oldms/text/doc_style.cpp @@ -30,7 +30,7 @@ std::optional ico_color(const std::uint8_t ico) { if (ico == 0) { return std::nullopt; // cvAuto } - return Color(ico_colors[ico]); + return Color::from_rgb(ico_colors[ico]); } /// ToggleOperand ([MS-DOC] 2.9.327) against the (unmodelled) style value: @@ -143,7 +143,7 @@ text::apply_character_sprms(TextStyle style, const std::string_view grpprl, throw std::runtime_error("doc: sprmCRgFtc0 font index out of range"); } if (!font_names.empty()) { - style.font_name = font_names[static_cast(ftc)].c_str(); + style.font_name = font_names[static_cast(ftc)]; } } break; case sprmCCv: { diff --git a/src/odr/internal/oldms/text/doc_style.hpp b/src/odr/internal/oldms/text/doc_style.hpp index b898dd2a8..d289baddb 100644 --- a/src/odr/internal/oldms/text/doc_style.hpp +++ b/src/odr/internal/oldms/text/doc_style.hpp @@ -29,9 +29,9 @@ class StyleRegistry final { [[nodiscard]] const TextStyle &text_style(std::uint32_t index) const; private: - /// Owns the font names: `TextStyle::font_name` (`const char *`) points into - /// them. Never modified after construction (moving the registry is fine — - /// the strings themselves do not move). + /// Owns the font names: `TextStyle::font_name` (a `std::string_view`) points + /// into them. Never modified after construction (moving the registry is fine + /// — the strings themselves do not move). std::vector m_font_names; std::vector m_styles; }; diff --git a/src/odr/internal/ooxml/AGENTS.md b/src/odr/internal/ooxml/AGENTS.md index 11143d072..c0d5e615e 100644 --- a/src/odr/internal/ooxml/AGENTS.md +++ b/src/odr/internal/ooxml/AGENTS.md @@ -5,7 +5,7 @@ The **why** for the OOXML engine; per-feature checklists live in the build/test, conventions) is in the top-level [`AGENTS.md`](../../../../AGENTS.md); read it first. -**Scope.** One engine (`DecoderEngine::odr`) reading the three OOXML document +**Scope.** Reading the three OOXML document types — word (`.docx`), presentation (`.pptx`), spreadsheet (`.xlsx`). The three formats **share almost nothing beyond packaging, encryption, and type detection**; each is a self-contained module with its own `AGENTS.md`: diff --git a/src/odr/internal/ooxml/ooxml_file.cpp b/src/odr/internal/ooxml/ooxml_file.cpp index 16cae8bbb..f60bd58ae 100644 --- a/src/odr/internal/ooxml/ooxml_file.cpp +++ b/src/odr/internal/ooxml/ooxml_file.cpp @@ -29,10 +29,6 @@ std::shared_ptr OfficeOpenXmlFile::file() const noexcept { return {}; } -DecoderEngine OfficeOpenXmlFile::decoder_engine() const noexcept { - return DecoderEngine::odr; -} - FileType OfficeOpenXmlFile::file_type() const noexcept { return m_file_meta.type; } diff --git a/src/odr/internal/ooxml/ooxml_file.hpp b/src/odr/internal/ooxml/ooxml_file.hpp index d7cb79aa9..5bc9a0faf 100644 --- a/src/odr/internal/ooxml/ooxml_file.hpp +++ b/src/odr/internal/ooxml/ooxml_file.hpp @@ -24,7 +24,6 @@ class OfficeOpenXmlFile final : public abstract::DocumentFile { [[nodiscard]] std::shared_ptr file() const noexcept override; - [[nodiscard]] DecoderEngine decoder_engine() const noexcept override; [[nodiscard]] FileType file_type() const noexcept override; [[nodiscard]] std::string_view mimetype() const noexcept override; [[nodiscard]] FileMeta file_meta() const noexcept override; diff --git a/src/odr/internal/ooxml/ooxml_util.cpp b/src/odr/internal/ooxml/ooxml_util.cpp index c9b6d73f4..ea3736c8a 100644 --- a/src/odr/internal/ooxml/ooxml_util.cpp +++ b/src/odr/internal/ooxml/ooxml_util.cpp @@ -46,7 +46,7 @@ ooxml::read_color_attribute(const pugi::xml_attribute attribute) { } if (std::strlen(value) == 6) { const std::uint32_t color = std::strtoull(value, nullptr, 16); - return Color(color); + return Color::from_rgb(color); } return {}; } diff --git a/src/odr/internal/ooxml/presentation/ooxml_presentation_document.cpp b/src/odr/internal/ooxml/presentation/ooxml_presentation_document.cpp index 4869232e2..ed1c2c0a4 100644 --- a/src/odr/internal/ooxml/presentation/ooxml_presentation_document.cpp +++ b/src/odr/internal/ooxml/presentation/ooxml_presentation_document.cpp @@ -468,39 +468,27 @@ class ElementAdapter final : public abstract::ElementAdapter, [[maybe_unused]] const ElementIdentifier element_id) const override { return AnchorType::at_page; } - [[nodiscard]] std::optional + [[nodiscard]] std::optional frame_x(const ElementIdentifier element_id) const override { - if (const std::optional x = read_emus_attribute( - get_frame_xfrm(element_id).child("a:off").attribute("x"))) { - return x->to_string(); - } - return {}; + return read_emus_attribute( + get_frame_xfrm(element_id).child("a:off").attribute("x")); } - [[nodiscard]] std::optional + [[nodiscard]] std::optional frame_y(const ElementIdentifier element_id) const override { - if (const std::optional y = read_emus_attribute( - get_frame_xfrm(element_id).child("a:off").attribute("y"))) { - return y->to_string(); - } - return {}; + return read_emus_attribute( + get_frame_xfrm(element_id).child("a:off").attribute("y")); } - [[nodiscard]] std::optional + [[nodiscard]] std::optional frame_width(const ElementIdentifier element_id) const override { - if (const std::optional cx = read_emus_attribute( - get_frame_xfrm(element_id).child("a:ext").attribute("cx"))) { - return cx->to_string(); - } - return {}; + return read_emus_attribute( + get_frame_xfrm(element_id).child("a:ext").attribute("cx")); } - [[nodiscard]] std::optional + [[nodiscard]] std::optional frame_height(const ElementIdentifier element_id) const override { - if (const std::optional cy = read_emus_attribute( - get_frame_xfrm(element_id).child("a:ext").attribute("cy"))) { - return cy->to_string(); - } - return {}; + return read_emus_attribute( + get_frame_xfrm(element_id).child("a:ext").attribute("cy")); } - [[nodiscard]] std::optional + [[nodiscard]] std::optional frame_z_index(const ElementIdentifier) const override { return std::nullopt; } diff --git a/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp b/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp index 6bb277dd3..4604f801b 100644 --- a/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp +++ b/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp @@ -358,59 +358,43 @@ class ElementAdapter final : public abstract::ElementAdapter, [[maybe_unused]] const ElementIdentifier element_id) const override { return AnchorType::at_page; } - [[nodiscard]] std::optional + [[nodiscard]] std::optional frame_x(const ElementIdentifier element_id) const override { - if (const std::optional x = - read_emus_attribute(get_node(element_id) - .child("xdr:pic") - .child("xdr:spPr") - .child("a:xfrm") - .child("a:off") - .attribute("x"))) { - return x->to_string(); - } - return {}; - } - [[nodiscard]] std::optional + return read_emus_attribute(get_node(element_id) + .child("xdr:pic") + .child("xdr:spPr") + .child("a:xfrm") + .child("a:off") + .attribute("x")); + } + [[nodiscard]] std::optional frame_y(const ElementIdentifier element_id) const override { - if (const std::optional y = - read_emus_attribute(get_node(element_id) - .child("xdr:pic") - .child("xdr:spPr") - .child("a:xfrm") - .child("a:off") - .attribute("y"))) { - return y->to_string(); - } - return {}; - } - [[nodiscard]] std::optional + return read_emus_attribute(get_node(element_id) + .child("xdr:pic") + .child("xdr:spPr") + .child("a:xfrm") + .child("a:off") + .attribute("y")); + } + [[nodiscard]] std::optional frame_width(const ElementIdentifier element_id) const override { - if (const std::optional width = - read_emus_attribute(get_node(element_id) - .child("xdr:pic") - .child("xdr:spPr") - .child("a:xfrm") - .child("a:ext") - .attribute("cx"))) { - return width->to_string(); - } - return {}; - } - [[nodiscard]] std::optional + return read_emus_attribute(get_node(element_id) + .child("xdr:pic") + .child("xdr:spPr") + .child("a:xfrm") + .child("a:ext") + .attribute("cx")); + } + [[nodiscard]] std::optional frame_height(const ElementIdentifier element_id) const override { - if (const std::optional height = - read_emus_attribute(get_node(element_id) - .child("xdr:pic") - .child("xdr:spPr") - .child("a:xfrm") - .child("a:ext") - .attribute("cy"))) { - return height->to_string(); - } - return {}; - } - [[nodiscard]] std::optional frame_z_index( + return read_emus_attribute(get_node(element_id) + .child("xdr:pic") + .child("xdr:spPr") + .child("a:xfrm") + .child("a:ext") + .attribute("cy")); + } + [[nodiscard]] std::optional frame_z_index( [[maybe_unused]] const ElementIdentifier element_id) const override { return std::nullopt; } diff --git a/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_style.cpp b/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_style.cpp index 5019a8881..97d632a20 100644 --- a/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_style.cpp +++ b/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_style.cpp @@ -55,11 +55,11 @@ std::optional read_color(const pugi::xml_node node) { const char *value = rgb.value(); if (std::strlen(value) == 8) { const std::uint32_t color = std::strtoull(value, nullptr, 16); - return Color(color, false); + return Color::from_argb(color); } if (std::strlen(value) == 6) { const std::uint32_t color = std::strtoull(value, nullptr, 16); - return Color(color); + return Color::from_rgb(color); } } return {}; diff --git a/src/odr/internal/ooxml/text/ooxml_text_document.cpp b/src/odr/internal/ooxml/text/ooxml_text_document.cpp index e559ee124..b2dc998ec 100644 --- a/src/odr/internal/ooxml/text/ooxml_text_document.cpp +++ b/src/odr/internal/ooxml/text/ooxml_text_document.cpp @@ -457,33 +457,25 @@ class ElementAdapter final : public abstract::ElementAdapter, } return AnchorType::as_char; // TODO default? } - [[nodiscard]] std::optional + [[nodiscard]] std::optional frame_x([[maybe_unused]] const ElementIdentifier element_id) const override { return std::nullopt; } - [[nodiscard]] std::optional + [[nodiscard]] std::optional frame_y([[maybe_unused]] const ElementIdentifier element_id) const override { return std::nullopt; } - [[nodiscard]] std::optional + [[nodiscard]] std::optional frame_width(const ElementIdentifier element_id) const override { const pugi::xml_node inner_node = get_frame_inner_node(element_id); - if (const std::optional width = read_emus_attribute( - inner_node.child("wp:extent").attribute("cx"))) { - return width->to_string(); - } - return {}; + return read_emus_attribute(inner_node.child("wp:extent").attribute("cx")); } - [[nodiscard]] std::optional + [[nodiscard]] std::optional frame_height(const ElementIdentifier element_id) const override { const pugi::xml_node inner_node = get_frame_inner_node(element_id); - if (const std::optional height = read_emus_attribute( - inner_node.child("wp:extent").attribute("cy"))) { - return height->to_string(); - } - return {}; + return read_emus_attribute(inner_node.child("wp:extent").attribute("cy")); } - [[nodiscard]] std::optional frame_z_index( + [[nodiscard]] std::optional frame_z_index( [[maybe_unused]] const ElementIdentifier element_id) const override { return std::nullopt; } diff --git a/src/odr/internal/open_strategy.cpp b/src/odr/internal/open_strategy.cpp index 31b656b8a..6efe3ddf7 100644 --- a/src/odr/internal/open_strategy.cpp +++ b/src/odr/internal/open_strategy.cpp @@ -46,6 +46,160 @@ template auto priority_comparator(const std::vector &priority) { }; } +/// Decodes @p file as exactly @p as, or throws the format's "not a ..." +/// 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) { + if (as == FileType::opendocument_text || + as == FileType::opendocument_presentation || + as == FileType::opendocument_spreadsheet || + as == FileType::opendocument_graphics) { + ODR_VERBOSE(logger, "open as odf"); + try { + auto zip_file = std::make_unique(file); + auto filesystem = zip_file->archive()->as_filesystem(); + return std::make_unique(filesystem); + } catch (...) { + ODR_VERBOSE(logger, "failed to open as odf"); + } + throw NoOpenDocumentFile(); + } + + if (as == FileType::office_open_xml_document || + as == FileType::office_open_xml_presentation || + as == FileType::office_open_xml_workbook || + as == FileType::office_open_xml_encrypted) { + ODR_VERBOSE(logger, "open as ooxml"); + try { + auto zip_file = std::make_unique(file); + auto filesystem = zip_file->archive()->as_filesystem(); + return std::make_unique(filesystem); + } catch (...) { + ODR_VERBOSE(logger, "failed to open as ooxml zip"); + } + try { + auto cfb_file = std::make_unique(file); + auto filesystem = cfb_file->archive()->as_filesystem(); + return std::make_unique(filesystem); + } catch (...) { + ODR_VERBOSE(logger, "failed to open as ooxml cfb"); + } + throw NoOfficeOpenXmlFile(); + } + + if (as == FileType::legacy_word_document || + as == FileType::legacy_powerpoint_presentation || + as == FileType::legacy_excel_worksheets) { + ODR_VERBOSE(logger, "open as legacy ms"); + try { + auto cfb_file = std::make_unique(file); + auto filesystem = cfb_file->archive()->as_filesystem(); + return std::make_unique(filesystem); + } catch (...) { + ODR_VERBOSE(logger, "failed to open as legacy ms"); + } + throw NoLegacyMicrosoftFile(); + } + + if (as == FileType::portable_document_format) { + ODR_VERBOSE(logger, "open as pdf"); + try { + return std::make_unique(file); + } catch (...) { + ODR_VERBOSE(logger, "failed to open as pdf"); + } + throw NoPdfFile(); + } + + if (as == FileType::portable_network_graphics || + as == FileType::graphics_interchange_format || as == FileType::jpeg || + as == FileType::bitmap_image_file) { + ODR_VERBOSE(logger, "open as image"); + try { + return std::make_unique(file, as); + } catch (...) { + ODR_VERBOSE(logger, "failed to open as image"); + } + throw NoImageFile(); + } + + if (as == FileType::starview_metafile) { + ODR_VERBOSE(logger, "open as svm"); + try { + return std::make_unique(file); + } catch (...) { + ODR_VERBOSE(logger, "failed to open as svm"); + } + throw NoSvmFile(); + } + + if (as == FileType::truetype_font || as == FileType::opentype_font) { + ODR_VERBOSE(logger, "open as font"); + try { + return std::make_unique(file, as); + } catch (...) { + ODR_VERBOSE(logger, "failed to open as font"); + } + throw NoFontFile(); + } + + if (as == FileType::text_file) { + ODR_VERBOSE(logger, "open as text file"); + try { + return std::make_unique(file); + } catch (...) { + ODR_VERBOSE(logger, "failed to open as text file"); + } + throw NoTextFile(); + } + + if (as == FileType::comma_separated_values) { + ODR_VERBOSE(logger, "open as csv"); + try { + auto text = std::make_shared(file); + return std::make_unique(text); + } catch (...) { + ODR_VERBOSE(logger, "failed to open as csv"); + } + throw NoCsvFile(); + } + + if (as == FileType::javascript_object_notation) { + ODR_VERBOSE(logger, "open as json"); + try { + auto text = std::make_shared(file); + return std::make_unique(text); + } catch (...) { + ODR_VERBOSE(logger, "failed to open as json"); + } + throw NoJsonFile(); + } + + if (as == FileType::zip) { + ODR_VERBOSE(logger, "open as zip"); + try { + return std::make_unique(file); + } catch (...) { + ODR_VERBOSE(logger, "failed to open as zip"); + } + throw NoZipFile(); + } + + if (as == FileType::compound_file_binary_format) { + ODR_VERBOSE(logger, "open as cfb"); + try { + return std::make_unique(file); + } catch (...) { + ODR_VERBOSE(logger, "failed to open as cfb"); + } + throw NoCfbFile(); + } + + ODR_ERROR(logger, "unsupported file type " << file_type_to_string(as)); + throw UnsupportedFileType(as); +} + } // namespace std::vector @@ -137,14 +291,6 @@ open_strategy::list_file_types(const std::shared_ptr &file, return result; } -std::vector open_strategy::list_decoder_engines(const FileType) { - std::vector result; - - result.push_back(DecoderEngine::odr); - - return result; -} - std::unique_ptr open_strategy::open_file(const std::shared_ptr &file, const Logger &logger) { @@ -262,233 +408,6 @@ open_strategy::open_file(const std::shared_ptr &file, return open_file(file, preference, logger); } -std::unique_ptr -open_strategy::open_file(const std::shared_ptr &file, - FileType as, DecoderEngine with, - const Logger &logger) { - if (as == FileType::opendocument_text || - as == FileType::opendocument_presentation || - as == FileType::opendocument_spreadsheet || - as == FileType::opendocument_graphics) { - ODR_VERBOSE(logger, "open as odf"); - if (with == DecoderEngine::odr) { - ODR_VERBOSE(logger, "using odr engine"); - try { - auto zip_file = std::make_unique(file); - auto filesystem = zip_file->archive()->as_filesystem(); - return std::make_unique(filesystem); - } catch (...) { - ODR_VERBOSE(logger, "failed to open as odf with odr engine"); - } - throw NoOpenDocumentFile(); - } - ODR_ERROR(logger, "unsupported decoder engine for odf " - << decoder_engine_to_string(with)); - throw UnsupportedDecoderEngine(with); - } - - if (as == FileType::office_open_xml_document || - as == FileType::office_open_xml_presentation || - as == FileType::office_open_xml_workbook || - as == FileType::office_open_xml_encrypted) { - ODR_VERBOSE(logger, "open as ooxml"); - if (with == DecoderEngine::odr) { - ODR_VERBOSE(logger, "using odr engine"); - try { - auto zip_file = std::make_unique(file); - auto filesystem = zip_file->archive()->as_filesystem(); - return std::make_unique(filesystem); - } catch (...) { - ODR_VERBOSE(logger, "failed to open as ooxml zip with odr engine"); - } - try { - auto cfb_file = std::make_unique(file); - auto filesystem = cfb_file->archive()->as_filesystem(); - return std::make_unique(filesystem); - } catch (...) { - ODR_VERBOSE(logger, "failed to open as ooxml cfb with odr engine"); - } - throw NoOfficeOpenXmlFile(); - } - ODR_ERROR(logger, "unsupported decoder engine for ooxml " - << decoder_engine_to_string(with)); - throw UnsupportedDecoderEngine(with); - } - - if (as == FileType::legacy_word_document || - as == FileType::legacy_powerpoint_presentation || - as == FileType::legacy_excel_worksheets) { - ODR_VERBOSE(logger, "open as legacy ms"); - if (with == DecoderEngine::odr) { - ODR_VERBOSE(logger, "using odr engine"); - try { - auto cfb_file = std::make_unique(file); - auto filesystem = cfb_file->archive()->as_filesystem(); - return std::make_unique(filesystem); - } catch (...) { - ODR_VERBOSE(logger, "failed to open as legacy ms with odr engine"); - } - throw NoLegacyMicrosoftFile(); - } - ODR_ERROR(logger, "unsupported decoder engine for legacy ms " - << decoder_engine_to_string(with)); - throw UnsupportedDecoderEngine(with); - } - - if (as == FileType::portable_document_format) { - ODR_VERBOSE(logger, "open as pdf"); - if (with == DecoderEngine::odr) { - ODR_VERBOSE(logger, "using odr engine"); - try { - return std::make_unique(file); - } catch (...) { - ODR_VERBOSE(logger, "failed to open as pdf with odr engine"); - } - throw NoPdfFile(); - } - ODR_ERROR(logger, "unsupported decoder engine for pdf " - << decoder_engine_to_string(with)); - throw UnsupportedDecoderEngine(with); - } - - if (as == FileType::portable_network_graphics || - as == FileType::graphics_interchange_format || as == FileType::jpeg || - as == FileType::bitmap_image_file) { - ODR_VERBOSE(logger, "open as image"); - if (with == DecoderEngine::odr) { - ODR_VERBOSE(logger, "using odr engine"); - try { - return std::make_unique(file, as); - } catch (...) { - ODR_VERBOSE(logger, "failed to open as image with odr engine"); - } - throw NoImageFile(); - } - ODR_ERROR(logger, "unsupported decoder engine for image " - << decoder_engine_to_string(with)); - throw UnsupportedDecoderEngine(with); - } - - if (as == FileType::starview_metafile) { - ODR_VERBOSE(logger, "open as svm"); - if (with == DecoderEngine::odr) { - ODR_VERBOSE(logger, "using odr engine"); - try { - return std::make_unique(file); - } catch (...) { - ODR_VERBOSE(logger, "failed to open as svm with odr engine"); - } - throw NoSvmFile(); - } - ODR_ERROR(logger, "unsupported decoder engine for svm " - << decoder_engine_to_string(with)); - throw UnsupportedDecoderEngine(with); - } - - if (as == FileType::truetype_font || as == FileType::opentype_font) { - ODR_VERBOSE(logger, "open as font"); - if (with == DecoderEngine::odr) { - ODR_VERBOSE(logger, "using odr engine"); - try { - return std::make_unique(file, as); - } catch (...) { - ODR_VERBOSE(logger, "failed to open as font with odr engine"); - } - throw NoFontFile(); - } - ODR_ERROR(logger, "unsupported decoder engine for font " - << decoder_engine_to_string(with)); - throw UnsupportedDecoderEngine(with); - } - - if (as == FileType::text_file) { - ODR_VERBOSE(logger, "open as text file"); - if (with == DecoderEngine::odr) { - ODR_VERBOSE(logger, "using odr engine"); - try { - return std::make_unique(file); - } catch (...) { - ODR_VERBOSE(logger, "failed to open as text file with odr engine"); - } - throw NoTextFile(); - } - ODR_ERROR(logger, "unsupported decoder engine for text file " - << decoder_engine_to_string(with)); - throw UnsupportedDecoderEngine(with); - } - - if (as == FileType::comma_separated_values) { - ODR_VERBOSE(logger, "open as csv"); - if (with == DecoderEngine::odr) { - ODR_VERBOSE(logger, "using odr engine"); - try { - auto text = std::make_shared(file); - return std::make_unique(text); - } catch (...) { - ODR_VERBOSE(logger, "failed to open as csv with odr engine"); - } - throw NoCsvFile(); - } - ODR_ERROR(logger, "unsupported decoder engine for csv " - << decoder_engine_to_string(with)); - throw UnsupportedDecoderEngine(with); - } - - if (as == FileType::javascript_object_notation) { - ODR_VERBOSE(logger, "open as json"); - if (with == DecoderEngine::odr) { - ODR_VERBOSE(logger, "using odr engine"); - try { - auto text = std::make_shared(file); - return std::make_unique(text); - } catch (...) { - ODR_VERBOSE(logger, "failed to open as json with odr engine"); - } - throw NoJsonFile(); - } - ODR_ERROR(logger, "unsupported decoder engine for json " - << decoder_engine_to_string(with)); - throw UnsupportedDecoderEngine(with); - } - - if (as == FileType::zip) { - ODR_VERBOSE(logger, "open as zip"); - if (with == DecoderEngine::odr) { - ODR_VERBOSE(logger, "using odr engine"); - try { - return std::make_unique(file); - } catch (...) { - ODR_VERBOSE(logger, "failed to open as zip with odr engine"); - } - throw NoZipFile(); - } - ODR_ERROR(logger, "unsupported decoder engine for zip " - << decoder_engine_to_string(with)); - throw UnsupportedDecoderEngine(with); - } - - if (as == FileType::compound_file_binary_format) { - ODR_VERBOSE(logger, "open as cfb"); - if (with == DecoderEngine::odr) { - ODR_VERBOSE(logger, "using odr engine"); - try { - return std::make_unique(file); - } catch (...) { - ODR_VERBOSE(logger, "failed to open as cfb with odr engine"); - } - throw NoCfbFile(); - } - ODR_ERROR(logger, "unsupported decoder engine for cfb " - << decoder_engine_to_string(with)); - throw UnsupportedDecoderEngine(with); - } - - ODR_ERROR(logger, "unsupported file type " << file_type_to_string(as) - << " with decoder engine " - << decoder_engine_to_string(with)); - throw UnsupportedFileType(as); -} - std::unique_ptr open_strategy::open_file(const std::shared_ptr &file, const DecodePreference &preference, @@ -514,36 +433,11 @@ open_strategy::open_file(const std::shared_ptr &file, for (FileType as : probe_types) { ODR_VERBOSE(logger, "try opening as file type " << file_type_to_string(as)); - std::vector probe_engines; - if (preference.with_engine.has_value()) { - ODR_VERBOSE(logger, - "using preferred decoder engine " - << decoder_engine_to_string(*preference.with_engine)); - probe_engines.push_back(*preference.with_engine); - } else { - ODR_VERBOSE(logger, "probe decoder engines"); - std::vector detected_engines = - open_strategy::list_decoder_engines(as); - probe_engines.insert(probe_engines.end(), detected_engines.begin(), - detected_engines.end()); - auto probe_engines_end = std::ranges::unique(probe_engines).begin(); - probe_engines.erase(probe_engines_end, probe_engines.end()); - } - - std::ranges::stable_sort(probe_engines, - priority_comparator(preference.engine_priority)); - - for (DecoderEngine with : probe_engines) { + try { + return open_file_as(file, as, logger); + } catch (...) { ODR_VERBOSE(logger, - "with decoder engine " << decoder_engine_to_string(with)); - try { - return open_file(file, as, with, logger); - } catch (...) { - ODR_VERBOSE(logger, "failed to open as file type " - << file_type_to_string(as) - << " with decoder engine " - << decoder_engine_to_string(with)); - } + "failed to open as file type " << file_type_to_string(as)); } } diff --git a/src/odr/internal/open_strategy.hpp b/src/odr/internal/open_strategy.hpp index 03513e681..b1b00bda7 100644 --- a/src/odr/internal/open_strategy.hpp +++ b/src/odr/internal/open_strategy.hpp @@ -5,7 +5,6 @@ namespace odr { enum class FileType; -enum class DecoderEngine; struct DecodePreference; class Logger; } // namespace odr @@ -21,17 +20,12 @@ namespace odr::internal::open_strategy { std::vector list_file_types(const std::shared_ptr &file, const Logger &logger); -std::vector list_decoder_engines(FileType as); 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, FileType as, - DecoderEngine with, const Logger &logger); std::unique_ptr open_file(const std::shared_ptr &file, const DecodePreference &preference, const Logger &logger); diff --git a/src/odr/internal/pdf/AGENTS.md b/src/odr/internal/pdf/AGENTS.md index cb566d0e8..41e3ef303 100644 --- a/src/odr/internal/pdf/AGENTS.md +++ b/src/odr/internal/pdf/AGENTS.md @@ -4,8 +4,6 @@ The **why** behind the `pdf/` module and the roadmap. What is concretely implemented is in the code; this file keeps the rationale, the non-obvious invariants, and where things live. Reference links live in [`README.md`](README.md). -This is the only PDF path; `DecoderEngine::odr` is the only engine. - **Goal.** Faithful read-only HTML for common real-world PDFs through a pure-serialization pipeline (no native renderer). The file-format, text-extraction, font and graphics foundations are in place; what remains is diff --git a/src/odr/internal/pdf/pdf_file.cpp b/src/odr/internal/pdf/pdf_file.cpp index 3ed30a46b..1d6e07946 100644 --- a/src/odr/internal/pdf/pdf_file.cpp +++ b/src/odr/internal/pdf/pdf_file.cpp @@ -131,10 +131,6 @@ std::shared_ptr PdfFile::file() const noexcept { return m_file; } -DecoderEngine PdfFile::decoder_engine() const noexcept { - return DecoderEngine::odr; -} - FileMeta PdfFile::file_meta() const noexcept { return m_file_meta; } bool PdfFile::password_encrypted() const noexcept { diff --git a/src/odr/internal/pdf/pdf_file.hpp b/src/odr/internal/pdf/pdf_file.hpp index 585ec19d6..6c36fe4d8 100644 --- a/src/odr/internal/pdf/pdf_file.hpp +++ b/src/odr/internal/pdf/pdf_file.hpp @@ -19,7 +19,6 @@ class PdfFile final : public abstract::PdfFile { [[nodiscard]] std::shared_ptr file() const noexcept override; - [[nodiscard]] DecoderEngine decoder_engine() const noexcept override; [[nodiscard]] FileMeta file_meta() const noexcept override; [[nodiscard]] bool password_encrypted() const noexcept override; diff --git a/src/odr/internal/svm/svm_file.cpp b/src/odr/internal/svm/svm_file.cpp index 89f7488cb..80dfd6770 100644 --- a/src/odr/internal/svm/svm_file.cpp +++ b/src/odr/internal/svm/svm_file.cpp @@ -23,10 +23,6 @@ std::shared_ptr SvmFile::file() const noexcept { return m_file; } -DecoderEngine SvmFile::decoder_engine() const noexcept { - return DecoderEngine::odr; -} - FileType SvmFile::file_type() const noexcept { return FileType::starview_metafile; } diff --git a/src/odr/internal/svm/svm_file.hpp b/src/odr/internal/svm/svm_file.hpp index b6e1c7c10..e03610420 100644 --- a/src/odr/internal/svm/svm_file.hpp +++ b/src/odr/internal/svm/svm_file.hpp @@ -17,7 +17,6 @@ class SvmFile final : public abstract::ImageFile { [[nodiscard]] std::shared_ptr file() const noexcept override; - [[nodiscard]] DecoderEngine decoder_engine() const noexcept override; [[nodiscard]] FileType file_type() const noexcept override; [[nodiscard]] std::string_view mimetype() const noexcept override; [[nodiscard]] FileMeta file_meta() const noexcept override; diff --git a/src/odr/internal/text/text_file.cpp b/src/odr/internal/text/text_file.cpp index 0f06cf129..7e8913cc9 100644 --- a/src/odr/internal/text/text_file.cpp +++ b/src/odr/internal/text/text_file.cpp @@ -14,10 +14,6 @@ std::shared_ptr TextFile::file() const noexcept { return m_file; } -DecoderEngine TextFile::decoder_engine() const noexcept { - return DecoderEngine::odr; -} - FileType TextFile::file_type() const noexcept { return FileType::text_file; } std::string_view TextFile::mimetype() const noexcept { return "text/plain"; } diff --git a/src/odr/internal/text/text_file.hpp b/src/odr/internal/text/text_file.hpp index bcf21324c..ff77487a1 100644 --- a/src/odr/internal/text/text_file.hpp +++ b/src/odr/internal/text/text_file.hpp @@ -15,7 +15,6 @@ class TextFile final : public abstract::TextFile { [[nodiscard]] std::shared_ptr file() const noexcept override; - [[nodiscard]] DecoderEngine decoder_engine() const noexcept override; [[nodiscard]] FileType file_type() const noexcept override; [[nodiscard]] std::string_view mimetype() const noexcept override; [[nodiscard]] FileMeta file_meta() const noexcept override; diff --git a/src/odr/internal/util/number_util.cpp b/src/odr/internal/util/number_util.cpp new file mode 100644 index 000000000..cda21d30c --- /dev/null +++ b/src/odr/internal/util/number_util.cpp @@ -0,0 +1,41 @@ +#include + +#include +#include +#include +#include + +namespace odr::internal::util { + +std::string number::to_string_significant(const double value, + const int significant_digits) { + if (!std::isfinite(value)) { + std::ostringstream ss; + ss << value; + return ss.str(); + } + + // `std::fixed` counts decimals, not significant digits, so shift by the size + // of the integer part. Clamped: a denormal would otherwise ask for hundreds + // of decimals, and a huge value for a negative count. + int integer_digits = 1; + if (value != 0.0) { + integer_digits = + static_cast(std::floor(std::log10(std::abs(value)))) + 1; + } + const int decimals = std::clamp(significant_digits - integer_digits, 0, 15); + + std::ostringstream ss; + ss << std::fixed << std::setprecision(decimals) << value; + std::string result = ss.str(); + + if (result.find('.') != std::string::npos) { + result.erase(result.find_last_not_of('0') + 1); + if (!result.empty() && result.back() == '.') { + result.pop_back(); + } + } + return result; +} + +} // namespace odr::internal::util diff --git a/src/odr/internal/util/number_util.hpp b/src/odr/internal/util/number_util.hpp new file mode 100644 index 000000000..42f0d0d1c --- /dev/null +++ b/src/odr/internal/util/number_util.hpp @@ -0,0 +1,19 @@ +#pragma once + +#include + +namespace odr::internal::util::number { + +/// Renders @p value with @p significant_digits significant digits, always in +/// positional notation and without trailing zeros. +/// +/// Unlike the iostream/printf default (`%g`), this never switches to +/// scientific notation — CSS and SVG lengths do not accept it. Non-finite +/// values are passed through to the stream default. +/// +/// @p significant_digits is what bounds the noise: a value that came from a +/// `float` carries about 7 digits (`FLT_DIG` is 6), so asking for more digits +/// than the source has turns `68.55` into `68.550003`. +std::string to_string_significant(double value, int significant_digits); + +} // namespace odr::internal::util::number diff --git a/src/odr/internal/zip/zip_file.cpp b/src/odr/internal/zip/zip_file.cpp index aac32eb33..f56227a1f 100644 --- a/src/odr/internal/zip/zip_file.cpp +++ b/src/odr/internal/zip/zip_file.cpp @@ -12,10 +12,6 @@ std::shared_ptr ZipFile::file() const noexcept { return m_zip->file(); } -DecoderEngine ZipFile::decoder_engine() const noexcept { - return DecoderEngine::odr; -} - FileType ZipFile::file_type() const noexcept { return FileType::zip; } std::string_view ZipFile::mimetype() const noexcept { diff --git a/src/odr/internal/zip/zip_file.hpp b/src/odr/internal/zip/zip_file.hpp index 4ba1d6bbd..7bd15f30f 100644 --- a/src/odr/internal/zip/zip_file.hpp +++ b/src/odr/internal/zip/zip_file.hpp @@ -18,7 +18,6 @@ class ZipFile final : public abstract::ArchiveFile { [[nodiscard]] std::shared_ptr file() const noexcept override; - [[nodiscard]] DecoderEngine decoder_engine() const noexcept override; [[nodiscard]] FileType file_type() const noexcept override; [[nodiscard]] std::string_view mimetype() const noexcept override; [[nodiscard]] FileMeta file_meta() const noexcept override; diff --git a/src/odr/odr.cpp b/src/odr/odr.cpp index 4371dcf4c..6a4bcb189 100644 --- a/src/odr/odr.cpp +++ b/src/odr/odr.cpp @@ -415,29 +415,11 @@ std::string_view odr::mimetype_by_file_type(const FileType type) { throw UnsupportedFileType(type); } -std::string odr::decoder_engine_to_string(const DecoderEngine engine) { - if (engine == DecoderEngine::odr) { - return "odr"; - } - throw UnknownDecoderEngine(); -} - -odr::DecoderEngine odr::decoder_engine_by_name(const std::string &engine) { - if (engine == "odr") { - return DecoderEngine::odr; - } - throw UnknownDecoderEngine(); -} - std::vector odr::list_file_types(const std::string &path, const Logger &logger) { return DecodedFile::list_file_types(path, logger); } -std::vector odr::list_decoder_engines(const FileType as) { - return DecodedFile::list_decoder_engines(as); -} - std::string_view odr::mimetype(const std::string &path, const Logger &logger) { return DecodedFile::mimetype(path, logger); } diff --git a/src/odr/odr.hpp b/src/odr/odr.hpp index b4ed8f1a1..acd97013a 100644 --- a/src/odr/odr.hpp +++ b/src/odr/odr.hpp @@ -8,7 +8,6 @@ namespace odr { enum class FileType; enum class FileCategory; -enum class DecoderEngine; struct DecodePreference; class DecodedFile; enum class DocumentType; @@ -64,25 +63,12 @@ file_type_by_mimetype(std::string_view mimetype) noexcept; /// @return The MIME type. [[nodiscard]] std::string_view mimetype_by_file_type(FileType type); -/// @brief Get the decoder engine as a string. -/// @param engine The decoder engine. -/// @return The decoder engine as a string. -[[nodiscard]] std::string decoder_engine_to_string(DecoderEngine engine); -/// @brief Get the decoder engine by the name. -/// @param engine The name of the decoder engine. -/// @return The decoder engine. -[[nodiscard]] DecoderEngine decoder_engine_by_name(const std::string &engine); - /// @brief Determine the file types by the file path. /// @param path The file path. /// @param logger The logger to use. /// @return The file types. [[nodiscard]] std::vector list_file_types(const std::string &path, const Logger &logger = Logger::null()); -/// @brief Determine the decoder engines for a file path and file type. -/// @param as The file type. -/// @return The decoder engines. -[[nodiscard]] std::vector list_decoder_engines(FileType as); /// @brief Determine MIME types by the file path. /// @param path The file path. /// @param logger The logger to use. diff --git a/src/odr/quantity.cpp b/src/odr/quantity.cpp index 1490e4c5f..c146e9d4c 100644 --- a/src/odr/quantity.cpp +++ b/src/odr/quantity.cpp @@ -1,10 +1,21 @@ #include +#include + #include #include namespace odr { +/// 7 significant digits: enough that document geometry survives a +/// parse/format round trip (drawing coordinates reach the thousands of mm, +/// which the stream default of 6 would round), and no more than a `float` +/// carries, so an xlsx column width stored as float `68.55` does not come back +/// as `68.550003`. +std::string QuantityBase::format_magnitude(const double magnitude) { + return internal::util::number::to_string_significant(magnitude, 7); +} + struct DynamicUnit::Unit final { std::string name; }; diff --git a/src/odr/quantity.hpp b/src/odr/quantity.hpp index 0a99bd1fb..f584b1ee7 100644 --- a/src/odr/quantity.hpp +++ b/src/odr/quantity.hpp @@ -1,8 +1,8 @@ #pragma once -#include #include #include +#include namespace odr { @@ -27,8 +27,17 @@ class DynamicUnit { const Unit *m_unit{nullptr}; }; +/// @brief The magnitude-type-independent part of @ref Quantity, so it can be +/// compiled once in a source file instead of inline in every instantiation. +class QuantityBase { +protected: + /// Renders @p magnitude with 7 significant digits, always positional. + static std::string format_magnitude(double magnitude); +}; + /// @brief Represents a quantity with a magnitude and a unit of measure. -template class Quantity { +template +class Quantity : private QuantityBase { public: Quantity(Magnitude magnitude, Unit unit) : m_magnitude{std::move(magnitude)}, m_unit{std::move(unit)} {} @@ -54,14 +63,17 @@ template class Quantity { Magnitude magnitude() const { return m_magnitude; } Unit unit() const { return m_unit; } - void to_stream(std::ostream &out) const { - out << m_magnitude << m_unit.to_string(); - } + void to_stream(std::ostream &out) const { out << to_string(); } [[nodiscard]] std::string to_string() const { - std::ostringstream ss; - ss << std::setprecision(4) << m_magnitude << m_unit.to_string(); - return ss.str(); + if constexpr (std::is_floating_point_v) { + return format_magnitude(static_cast(m_magnitude)) + + m_unit.to_string(); + } else { + std::ostringstream ss; + ss << m_magnitude << m_unit.to_string(); + return ss.str(); + } } private: @@ -69,6 +81,9 @@ template class Quantity { Unit m_unit; }; +/// @brief Represents a quantity: a magnitude and a unit of measure. +using Measure = Quantity; + } // namespace odr std::ostream &operator<<(std::ostream &, const odr::DynamicUnit &); diff --git a/src/odr/style.cpp b/src/odr/style.cpp index c10a8972f..e536bcc03 100644 --- a/src/odr/style.cpp +++ b/src/odr/style.cpp @@ -2,18 +2,20 @@ namespace odr { -Color::Color() = default; +Color Color::from_rgb(const std::uint32_t rgb) { + return {static_cast(rgb >> 16), + static_cast(rgb >> 8), + static_cast(rgb >> 0)}; +} -Color::Color(const std::uint32_t rgb) - : red{static_cast(rgb >> 16)}, - green{static_cast(rgb >> 8)}, - blue{static_cast(rgb >> 0)} {} +Color Color::from_argb(const std::uint32_t argb) { + return {static_cast(argb >> 16), + static_cast(argb >> 8), + static_cast(argb >> 0), + static_cast(argb >> 24)}; +} -Color::Color(const std::uint32_t argb, bool) - : red{static_cast(argb >> 16)}, - green{static_cast(argb >> 8)}, - blue{static_cast(argb >> 0)}, - alpha{static_cast(argb >> 24)} {} +Color::Color() = default; Color::Color(const std::uint8_t red, const std::uint8_t green, const std::uint8_t blue) @@ -41,7 +43,7 @@ std::uint32_t Color::argb() const { } void TextStyle::override(const TextStyle &other) { - if (other.font_name != nullptr) { + if (other.font_name.has_value()) { font_name = other.font_name; } if (other.font_size.has_value()) { diff --git a/src/odr/style.hpp b/src/odr/style.hpp index d298bf00d..5bfa15163 100644 --- a/src/odr/style.hpp +++ b/src/odr/style.hpp @@ -5,13 +5,11 @@ #include #include #include +#include #include namespace odr { -/// @brief Represents a quantity: a magnitude and a unit of measure. -using Measure = Quantity; - /// @brief Collection of font weights. enum class FontWeight { normal, @@ -74,9 +72,12 @@ struct Color final { std::uint8_t blue{0}; std::uint8_t alpha{255}; + /// @brief Builds an opaque color from a packed `0xRRGGBB` value. + static Color from_rgb(std::uint32_t rgb); + /// @brief Builds a color from a packed `0xAARRGGBB` value. + static Color from_argb(std::uint32_t argb); + Color(); - explicit Color(std::uint32_t rgb); - Color(std::uint32_t argb, bool dummy); Color(std::uint8_t red, std::uint8_t green, std::uint8_t blue); Color(std::uint8_t red, std::uint8_t green, std::uint8_t blue, std::uint8_t alpha); @@ -85,7 +86,9 @@ struct Color final { [[nodiscard]] std::uint32_t argb() const; }; -inline Color operator""_rgb(const unsigned long long rgb) { return Color(rgb); } +inline Color operator""_rgb(const unsigned long long rgb) { + return Color::from_rgb(static_cast(rgb)); +} /// @brief Represents a directional style. template struct DirectionalStyle final { @@ -130,8 +133,11 @@ template struct DirectionalStyle final { }; /// @brief Represents a style for text. +/// +/// @note `font_name` borrows from the document that produced the style and is +/// only valid for as long as that document is alive. struct TextStyle final { - const char *font_name{nullptr}; + std::optional font_name; std::optional font_size; std::optional font_weight; std::optional font_style; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 2a392c6e6..ad4ff6120 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -76,6 +76,7 @@ add_executable(odr_test "src/internal/text/text_file_test.cpp" "src/internal/util/map_util_test.cpp" + "src/internal/util/number_util_test.cpp" "src/internal/util/string_util_test.cpp" "src/internal/util/xml_util_test.cpp" diff --git a/test/data/reference-output/odr-private b/test/data/reference-output/odr-private index ba4be9ff5..7ed25fa75 160000 --- a/test/data/reference-output/odr-private +++ b/test/data/reference-output/odr-private @@ -1 +1 @@ -Subproject commit ba4be9ff59f636d91144a35d2d90961748204e3a +Subproject commit 7ed25fa753206f782af4b6495a2a4c4ff03478eb diff --git a/test/data/reference-output/odr-public b/test/data/reference-output/odr-public index e74d3332c..6581dcd1e 160000 --- a/test/data/reference-output/odr-public +++ b/test/data/reference-output/odr-public @@ -1 +1 @@ -Subproject commit e74d3332c7ef2d0537cfc51f93ea59cc3e977555 +Subproject commit 6581dcd1ecf121e030b4ce0221e8f0a119d1f604 diff --git a/test/src/html_output_test.cpp b/test/src/html_output_test.cpp index 8958983ff..bb5c56bad 100644 --- a/test/src/html_output_test.cpp +++ b/test/src/html_output_test.cpp @@ -40,7 +40,6 @@ FileType expected_file_type_post_decryption(const TestFile &test_file) { struct TestParams { TestFile test_file; std::string path; - DecoderEngine engine{DecoderEngine::odr}; PdfTextMode pdf_text_mode{PdfTextMode::dual_layer}; std::string test_repo; std::string output_path; @@ -54,14 +53,12 @@ TEST_P(HtmlOutputTests, html_meta) { const TestParams ¶ms = GetParam(); const TestFile &test_file = params.test_file; - const DecoderEngine engine = params.engine; const std::string &output_path = params.output_path; const std::string &output_path_prefix = params.output_path_prefix; const FileCategory file_category = file_category_by_file_type(test_file.type); - ODR_INFO(logger, "Testing file: " << test_file.short_path << " with engine: " - << odr::decoder_engine_to_string(engine) + ODR_INFO(logger, "Testing file: " << test_file.short_path << " output to: " << output_path); // these files cannot be opened @@ -73,7 +70,6 @@ TEST_P(HtmlOutputTests, html_meta) { DecodePreference decode_preference; decode_preference.as_file_type = test_file.type; - decode_preference.with_engine = engine; DecodedFile file = open(test_file.absolute_path, decode_preference, logger); FileMeta file_meta = file.file_meta(); @@ -112,8 +108,7 @@ TEST_P(HtmlOutputTests, html_meta) { // TODO oldms decryption if (test_file.password.has_value() && - test_file.type == FileType::legacy_word_document && - engine == DecoderEngine::odr) { + test_file.type == FileType::legacy_word_document) { GTEST_SKIP(); } @@ -184,11 +179,6 @@ TEST_P(HtmlOutputTests, html_meta) { namespace { -std::string engine_suffix(const DecoderEngine engine) { - return engine == DecoderEngine::odr ? "" - : "-" + decoder_engine_to_string(engine); -} - /// The default `dual_layer` mode carries no suffix so existing (non-PDF and /// dual-layer) reference outputs keep their paths; single-layer variants are /// disambiguated with `-single`. @@ -197,8 +187,7 @@ std::string text_mode_suffix(const PdfTextMode pdf_text_mode) { } std::string test_params_to_name(const TestParams ¶ms) { - std::string path = params.path + engine_suffix(params.engine) + - text_mode_suffix(params.pdf_text_mode); + std::string path = params.path + text_mode_suffix(params.pdf_text_mode); util::string::replace_all(path, "/", "_"); util::string::replace_all(path, "-", "_"); util::string::replace_all(path, "+", "_"); @@ -209,7 +198,6 @@ std::string test_params_to_name(const TestParams ¶ms) { } TestParams create_test_params(const TestFile &test_file, - const DecoderEngine engine, const PdfTextMode pdf_text_mode) { const std::string test_file_path = test_file.short_path; @@ -219,8 +207,7 @@ TestParams create_test_params(const TestFile &test_file, .join(RelPath(test_repo)) .join(RelPath("output")) .string(); - const std::string output_path_suffix = - engine_suffix(engine) + text_mode_suffix(pdf_text_mode); + const std::string output_path_suffix = text_mode_suffix(pdf_text_mode); const std::string output_path = AbsPath(output_path_prefix) .join(RelPath(test_file_path).rebase(RelPath(test_repo))) @@ -230,7 +217,6 @@ TestParams create_test_params(const TestFile &test_file, return { .test_file = test_file, .path = test_file_path, - .engine = engine, .pdf_text_mode = pdf_text_mode, .test_repo = test_repo, .output_path = output_path, @@ -241,16 +227,15 @@ TestParams create_test_params(const TestFile &test_file, std::vector list_test_params() { std::vector params; for (const TestFile &test_file : TestData::test_files()) { - params.push_back(create_test_params(test_file, DecoderEngine::odr, - PdfTextMode::dual_layer)); + params.push_back(create_test_params(test_file, PdfTextMode::dual_layer)); // PDFs default to `PdfTextMode::dual_layer`. To keep the single-layer path // under reference-output coverage too, eject an extra `-single` test case // for one representative PDF (odr engine only, since the text mode only // affects odr's PDF rendering). if (test_file.short_path == "odr-private/pdf/978-3-030-65771-0.pdf") { - params.push_back(create_test_params(test_file, DecoderEngine::odr, - PdfTextMode::single_layer)); + params.push_back( + create_test_params(test_file, PdfTextMode::single_layer)); } } return params; diff --git a/test/src/internal/oldms/doc_test.cpp b/test/src/internal/oldms/doc_test.cpp index b5cb24e9e..8c8c0c321 100644 --- a/test/src/internal/oldms/doc_test.cpp +++ b/test/src/internal/oldms/doc_test.cpp @@ -161,7 +161,7 @@ TEST(OldMs, doc_apply_character_sprms) { EXPECT_EQ(style.font_line_through, false); EXPECT_EQ(style.font_underline, true); EXPECT_EQ(style.font_size, Measure("16pt")); - EXPECT_STREQ(style.font_name, "Courier New"); + EXPECT_EQ(style.font_name, "Courier New"); ASSERT_TRUE(style.font_color.has_value()); EXPECT_EQ(style.font_color->rgb(), 0x112233u); ASSERT_TRUE(style.background_color.has_value()); @@ -185,7 +185,7 @@ TEST(OldMs, doc_apply_character_sprms) { { // cvAuto ([MS-DOC] 2.9.43) resets an earlier explicit color. TextStyle colored = base; - colored.font_color = Color(std::uint32_t{0x123456}); + colored.font_color = Color::from_rgb(0x123456); const TextStyle style = apply_character_sprms( colored, doc_prl(0x6870, std::string("\x00\x00\x00\xFF", 4)), fonts); EXPECT_FALSE(style.font_color.has_value()); @@ -197,7 +197,7 @@ TEST(OldMs, doc_apply_character_sprms) { const std::vector no_fonts; const TextStyle style = apply_character_sprms( base, doc_prl(0x4A4F, std::string("\x00\x00", 2)), no_fonts); - EXPECT_EQ(style.font_name, nullptr); + EXPECT_FALSE(style.font_name.has_value()); EXPECT_THROW( apply_character_sprms(base, doc_prl(0x4A4F, std::string("\x01\x00", 2)), no_fonts), @@ -318,12 +318,12 @@ TEST(OldMs, doc_character_formatting) { const TextStyle plain = spans[0].as_span().style(); EXPECT_EQ(plain.font_size, Measure("10pt")); EXPECT_FALSE(plain.font_weight.has_value()); - EXPECT_EQ(plain.font_name, nullptr); + EXPECT_FALSE(plain.font_name.has_value()); EXPECT_EQ(collect_text(spans[1]), "bold"); const TextStyle bold = spans[1].as_span().style(); EXPECT_EQ(bold.font_weight, FontWeight::bold); - EXPECT_STREQ(bold.font_name, "Arial"); + EXPECT_EQ(bold.font_name, "Arial"); EXPECT_EQ(bold.font_size, Measure("10pt")); EXPECT_EQ(paragraphs[0].as_paragraph().text_style().font_size, diff --git a/test/src/internal/oldms/ppt_test.cpp b/test/src/internal/oldms/ppt_test.cpp index bf383c459..e044a6ee7 100644 --- a/test/src/internal/oldms/ppt_test.cpp +++ b/test/src/internal/oldms/ppt_test.cpp @@ -156,8 +156,8 @@ TEST(OldMs, ppt_style_various) { // A non-empty pseudo-path names the BLIP so the HTML renderer can emit a // distinct resource per picture when images are not embedded. EXPECT_EQ(image.href(), "Pictures/1.png"); - EXPECT_EQ(background.as_frame().x(), "0in"); - EXPECT_EQ(background.as_frame().y(), "0in"); + EXPECT_EQ(background.as_frame().x(), Measure("0in")); + EXPECT_EQ(background.as_frame().y(), Measure("0in")); // Slide 0 is a title + subtitle box, at different vertical positions. ASSERT_EQ(slides[0].size(), 2); @@ -175,7 +175,7 @@ TEST(OldMs, ppt_style_various) { // The title is 44pt Arial with an explicit black color. const TextStyle title = first_span(slides[0][0]).style(); - EXPECT_STREQ(title.font_name, "Arial"); + EXPECT_EQ(title.font_name, "Arial"); EXPECT_EQ(title.font_size, Measure("44pt")); ASSERT_TRUE(title.font_color.has_value()); EXPECT_EQ(title.font_color->rgb(), 0x000000u); diff --git a/test/src/internal/oldms/xls_test.cpp b/test/src/internal/oldms/xls_test.cpp index 25c481d6e..7ca22248d 100644 --- a/test/src/internal/oldms/xls_test.cpp +++ b/test/src/internal/oldms/xls_test.cpp @@ -250,7 +250,7 @@ TEST(OldMs, xls_cell_styles) { const Sheet sheet = document.root_element().first_child().as_sheet(); const TextStyle plain = first_text(sheet.cell(0, 0)).style(); - EXPECT_STREQ(plain.font_name, "Arial"); + EXPECT_EQ(plain.font_name, "Arial"); EXPECT_EQ(plain.font_size, Measure("10pt")); EXPECT_EQ(plain.font_weight, FontWeight::normal); EXPECT_EQ(plain.font_style, FontStyle::normal); @@ -260,7 +260,7 @@ TEST(OldMs, xls_cell_styles) { EXPECT_FALSE(sheet.cell_style(0, 0).background_color.has_value()); const TextStyle fancy = first_text(sheet.cell(1, 0)).style(); - EXPECT_STREQ(fancy.font_name, "Comic Sans MS"); + EXPECT_EQ(fancy.font_name, "Comic Sans MS"); EXPECT_EQ(fancy.font_size, Measure("16pt")); EXPECT_EQ(fancy.font_weight, FontWeight::bold); EXPECT_EQ(fancy.font_style, FontStyle::italic); diff --git a/test/src/internal/util/number_util_test.cpp b/test/src/internal/util/number_util_test.cpp new file mode 100644 index 000000000..8d3636421 --- /dev/null +++ b/test/src/internal/util/number_util_test.cpp @@ -0,0 +1,46 @@ +#include + +#include + +#include + +using namespace odr::internal::util::number; + +TEST(ToStringSignificant, trims_trailing_zeros) { + EXPECT_EQ(to_string_significant(1.5, 7), "1.5"); + EXPECT_EQ(to_string_significant(2.0, 7), "2"); + EXPECT_EQ(to_string_significant(0.0, 7), "0"); +} + +TEST(ToStringSignificant, keeps_requested_significant_digits) { + EXPECT_EQ(to_string_significant(1.0429, 7), "1.0429"); + EXPECT_EQ(to_string_significant(-6734.61, 7), "-6734.61"); + EXPECT_EQ(to_string_significant(0.007, 7), "0.007"); +} + +TEST(ToStringSignificant, rounds_beyond_the_requested_digits) { + EXPECT_EQ(to_string_significant(1.23456789, 4), "1.235"); + EXPECT_EQ(to_string_significant(191.31, 4), "191.3"); +} + +/// The reason the digit count is bounded: a value that arrived as a `float` +/// carries about 7 digits, and asking for more exposes the binary +/// representation (an xlsx column width of `68.55` becomes `68.550003`). +TEST(ToStringSignificant, hides_float_representation_noise) { + const auto from_float = static_cast(68.55F); + EXPECT_EQ(to_string_significant(from_float, 7), "68.55"); + EXPECT_EQ(to_string_significant(from_float, 10), "68.55000305"); +} + +/// CSS and SVG lengths reject scientific notation, so it must never appear +/// however large or small the value is. +TEST(ToStringSignificant, never_uses_scientific_notation) { + EXPECT_EQ(to_string_significant(13421095.26, 7), "13421095"); + EXPECT_EQ(to_string_significant(1e21, 7), "1000000000000000000000"); + EXPECT_EQ(to_string_significant(0.00001, 7), "0.00001"); +} + +TEST(ToStringSignificant, passes_through_non_finite) { + EXPECT_EQ(to_string_significant(std::nan(""), 7), "nan"); + EXPECT_EQ(to_string_significant(HUGE_VAL, 7), "inf"); +} From 5e17e21ca2ffc6c7138177c979b8b4560b8fd13d Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 27 Jul 2026 23:35:09 +0200 Subject: [PATCH 2/4] docs: reference the PR from the 6.0.0 changelog entries Every other entry cites its PR; the ones added here had no number until the PR existed. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FoqZKnajAkjWg1tdX3EqBq --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d2831cef2..6e9bb39ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ described at the bottom of this file. A breaking release that cleans up the public API. Everything below is a source-incompatible change to `odr::`; see the migration notes per entry. +Entries without a PR reference come from #627. ### Removed From 70ca31630488b1254768d6c9535dc19ae54e6f7a Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 27 Jul 2026 23:54:41 +0200 Subject: [PATCH 3/4] fix!: never leave a DynamicUnit null, and drop the public const char * A default-constructed `DynamicUnit` left `m_unit` null while `name()`, `to_string()` and `to_stream()` all dereferenced it unconditionally. The geometry work in this branch made that reachable: an ODF shape missing one of its `svg:*` attributes fell back to `Measure(0, DynamicUnit())`, and rendering it segfaulted instead of emitting a zero length. Reported by Codex on #627. Fixed at the invariant rather than the call sites: the default constructor now takes the registered empty unit, the same one `Measure("5")` parses out of a bare number. `m_unit` is therefore never null, and a fallback measure compares equal to the parsed equivalent instead of being a look-alike that isn't. Also replaces the remaining `const char *` in the public headers: * `DynamicUnit` and `Quantity` take `std::string_view`. `Quantity` copies into a buffer first, since `std::strtod` needs a terminator a view does not promise, and stops treating a negative `char` as UB in `std::isspace`. * `DocumentPath`'s `const char *` and `const std::string &` constructors collapse into one taking `std::string_view`. * `CfbError` takes `const std::string &`, like its forty siblings. `File::memory_data()` keeps `const char *`: it is a pointer to bytes paired with `size()`, not a string. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FoqZKnajAkjWg1tdX3EqBq --- CHANGELOG.md | 85 -------------------------------------- README.md | 4 +- src/odr/document_path.cpp | 11 ++--- src/odr/document_path.hpp | 4 +- src/odr/exceptions.cpp | 2 +- src/odr/exceptions.hpp | 2 +- src/odr/quantity.cpp | 17 +++++--- src/odr/quantity.hpp | 14 +++++-- test/src/quantity_test.cpp | 22 ++++++++++ 9 files changed, 53 insertions(+), 108 deletions(-) delete mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md deleted file mode 100644 index 6e9bb39ef..000000000 --- a/CHANGELOG.md +++ /dev/null @@ -1,85 +0,0 @@ -# Changelog - -Notable changes to `odrcore`. Releases before 6.0.0 are documented only in the -[GitHub releases](https://github.com/opendocument-app/OpenDocument.core/releases). - -This project follows [semantic versioning](#versioning-and-api-stability) as -described at the bottom of this file. - -## Unreleased — 6.0.0 - -A breaking release that cleans up the public API. Everything below is a -source-incompatible change to `odr::`; see the migration notes per entry. -Entries without a PR reference come from #627. - -### Removed - -- **Deprecated API and the pdf2htmlEX / wvWare backends** (#622). -- **`DecoderEngine` and the whole decoder-selection dimension.** Since the - external backends were dropped in #622, the enum had exactly one value and - could not select anything. Gone: `DecoderEngine`, - `DecodePreference::with_engine`, `DecodePreference::engine_priority`, - `list_decoder_engines`, `decoder_engine_by_name`, `decoder_engine_to_string`, - `DecodedFile::decoder_engine`, `UnknownDecoderEngine` and - `UnsupportedDecoderEngine`. Also removed from the Python and Java bindings - (`DecoderEngine.java` is deleted). - *Migration:* drop the arguments; there is only one decoder. - -### Changed - -- **`FileMeta` collapsed into a single type** (#626). -- **`Logger` is a value type with a public `ILogger` sink** (#625). -- **`HttpServer::listen` split into `bind` and `listen`** (#624). -- **Shape geometry is typed.** `Frame`, `Rect`, `Line`, `Circle` and - `CustomShape` return `Measure` instead of `std::string` for - `x`/`y`/`width`/`height`/`x1`/`y1`/`x2`/`y2`, and `Frame::z_index` returns - `std::optional` instead of a string. - *Migration:* call `.to_string()` where you previously got a string. -- **`Measure::to_string` keeps 10 significant digits** instead of 4. The old - precision silently rounded document geometry (a drawing coordinate of - `-6734.61mm` was emitted as `-6735mm`). Emitted HTML now carries the more - accurate value; reference outputs change numerically but not structurally. -- **`TextStyle::font_name` is `std::optional`** instead of - `const char *`, matching its sibling fields. - *Migration:* `style.font_name != nullptr` becomes - `style.font_name.has_value()`. -- **Every exception derives from `odr::Exception`**, so `catch (const - odr::Exception &)` catches all typed library errors. Note the decoders still - throw plain `std::runtime_error` for malformed input without a dedicated - type. The Python bindings mirror this with a base `pyodr.Error`; the Java - bindings already had `OdrException`. -- **`Color` uses named factories.** `Color(std::uint32_t rgb)` and - `Color(std::uint32_t argb, bool dummy)` are replaced by `Color::from_rgb` and - `Color::from_argb`. -- **`html::edit` takes `std::string_view`** instead of `const char *`. -- **`Html::config()` is `const`**, matching `HtmlService::config()`. -- **`Measure` moved from `` to ``.** Both - headers still provide it; `style.hpp` includes `quantity.hpp`. - -### Fixed - -- **`ElementIterator::operator++(int)` advances the iterator.** It was `const`, - so it could not advance, and it returned the *successor* rather than the - pre-increment value — meaning `it++` silently violated the forward-iterator - contract the class advertises. -- **`Element` and `ElementIterator` equality compare the document.** They - previously compared only the element identifier, so element *n* of one - document compared equal to element *n* of an unrelated one. -- `FileType::word_perfect` and `FileType::rich_text_format` are documented as - detection-only: magic recognises them so a caller can report the type, but - there is no decoder and opening one throws. - -## Versioning and API stability - -`odrcore` uses semantic versioning. - -- The **public API** is everything under `src/odr/*.hpp` — the headers directly - in the `odr/` directory. Within a major version these stay - source-compatible; additions are minor releases. -- **`odr::internal`** (`src/odr/internal/**`) is *not* public. These headers are - installed because parts of the public API hand back `internal::abstract` - types, but anything in that namespace may change in any release. -- **ABI** is not guaranteed between releases, including patch releases. Link - against a matching build. -- The **bindings** (`pyodr`, `app.opendocument.core`) track the C++ API and - break with it. diff --git a/README.md b/README.md index 8a8882ae0..6c7669e71 100644 --- a/README.md +++ b/README.md @@ -55,9 +55,7 @@ As an alternative to the Conan remote you can also export the package locally vi ## Version -Notable changes are in [`CHANGELOG.md`](CHANGELOG.md), which also states what -counts as public API and what stability to expect. Release history is on -[GitHub](https://github.com/opendocument-app/OpenDocument.core/releases). +Versions and history are tracked on [GitHub](https://github.com/opendocument-app/OpenDocument.core). ## Testing diff --git a/src/odr/document_path.cpp b/src/odr/document_path.cpp index f61d98d5c..4b847bcb4 100644 --- a/src/odr/document_path.cpp +++ b/src/odr/document_path.cpp @@ -68,10 +68,7 @@ DocumentPath::DocumentPath(const Container &components) DocumentPath::DocumentPath(Container &&components) : m_components{std::move(components)} {} -DocumentPath::DocumentPath(const char *c_string) - : DocumentPath(std::string(c_string)) {} - -DocumentPath::DocumentPath(const std::string &string) { +DocumentPath::DocumentPath(const std::string_view string) { if (string.empty()) { return; } @@ -82,11 +79,11 @@ DocumentPath::DocumentPath(const std::string &string) { throw std::invalid_argument("missing /"); } std::size_t next = string.find('/', pos + 1); - if (next == std::string::npos) { + if (next == std::string_view::npos) { next = string.size(); } - m_components.push_back( - component_from_string(string.substr(pos + 1, next - pos - 1))); + m_components.push_back(component_from_string( + std::string(string.substr(pos + 1, next - pos - 1)))); pos = next; } } diff --git a/src/odr/document_path.hpp b/src/odr/document_path.hpp index 025d92d43..1707d0ecf 100644 --- a/src/odr/document_path.hpp +++ b/src/odr/document_path.hpp @@ -4,6 +4,7 @@ #include #include +#include #include #include @@ -55,8 +56,7 @@ class DocumentPath final { DocumentPath() noexcept; explicit DocumentPath(const Container &components); explicit DocumentPath(Container &&components); - explicit DocumentPath(const char *c_string); - explicit DocumentPath(const std::string &string); + explicit DocumentPath(std::string_view string); bool operator==(const DocumentPath &other) const noexcept; bool operator!=(const DocumentPath &other) const noexcept; diff --git a/src/odr/exceptions.cpp b/src/odr/exceptions.cpp index 30165a769..f4d1a8fe9 100644 --- a/src/odr/exceptions.cpp +++ b/src/odr/exceptions.cpp @@ -31,7 +31,7 @@ NoZipFile::NoZipFile() : Exception("not a zip file") {} ZipSaveError::ZipSaveError() : Exception("zip save error") {} -CfbError::CfbError(const char *desc) : Exception(desc) {} +CfbError::CfbError(const std::string &desc) : Exception(desc) {} NoCfbFile::NoCfbFile() : CfbError("no cfb file") {} diff --git a/src/odr/exceptions.hpp b/src/odr/exceptions.hpp index 3377688f3..a5408c706 100644 --- a/src/odr/exceptions.hpp +++ b/src/odr/exceptions.hpp @@ -61,7 +61,7 @@ struct ZipSaveError : Exception { /// @brief CFB error base; NoCfbFile and CfbFileCorrupted refine it. struct CfbError : Exception { - explicit CfbError(const char *desc); + explicit CfbError(const std::string &desc); }; /// @brief No CFB file exception base diff --git a/src/odr/quantity.cpp b/src/odr/quantity.cpp index c146e9d4c..ed5390d67 100644 --- a/src/odr/quantity.cpp +++ b/src/odr/quantity.cpp @@ -22,7 +22,9 @@ struct DynamicUnit::Unit final { class DynamicUnit::Registry final { public: - static const Unit *unit(const char *name) { return registry_().unit_(name); } + static const Unit *unit(const std::string_view name) { + return registry_().unit_(name); + } private: static Registry ®istry_() { @@ -34,8 +36,8 @@ class DynamicUnit::Registry final { std::unordered_map> m_registry; - const Unit *unit_(const char *name) { - std::unique_ptr &unit = m_registry[name]; + const Unit *unit_(const std::string_view name) { + std::unique_ptr &unit = m_registry[std::string(name)]; if (unit == nullptr) { unit = std::make_unique(); unit->name = name; @@ -44,9 +46,14 @@ class DynamicUnit::Registry final { } }; -DynamicUnit::DynamicUnit() = default; +/// The unitless unit, i.e. the same one `Measure("5")` parses out of a bare +/// number. Registered rather than left null so that `m_unit` is never null and +/// every accessor below can dereference it, and so that a default-constructed +/// unit compares equal to a parsed one. +DynamicUnit::DynamicUnit() : m_unit{Registry::unit("")} {} -DynamicUnit::DynamicUnit(const char *name) : m_unit{Registry::unit(name)} {} +DynamicUnit::DynamicUnit(const std::string_view name) + : m_unit{Registry::unit(name)} {} bool DynamicUnit::operator==(const DynamicUnit &rhs) const { return m_unit == rhs.m_unit; diff --git a/src/odr/quantity.hpp b/src/odr/quantity.hpp index f584b1ee7..b41e1487e 100644 --- a/src/odr/quantity.hpp +++ b/src/odr/quantity.hpp @@ -1,7 +1,10 @@ #pragma once +#include +#include #include #include +#include #include namespace odr { @@ -9,8 +12,9 @@ namespace odr { /// @brief Represents a runtime unit of measure. class DynamicUnit { public: + /// Constructs the unitless unit, equal to `DynamicUnit("")`. DynamicUnit(); - explicit DynamicUnit(const char *name); + explicit DynamicUnit(std::string_view name); bool operator==(const DynamicUnit &rhs) const; bool operator!=(const DynamicUnit &rhs) const; @@ -42,10 +46,12 @@ class Quantity : private QuantityBase { Quantity(Magnitude magnitude, Unit unit) : m_magnitude{std::move(magnitude)}, m_unit{std::move(unit)} {} - explicit Quantity(const char *string) { + explicit Quantity(const std::string_view string) { + // `std::strtod` needs a terminator, which a view does not promise + const std::string buffer(string); char *end{nullptr}; - m_magnitude = std::strtod(string, &end); - while (*end && std::isspace(*end)) { + m_magnitude = std::strtod(buffer.c_str(), &end); + while (*end != '\0' && std::isspace(static_cast(*end))) { ++end; } m_unit = DynamicUnit(end); diff --git a/test/src/quantity_test.cpp b/test/src/quantity_test.cpp index 369eed7a0..7d81306c3 100644 --- a/test/src/quantity_test.cpp +++ b/test/src/quantity_test.cpp @@ -8,3 +8,25 @@ TEST(Quantity, construct) { Quantity{"10ms"}; Quantity{"10 ms"}; } + +/// A default-constructed unit used to leave the unit pointer null, so anything +/// that rendered such a quantity — the geometry fallback for a shape missing +/// its `svg:*` attribute, for one — dereferenced null instead of emitting a +/// bare number. +TEST(DynamicUnit, default_constructed_is_the_unitless_unit) { + EXPECT_EQ(DynamicUnit(), DynamicUnit("")); + EXPECT_EQ(DynamicUnit().name(), ""); + EXPECT_EQ(DynamicUnit().to_string(), ""); +} + +TEST(Quantity, default_unit_renders_a_bare_number) { + EXPECT_EQ(Measure(0, DynamicUnit()).to_string(), "0"); + EXPECT_EQ(Measure(1.5, {}).to_string(), "1.5"); +} + +/// A fallback measure has to compare equal to the same value parsed from text, +/// which only holds if both end up on the registered empty unit. +TEST(Quantity, default_unit_equals_parsed_unitless) { + EXPECT_EQ(Measure(0, DynamicUnit()), Measure("0")); + EXPECT_EQ(Measure(2.5, {}), Measure("2.5")); +} From 8ddc02ae97d3fcc316acbb610fe81a26762adf2e Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 27 Jul 2026 23:58:30 +0200 Subject: [PATCH 4/4] fix!: return the file's bytes as a view, and report MemoryFile as memory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `File::memory_data()` returned a bare `const char *` that only meant anything alongside `size()`, and used null to signal "not in memory". It now returns `std::optional`, mirroring the `disk_path()` next to it: the path if the file is on disk, the bytes if it is in memory. The view carries its own length, so the pointer can no longer be read without one. Also fixes `MemoryFile::location()`, which reported `FileLocation::disk`. Nothing branched on it internally — it is only passed through to the bindings — so this corrects what the API reports rather than changing any behaviour. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FoqZKnajAkjWg1tdX3EqBq --- src/odr/file.cpp | 4 +++- src/odr/file.hpp | 2 +- src/odr/internal/abstract/file.hpp | 4 +++- src/odr/internal/cfb/cfb_util.cpp | 4 +++- src/odr/internal/common/file.cpp | 10 +++++++--- src/odr/internal/common/file.hpp | 4 ++-- src/odr/internal/zip/zip_util.cpp | 4 +++- test/src/file_test.cpp | 27 +++++++++++++++++++++++++++ 8 files changed, 49 insertions(+), 10 deletions(-) diff --git a/src/odr/file.cpp b/src/odr/file.cpp index 9baea9b6b..57e467a2e 100644 --- a/src/odr/file.cpp +++ b/src/odr/file.cpp @@ -34,7 +34,9 @@ std::optional File::disk_path() const { return {}; } -const char *File::memory_data() const { return m_impl->memory_data(); } +std::optional File::memory_data() const { + return m_impl->memory_data(); +} std::unique_ptr File::stream() const { return m_impl->stream(); } diff --git a/src/odr/file.hpp b/src/odr/file.hpp index c8f703241..98465f62c 100644 --- a/src/odr/file.hpp +++ b/src/odr/file.hpp @@ -163,7 +163,7 @@ class File final { [[nodiscard]] std::size_t size() const; [[nodiscard]] std::optional disk_path() const; - [[nodiscard]] const char *memory_data() const; + [[nodiscard]] std::optional memory_data() const; [[nodiscard]] std::unique_ptr stream() const; void pipe(std::ostream &out) const; diff --git a/src/odr/internal/abstract/file.hpp b/src/odr/internal/abstract/file.hpp index ca199949c..94a9ea519 100644 --- a/src/odr/internal/abstract/file.hpp +++ b/src/odr/internal/abstract/file.hpp @@ -6,6 +6,7 @@ #include #include #include +#include namespace odr::internal { class AbsPath; @@ -25,7 +26,8 @@ class File { [[nodiscard]] virtual std::size_t size() const = 0; [[nodiscard]] virtual std::optional disk_path() const = 0; - [[nodiscard]] virtual const char *memory_data() const = 0; + /// The file's bytes if it is held in memory, else nullopt. + [[nodiscard]] virtual std::optional memory_data() const = 0; [[nodiscard]] virtual std::unique_ptr stream() const = 0; }; diff --git a/src/odr/internal/cfb/cfb_util.cpp b/src/odr/internal/cfb/cfb_util.cpp index 3330363ed..f1ad9643b 100644 --- a/src/odr/internal/cfb/cfb_util.cpp +++ b/src/odr/internal/cfb/cfb_util.cpp @@ -116,7 +116,9 @@ class FileInCfb final : public abstract::File { [[nodiscard]] std::optional disk_path() const override { return std::nullopt; } - [[nodiscard]] const char *memory_data() const override { return nullptr; } + [[nodiscard]] std::optional memory_data() const override { + return std::nullopt; + } [[nodiscard]] std::unique_ptr stream() const override { return std::make_unique(m_archive, m_archive->cfb(), diff --git a/src/odr/internal/common/file.cpp b/src/odr/internal/common/file.cpp index b5da6fb9a..8168e05dd 100644 --- a/src/odr/internal/common/file.cpp +++ b/src/odr/internal/common/file.cpp @@ -28,7 +28,9 @@ std::size_t DiskFile::size() const { std::optional DiskFile::disk_path() const { return m_path; } -const char *DiskFile::memory_data() const { return nullptr; } +std::optional DiskFile::memory_data() const { + return std::nullopt; +} std::unique_ptr DiskFile::stream() const { return std::make_unique(util::file::open(m_path.string())); @@ -46,14 +48,16 @@ MemoryFile::MemoryFile(const File &file) : m_data(file.size(), ' ') { } FileLocation MemoryFile::location() const noexcept { - return FileLocation::disk; + return FileLocation::memory; } std::size_t MemoryFile::size() const { return m_data.size(); } std::optional MemoryFile::disk_path() const { return std::nullopt; } -const char *MemoryFile::memory_data() const { return m_data.data(); } +std::optional MemoryFile::memory_data() const { + return m_data; +} std::unique_ptr MemoryFile::stream() const { return std::make_unique(m_data); diff --git a/src/odr/internal/common/file.hpp b/src/odr/internal/common/file.hpp index 05fb1cabd..86f5d493e 100644 --- a/src/odr/internal/common/file.hpp +++ b/src/odr/internal/common/file.hpp @@ -23,7 +23,7 @@ class DiskFile : public abstract::File { [[nodiscard]] std::size_t size() const final; [[nodiscard]] std::optional disk_path() const final; - [[nodiscard]] const char *memory_data() const final; + [[nodiscard]] std::optional memory_data() const final; [[nodiscard]] std::unique_ptr stream() const final; @@ -40,7 +40,7 @@ class MemoryFile final : public abstract::File { [[nodiscard]] std::size_t size() const override; [[nodiscard]] std::optional disk_path() const override; - [[nodiscard]] const char *memory_data() const override; + [[nodiscard]] std::optional memory_data() const override; [[nodiscard]] std::unique_ptr stream() const override; diff --git a/src/odr/internal/zip/zip_util.cpp b/src/odr/internal/zip/zip_util.cpp index 1d171e853..18b475631 100644 --- a/src/odr/internal/zip/zip_util.cpp +++ b/src/odr/internal/zip/zip_util.cpp @@ -86,7 +86,9 @@ class FileInZip final : public abstract::File { [[nodiscard]] std::optional disk_path() const override { return std::nullopt; } - [[nodiscard]] const char *memory_data() const override { return nullptr; } + [[nodiscard]] std::optional memory_data() const override { + return std::nullopt; + } [[nodiscard]] std::unique_ptr stream() const override { std::lock_guard lock(m_archive->mutex()); diff --git a/test/src/file_test.cpp b/test/src/file_test.cpp index 7c7f7d199..25cb9972f 100644 --- a/test/src/file_test.cpp +++ b/test/src/file_test.cpp @@ -1,7 +1,13 @@ #include +#include + +#include #include +#include +#include + #include using namespace odr; @@ -9,6 +15,27 @@ using namespace odr::test; TEST(File, open) { EXPECT_THROW(File("/"), FileNotFound); } +/// `MemoryFile` used to report itself as `disk`, and `memory_data()` handed +/// back a bare pointer that only meant something alongside `size()`. +TEST(File, memory_file_reports_memory_and_its_bytes) { + const File file(std::make_shared(std::string("hello"))); + + EXPECT_EQ(file.location(), FileLocation::memory); + EXPECT_EQ(file.size(), 5); + EXPECT_FALSE(file.disk_path().has_value()); + ASSERT_TRUE(file.memory_data().has_value()); + EXPECT_EQ(*file.memory_data(), "hello"); +} + +TEST(File, disk_file_has_no_memory_data) { + const File file(std::make_shared( + TestData::test_file_path("odr-public/odt/about.odt"))); + + EXPECT_EQ(file.location(), FileLocation::disk); + EXPECT_TRUE(file.disk_path().has_value()); + EXPECT_FALSE(file.memory_data().has_value()); +} + TEST(DocumentFile, open) { EXPECT_THROW(DocumentFile("/"), FileNotFound); } TEST(DecodedFile, wpd) {