From 1d079f44bacf5fc498e0a3a44d2feb309c19f915 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Wed, 31 Dec 2025 12:23:09 +0100 Subject: [PATCH 1/3] cleanup more --- src/odr/document.cpp | 2 +- src/odr/document_element.hpp | 4 - src/odr/file.cpp | 25 +-- src/odr/filesystem.cpp | 2 +- src/odr/html.cpp | 6 +- src/odr/http_server.cpp | 12 +- src/odr/internal/cfb/cfb_util.cpp | 12 +- src/odr/internal/common/path.hpp | 6 +- src/odr/internal/common/table_range.cpp | 5 +- src/odr/internal/html/document_element.cpp | 3 +- src/odr/internal/html/document_style.cpp | 148 +++++++++++------- src/odr/internal/libmagic/libmagic.cpp | 4 +- src/odr/internal/odf/odf_document.cpp | 2 +- .../ooxml_spreadsheet_document.cpp | 20 +-- .../spreadsheet/ooxml_spreadsheet_parser.cpp | 5 +- .../internal/pdf_poppler/poppler_pdf_file.cpp | 9 +- src/odr/internal/util/document_util.cpp | 4 +- src/odr/logger.cpp | 4 +- src/odr/style.cpp | 46 +++--- src/odr/style.hpp | 16 +- src/odr/table_position.cpp | 28 +--- src/odr/table_position.hpp | 20 +-- test/src/internal/common/table_range_test.cpp | 16 +- test/src/table_position_test.cpp | 24 +-- 24 files changed, 221 insertions(+), 202 deletions(-) diff --git a/src/odr/document.cpp b/src/odr/document.cpp index 23b0b98a..37398a1e 100644 --- a/src/odr/document.cpp +++ b/src/odr/document.cpp @@ -12,7 +12,7 @@ namespace odr { Document::Document(std::shared_ptr impl) : m_impl{std::move(impl)} { - if (!m_impl) { + if (m_impl == nullptr) { throw NullPointerError("document is null"); } } diff --git a/src/odr/document_element.hpp b/src/odr/document_element.hpp index d7010cb2..7e55ab04 100644 --- a/src/odr/document_element.hpp +++ b/src/odr/document_element.hpp @@ -29,8 +29,6 @@ class TextRootAdapter; class SlideAdapter; class PageAdapter; class SheetAdapter; -class SheetColumnAdapter; -class SheetRowAdapter; class SheetCellAdapter; class MasterPageAdapter; class LineBreakAdapter; @@ -60,8 +58,6 @@ class ElementRange; class TextRoot; class Slide; class Sheet; -class SheetColumn; -class SheetRow; class SheetCell; class Page; class MasterPage; diff --git a/src/odr/file.cpp b/src/odr/file.cpp index f9b384ec..a1c19640 100644 --- a/src/odr/file.cpp +++ b/src/odr/file.cpp @@ -167,40 +167,45 @@ bool DecodedFile::is_pdf_file() const { } TextFile DecodedFile::as_text_file() const { - if (const auto text_file = - std::dynamic_pointer_cast(m_impl)) { + if (const std::shared_ptr text_file = + std::dynamic_pointer_cast(m_impl); + text_file != nullptr) { return TextFile(text_file); } throw NoTextFile(); } ImageFile DecodedFile::as_image_file() const { - if (const auto image_file = - std::dynamic_pointer_cast(m_impl)) { + if (const std::shared_ptr image_file = + std::dynamic_pointer_cast(m_impl); + image_file != nullptr) { return ImageFile(image_file); } throw NoImageFile(); } ArchiveFile DecodedFile::as_archive_file() const { - if (const auto archive_file = - std::dynamic_pointer_cast(m_impl)) { + if (const std::shared_ptr archive_file = + std::dynamic_pointer_cast(m_impl); + archive_file != nullptr) { return ArchiveFile(archive_file); } throw NoArchiveFile(); } DocumentFile DecodedFile::as_document_file() const { - if (const auto document_file = - std::dynamic_pointer_cast(m_impl)) { + if (const std::shared_ptr document_file = + std::dynamic_pointer_cast(m_impl); + document_file != nullptr) { return DocumentFile(document_file); } throw NoDocumentFile(); } PdfFile DecodedFile::as_pdf_file() const { - if (const auto pdf_file = - std::dynamic_pointer_cast(m_impl)) { + if (const std::shared_ptr pdf_file = + std::dynamic_pointer_cast(m_impl); + pdf_file != nullptr) { return PdfFile(pdf_file); } throw NoPdfFile(); diff --git a/src/odr/filesystem.cpp b/src/odr/filesystem.cpp index 37ee588d..c7a996bc 100644 --- a/src/odr/filesystem.cpp +++ b/src/odr/filesystem.cpp @@ -22,7 +22,7 @@ FileWalker::FileWalker(FileWalker &&other) noexcept = default; FileWalker::~FileWalker() = default; FileWalker &FileWalker::operator=(const FileWalker &other) { - if (this == &other) { + if (&other == this) { return *this; } m_impl = other.m_impl->clone(); diff --git a/src/odr/html.cpp b/src/odr/html.cpp index 9b9bc942..3ae6e08f 100644 --- a/src/odr/html.cpp +++ b/src/odr/html.cpp @@ -296,7 +296,8 @@ HtmlService html::translate(const DocumentFile &document_file, #ifdef ODR_WITH_WVWARE if (const std::shared_ptr wv_document_file = std::dynamic_pointer_cast( - document_file_impl)) { + document_file_impl); + wv_document_file != nullptr) { std::filesystem::create_directories(cache_path); return internal::html::create_wvware_oldms_service( *wv_document_file, cache_path, config, std::move(logger)); @@ -315,7 +316,8 @@ HtmlService html::translate(const PdfFile &pdf_file, #ifdef ODR_WITH_PDF2HTMLEX if (const std::shared_ptr poppler_pdf_file = - std::dynamic_pointer_cast(pdf_file_impl)) { + std::dynamic_pointer_cast(pdf_file_impl); + poppler_pdf_file != nullptr) { std::filesystem::create_directories(cache_path); return internal::html::create_poppler_pdf_service( *poppler_pdf_file, cache_path, config, std::move(logger)); diff --git a/src/odr/http_server.cpp b/src/odr/http_server.cpp index 9769e3e8..3205279b 100644 --- a/src/odr/http_server.cpp +++ b/src/odr/http_server.cpp @@ -2,9 +2,7 @@ #include #include -#include #include -#include #include @@ -23,9 +21,9 @@ class HttpServer::Impl { // This prevents crashes when exceptions occur during request processing. m_server->set_exception_handler([this](const httplib::Request & /*req*/, httplib::Response &res, - std::exception_ptr ep) { + const std::exception_ptr &ep) { try { - if (ep) { + if (ep != nullptr) { std::rethrow_exception(ep); } } catch (const std::exception &e) { @@ -73,7 +71,7 @@ class HttpServer::Impl { // here (via unique_ptr::reset), we ensure all threads are joined // before any other members are destroyed. m_stopping.store(true, std::memory_order_release); - if (m_server) { + if (m_server != nullptr) { m_server->stop(); m_server.reset(); // Destroy server, join all thread pool threads } @@ -160,7 +158,7 @@ class HttpServer::Impl { m_content.emplace(prefix, Content{prefix, std::move(service)}); } - void listen(const std::string &host, const std::uint32_t port) { + void listen(const std::string &host, const std::uint32_t port) const { ODR_VERBOSE(*m_logger, "Listening on " << host << ":" << port); m_server->listen(host, static_cast(port)); @@ -186,7 +184,7 @@ class HttpServer::Impl { // This prevents new requests from starting while we're shutting down. m_stopping.store(true, std::memory_order_release); - if (m_server) { + if (m_server != nullptr) { // Stop the server to prevent new connections. // Note: httplib::Server::stop() signals shutdown but thread pool // threads may still be running. They only fully stop when the diff --git a/src/odr/internal/cfb/cfb_util.cpp b/src/odr/internal/cfb/cfb_util.cpp index ff52ee6b..745d6791 100644 --- a/src/odr/internal/cfb/cfb_util.cpp +++ b/src/odr/internal/cfb/cfb_util.cpp @@ -133,13 +133,13 @@ std::optional Archive::Entry::child() const { } void Archive::Iterator::dig_left_() { - if (!m_entry) { + if (!m_entry.has_value()) { return; } while (true) { const std::optional left = m_entry->left(); - if (!left) { + if (!left.has_value()) { break; } m_ancestors.push_back(*m_entry); @@ -148,11 +148,11 @@ void Archive::Iterator::dig_left_() { } void Archive::Iterator::next_() { - if (!m_entry) { + if (!m_entry.has_value()) { return; } - if (const std::optional child = m_entry->child()) { + if (const std::optional child = m_entry->child(); child.has_value()) { m_directories.push_back(*m_entry); m_entry = child; dig_left_(); @@ -163,11 +163,11 @@ void Archive::Iterator::next_() { } void Archive::Iterator::next_flat_() { - if (!m_entry) { + if (!m_entry.has_value()) { return; } - if (const std::optional right = m_entry->right()) { + if (const std::optional right = m_entry->right(); right.has_value()) { m_entry = right; dig_left_(); return; diff --git a/src/odr/internal/common/path.hpp b/src/odr/internal/common/path.hpp index c04c1db6..466c89fc 100644 --- a/src/odr/internal/common/path.hpp +++ b/src/odr/internal/common/path.hpp @@ -94,9 +94,9 @@ class Path { private: std::string m_path; - std::uint32_t m_upwards; - std::uint32_t m_downwards; - bool m_absolute; + std::uint32_t m_upwards{0}; + std::uint32_t m_downwards{0}; + bool m_absolute{false}; friend std::ostream &operator<<(std::ostream &os, const Path &p) { return os << p.m_path; diff --git a/src/odr/internal/common/table_range.cpp b/src/odr/internal/common/table_range.cpp index 307a41fa..5c783992 100644 --- a/src/odr/internal/common/table_range.cpp +++ b/src/odr/internal/common/table_range.cpp @@ -28,9 +28,8 @@ std::string TableRange::to_string() const noexcept { } bool TableRange::contains(const TablePosition &position) const noexcept { - return m_from.column() <= position.column() && - m_to.column() > position.column() && m_from.row() <= position.row() && - m_to.row() > position.row(); + return m_from.column <= position.column && m_to.column > position.column && + m_from.row <= position.row && m_to.row > position.row; } } // namespace odr::internal diff --git a/src/odr/internal/html/document_element.cpp b/src/odr/internal/html/document_element.cpp index 75519a2f..ae5d85d9 100644 --- a/src/odr/internal/html/document_element.cpp +++ b/src/odr/internal/html/document_element.cpp @@ -130,7 +130,8 @@ void html::translate_sheet(const Sheet &sheet, const WritingState &state) { state.out().write_element_begin( "td", HtmlElementOptions().set_inline(true).set_style([&] { std::string style = "text-align:center;vertical-align:middle;"; - if (const std::optional height = table_row_style.height) { + if (const std::optional height = table_row_style.height; + height.has_value()) { style += "height:" + height->to_string() + ";"; style += "max-height:" + height->to_string() + ";"; } diff --git a/src/odr/internal/html/document_style.cpp b/src/odr/internal/html/document_style.cpp index 201ecb20..ef159b49 100644 --- a/src/odr/internal/html/document_style.cpp +++ b/src/odr/internal/html/document_style.cpp @@ -73,10 +73,12 @@ const char *html::translate_font_style(const FontStyle font_style) { std::string html::translate_outer_page_style(const PageLayout &page_layout) { std::string result; - if (const std::optional width = page_layout.width) { + if (const std::optional width = page_layout.width; + width.has_value()) { result.append("width:").append(width->to_string()).append(";"); } - if (const std::optional height = page_layout.height) { + if (const std::optional height = page_layout.height; + height.has_value()) { result.append("height:").append(height->to_string()).append(";"); } return result; @@ -85,21 +87,24 @@ std::string html::translate_outer_page_style(const PageLayout &page_layout) { std::string html::translate_inner_page_style(const PageLayout &page_layout) { std::string result; if (const std::optional> margin_right = - page_layout.margin.right) { + page_layout.margin.right; + margin_right.has_value()) { result.append("margin-right:") .append(margin_right->to_string()) .append(";"); } - if (const std::optional> margin_top = - page_layout.margin.top) { + if (const std::optional> margin_top = page_layout.margin.top; + margin_top.has_value()) { result.append("margin-top:").append(margin_top->to_string()).append(";"); } if (const std::optional> margin_left = - page_layout.margin.left) { + page_layout.margin.left; + margin_left.has_value()) { result.append("margin-left:").append(margin_left->to_string()).append(";"); } if (const std::optional> margin_bottom = - page_layout.margin.bottom) { + page_layout.margin.bottom; + margin_bottom.has_value()) { result.append("margin-bottom:") .append(margin_bottom->to_string()) .append(";"); @@ -109,18 +114,21 @@ 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) { + 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_size = text_style.font_size) { + if (const std::optional font_size = text_style.font_size; + font_size.has_value()) { result.append("font-size:").append(font_size->to_string()).append(";"); } - if (const std::optional font_weight = text_style.font_weight) { + if (const std::optional font_weight = text_style.font_weight; + font_weight.has_value()) { result.append("font-weight:") .append(translate_font_weight(*font_weight)) .append(";"); } - if (const std::optional font_style = text_style.font_style) { + if (const std::optional font_style = text_style.font_style; + font_style.has_value()) { result.append("font-style:") .append(translate_font_style(*font_style)) .append(";"); @@ -131,14 +139,16 @@ std::string html::translate_text_style(const TextStyle &text_style) { if (text_style.font_line_through && *text_style.font_line_through) { result += "text-decoration:line-through;"; } - if (const std::optional font_shadow = text_style.font_shadow) { + if (const std::optional font_shadow = text_style.font_shadow; + font_shadow.has_value()) { result.append("text-shadow:").append(*font_shadow).append(";"); } - if (const std::optional font_color = text_style.font_color) { + if (const std::optional font_color = text_style.font_color; + font_color.has_value()) { result.append("color:").append(color(*font_color)).append(";"); } - if (const std::optional background_color = - text_style.background_color) { + if (const std::optional background_color = text_style.background_color; + background_color.has_value()) { result.append("background-color:") .append(color(*background_color)) .append(";"); @@ -149,32 +159,38 @@ std::string html::translate_text_style(const TextStyle &text_style) { std::string html::translate_paragraph_style(const ParagraphStyle ¶graph_style) { std::string result; - if (const std::optional text_align = paragraph_style.text_align) { + if (const std::optional text_align = paragraph_style.text_align; + text_align.has_value()) { result.append("text-align:") .append(translate_text_align(*text_align)) .append(";"); } if (const std::optional> margin_right = - paragraph_style.margin.right) { + paragraph_style.margin.right; + margin_right.has_value()) { result.append("margin-right:") .append(margin_right->to_string()) .append(";"); } if (const std::optional> margin_top = - paragraph_style.margin.top) { + paragraph_style.margin.top; + margin_top.has_value()) { result.append("margin-top:").append(margin_top->to_string()).append(";"); } if (const std::optional> margin_left = - paragraph_style.margin.left) { + paragraph_style.margin.left; + margin_left.has_value()) { result.append("margin-left:").append(margin_left->to_string()).append(";"); } if (const std::optional> margin_bottom = - paragraph_style.margin.bottom) { + paragraph_style.margin.bottom; + margin_bottom.has_value()) { result.append("margin-bottom:") .append(margin_bottom->to_string()) .append(";"); } - if (const std::optional line_height = paragraph_style.line_height) { + if (const std::optional line_height = paragraph_style.line_height; + line_height.has_value()) { result.append("line-height:").append(line_height->to_string()).append(";"); } return result; @@ -182,7 +198,8 @@ html::translate_paragraph_style(const ParagraphStyle ¶graph_style) { std::string html::translate_table_style(const TableStyle &table_style) { std::string result; - if (const std::optional width = table_style.width) { + if (const std::optional width = table_style.width; + width.has_value()) { result.append("width:").append(width->to_string()).append(";"); } return result; @@ -191,7 +208,8 @@ std::string html::translate_table_style(const TableStyle &table_style) { std::string html::translate_table_column_style(const TableColumnStyle &table_column_style) { std::string result; - if (const std::optional width = table_column_style.width) { + if (const std::optional width = table_column_style.width; + width.has_value()) { result.append("width:").append(width->to_string()).append(";"); result.append("min-width:").append(width->to_string()).append(";"); } @@ -203,7 +221,8 @@ html::translate_table_row_style(const TableRowStyle &table_row_style) { std::string result; // TODO that does not work with HTML; height would need to be applied to the // cells - if (const std::optional height = table_row_style.height) { + if (const std::optional height = table_row_style.height; + height.has_value()) { result.append("height:").append(height->to_string()).append(";"); } return result; @@ -213,64 +232,74 @@ std::string html::translate_table_cell_style(const TableCellStyle &table_cell_style) { std::string result; if (const std::optional horizontal_align = - table_cell_style.horizontal_align) { + table_cell_style.horizontal_align; + horizontal_align.has_value()) { result.append("text-align:") .append(translate_horizontal_align(*horizontal_align)) .append(";"); } if (const std::optional vertical_align = - table_cell_style.vertical_align) { + table_cell_style.vertical_align; + vertical_align.has_value()) { result.append("vertical-align:") .append(translate_vertical_align(*vertical_align)) .append(";"); } if (const std::optional background_color = - table_cell_style.background_color) { + table_cell_style.background_color; + background_color.has_value()) { result.append("background-color:") .append(color(*background_color)) .append(";"); } if (const std::optional> padding_right = - table_cell_style.padding.right) { + table_cell_style.padding.right; + padding_right.has_value()) { result.append("padding-right:") .append(padding_right->to_string()) .append(";"); } if (const std::optional> padding_top = - table_cell_style.padding.top) { + table_cell_style.padding.top; + padding_top.has_value()) { result.append("padding-top:").append(padding_top->to_string()).append(";"); } if (const std::optional> padding_left = - table_cell_style.padding.left) { + table_cell_style.padding.left; + padding_left.has_value()) { result.append("padding-left:") .append(padding_left->to_string()) .append(";"); } if (const std::optional> padding_bottom = - table_cell_style.padding.bottom) { + table_cell_style.padding.bottom; + padding_bottom.has_value()) { result.append("padding-bottom:") .append(padding_bottom->to_string()) .append(";"); } if (const std::optional border_right = - table_cell_style.border.right) { + table_cell_style.border.right; + border_right.has_value()) { result.append("border-right:").append(*border_right).append(";"); } - if (const std::optional border_top = - table_cell_style.border.top) { + if (const std::optional border_top = table_cell_style.border.top; + border_top.has_value()) { result.append("border-top:").append(*border_top).append(";"); } if (const std::optional border_left = - table_cell_style.border.left) { + table_cell_style.border.left; + border_left.has_value()) { result.append("border-left:").append(*border_left).append(";"); } if (const std::optional border_bottom = - table_cell_style.border.bottom) { + table_cell_style.border.bottom; + border_bottom.has_value()) { result.append("border-bottom:").append(*border_bottom).append(";"); } if (const std::optional text_rotation = table_cell_style.text_rotation; - text_rotation && *text_rotation != 0) { + text_rotation.has_value() && *text_rotation != 0) { // TODO covers only the -90° case result.append("writing-mode:vertical-lr;"); } @@ -279,19 +308,23 @@ html::translate_table_cell_style(const TableCellStyle &table_cell_style) { std::string html::translate_drawing_style(const GraphicStyle &graphic_style) { std::string result; - if (const std::optional stroke_width = graphic_style.stroke_width) { + if (const std::optional stroke_width = graphic_style.stroke_width; + stroke_width.has_value()) { result.append("stroke-width:") .append(stroke_width->to_string()) .append(";"); } - if (const std::optional stroke_color = graphic_style.stroke_color) { + if (const std::optional stroke_color = graphic_style.stroke_color; + stroke_color.has_value()) { result.append("stroke:").append(color(*stroke_color)).append(";"); } - if (const std::optional fill_color = graphic_style.fill_color) { + if (const std::optional fill_color = graphic_style.fill_color; + fill_color.has_value()) { result.append("fill:").append(color(*fill_color)).append(";"); } if (const std::optional vertical_align = - graphic_style.vertical_align) { + graphic_style.vertical_align; + vertical_align.has_value()) { if (vertical_align == VerticalAlign::middle) { result += "display:flex;justify-content:center;flex-direction:column;"; } @@ -302,22 +335,22 @@ std::string html::translate_drawing_style(const GraphicStyle &graphic_style) { std::string html::translate_frame_properties(const Frame &frame) { auto text_wrap = TextWrap::run_through; - if (auto style = frame.style(); style.text_wrap) { + if (const GraphicStyle style = frame.style(); style.text_wrap.has_value()) { text_wrap = *style.text_wrap; } std::string result; - if (auto anchor_type = frame.anchor_type(); + if (const AnchorType anchor_type = frame.anchor_type(); anchor_type == AnchorType::as_char) { result += "display:inline-block;"; } else if (text_wrap == TextWrap::before) { result += "display:block;"; result += "float:right;clear:both;"; result += "shape-outside:content-box;"; - if (auto x = frame.x()) { + if (const std::optional x = frame.x(); x.has_value()) { result += "margin-left:" + *x + ";"; } - if (auto y = frame.y()) { + if (const std::optional y = frame.y(); y.has_value()) { result += "margin-top:" + *y + ";"; } result += "margin-right:calc(100% - "; @@ -329,37 +362,40 @@ std::string html::translate_frame_properties(const Frame &frame) { result += "display:block;"; result += "float:left;clear:both;"; result += "shape-outside:content-box;"; - if (auto x = frame.x()) { + if (const std::optional x = frame.x(); x.has_value()) { result += "margin-left:" + *x + ";"; } - if (auto y = frame.y()) { + if (const std::optional y = frame.y(); y.has_value()) { result += "margin-top:" + *y + ";"; } } else if (text_wrap == TextWrap::none) { result += "display:block;"; - if (auto x = frame.x()) { + if (const std::optional x = frame.x(); x.has_value()) { result += "margin-left:" + *x + ";"; } - if (auto y = frame.y()) { + if (const std::optional y = frame.y(); y.has_value()) { result += "margin-top:" + *y + ";"; } } else { result += "display:block;"; result += "position:absolute;"; - if (auto x = frame.x()) { + if (const std::optional x = frame.x(); x.has_value()) { result += "left:" + *x + ";"; } - if (auto y = frame.y()) { + if (const std::optional y = frame.y(); y.has_value()) { result += "top:" + *y + ";"; } } - if (auto width = frame.width()) { + if (const std::optional width = frame.width(); + width.has_value()) { result += "width:" + *width + ";"; } - if (auto height = frame.height()) { + if (const std::optional height = frame.height(); + height.has_value()) { result += "height:" + *height + ";"; } - if (auto z_index = frame.z_index()) { + if (const std::optional z_index = frame.z_index(); + z_index.has_value()) { result += "z-index:" + *z_index + ";"; } return result; @@ -389,12 +425,12 @@ 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()) { + if (const std::optional x = custom_shape.x(); x.has_value()) { result += "left:" + *x + ";"; } else { result += "left:0;"; } - if (const std::optional y = custom_shape.y()) { + if (const std::optional y = custom_shape.y(); y.has_value()) { result += "top:" + *y + ";"; } else { result += "top:0;"; diff --git a/src/odr/internal/libmagic/libmagic.cpp b/src/odr/internal/libmagic/libmagic.cpp index 64ddbc0d..4c2bc212 100644 --- a/src/odr/internal/libmagic/libmagic.cpp +++ b/src/odr/internal/libmagic/libmagic.cpp @@ -21,12 +21,12 @@ magic_t get_magic_cookie() { std::unique_ptr, decltype(&magic_deleter)>; static Holder magic_cookie(nullptr, &magic_deleter); - if (magic_cookie) { + if (magic_cookie != nullptr) { return magic_cookie.get(); } magic_cookie = Holder(magic_open(MAGIC_MIME_TYPE), &magic_deleter); - if (!magic_cookie) { + if (magic_cookie == nullptr) { throw std::runtime_error("magic_open failed"); } if (magic_load(magic_cookie.get(), diff --git a/src/odr/internal/odf/odf_document.cpp b/src/odr/internal/odf/odf_document.cpp index f3606d57..c5b505bc 100644 --- a/src/odr/internal/odf/odf_document.cpp +++ b/src/odr/internal/odf/odf_document.cpp @@ -973,7 +973,7 @@ class ElementAdapter final : public abstract::ElementAdapter, } } - const auto [column, row] = position.pair(); + const auto [column, row] = position; const ElementRegistry::Sheet &sheet_registry = m_registry->sheet_element_at(sheet_id); diff --git a/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp b/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp index 02b9030e..a67503b8 100644 --- a/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp +++ b/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp @@ -24,27 +24,29 @@ Document::Document(std::shared_ptr files) : internal::Document(FileType::office_open_xml_workbook, DocumentType::spreadsheet, std::move(files)) { const AbsPath workbook_path("/xl/workbook.xml"); - auto [workbook_xml, workbook_relations] = parse_xml_(workbook_path); - auto [styles_xml, _] = parse_xml_(AbsPath("/xl/styles.xml")); + const auto [workbook_xml, workbook_relations] = parse_xml_(workbook_path); + const auto [styles_xml, _] = parse_xml_(AbsPath("/xl/styles.xml")); for (pugi::xml_node sheet_node : workbook_xml.document_element().child("sheets").children("sheet")) { const char *id = sheet_node.attribute("r:id").value(); - AbsPath sheet_path = + const AbsPath sheet_path = workbook_path.parent().join(RelPath(workbook_relations.at(id))); - auto [sheet_xml, sheet_relationships] = parse_xml_(sheet_path); + const auto [sheet_xml, sheet_relationships] = parse_xml_(sheet_path); - if (auto drawing = sheet_xml.document_element().child("drawing")) { - AbsPath drawing_path = sheet_path.parent().join( + if (const pugi::xml_node drawing = + sheet_xml.document_element().child("drawing")) { + const AbsPath drawing_path = sheet_path.parent().join( RelPath(sheet_relationships.at(drawing.attribute("r:id").value()))); parse_xml_(drawing_path); } } if (m_files->exists(AbsPath("/xl/sharedStrings.xml"))) { - for (auto [shared_strings_xml, _] = - parse_xml_(AbsPath("/xl/sharedStrings.xml")); - auto shared_string : shared_strings_xml.document_element()) { + const auto [shared_strings_xml, _] = + parse_xml_(AbsPath("/xl/sharedStrings.xml")); + for (const pugi::xml_node shared_string : + shared_strings_xml.document_element()) { m_shared_strings.push_back(shared_string); } } diff --git a/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_parser.cpp b/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_parser.cpp index 9dea058f..0e4686b5 100644 --- a/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_parser.cpp +++ b/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_parser.cpp @@ -118,8 +118,7 @@ parse_sheet_element(ElementRegistry ®istry, const ParseContext &context, const auto &[cell_id, _, __] = registry.create_sheet_cell_element(cell_node, position); registry.append_sheet_cell(element_id, cell_id); - sheet.register_cell(position.column(), position.row(), cell_node, - cell_id); + sheet.register_cell(position.column, position.row, cell_node, cell_id); parse_sheet_cell_children(registry, context, cell_id, cell_node); } } @@ -134,7 +133,7 @@ parse_sheet_element(ElementRegistry ®istry, const ParseContext &context, position_to = TableRange(dimension_ref).to(); } sheet.dimensions = - TableDimensions(position_to.row() + 1, position_to.column() + 1); + TableDimensions(position_to.row + 1, position_to.column + 1); } if (const pugi::xml_node drawing_node = node.child("drawing")) { diff --git a/src/odr/internal/pdf_poppler/poppler_pdf_file.cpp b/src/odr/internal/pdf_poppler/poppler_pdf_file.cpp index 0232bb7e..82ade1c8 100644 --- a/src/odr/internal/pdf_poppler/poppler_pdf_file.cpp +++ b/src/odr/internal/pdf_poppler/poppler_pdf_file.cpp @@ -24,13 +24,16 @@ void PopplerPdfFile::open(const std::optional &password) { password_goo = GooString(password.value().c_str()); } - if (const auto disk_file = std::dynamic_pointer_cast(m_file)) { + if (const std::shared_ptr disk_file = + std::dynamic_pointer_cast(m_file); + disk_file != nullptr) { auto file_path_goo = std::make_unique(disk_file->disk_path()->string().c_str()); m_pdf_doc = std::make_shared(std::move(file_path_goo), password_goo, password_goo); - } else if (const auto memory_file = - std::dynamic_pointer_cast(m_file)) { + } else if (const std::shared_ptr memory_file = + std::dynamic_pointer_cast(m_file); + memory_file != nullptr) { // `stream` is freed by `m_pdf_doc` auto stream = new MemStream(memory_file->memory_data(), 0, static_cast(memory_file->size()), diff --git a/src/odr/internal/util/document_util.cpp b/src/odr/internal/util/document_util.cpp index b91413d2..23fac514 100644 --- a/src/odr/internal/util/document_util.cpp +++ b/src/odr/internal/util/document_util.cpp @@ -64,8 +64,8 @@ navigate_path_component(const abstract::ElementAdapter &element_adapter, if (sheet_adapter == nullptr) { throw std::invalid_argument("sheet adapter not found"); } - return sheet_adapter->sheet_cell(element_id, cell->position().column(), - cell->position().row()); + return sheet_adapter->sheet_cell(element_id, cell->position().column, + cell->position().row); } throw std::invalid_argument("unknown document path component"); diff --git a/src/odr/logger.cpp b/src/odr/logger.cpp index c346c93f..acbc8b8d 100644 --- a/src/odr/logger.cpp +++ b/src/odr/logger.cpp @@ -167,9 +167,9 @@ void Logger::print_head(std::ostream &out, Time time, LogLevel level, } if (format.location_width > 0) { - std::string file_name = + const std::string file_name = std::filesystem::path(location.file_name()).filename().string(); - std::string line_number = std::to_string(location.line()); + const std::string line_number = std::to_string(location.line()); std::stringstream location_ss; if (file_name.size() + 1 + line_number.size() > format.location_width) { if (1 + line_number.size() < format.location_width) { diff --git a/src/odr/style.cpp b/src/odr/style.cpp index bfd002ad..96a95053 100644 --- a/src/odr/style.cpp +++ b/src/odr/style.cpp @@ -41,94 +41,94 @@ std::uint32_t Color::argb() const { } void TextStyle::override(const TextStyle &other) { - if (other.font_name) { + if (other.font_name != nullptr) { font_name = other.font_name; } - if (other.font_size) { + if (other.font_size.has_value()) { font_size = other.font_size; } - if (other.font_weight) { + if (other.font_weight.has_value()) { font_weight = other.font_weight; } - if (other.font_style) { + if (other.font_style.has_value()) { font_style = other.font_style; } - if (other.font_underline) { + if (other.font_underline.has_value()) { font_underline = other.font_underline; } - if (other.font_line_through) { + if (other.font_line_through.has_value()) { font_line_through = other.font_line_through; } - if (other.font_shadow) { + if (other.font_shadow.has_value()) { font_shadow = other.font_shadow; } - if (other.font_color) { + if (other.font_color.has_value()) { font_color = other.font_color; } - if (other.background_color) { + if (other.background_color.has_value()) { background_color = other.background_color; } } void ParagraphStyle::override(const ParagraphStyle &other) { - if (other.text_align) { + if (other.text_align.has_value()) { text_align = other.text_align; } margin.override(other.margin); - if (other.line_height) { + if (other.line_height.has_value()) { line_height = other.line_height; } } void TableStyle::override(const TableStyle &other) { - if (other.width) { + if (other.width.has_value()) { width = other.width; } } void TableColumnStyle::override(const TableColumnStyle &other) { - if (other.width) { + if (other.width.has_value()) { width = other.width; } } void TableRowStyle::override(const TableRowStyle &other) { - if (other.height) { + if (other.height.has_value()) { height = other.height; } } void TableCellStyle::override(const TableCellStyle &other) { - if (other.horizontal_align) { + if (other.horizontal_align.has_value()) { horizontal_align = other.horizontal_align; } - if (other.vertical_align) { + if (other.vertical_align.has_value()) { vertical_align = other.vertical_align; } - if (other.background_color) { + if (other.background_color.has_value()) { background_color = other.background_color; } padding.override(other.padding); border.override(other.border); - if (other.text_rotation) { + if (other.text_rotation.has_value()) { text_rotation = other.text_rotation; } } void GraphicStyle::override(const GraphicStyle &other) { - if (other.stroke_width) { + if (other.stroke_width.has_value()) { stroke_width = other.stroke_width; } - if (other.stroke_color) { + if (other.stroke_color.has_value()) { stroke_color = other.stroke_color; } - if (other.fill_color) { + if (other.fill_color.has_value()) { fill_color = other.fill_color; } - if (other.vertical_align) { + if (other.vertical_align.has_value()) { vertical_align = other.vertical_align; } - if (other.text_wrap) { + if (other.text_wrap.has_value()) { text_wrap = other.text_wrap; } } diff --git a/src/odr/style.hpp b/src/odr/style.hpp index 83b7994a..16a0c005 100644 --- a/src/odr/style.hpp +++ b/src/odr/style.hpp @@ -92,31 +92,31 @@ template struct DirectionalStyle final { : right{all}, top{all}, left{all}, bottom{all} {} void override(const DirectionalStyle &other) { - if (other.right) { + if (other.right.has_value()) { right = other.right; } - if (other.top) { + if (other.top.has_value()) { top = other.top; } - if (other.left) { + if (other.left.has_value()) { left = other.left; } - if (other.bottom) { + if (other.bottom.has_value()) { bottom = other.bottom; } } void override(DirectionalStyle &&other) { - if (other.right) { + if (other.right.has_value()) { right = std::move(other.right); } - if (other.top) { + if (other.top.has_value()) { top = std::move(other.top); } - if (other.left) { + if (other.left.has_value()) { left = std::move(other.left); } - if (other.bottom) { + if (other.bottom.has_value()) { bottom = std::move(other.bottom); } } diff --git a/src/odr/table_position.cpp b/src/odr/table_position.cpp index 0e938067..3b8dca69 100644 --- a/src/odr/table_position.cpp +++ b/src/odr/table_position.cpp @@ -46,44 +46,26 @@ std::string TablePosition::to_row_string(const std::uint32_t row) { return std::to_string(row + 1); } -TablePosition::TablePosition() noexcept = default; - -TablePosition::TablePosition(const std::uint32_t column, - const std::uint32_t row) noexcept - : m_column{column}, m_row{row} {} - TablePosition::TablePosition(const std::string &s) { const auto pos = s.find_first_of("0123456789"); if (pos == std::string::npos) { throw std::invalid_argument("malformed table position " + s); } - m_row = to_row_num(s.substr(pos)); - m_column = to_column_num(s.substr(0, pos)); + row = to_row_num(s.substr(pos)); + column = to_column_num(s.substr(0, pos)); } bool TablePosition::operator==(const TablePosition &rhs) const { - return m_column == rhs.m_column && m_row == rhs.m_row; -} - -bool TablePosition::operator!=(const TablePosition &rhs) const { - return m_column != rhs.m_column || m_row != rhs.m_row; -} - -std::uint32_t TablePosition::column() const noexcept { return m_column; } - -std::uint32_t TablePosition::row() const noexcept { return m_row; } - -std::pair TablePosition::pair() const noexcept { - return {m_column, m_row}; + return column == rhs.column && row == rhs.row; } std::string TablePosition::to_string() const noexcept { - return to_column_string(m_column) + to_row_string(m_row); + return to_column_string(column) + to_row_string(row); } std::size_t TablePosition::hash() const noexcept { std::size_t result = 0; - internal::util::hash::hash_combine(result, m_row, m_column); + internal::util::hash::hash_combine(result, row, column); return result; } diff --git a/src/odr/table_position.hpp b/src/odr/table_position.hpp index af9ea627..2b810c81 100644 --- a/src/odr/table_position.hpp +++ b/src/odr/table_position.hpp @@ -6,29 +6,25 @@ namespace odr { -class TablePosition final { -public: +struct TablePosition final { static std::uint32_t to_column_num(const std::string &string); static std::uint32_t to_row_num(const std::string &string); static std::string to_column_string(std::uint32_t column); static std::string to_row_string(std::uint32_t row); - TablePosition() noexcept; - TablePosition(std::uint32_t column, std::uint32_t row) noexcept; + std::uint32_t column{0}; + std::uint32_t row{0}; + + constexpr TablePosition() noexcept = default; + constexpr TablePosition(const std::uint32_t column_, + const std::uint32_t row_) noexcept + : column{column_}, row{row_} {} explicit TablePosition(const std::string &string); bool operator==(const TablePosition &rhs) const; - bool operator!=(const TablePosition &rhs) const; - [[nodiscard]] std::uint32_t column() const noexcept; - [[nodiscard]] std::uint32_t row() const noexcept; - [[nodiscard]] std::pair pair() const noexcept; [[nodiscard]] std::string to_string() const noexcept; [[nodiscard]] std::size_t hash() const noexcept; - -private: - std::uint32_t m_column{0}; - std::uint32_t m_row{0}; }; } // namespace odr diff --git a/test/src/internal/common/table_range_test.cpp b/test/src/internal/common/table_range_test.cpp index 165e413f..823388be 100644 --- a/test/src/internal/common/table_range_test.cpp +++ b/test/src/internal/common/table_range_test.cpp @@ -8,19 +8,19 @@ using namespace odr::internal; TEST(TableRange, default) { const TableRange tr; - EXPECT_EQ(0, tr.from().column()); - EXPECT_EQ(0, tr.from().row()); - EXPECT_EQ(0, tr.to().column()); - EXPECT_EQ(0, tr.to().row()); + EXPECT_EQ(0, tr.from().column); + EXPECT_EQ(0, tr.from().row); + EXPECT_EQ(0, tr.to().column); + EXPECT_EQ(0, tr.to().row); EXPECT_EQ("A1:A1", tr.to_string()); } TEST(TableRange, string1) { const std::string input = "A1:C55"; const TableRange tr(input); - EXPECT_EQ(0, tr.from().column()); - EXPECT_EQ(0, tr.from().row()); - EXPECT_EQ(2, tr.to().column()); - EXPECT_EQ(54, tr.to().row()); + EXPECT_EQ(0, tr.from().column); + EXPECT_EQ(0, tr.from().row); + EXPECT_EQ(2, tr.to().column); + EXPECT_EQ(54, tr.to().row); EXPECT_EQ(input, tr.to_string()); } diff --git a/test/src/table_position_test.cpp b/test/src/table_position_test.cpp index 21144bea..2bfe8358 100644 --- a/test/src/table_position_test.cpp +++ b/test/src/table_position_test.cpp @@ -8,46 +8,46 @@ using namespace odr; TEST(TablePosition, default) { const TablePosition tp; - EXPECT_EQ(0, tp.row()); - EXPECT_EQ(0, tp.column()); + EXPECT_EQ(0, tp.row); + EXPECT_EQ(0, tp.column); EXPECT_EQ("A1", tp.to_string()); } TEST(TablePosition, direct) { const TablePosition tp(2, 1); - EXPECT_EQ(2, tp.column()); - EXPECT_EQ(1, tp.row()); + EXPECT_EQ(2, tp.column); + EXPECT_EQ(1, tp.row); EXPECT_EQ("C2", tp.to_string()); } TEST(TablePosition, string1) { const std::string input = "A1"; const TablePosition tp(input); - EXPECT_EQ(0, tp.column()); - EXPECT_EQ(0, tp.row()); + EXPECT_EQ(0, tp.column); + EXPECT_EQ(0, tp.row); EXPECT_EQ(input, tp.to_string()); } TEST(TablePosition, string2) { const std::string input = "AA11"; const TablePosition tp(input); - EXPECT_EQ(26, tp.column()); - EXPECT_EQ(10, tp.row()); + EXPECT_EQ(26, tp.column); + EXPECT_EQ(10, tp.row); EXPECT_EQ(input, tp.to_string()); } TEST(TablePosition, string3) { const std::string input = "ZZ1"; const TablePosition tp(input); - EXPECT_EQ(701, tp.column()); - EXPECT_EQ(0, tp.row()); + EXPECT_EQ(701, tp.column); + EXPECT_EQ(0, tp.row); EXPECT_EQ(input, tp.to_string()); } TEST(TablePosition, string4) { const std::string input = "AAA1"; const TablePosition tp(input); - EXPECT_EQ(702, tp.column()); - EXPECT_EQ(0, tp.row()); + EXPECT_EQ(702, tp.column); + EXPECT_EQ(0, tp.row); EXPECT_EQ(input, tp.to_string()); } From a78d14dd6a9045f8abdbdf190761db086077453a Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Wed, 31 Dec 2025 12:26:02 +0100 Subject: [PATCH 2/3] fix build --- src/odr/document_element.hpp | 2 +- src/odr/internal/abstract/document.hpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/odr/document_element.hpp b/src/odr/document_element.hpp index 7e55ab04..8f8e4e5c 100644 --- a/src/odr/document_element.hpp +++ b/src/odr/document_element.hpp @@ -7,7 +7,7 @@ #include namespace odr { -class TablePosition; +struct TablePosition; struct TableDimensions; class DocumentPath; class File; diff --git a/src/odr/internal/abstract/document.hpp b/src/odr/internal/abstract/document.hpp index 8aa03e6f..6ba013d0 100644 --- a/src/odr/internal/abstract/document.hpp +++ b/src/odr/internal/abstract/document.hpp @@ -13,7 +13,7 @@ enum class ValueType; enum class AnchorType; struct PageLayout; struct TableDimensions; -class TablePosition; +struct TablePosition; struct TableStyle; struct TableColumnStyle; struct TableRowStyle; From 04a33fdfbb46dfa9cbcf6e3f8c6fead9941d7256 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Wed, 31 Dec 2025 12:29:40 +0100 Subject: [PATCH 3/3] fix build --- src/odr/http_server.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/odr/http_server.cpp b/src/odr/http_server.cpp index 3205279b..74d1091c 100644 --- a/src/odr/http_server.cpp +++ b/src/odr/http_server.cpp @@ -7,7 +7,7 @@ #include #include -#include +#include #include namespace odr {