From 530ba249b9d6309df6610035c9afb3c119964065 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 30 Aug 2026 21:09:02 +0200 Subject: [PATCH] refactor(xml, png): give the xml and png helpers their own internal packages `util/xml_util` moves into the existing `xml` package, next to `XmlFile`, and `util/png_util` becomes a package of its own. The namespaces follow: `util::xml` -> `xml`, `util::png` -> `png`. `util` is for helpers that belong to no engine. Neither of these does: every xml-reading and xml-writing engine goes through the parse and the escaping, and the png writer is the encoder pdf image extraction and svm bitmaps hand their pixels to. The xml half also lands where `xml::` already resolved for `open_strategy`, `svg_file` and `odf_flat_file`. Contents are unchanged apart from the namespace. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01MjtSRoykPTFpvHzaJmVLJk --- AGENTS.md | 7 +++--- CMakeLists.txt | 5 +++-- docs/design/editing.md | 2 +- src/odr/internal/html/common.cpp | 6 ++--- src/odr/internal/html/common.hpp | 4 ++-- src/odr/internal/html/document_element.cpp | 9 ++++---- src/odr/internal/html/document_style.cpp | 16 +++++++------- src/odr/internal/html/filesystem.cpp | 22 +++++++++---------- src/odr/internal/html/frontend.cpp | 7 +++--- src/odr/internal/html/media_file.cpp | 4 ++-- src/odr/internal/html/pdf_file.cpp | 6 ++--- src/odr/internal/odf/odf_document.cpp | 19 ++++++++-------- src/odr/internal/odf/odf_file.cpp | 4 ++-- src/odr/internal/odf/odf_flat_file.cpp | 4 ++-- src/odr/internal/odf/odf_meta.cpp | 7 +++--- src/odr/internal/ooxml/ooxml_util.cpp | 8 +++---- .../ooxml_presentation_document.cpp | 16 +++++++------- .../presentation/ooxml_presentation_style.cpp | 4 ++-- .../ooxml_spreadsheet_document.cpp | 4 ++-- .../ooxml/text/ooxml_text_document.cpp | 19 ++++++++-------- src/odr/internal/pdf/pdf_image.cpp | 6 ++--- src/odr/internal/{util => png}/png_util.cpp | 8 +++---- src/odr/internal/{util => png}/png_util.hpp | 4 ++-- src/odr/internal/svg/AGENTS.md | 2 +- src/odr/internal/svg/svg_writer.cpp | 10 ++++----- src/odr/internal/svm/svm_format.cpp | 5 ++--- src/odr/internal/xml/AGENTS.md | 13 +++++++++-- src/odr/internal/xml/xml_file.cpp | 4 ++-- src/odr/internal/{util => xml}/xml_util.cpp | 6 ++--- src/odr/internal/{util => xml}/xml_util.hpp | 4 ++-- test/CMakeLists.txt | 5 +++-- test/src/internal/pdf/pdf_image.cpp | 4 ++-- .../internal/{util => png}/png_util_test.cpp | 16 +++++++------- test/src/internal/xml/xml_file_test.cpp | 4 ++-- .../internal/{util => xml}/xml_util_test.cpp | 4 ++-- 35 files changed, 135 insertions(+), 133 deletions(-) rename src/odr/internal/{util => png}/png_util.cpp (88%) rename src/odr/internal/{util => png}/png_util.hpp (83%) rename src/odr/internal/{util => xml}/xml_util.cpp (98%) rename src/odr/internal/{util => xml}/xml_util.hpp (95%) rename test/src/internal/{util => png}/png_util_test.cpp (83%) rename test/src/internal/{util => xml}/xml_util_test.cpp (96%) diff --git a/AGENTS.md b/AGENTS.md index 3bf16506a..c8fc4033b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -57,7 +57,7 @@ bytes ─▶ magic/open_strategy ─▶ DecodedFile ─▶ Document ─▶ Eleme | `src/odr/*.hpp` | **Public API**: `file`, `document`, `document_element`, `html`, `style`, `quantity` (`Measure`), `odr`. | | `src/odr/internal/abstract/` | Core interfaces: `File`/`DecodedFile`, `Document` + `ElementAdapter`, `Filesystem`, `Archive`, `HtmlService`. | | `src/odr/internal/common/` | Reusable impls: `Path`/`AbsPath`, base `Document`, filesystem, `style`, table cursor/range, temp files. | -| `src/odr/internal/util/` | Helpers: `byte_stream_util`, `string_util`, `stream_util`, `document_util`, `xml_util`. | +| `src/odr/internal/util/` | Helpers: `byte_stream_util`, `string_util`, `stream_util`, `document_util`. | | `src/odr/internal/magic.*`, `open_strategy.*` | File-type detection + open/dispatch. | | `src/odr/internal/file_type_table.*` | **The** per-`FileType` table: extensions, MIME types, category, document type, `FileTypeCapabilities`. Every public lookup in `odr.hpp` is a thin forward into it — extend the table, not the lookups. | | `src/odr/internal/html/` | Generic HTML renderer. | @@ -67,9 +67,10 @@ bytes ─▶ magic/open_strategy ─▶ DecodedFile ─▶ Document ─▶ Eleme | `src/odr/internal/oldms/` | **Legacy MS binary** (.doc/.ppt/.xls). | | `src/odr/internal/iwork/` | Apple iWork (`.pages`, `.key`, `.numbers`); see [`iwork/AGENTS.md`](src/odr/internal/iwork/AGENTS.md) + [`iwork/PLAN.md`](src/odr/internal/iwork/PLAN.md). | | `src/odr/internal/pdf/` | PDF (own parser). | +| `src/odr/internal/png/` | PNG encoder: the writer `pdf` image extraction and `svm` bitmaps hand their pixels to. | | `src/odr/internal/rtf/` | RTF, read as a text document; see [`rtf/AGENTS.md`](src/odr/internal/rtf/AGENTS.md) + [`rtf/PLAN.md`](src/odr/internal/rtf/PLAN.md). | | `src/odr/internal/markdown/` | Markdown (CommonMark + GFM via md4c), decoded to a text document; see [`markdown/AGENTS.md`](src/odr/internal/markdown/AGENTS.md) + [`markdown/PLAN.md`](src/odr/internal/markdown/PLAN.md). | -| `src/odr/internal/xml/` | XML, rendered as a source view; see [`xml/AGENTS.md`](src/odr/internal/xml/AGENTS.md). | +| `src/odr/internal/xml/` | XML: the pugixml parse and the escaping every xml-writing engine shares (`xml_util`), plus the source view (`xml_file`); see [`xml/AGENTS.md`](src/odr/internal/xml/AGENTS.md). | | `src/odr/internal/svg/` | SVG, detected by reading it as xml; see [`svg/AGENTS.md`](src/odr/internal/svg/AGENTS.md). | | `src/odr/internal/svm/` | StarView metafile, the vector image odf/ooxml packages carry for charts and OLE objects; translated to svg. See [`svm/AGENTS.md`](src/odr/internal/svm/AGENTS.md) + [`svm/PLAN.md`](src/odr/internal/svm/PLAN.md). | | `src/odr/internal/{csv,json,text}/` | Smaller formats. | @@ -120,7 +121,7 @@ cmake --build cmake-build-relwithdebinfo --target translate # CLI: file → HTM link error — hence the imported target, and no prebuilt library to mismatch against. A new target that includes `pugixml.hpp` has to get it too, and the installed internal headers expose pugixml types, so `odr` carries the define - INTERFACE and `package_info` declares it. `util/xml_util.cpp` asserts the + INTERFACE and `package_info` declares it. `xml/xml_util.cpp` asserts the layout it compiled against. ## Releasing diff --git a/CMakeLists.txt b/CMakeLists.txt index 906e17f59..d635ded0b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -252,6 +252,8 @@ set(ODR_SOURCE_FILES "src/odr/internal/pdf/pdf_page_extractor.cpp" "src/odr/internal/pdf/pdf_shading.cpp" + "src/odr/internal/png/png_util.cpp" + "src/odr/internal/font/cff_builder.cpp" "src/odr/internal/font/cff_font.cpp" "src/odr/internal/font/cff_standard_strings.cpp" @@ -289,11 +291,10 @@ set(ODR_SOURCE_FILES "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/png_util.cpp" "src/odr/internal/util/string_util.cpp" - "src/odr/internal/util/xml_util.cpp" "src/odr/internal/xml/xml_file.cpp" + "src/odr/internal/xml/xml_util.cpp" "src/odr/internal/zip/zip_archive.cpp" "src/odr/internal/zip/zip_exceptions.cpp" diff --git a/docs/design/editing.md b/docs/design/editing.md index 67596cf8f..6c8ea672e 100644 --- a/docs/design/editing.md +++ b/docs/design/editing.md @@ -273,7 +273,7 @@ all against an in-memory copy, writing only on full success (decision 7). | `toggleMark(range, bold)` | split run; assign an **automatic style** (`text:style-name`) — find or create a `` with `fo:font-weight="bold"` | split `w:r`; set `` on the middle run | | `splitParagraph` | clone `` (copy style-name), move trailing nodes into the clone | clone `` incl. `w:pPr`, move trailing `w:r` | | `insertParagraph` | `insert_child_after` a fresh `` at anchor; **append** registry entry | fresh ``; append | -| `insertText(range)` | reuse the `text_set_content` tokenizer (`util::xml::tokenize_text`) on a sub-range | same, `w:t` tokenizer | +| `insertText(range)` | reuse the `text_set_content` tokenizer (`xml::tokenize_text`) on a sub-range | same, `w:t` tokenizer | Two genuinely format-specific complications, both already visible in the code: diff --git a/src/odr/internal/html/common.cpp b/src/odr/internal/html/common.cpp index 60233cde0..10b5a1c7d 100644 --- a/src/odr/internal/html/common.cpp +++ b/src/odr/internal/html/common.cpp @@ -5,7 +5,7 @@ #include #include #include -#include +#include #include #include @@ -32,7 +32,7 @@ void html::write_viewport_meta( const std::optional mode_override) { if (config.viewport_content.has_value()) { out.write_header_viewport( - util::xml::escape_attribute(config.viewport_content.value())); + xml::escape_attribute(config.viewport_content.value())); return; } @@ -257,7 +257,7 @@ std::string html::escape_text(std::string text) { return text; } - text = util::xml::escape_text(text); + text = xml::escape_text(text); if (text.front() == ' ') { text = " " + text.substr(1); diff --git a/src/odr/internal/html/common.hpp b/src/odr/internal/html/common.hpp index f55ffdfa7..1496840f7 100644 --- a/src/odr/internal/html/common.hpp +++ b/src/odr/internal/html/common.hpp @@ -82,9 +82,9 @@ void write_zoom_style(HtmlWriter &out, const HtmlConfig &config, WidthFit fits, /// length, which leaves those insets as shipped. void write_content_margin_style(HtmlWriter &out, const HtmlConfig &config); -/// @ref util::xml::escape_text, plus the ` ` and ` ` that keep html +/// @ref xml::escape_text, plus the ` ` and ` ` that keep html /// from collapsing the run's own whitespace. An attribute value wants -/// @ref util::xml::escape_attribute instead, which leaves spaces intact. +/// @ref xml::escape_attribute instead, which leaves spaces intact. std::string escape_text(std::string text); /// What a target is, as an `href` would be dispatched. Whitespace and control diff --git a/src/odr/internal/html/document_element.cpp b/src/odr/internal/html/document_element.cpp index 1e825b712..bb2ece79f 100644 --- a/src/odr/internal/html/document_element.cpp +++ b/src/odr/internal/html/document_element.cpp @@ -12,7 +12,7 @@ #include #include #include -#include +#include namespace odr::internal { @@ -414,7 +414,7 @@ void html::translate_link(const Element &element, const WritingState &state) { // A refused target loses the attribute, not the element. HtmlAttributesVector attributes; if (kind != UriKind::refused) { - attributes.emplace_back("href", util::xml::escape_attribute(href)); + attributes.emplace_back("href", xml::escape_attribute(href)); } HtmlElementOptions options = @@ -437,7 +437,7 @@ void html::translate_bookmark(const Element &element, state.out().write_element_begin( "a", HtmlElementOptions().set_inline(true).set_attributes(HtmlAttributesVector{ - {"id", util::xml::escape_attribute(bookmark.name())}})); + {"id", xml::escape_attribute(bookmark.name())}})); state.out().write_element_end("a"); } @@ -566,8 +566,7 @@ void html::translate_image(const Element &element, const WritingState &state) { .set_attributes([&](const HtmlAttributeWriterCallback &clb) { clb("alt", "Error: image not found or unsupported"); if (resource_location.has_value()) { - clb("src", - util::xml::escape_attribute(resource_location.value())); + clb("src", xml::escape_attribute(resource_location.value())); } else { clb("src", [&](std::ostream &o) { // reached only for internal images, which have a file diff --git a/src/odr/internal/html/document_style.cpp b/src/odr/internal/html/document_style.cpp index 243b307d1..3da69f987 100644 --- a/src/odr/internal/html/document_style.cpp +++ b/src/odr/internal/html/document_style.cpp @@ -4,7 +4,7 @@ #include #include -#include +#include namespace odr::internal { @@ -151,7 +151,7 @@ std::string html::translate_text_style(const TextStyle &text_style) { if (const std::optional font_name = text_style.font_name; font_name.has_value()) { result.append("font-family:") - .append(util::xml::escape_attribute(std::string(*font_name))) + .append(xml::escape_attribute(std::string(*font_name))) .append(";"); } if (const std::optional font_size = text_style.font_size; @@ -179,7 +179,7 @@ std::string html::translate_text_style(const TextStyle &text_style) { if (const std::optional font_shadow = text_style.font_shadow; font_shadow.has_value()) { result.append("text-shadow:") - .append(util::xml::escape_attribute(*font_shadow)) + .append(xml::escape_attribute(*font_shadow)) .append(";"); } if (const std::optional font_color = text_style.font_color; @@ -207,7 +207,7 @@ std::string html::translate_block_font_style(const TextStyle &text_style) { if (const std::optional font_name = text_style.font_name; font_name.has_value()) { result.append("font-family:") - .append(util::xml::escape_attribute(std::string(*font_name))) + .append(xml::escape_attribute(std::string(*font_name))) .append(";"); } if (const std::optional font_size = text_style.font_size; @@ -357,27 +357,27 @@ html::translate_table_cell_style(const TableCellStyle &table_cell_style) { table_cell_style.border.right; border_right.has_value()) { result.append("border-right:") - .append(util::xml::escape_attribute(*border_right)) + .append(xml::escape_attribute(*border_right)) .append(";"); } if (const std::optional border_top = table_cell_style.border.top; border_top.has_value()) { result.append("border-top:") - .append(util::xml::escape_attribute(*border_top)) + .append(xml::escape_attribute(*border_top)) .append(";"); } if (const std::optional border_left = table_cell_style.border.left; border_left.has_value()) { result.append("border-left:") - .append(util::xml::escape_attribute(*border_left)) + .append(xml::escape_attribute(*border_left)) .append(";"); } if (const std::optional border_bottom = table_cell_style.border.bottom; border_bottom.has_value()) { result.append("border-bottom:") - .append(util::xml::escape_attribute(*border_bottom)) + .append(xml::escape_attribute(*border_bottom)) .append(";"); } if (const std::optional text_rotation = diff --git a/src/odr/internal/html/filesystem.cpp b/src/odr/internal/html/filesystem.cpp index be8bcbf03..834abd1e9 100644 --- a/src/odr/internal/html/filesystem.cpp +++ b/src/odr/internal/html/filesystem.cpp @@ -12,7 +12,7 @@ #include #include #include -#include +#include #include #include @@ -226,10 +226,10 @@ class HtmlServiceImpl final : public HtmlService { HtmlElementOptions().set_inline(true).set_class("odr-files-name")); if (location.has_value()) { out.write_element_begin( - "a", HtmlElementOptions().set_inline(true).set_attributes( - HtmlAttributesVector{ - {"href", util::xml::escape_attribute(*location)}, - {"title", util::xml::escape_attribute(name)}})); + "a", + HtmlElementOptions().set_inline(true).set_attributes( + HtmlAttributesVector{{"href", xml::escape_attribute(*location)}, + {"title", xml::escape_attribute(name)}})); out.write_raw(escape_text(file_path.string())); out.write_element_end("a"); } else { @@ -250,15 +250,15 @@ class HtmlServiceImpl final : public HtmlService { HtmlElementOptions().set_inline(true).set_class("odr-files-action")); if (const std::optional href = location.has_value() - ? std::optional(util::xml::escape_attribute(*location)) + ? std::optional(xml::escape_attribute(*location)) : entry_data_url(file, mime_type_of(file_path)); href.has_value()) { out.write_element_begin( - "a", HtmlElementOptions().set_inline(true).set_attributes( - HtmlAttributesVector{ - {"href", *href}, - {"download", util::xml::escape_attribute(name)}, - {"title", util::xml::escape_attribute(name)}})); + "a", + HtmlElementOptions().set_inline(true).set_attributes( + HtmlAttributesVector{{"href", *href}, + {"download", xml::escape_attribute(name)}, + {"title", xml::escape_attribute(name)}})); out.write_raw("\u2193"); out.write_element_end("a"); } diff --git a/src/odr/internal/html/frontend.cpp b/src/odr/internal/html/frontend.cpp index ee85b59fd..b9a5f2d2a 100644 --- a/src/odr/internal/html/frontend.cpp +++ b/src/odr/internal/html/frontend.cpp @@ -6,7 +6,7 @@ #include #include #include -#include +#include #include #include @@ -1561,8 +1561,7 @@ void write_style(const Asset &asset, const WritingState &state, if (const HtmlResourceLocation location = locate(asset, state.config(), state.resources()); location.has_value()) { - state.out().write_header_style(util::xml::escape_attribute(*location), - media); + state.out().write_header_style(xml::escape_attribute(*location), media); return; } @@ -1581,7 +1580,7 @@ void write_script(const Asset &asset, const WritingState &state) { if (const HtmlResourceLocation location = locate(asset, state.config(), state.resources()); location.has_value()) { - state.out().write_script(util::xml::escape_attribute(*location)); + state.out().write_script(xml::escape_attribute(*location)); return; } diff --git a/src/odr/internal/html/media_file.cpp b/src/odr/internal/html/media_file.cpp index 4a340821d..86479e821 100644 --- a/src/odr/internal/html/media_file.cpp +++ b/src/odr/internal/html/media_file.cpp @@ -11,7 +11,7 @@ #include #include #include -#include +#include #include #include @@ -182,7 +182,7 @@ class HtmlServiceImpl final : public HtmlService { : "controls preload=\"metadata\"") .set_attributes([&](const HtmlAttributeWriterCallback &clb) { if (location.has_value()) { - clb("src", util::xml::escape_attribute(*location)); + clb("src", xml::escape_attribute(*location)); } else { clb("src", [&](std::ostream &o) { o << file_to_url(*m_media_file.file().impl(), m_mime_type); diff --git a/src/odr/internal/html/pdf_file.cpp b/src/odr/internal/html/pdf_file.cpp index 575ec1851..761e29a3b 100644 --- a/src/odr/internal/html/pdf_file.cpp +++ b/src/odr/internal/html/pdf_file.cpp @@ -25,7 +25,7 @@ #include #include #include -#include +#include #include @@ -246,7 +246,7 @@ std::vector collect_page_links(const pdf::Page &page, link.top = std::min(p0[1], p1[1]); link.width = std::abs(p1[0] - p0[0]); link.height = std::abs(p1[1] - p0[1]); - link.href = util::xml::escape_attribute(std::move(href)); + link.href = xml::escape_attribute(std::move(href)); link.internal = internal; links.push_back(std::move(link)); } @@ -675,7 +675,7 @@ std::string svg_image_fragment(const pdf::ImageElement &image, !blend.empty()) { f << " style=\"mix-blend-mode:" << blend << '"'; } - f << " href=\"" << util::xml::escape_attribute(images.url(image)) << "\"/>"; + f << " href=\"" << xml::escape_attribute(images.url(image)) << "\"/>"; if (!clip_id.empty()) { f << ""; } diff --git a/src/odr/internal/odf/odf_document.cpp b/src/odr/internal/odf/odf_document.cpp index 74fb5be06..3f5d32814 100644 --- a/src/odr/internal/odf/odf_document.cpp +++ b/src/odr/internal/odf/odf_document.cpp @@ -14,7 +14,7 @@ #include #include #include -#include +#include #include #include @@ -31,10 +31,10 @@ create_element_adapter(const Document &document, ElementRegistry ®istry); Document::Document(const FileType file_type, const DocumentType document_type, std::shared_ptr files) : internal::Document(file_type, document_type, std::move(files)) { - m_content_xml = util::xml::parse(*m_files, AbsPath("/content.xml")); + m_content_xml = xml::parse(*m_files, AbsPath("/content.xml")); if (m_files->exists(AbsPath("/styles.xml"))) { - m_styles_xml = util::xml::parse(*m_files, AbsPath("/styles.xml")); + m_styles_xml = xml::parse(*m_files, AbsPath("/styles.xml")); } init_(m_content_xml.document_element(), m_styles_xml.document_element()); @@ -121,8 +121,7 @@ void Document::save(std::ostream &out) const { } if (abs_path == Path("/META-INF/manifest.xml")) { // TODO - auto manifest = - util::xml::parse(*m_files, AbsPath("/META-INF/manifest.xml")); + auto manifest = xml::parse(*m_files, AbsPath("/META-INF/manifest.xml")); for (auto &&node : manifest.select_nodes("//manifest:encryption-data")) { node.node().parent().remove_child(node.node()); @@ -651,19 +650,19 @@ class ElementAdapter final : public abstract::ElementAdapter, return track(parent.insert_child_before(node, old_first)); }; - for (const util::xml::StringToken &token : util::xml::tokenize_text(text)) { + for (const xml::StringToken &token : xml::tokenize_text(text)) { switch (token.type) { - case util::xml::StringToken::Type::none: + case xml::StringToken::Type::none: break; - case util::xml::StringToken::Type::string: { + case xml::StringToken::Type::string: { auto text_node = insert_pcdata(); text_node.text().set(token.string.c_str()); } break; - case util::xml::StringToken::Type::spaces: { + case xml::StringToken::Type::spaces: { auto space_node = insert_node("text:s"); space_node.prepend_attribute("text:c").set_value(token.string.size()); } break; - case util::xml::StringToken::Type::tabs: { + case xml::StringToken::Type::tabs: { for (std::size_t i = 0; i < token.string.size(); ++i) { insert_node("text:tab"); } diff --git a/src/odr/internal/odf/odf_file.cpp b/src/odr/internal/odf/odf_file.cpp index 1f7280b60..b4c7657f8 100644 --- a/src/odr/internal/odf/odf_file.cpp +++ b/src/odr/internal/odf/odf_file.cpp @@ -5,7 +5,7 @@ #include #include #include -#include +#include namespace odr::internal::abstract { class Document; @@ -19,7 +19,7 @@ OpenDocumentFile::OpenDocumentFile( : m_filesystem{std::move(filesystem)} { if (m_filesystem->exists(AbsPath("/META-INF/manifest.xml"))) { const pugi::xml_document manifest = - util::xml::parse(*m_filesystem, AbsPath("/META-INF/manifest.xml")); + xml::parse(*m_filesystem, AbsPath("/META-INF/manifest.xml")); m_file_meta = parse_file_meta(*m_filesystem, &manifest, false); m_manifest = parse_manifest(manifest); diff --git a/src/odr/internal/odf/odf_flat_file.cpp b/src/odr/internal/odf/odf_flat_file.cpp index 23f660b05..b56514901 100644 --- a/src/odr/internal/odf/odf_flat_file.cpp +++ b/src/odr/internal/odf/odf_flat_file.cpp @@ -5,8 +5,8 @@ #include #include #include -#include #include +#include #include @@ -65,7 +65,7 @@ std::shared_ptr FlatOpenDocumentFile::document() const { // the recogniser's tree is parsed for a source view; the document model // needs its own parse return std::make_shared(m_file_meta.type, m_file_meta.document_type, - util::xml::parse(m_file->text())); + xml::parse(m_file->text())); } } // namespace odr::internal::odf diff --git a/src/odr/internal/odf/odf_meta.cpp b/src/odr/internal/odf/odf_meta.cpp index 34e6303da..76bc73e3f 100644 --- a/src/odr/internal/odf/odf_meta.cpp +++ b/src/odr/internal/odf/odf_meta.cpp @@ -8,7 +8,7 @@ #include #include #include -#include +#include #include #include @@ -127,8 +127,7 @@ FileMeta parse_file_meta(const abstract::ReadableFilesystem &filesystem, pugi::xml_document manifest_xml; if (manifest == nullptr && filesystem.is_file(AbsPath("/META-INF/manifest.xml"))) { - manifest_xml = - util::xml::parse(filesystem, AbsPath("/META-INF/manifest.xml")); + manifest_xml = xml::parse(filesystem, AbsPath("/META-INF/manifest.xml")); manifest = &manifest_xml; } @@ -152,7 +151,7 @@ FileMeta parse_file_meta(const abstract::ReadableFilesystem &filesystem, if (result.password_encrypted == decrypted && filesystem.is_file(AbsPath("/meta.xml"))) { const pugi::xml_document meta_xml = - util::xml::parse(filesystem, AbsPath("/meta.xml")); + xml::parse(filesystem, AbsPath("/meta.xml")); const pugi::xml_node statistics = meta_xml.child("office:document-meta") .child("office:meta") diff --git a/src/odr/internal/ooxml/ooxml_util.cpp b/src/odr/internal/ooxml/ooxml_util.cpp index f42aced82..e0d99cc49 100644 --- a/src/odr/internal/ooxml/ooxml_util.cpp +++ b/src/odr/internal/ooxml/ooxml_util.cpp @@ -4,7 +4,7 @@ #include #include #include -#include +#include #include #include @@ -373,8 +373,7 @@ ooxml::parse_relationships(const abstract::ReadableFilesystem &filesystem, return {}; } - const pugi::xml_document relationships = - util::xml::parse(filesystem, rel_path); + const pugi::xml_document relationships = xml::parse(filesystem, rel_path); return parse_relationships(relationships); } @@ -389,8 +388,7 @@ ooxml::parse_relationship_target(const abstract::ReadableFilesystem &filesystem, return {}; } - const pugi::xml_document relationships = - util::xml::parse(filesystem, rel_path); + const pugi::xml_document relationships = xml::parse(filesystem, rel_path); for (const pugi::xpath_node e : relationships.select_nodes("//Relationship")) { // the type is a uri, so `type` has to match a whole trailing segment diff --git a/src/odr/internal/ooxml/presentation/ooxml_presentation_document.cpp b/src/odr/internal/ooxml/presentation/ooxml_presentation_document.cpp index fa2755200..8d28da329 100644 --- a/src/odr/internal/ooxml/presentation/ooxml_presentation_document.cpp +++ b/src/odr/internal/ooxml/presentation/ooxml_presentation_document.cpp @@ -12,7 +12,7 @@ #include #include #include -#include +#include #include @@ -27,7 +27,7 @@ create_element_adapter(const Document &document, ElementRegistry ®istry); Document::Document(std::shared_ptr files) : internal::Document(FileType::office_open_xml_presentation, DocumentType::presentation, std::move(files)) { - m_document_xml = util::xml::parse(*m_files, AbsPath("/ppt/presentation.xml")); + m_document_xml = xml::parse(*m_files, AbsPath("/ppt/presentation.xml")); // Only the parts the slide-id list names: a package may relate anything at // all to the presentation, and a Google Slides export relates a protobuf. @@ -39,7 +39,7 @@ Document::Document(std::shared_ptr files) .children("p:sldId")) { const std::string id = slide_id.attribute("r:id").value(); AbsPath slide_path = AbsPath("/ppt").join(RelPath(relations.at(id))); - m_slides_xml[id] = util::xml::parse(*m_files, slide_path); + m_slides_xml[id] = xml::parse(*m_files, slide_path); slides.push_back(std::move(slide_path)); } @@ -348,24 +348,24 @@ class ElementAdapter final : public abstract::ElementAdapter, return new_node; }; - for (const util::xml::StringToken &token : util::xml::tokenize_text(text)) { + for (const xml::StringToken &token : xml::tokenize_text(text)) { switch (token.type) { - case util::xml::StringToken::Type::none: + case xml::StringToken::Type::none: break; - case util::xml::StringToken::Type::string: { + case xml::StringToken::Type::string: { auto text_node = insert_node("a:t"); text_node.append_child(pugi::xml_node_type::node_pcdata) .text() .set(token.string.c_str()); } break; - case util::xml::StringToken::Type::spaces: { + case xml::StringToken::Type::spaces: { auto text_node = insert_node("a:t"); text_node.append_attribute("xml:space").set_value("preserve"); text_node.append_child(pugi::xml_node_type::node_pcdata) .text() .set(token.string.c_str()); } break; - case util::xml::StringToken::Type::tabs: { + case xml::StringToken::Type::tabs: { for (std::size_t i = 0; i < token.string.size(); ++i) { insert_node("a:tab"); } diff --git a/src/odr/internal/ooxml/presentation/ooxml_presentation_style.cpp b/src/odr/internal/ooxml/presentation/ooxml_presentation_style.cpp index 2829b6bc3..ce6b74de5 100644 --- a/src/odr/internal/ooxml/presentation/ooxml_presentation_style.cpp +++ b/src/odr/internal/ooxml/presentation/ooxml_presentation_style.cpp @@ -3,7 +3,7 @@ #include #include #include -#include +#include #include #include @@ -42,7 +42,7 @@ bool parse_optional_part(const abstract::ReadableFilesystem &files, return false; } try { - result = util::xml::parse(files, path); + result = xml::parse(files, path); } catch (const std::exception &) { return false; } diff --git a/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp b/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp index 3f52c6a14..58e3e412a 100644 --- a/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp +++ b/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp @@ -8,7 +8,7 @@ #include #include #include -#include +#include #include @@ -85,7 +85,7 @@ void Document::save(std::ostream & /*out*/, const char * /*password*/) const { std::pair Document::parse_xml_(const AbsPath &path) { - pugi::xml_document document = util::xml::parse(*m_files, path); + pugi::xml_document document = xml::parse(*m_files, path); Relations relations = parse_relationships(*m_files, path); auto [it, _] = m_xml_documents_and_relations.emplace( diff --git a/src/odr/internal/ooxml/text/ooxml_text_document.cpp b/src/odr/internal/ooxml/text/ooxml_text_document.cpp index f42bda52f..9095afea9 100644 --- a/src/odr/internal/ooxml/text/ooxml_text_document.cpp +++ b/src/odr/internal/ooxml/text/ooxml_text_document.cpp @@ -9,7 +9,7 @@ #include #include #include -#include +#include #include #include @@ -64,13 +64,12 @@ PageLayout read_page_layout(const pugi::xml_node body) { Document::Document(std::shared_ptr files) : internal::Document(FileType::office_open_xml_document, DocumentType::text, std::move(files)) { - m_document_xml = util::xml::parse(*m_files, AbsPath("/word/document.xml")); - m_styles_xml = util::xml::parse(*m_files, AbsPath("/word/styles.xml")); + m_document_xml = xml::parse(*m_files, AbsPath("/word/document.xml")); + m_styles_xml = xml::parse(*m_files, AbsPath("/word/styles.xml")); // Optional: a document without a single list carries no numbering part. if (m_files->exists(AbsPath("/word/numbering.xml"))) { - m_numbering_xml = - util::xml::parse(*m_files, AbsPath("/word/numbering.xml")); + m_numbering_xml = xml::parse(*m_files, AbsPath("/word/numbering.xml")); } m_document_relations = @@ -348,24 +347,24 @@ class ElementAdapter final : public abstract::ElementAdapter, return new_node; }; - for (const util::xml::StringToken &token : util::xml::tokenize_text(text)) { + for (const xml::StringToken &token : xml::tokenize_text(text)) { switch (token.type) { - case util::xml::StringToken::Type::none: + case xml::StringToken::Type::none: break; - case util::xml::StringToken::Type::string: { + case xml::StringToken::Type::string: { auto text_node = insert_node("w:t"); text_node.append_child(pugi::xml_node_type::node_pcdata) .text() .set(token.string.c_str()); } break; - case util::xml::StringToken::Type::spaces: { + case xml::StringToken::Type::spaces: { auto text_node = insert_node("w:t"); text_node.append_attribute("xml:space").set_value("preserve"); text_node.append_child(pugi::xml_node_type::node_pcdata) .text() .set(token.string.c_str()); } break; - case util::xml::StringToken::Type::tabs: { + case xml::StringToken::Type::tabs: { for (std::size_t i = 0; i < token.string.size(); ++i) { insert_node("w:tab"); } diff --git a/src/odr/internal/pdf/pdf_image.cpp b/src/odr/internal/pdf/pdf_image.cpp index c0eb4e0f0..e5a89e90d 100644 --- a/src/odr/internal/pdf/pdf_image.cpp +++ b/src/odr/internal/pdf/pdf_image.cpp @@ -5,8 +5,8 @@ #include #include #include +#include #include -#include #include #include @@ -218,7 +218,7 @@ std::string pdf::encode_image_png(const std::string &samples, } } - return util::png::write(out, width, height, has_alpha ? 4 : 3); + return png::write(out, width, height, has_alpha ? 4 : 3); } std::vector pdf::decode_mask_alpha( @@ -311,7 +311,7 @@ std::string pdf::encode_stencil_png(const std::string &samples, rgba[out_index++] = static_cast(paint ? 0xFF : 0x00); } } - return util::png::write(rgba, width, height, 4); + return png::write(rgba, width, height, 4); } std::optional pdf::encode_image( diff --git a/src/odr/internal/util/png_util.cpp b/src/odr/internal/png/png_util.cpp similarity index 88% rename from src/odr/internal/util/png_util.cpp rename to src/odr/internal/png/png_util.cpp index 39dfd44fb..acacc696f 100644 --- a/src/odr/internal/util/png_util.cpp +++ b/src/odr/internal/png/png_util.cpp @@ -1,4 +1,4 @@ -#include +#include #include #include @@ -23,10 +23,8 @@ void write_chunk(std::string &out, const std::string_view type, } // namespace -std::string util::png::write(const std::string &pixels, - const std::int32_t width, - const std::int32_t height, - const std::int32_t channels) { +std::string png::write(const std::string &pixels, const std::int32_t width, + const std::int32_t height, const std::int32_t channels) { if (width <= 0 || height <= 0 || (channels != 3 && channels != 4)) { return {}; } diff --git a/src/odr/internal/util/png_util.hpp b/src/odr/internal/png/png_util.hpp similarity index 83% rename from src/odr/internal/util/png_util.hpp rename to src/odr/internal/png/png_util.hpp index 9d1735de8..95487fe40 100644 --- a/src/odr/internal/util/png_util.hpp +++ b/src/odr/internal/png/png_util.hpp @@ -3,7 +3,7 @@ #include #include -namespace odr::internal::util::png { +namespace odr::internal::png { /// Wraps 8-bit pixels (row-major, unpadded, top to bottom) into a png: one /// `IDAT`, no interlacing, every scanline filtered as `None` (PNG 9.2). @@ -11,4 +11,4 @@ namespace odr::internal::util::png { std::string write(const std::string &pixels, std::int32_t width, std::int32_t height, std::int32_t channels); -} // namespace odr::internal::util::png +} // namespace odr::internal::png diff --git a/src/odr/internal/svg/AGENTS.md b/src/odr/internal/svg/AGENTS.md index ecf2d5198..9396678d3 100644 --- a/src/odr/internal/svg/AGENTS.md +++ b/src/odr/internal/svg/AGENTS.md @@ -76,7 +76,7 @@ svg. Today that is `svm/svm_to_svg.cpp`, translating a StarView metafile. Two things it does that a raw `operator<<` does not: -- **It escapes**, through `util::xml::escape` — the same single pass +- **It escapes**, through `xml::escape_attribute` — the same single pass `html::escape_attribute` uses, asked to drop the control characters xml forbids as well. svg is xml, so an unescaped `&` in a chart label does not spoil one label, it costs the whole image: an xml parse error renders nothing diff --git a/src/odr/internal/svg/svg_writer.cpp b/src/odr/internal/svg/svg_writer.cpp index 200d5777d..d1f98eeb5 100644 --- a/src/odr/internal/svg/svg_writer.cpp +++ b/src/odr/internal/svg/svg_writer.cpp @@ -1,7 +1,7 @@ #include #include -#include +#include #include #include @@ -59,7 +59,7 @@ void svg::SvgWriter::write_attribute(const std::string_view name, if (!m_tag_open) { throw std::runtime_error("no open tag to write an attribute to"); } - *m_out << " " << name << "=\"" << util::xml::escape_attribute(value) << "\""; + *m_out << " " << name << "=\"" << xml::escape_attribute(value) << "\""; } void svg::SvgWriter::write_attribute(const std::string_view name, @@ -72,13 +72,13 @@ void svg::SvgWriter::write_style(const std::string_view property, if (!m_tag_open) { throw std::runtime_error("no open tag to write a style to"); } - m_style += util::xml::escape_attribute(property); + m_style += xml::escape_attribute(property); m_style += ":"; // a `;` the file wrote would open a declaration of its own; dropped before // escaping, which writes `;` of its own std::string sanitized(value); std::erase(sanitized, ';'); - m_style += util::xml::escape_attribute(sanitized); + m_style += xml::escape_attribute(sanitized); m_style += ";"; } @@ -89,7 +89,7 @@ void svg::SvgWriter::write_style(const std::string_view property, void svg::SvgWriter::write_text(const std::string_view text) { close_tag(true); - *m_out << util::xml::escape_text(text); + *m_out << xml::escape_text(text); } } // namespace odr::internal diff --git a/src/odr/internal/svm/svm_format.cpp b/src/odr/internal/svm/svm_format.cpp index 42458da6d..89bc1e64b 100644 --- a/src/odr/internal/svm/svm_format.cpp +++ b/src/odr/internal/svm/svm_format.cpp @@ -2,8 +2,8 @@ #include +#include #include -#include #include #include @@ -791,8 +791,7 @@ svm::Image svm::read_dib(std::istream &in, const std::uint32_t limit) { bytes += read_bytes(in, pixels); if (const std::string rows = get_rgb_rows(bytes, layout); !rows.empty()) { - result.data = - util::png::write(rows, result.size_pixel.x, result.size_pixel.y, 3); + result.data = png::write(rows, result.size_pixel.x, result.size_pixel.y, 3); result.mime_type = "image/png"; } if (result.data.empty()) { diff --git a/src/odr/internal/xml/AGENTS.md b/src/odr/internal/xml/AGENTS.md index 5c6812557..701f19817 100644 --- a/src/odr/internal/xml/AGENTS.md +++ b/src/odr/internal/xml/AGENTS.md @@ -3,6 +3,15 @@ Read the root [`AGENTS.md`](../../../../AGENTS.md) first. This file covers what xml does differently, and why. What is not built yet is in [`PLAN.md`](PLAN.md). +## Two things live here + +`xml_util` is the shared plumbing — `parse`, `escape_text` / +`escape_attribute`, `read_declared_encoding`, `tokenize_text` — that odf, +ooxml, svg and the html writer all go through; it depends on no other engine, +and every other engine depends on it. `xml_file` is the format: xml opened as +a file of its own and rendered as a source view. The rest of this file is +about the latter. + ## A source view, not a document Xml has no document semantics: no paragraph, no page, no sheet, only nesting. @@ -42,7 +51,7 @@ is most wanted. ## The parse flags, and where they live `parse_source` (`xml_file.cpp`, file-local) is the only place they are written, -and `XmlFile`'s constructor is its only caller. Not `util::xml::parse`, whose +and `XmlFile`'s constructor is its only caller. Not `xml::parse`, whose every other caller wants pugixml's defaults. - `parse_full` adds the four node kinds `parse_default` drops — comments, @@ -62,7 +71,7 @@ pugixml's `encoding_auto` resolves UTF-8/16/32 from a BOM or the ` #include -#include +#include #include @@ -21,7 +21,7 @@ namespace { /// leaves the guess in place. TextEncoding resolve_encoding(const text::TextFile &file) { const std::unique_ptr in = file.file()->stream(); - const std::string declared = util::xml::read_declared_encoding(*in); + const std::string declared = xml::read_declared_encoding(*in); if (const TextEncoding encoding = text_encoding_by_name(declared); encoding != TextEncoding::unknown) { diff --git a/src/odr/internal/util/xml_util.cpp b/src/odr/internal/xml/xml_util.cpp similarity index 98% rename from src/odr/internal/util/xml_util.cpp rename to src/odr/internal/xml/xml_util.cpp index 88f3b849c..caf471864 100644 --- a/src/odr/internal/util/xml_util.cpp +++ b/src/odr/internal/xml/xml_util.cpp @@ -1,4 +1,4 @@ -#include +#include #include @@ -16,7 +16,7 @@ #include #include -namespace odr::internal::util { +namespace odr::internal { // `PUGIXML_COMPACT` (CMakeLists.txt) changes the size of every node, and a // translation unit that misses it links fine and then reads the wrong layout. @@ -210,4 +210,4 @@ std::vector xml::tokenize_text(const std::string &text) { return result; } -} // namespace odr::internal::util +} // namespace odr::internal diff --git a/src/odr/internal/util/xml_util.hpp b/src/odr/internal/xml/xml_util.hpp similarity index 95% rename from src/odr/internal/util/xml_util.hpp rename to src/odr/internal/xml/xml_util.hpp index 858a53210..9ad1d9adc 100644 --- a/src/odr/internal/util/xml_util.hpp +++ b/src/odr/internal/xml/xml_util.hpp @@ -18,7 +18,7 @@ namespace odr::internal { class AbsPath; } -namespace odr::internal::util::xml { +namespace odr::internal::xml { /// Escapes `&`, `<` and `>` for element content, and drops the control /// characters xml 1.0 cannot carry at all. @@ -57,4 +57,4 @@ struct StringToken { std::vector tokenize_text(const std::string &text); -} // namespace odr::internal::util::xml +} // namespace odr::internal::xml diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index a2ab2e65c..19f618cc8 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -59,6 +59,7 @@ add_executable(odr_test "src/internal/svg/svg_file_test.cpp" "src/internal/svg/svg_writer_test.cpp" "src/internal/xml/xml_file_test.cpp" + "src/internal/xml/xml_util_test.cpp" "src/internal/markdown/markdown_file_test.cpp" @@ -109,6 +110,8 @@ add_executable(odr_test "src/internal/pdf/pdf_shading.cpp" "src/internal/pdf/pdf_test_file_builder.cpp" + "src/internal/png/png_util_test.cpp" + "src/internal/font/cff_font.cpp" "src/internal/font/type1_charstring.cpp" "src/internal/font/type1_crypt.cpp" @@ -124,9 +127,7 @@ add_executable(odr_test "src/internal/util/map_util_test.cpp" "src/internal/util/number_util_test.cpp" "src/internal/util/stream_util_test.cpp" - "src/internal/util/png_util_test.cpp" "src/internal/util/string_util_test.cpp" - "src/internal/util/xml_util_test.cpp" "src/internal/zip/miniz_test.cpp" "src/internal/zip/zip_archive_test.cpp" diff --git a/test/src/internal/pdf/pdf_image.cpp b/test/src/internal/pdf/pdf_image.cpp index 7b82e7122..2ca091423 100644 --- a/test/src/internal/pdf/pdf_image.cpp +++ b/test/src/internal/pdf/pdf_image.cpp @@ -2,7 +2,7 @@ #include #include -#include +#include #include #include @@ -233,7 +233,7 @@ TEST(PdfImage, png_rgba_round_trip) { // 2x1: opaque red, half-transparent green. const std::string rgba = bytes({255, 0, 0, 255, 0, 255, 0, 128}); const DecodedPngRgba png = - decode_png_rgba(odr::internal::util::png::write(rgba, 2, 1, 4)); + decode_png_rgba(odr::internal::png::write(rgba, 2, 1, 4)); EXPECT_EQ(png.width, 2); EXPECT_EQ(png.height, 1); EXPECT_EQ(png.rgba, rgba); diff --git a/test/src/internal/util/png_util_test.cpp b/test/src/internal/png/png_util_test.cpp similarity index 83% rename from test/src/internal/util/png_util_test.cpp rename to test/src/internal/png/png_util_test.cpp index 18e510c22..cb6bed94f 100644 --- a/test/src/internal/util/png_util_test.cpp +++ b/test/src/internal/png/png_util_test.cpp @@ -1,4 +1,4 @@ -#include +#include #include @@ -32,7 +32,7 @@ struct DecodedPng final { std::string rgb; }; -/// Reads back what @ref util::png::write wrote: the chunks, then the one +/// Reads back what @ref png::write wrote: the chunks, then the one /// zlib stream their `IDAT` holds, minus the filter byte per row. DecodedPng decode_png(const std::string &png) { DecodedPng result; @@ -74,7 +74,7 @@ TEST(PngUtil, rgb_round_trip) { const std::string rgb = bytes({255, 0, 0, 0, 255, 0, 0, 0, 255, 255, 255, 255}); - const DecodedPng png = decode_png(util::png::write(rgb, 2, 2, 3)); + const DecodedPng png = decode_png(png::write(rgb, 2, 2, 3)); EXPECT_EQ(2, png.width); EXPECT_EQ(2, png.height); @@ -82,13 +82,13 @@ TEST(PngUtil, rgb_round_trip) { } TEST(PngUtil, a_buffer_too_short_for_the_size_is_refused) { - EXPECT_TRUE(util::png::write(bytes({255, 0, 0}), 2, 2, 3).empty()); - EXPECT_TRUE(util::png::write("", 0, 0, 3).empty()); + EXPECT_TRUE(png::write(bytes({255, 0, 0}), 2, 2, 3).empty()); + EXPECT_TRUE(png::write("", 0, 0, 3).empty()); } TEST(PngUtil, only_three_or_four_channels) { const std::string pixels(2 * 2 * 4, '\0'); - EXPECT_TRUE(util::png::write(pixels, 2, 2, 1).empty()); - EXPECT_TRUE(util::png::write(pixels, 2, 2, 2).empty()); - EXPECT_FALSE(util::png::write(pixels, 2, 2, 4).empty()); + EXPECT_TRUE(png::write(pixels, 2, 2, 1).empty()); + EXPECT_TRUE(png::write(pixels, 2, 2, 2).empty()); + EXPECT_FALSE(png::write(pixels, 2, 2, 4).empty()); } diff --git a/test/src/internal/xml/xml_file_test.cpp b/test/src/internal/xml/xml_file_test.cpp index 8208836bc..2dab7cbce 100644 --- a/test/src/internal/xml/xml_file_test.cpp +++ b/test/src/internal/xml/xml_file_test.cpp @@ -5,8 +5,8 @@ #include #include -#include #include +#include #include #include @@ -39,7 +39,7 @@ std::string xml_html(const std::string &content) { std::string declared_encoding(const std::string &content) { std::istringstream in(content); - return util::xml::read_declared_encoding(in); + return xml::read_declared_encoding(in); } } // namespace diff --git a/test/src/internal/util/xml_util_test.cpp b/test/src/internal/xml/xml_util_test.cpp similarity index 96% rename from test/src/internal/util/xml_util_test.cpp rename to test/src/internal/xml/xml_util_test.cpp index 04097ca04..66613304b 100644 --- a/test/src/internal/util/xml_util_test.cpp +++ b/test/src/internal/xml/xml_util_test.cpp @@ -1,11 +1,11 @@ -#include +#include #include #include #include -using namespace odr::internal::util::xml; +using namespace odr::internal::xml; TEST(xml_util, escape_text) { EXPECT_EQ("a & b <c> \"d\"", escape_text("a & b \"d\""));