From bb04205dacaac7debed6dae29dc0816f7165cc01 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Mon, 2 Jul 2018 21:33:27 +1200 Subject: [PATCH 01/23] workbook parameterised constructors Issue #298 - all 4 are simply duplicating existing behaviour, but perhaps we can get a more optimal version in future - istream ctor is intended as an extension point that can then be used to create free/static functions to work with any future data source (vector), while the path ctor is a convenience function for the common case (from file) --- include/xlnt/workbook/workbook.hpp | 25 +++++++++++++++++++++++++ source/workbook/workbook.cpp | 28 +++++++++++++++++++++++++--- 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/include/xlnt/workbook/workbook.hpp b/include/xlnt/workbook/workbook.hpp index b5570c49..813b6853 100644 --- a/include/xlnt/workbook/workbook.hpp +++ b/include/xlnt/workbook/workbook.hpp @@ -130,6 +130,31 @@ public: /// workbook(); + /// + /// load the xlsx file at path + /// + workbook(const xlnt::path &file); + + /// + /// load the encrpyted xlsx file at path + /// + workbook(const xlnt::path &file, const std::string& password); + + /// + /// construct the workbook from any data stream where the data is the binary form of a workbook + /// + workbook(std::istream & data); + + /// + /// construct the workbook from any data stream where the data is the binary form of an encrypted workbook + /// + workbook(std::istream &data, const std::string& password); + + /// + /// Move constructor. Constructs a workbook from existing workbook, other. + /// + workbook(workbook &&other); + /// /// Move constructor. Constructs a workbook from existing workbook, other. /// diff --git a/source/workbook/workbook.cpp b/source/workbook/workbook.cpp index b783749a..77d628c8 100644 --- a/source/workbook/workbook.cpp +++ b/source/workbook/workbook.cpp @@ -500,6 +500,30 @@ workbook::workbook() swap(wb_template); } +workbook::workbook(const xlnt::path &file) +{ + *this = empty(); + load(file); +} + +workbook::workbook(const xlnt::path &file, const std::string &password) +{ + *this = empty(); + load(file, password); +} + +workbook::workbook(std::istream &data) +{ + *this = empty(); + load(data); +} + +workbook::workbook(std::istream &data, const std::string &password) +{ + *this = empty(); + load(data, password); +} + workbook::workbook(detail::workbook_impl *impl) : d_(impl) { @@ -1178,9 +1202,7 @@ workbook::workbook(const workbook &other) d_->stylesheet_.get().parent = this; } -workbook::~workbook() -{ -} +workbook::~workbook() = default; bool workbook::has_theme() const { From fb195e277751722388374d466a55a95068afab6d Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Tue, 3 Jul 2018 11:09:33 +1200 Subject: [PATCH 02/23] fix accidentally copied ctor declaration --- include/xlnt/workbook/workbook.hpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/include/xlnt/workbook/workbook.hpp b/include/xlnt/workbook/workbook.hpp index 813b6853..7b98f622 100644 --- a/include/xlnt/workbook/workbook.hpp +++ b/include/xlnt/workbook/workbook.hpp @@ -155,11 +155,6 @@ public: /// workbook(workbook &&other); - /// - /// Move constructor. Constructs a workbook from existing workbook, other. - /// - workbook(workbook &&other); - /// /// Copy constructor. Constructs this workbook from existing workbook, other. /// From 688c8c7f33e6d32183b39a5e49aed4155b4a1fee Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Tue, 3 Jul 2018 11:10:47 +1200 Subject: [PATCH 03/23] add column_index to cell to be consistent with cell_reference Issue #298 The consistency argument is a good one in my opinion --- include/xlnt/cell/cell.hpp | 5 +++++ source/cell/cell.cpp | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/include/xlnt/cell/cell.hpp b/include/xlnt/cell/cell.hpp index 74a5b6cb..f77f6d31 100644 --- a/include/xlnt/cell/cell.hpp +++ b/include/xlnt/cell/cell.hpp @@ -240,6 +240,11 @@ public: /// column_t column() const; + /// + /// Returns the numeric index (A == 1) of the column of this cell. + /// + column_t::index_t column_index() const; + /// /// Returns the row of this cell. /// diff --git a/source/cell/cell.cpp b/source/cell/cell.cpp index 22b10f49..4426a5dc 100644 --- a/source/cell/cell.cpp +++ b/source/cell/cell.cpp @@ -314,6 +314,11 @@ column_t cell::column() const return d_->column_; } +column_t::index_t cell::column_index() const +{ + return d_->column_.index; +} + void cell::merged(bool merged) { d_->is_merged_ = merged; From 4eecf84713f6a0b980f39093e442aea599cae252 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Tue, 3 Jul 2018 13:16:33 +1200 Subject: [PATCH 04/23] add simple test for cell::column_index --- tests/cell/cell_test_suite.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/cell/cell_test_suite.hpp b/tests/cell/cell_test_suite.hpp index 04294818..972de016 100644 --- a/tests/cell/cell_test_suite.hpp +++ b/tests/cell/cell_test_suite.hpp @@ -123,6 +123,7 @@ private: xlnt_assert(cell.data_type() == xlnt::cell::type::empty); xlnt_assert(cell.column() == "A"); + xlnt_assert(cell.column_index() == 1); xlnt_assert(cell.row() == 1); xlnt_assert(cell.reference() == "A1"); xlnt_assert(!cell.has_value()); From a8fa6637fe5d3631851e34dbc6d679c065babaf0 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Tue, 3 Jul 2018 14:05:07 +1200 Subject: [PATCH 05/23] fix workbook::operator== always returning false, add tests for workbook::load workbook::operator== was comparing the value of the raw pointer held by two std::unique_ptr's. By definition, this is always false in a well behaved program (if it's true, things go bang...). This then led to adding equality operators to nearly every other struct/class in xlnt to support workbook::operator== workbook::load and the non-default ctors for loading data from a file are tested using the now functional equality operator NOTE: a large number of copy ctors need updates/fixing. Many should be defaulted --- include/xlnt/packaging/ext_list.hpp | 8 +- include/xlnt/packaging/manifest.hpp | 2 + include/xlnt/styles/conditional_format.hpp | 5 ++ include/xlnt/utils/optional.hpp | 10 ++- include/xlnt/utils/variant.hpp | 2 + .../xlnt/workbook/calculation_properties.hpp | 6 ++ include/xlnt/workbook/named_range.hpp | 2 + include/xlnt/workbook/theme.hpp | 5 ++ include/xlnt/workbook/workbook_view.hpp | 13 ++++ include/xlnt/worksheet/column_properties.hpp | 9 +++ include/xlnt/worksheet/header_footer.hpp | 2 + include/xlnt/worksheet/page_margins.hpp | 2 + include/xlnt/worksheet/page_setup.hpp | 2 + include/xlnt/worksheet/phonetic_pr.hpp | 2 + include/xlnt/worksheet/print_options.hpp | 9 +++ include/xlnt/worksheet/row_properties.hpp | 10 +++ .../worksheet/sheet_format_properties.hpp | 8 ++ include/xlnt/worksheet/sheet_pr.hpp | 13 ++++ source/detail/implementations/cell_impl.hpp | 16 ++++ .../conditional_format_impl.hpp | 15 +++- .../detail/implementations/hyperlink_impl.hpp | 7 ++ source/detail/implementations/style_impl.hpp | 24 ++++++ source/detail/implementations/stylesheet.hpp | 19 +++++ .../detail/implementations/workbook_impl.hpp | 32 ++++++++ .../detail/implementations/worksheet_impl.hpp | 27 +++++++ source/packaging/ext_list.cpp | 5 ++ source/packaging/manifest.cpp | 7 ++ source/utils/variant.cpp | 24 +++++- source/workbook/named_range.cpp | 6 ++ source/workbook/workbook.cpp | 4 +- source/worksheet/header_footer.cpp | 13 ++++ source/worksheet/page_margins.cpp | 9 +++ source/worksheet/page_setup.cpp | 11 +++ source/worksheet/phonetic_pr.cpp | 7 ++ tests/workbook/workbook_test_suite.hpp | 77 ++++++++++++++----- 35 files changed, 386 insertions(+), 27 deletions(-) diff --git a/include/xlnt/packaging/ext_list.hpp b/include/xlnt/packaging/ext_list.hpp index 9634daa4..65c98945 100644 --- a/include/xlnt/packaging/ext_list.hpp +++ b/include/xlnt/packaging/ext_list.hpp @@ -46,7 +46,6 @@ class XLNT_API ext_list public: struct ext { - public: ext(xml::parser &parser, const std::string& ns); ext(const uri& ID, const std::string& serialised); void serialise(xml::serializer &serialiser, const std::string& ns); @@ -66,7 +65,14 @@ public: const std::vector &extensions() const; + bool operator==(const ext_list& rhs) const; private: std::vector extensions_; }; + +inline bool operator==(const ext_list::ext& lhs, const ext_list::ext& rhs) +{ + return lhs.extension_ID_ == rhs.extension_ID_ + && lhs.serialised_value_ == rhs.serialised_value_; +} } // namespace xlnt \ No newline at end of file diff --git a/include/xlnt/packaging/manifest.hpp b/include/xlnt/packaging/manifest.hpp index f7c6ca79..03d19ded 100644 --- a/include/xlnt/packaging/manifest.hpp +++ b/include/xlnt/packaging/manifest.hpp @@ -172,6 +172,8 @@ public: /// void unregister_override_type(const path &part); + bool operator==(const manifest &other) const; + private: /// /// Returns the lowest rId for the given part that hasn't already been registered. diff --git a/include/xlnt/styles/conditional_format.hpp b/include/xlnt/styles/conditional_format.hpp index db3782d9..0260b6b5 100644 --- a/include/xlnt/styles/conditional_format.hpp +++ b/include/xlnt/styles/conditional_format.hpp @@ -53,6 +53,11 @@ public: static condition text_contains(const std::string &start); static condition text_does_not_contain(const std::string &start); + bool operator==(const condition& rhs) const + { + return text_comparand_ == rhs.text_comparand_; + } + private: friend class detail::xlsx_producer; diff --git a/include/xlnt/utils/optional.hpp b/include/xlnt/utils/optional.hpp index 3df95451..143c38ec 100644 --- a/include/xlnt/utils/optional.hpp +++ b/include/xlnt/utils/optional.hpp @@ -125,8 +125,14 @@ public: /// bool operator==(const optional &other) const { - return has_value_ == other.has_value_ - && (!has_value_ || value_ == other.value_); + if (has_value_ != other.has_value_) { + return false; + } + if (!has_value_) + { + return true; + } + return value_ == other.value_; } private: diff --git a/include/xlnt/utils/variant.hpp b/include/xlnt/utils/variant.hpp index e2d3f5e8..a7c4c306 100644 --- a/include/xlnt/utils/variant.hpp +++ b/include/xlnt/utils/variant.hpp @@ -162,6 +162,8 @@ public: /// type value_type() const; + bool operator==(const variant &rhs) const; + private: type type_; std::vector vector_value_; diff --git a/include/xlnt/workbook/calculation_properties.hpp b/include/xlnt/workbook/calculation_properties.hpp index e0335f2a..22026641 100644 --- a/include/xlnt/workbook/calculation_properties.hpp +++ b/include/xlnt/workbook/calculation_properties.hpp @@ -46,4 +46,10 @@ public: bool concurrent_calc; }; +inline bool operator==(const calculation_properties &lhs, const calculation_properties &rhs) +{ + return lhs.calc_id == rhs.calc_id + && lhs.concurrent_calc == rhs.concurrent_calc; +} + } // namespace xlnt diff --git a/include/xlnt/workbook/named_range.hpp b/include/xlnt/workbook/named_range.hpp index 86f8497f..bc90b201 100644 --- a/include/xlnt/workbook/named_range.hpp +++ b/include/xlnt/workbook/named_range.hpp @@ -76,6 +76,8 @@ public: /// named_range &operator=(const named_range &other); + bool operator==(const named_range &rhs) const; + private: /// /// The name of this named range. diff --git a/include/xlnt/workbook/theme.hpp b/include/xlnt/workbook/theme.hpp index 70bf093a..96b0434e 100644 --- a/include/xlnt/workbook/theme.hpp +++ b/include/xlnt/workbook/theme.hpp @@ -33,6 +33,11 @@ namespace xlnt { /// class XLNT_API theme { +public: + bool operator==(const theme&) const + { + return true; + } }; } // namespace xlnt diff --git a/include/xlnt/workbook/workbook_view.hpp b/include/xlnt/workbook/workbook_view.hpp index e79384c9..6d1efc68 100644 --- a/include/xlnt/workbook/workbook_view.hpp +++ b/include/xlnt/workbook/workbook_view.hpp @@ -102,4 +102,17 @@ public: optional y_window; }; +inline bool operator==(const workbook_view &lhs, const workbook_view &rhs) +{ + return lhs.active_tab == rhs.active_tab + && lhs.auto_filter_date_grouping == rhs.auto_filter_date_grouping + && lhs.first_sheet == rhs.first_sheet + && lhs.minimized == rhs.minimized + && lhs.show_horizontal_scroll == rhs.show_horizontal_scroll + && lhs.show_sheet_tabs == rhs.show_sheet_tabs + && lhs.show_vertical_scroll == rhs.show_vertical_scroll + && lhs.tab_ratio == rhs.tab_ratio + && lhs.visible == rhs.visible; +} + } // namespace xlnt diff --git a/include/xlnt/worksheet/column_properties.hpp b/include/xlnt/worksheet/column_properties.hpp index 36a8d386..32549933 100644 --- a/include/xlnt/worksheet/column_properties.hpp +++ b/include/xlnt/worksheet/column_properties.hpp @@ -63,4 +63,13 @@ public: bool hidden = false; }; +inline bool operator==(const column_properties &lhs, const column_properties &rhs) +{ + return lhs.width == rhs.width + && lhs.custom_width == rhs.custom_width + && lhs.style == rhs.style + && lhs.best_fit == rhs.best_fit + && lhs.hidden == rhs.hidden; +} + } // namespace xlnt diff --git a/include/xlnt/worksheet/header_footer.hpp b/include/xlnt/worksheet/header_footer.hpp index 8633c750..3b70b1fa 100644 --- a/include/xlnt/worksheet/header_footer.hpp +++ b/include/xlnt/worksheet/header_footer.hpp @@ -312,6 +312,8 @@ public: /// rich_text even_footer(location where) const; + bool operator==(const header_footer &rhs) const; + private: bool align_with_margins_ = false; bool different_odd_even_ = false; diff --git a/include/xlnt/worksheet/page_margins.hpp b/include/xlnt/worksheet/page_margins.hpp index a4118e52..7f38f8bb 100644 --- a/include/xlnt/worksheet/page_margins.hpp +++ b/include/xlnt/worksheet/page_margins.hpp @@ -99,6 +99,8 @@ public: /// void footer(double footer); + bool operator==(const page_margins &rhs) const; + private: /// /// The top margin diff --git a/include/xlnt/worksheet/page_setup.hpp b/include/xlnt/worksheet/page_setup.hpp index afc2f31f..03d6a01d 100644 --- a/include/xlnt/worksheet/page_setup.hpp +++ b/include/xlnt/worksheet/page_setup.hpp @@ -172,6 +172,8 @@ public: /// xlnt::optional vertical_dpi_; + bool operator==(const page_setup &rhs) const; + private: /// /// The break diff --git a/include/xlnt/worksheet/phonetic_pr.hpp b/include/xlnt/worksheet/phonetic_pr.hpp index 3ac802e0..592cb8e8 100644 --- a/include/xlnt/worksheet/phonetic_pr.hpp +++ b/include/xlnt/worksheet/phonetic_pr.hpp @@ -143,6 +143,8 @@ public: /// static align alignment_from_string(const std::string &str); + bool operator==(const phonetic_pr &rhs) const; + private: /// /// zero based index into style sheet font record. diff --git a/include/xlnt/worksheet/print_options.hpp b/include/xlnt/worksheet/print_options.hpp index bcc8ca47..820b6966 100644 --- a/include/xlnt/worksheet/print_options.hpp +++ b/include/xlnt/worksheet/print_options.hpp @@ -55,4 +55,13 @@ struct XLNT_API print_options /// optional vertical_centered; }; + +inline bool operator==(const print_options& lhs, const print_options &rhs) +{ + return lhs.grid_lines_set == rhs.grid_lines_set + && lhs.horizontal_centered == rhs.horizontal_centered + && lhs.print_grid_lines == rhs.print_grid_lines + && lhs.print_headings == rhs.print_headings + && lhs.vertical_centered == rhs.vertical_centered; +} } // namespace xlnt \ No newline at end of file diff --git a/include/xlnt/worksheet/row_properties.hpp b/include/xlnt/worksheet/row_properties.hpp index 2e724f20..d78c4313 100644 --- a/include/xlnt/worksheet/row_properties.hpp +++ b/include/xlnt/worksheet/row_properties.hpp @@ -65,4 +65,14 @@ public: optional style; }; +inline bool operator==(const row_properties &lhs, const row_properties &rhs) +{ + return lhs.height == rhs.height + && lhs.dy_descent == rhs.dy_descent + && lhs.custom_height == rhs.custom_height + && lhs.hidden == rhs.hidden + && lhs.custom_format == rhs.custom_format + && lhs.style == rhs.style; +} + } // namespace xlnt diff --git a/include/xlnt/worksheet/sheet_format_properties.hpp b/include/xlnt/worksheet/sheet_format_properties.hpp index 82be43ca..c71eefbe 100644 --- a/include/xlnt/worksheet/sheet_format_properties.hpp +++ b/include/xlnt/worksheet/sheet_format_properties.hpp @@ -55,4 +55,12 @@ public: optional dy_descent; }; +inline bool operator==(const sheet_format_properties &lhs, const sheet_format_properties &rhs) +{ + return lhs.base_col_width == rhs.base_col_width + && lhs.default_column_width == rhs.default_column_width + && lhs.default_row_height == rhs.default_row_height + && lhs.dy_descent == rhs.dy_descent; +} + } // namespace xlnt diff --git a/include/xlnt/worksheet/sheet_pr.hpp b/include/xlnt/worksheet/sheet_pr.hpp index 86097274..417b335c 100644 --- a/include/xlnt/worksheet/sheet_pr.hpp +++ b/include/xlnt/worksheet/sheet_pr.hpp @@ -78,4 +78,17 @@ struct XLNT_API sheet_pr /// optional enable_format_condition_calculation; }; + +inline bool operator==(const sheet_pr &lhs, const sheet_pr &rhs) +{ + return lhs.sync_horizontal == rhs.sync_horizontal + && lhs.sync_vertical == rhs.sync_vertical + && lhs.sync_ref == rhs.sync_ref + && lhs.transition_evaluation == rhs.transition_evaluation + && lhs.transition_entry == rhs.transition_entry + && lhs.published == rhs.published + && lhs.code_name == rhs.code_name + && lhs.filter_mode == rhs.filter_mode + && lhs.enable_format_condition_calculation == rhs.enable_format_condition_calculation; +} } // namespace xlnt \ No newline at end of file diff --git a/source/detail/implementations/cell_impl.hpp b/source/detail/implementations/cell_impl.hpp index 4553d133..ed1833ed 100644 --- a/source/detail/implementations/cell_impl.hpp +++ b/source/detail/implementations/cell_impl.hpp @@ -61,5 +61,21 @@ struct cell_impl optional comment_; }; +inline bool operator==(const cell_impl &lhs, const cell_impl &rhs) +{ + // not comparing parent + return lhs.type_ == rhs.type_ + && lhs.column_ == rhs.column_ + && lhs.row_ == rhs.row_ + && lhs.is_merged_ == rhs.is_merged_ + && lhs.value_text_ == rhs.value_text_ + && lhs.value_numeric_ == rhs.value_numeric_ + && lhs.formula_ == rhs.formula_ + && lhs.hyperlink_ == rhs.hyperlink_ + // comparing pointers is unlikely to be what is wanted + /*&& lhs.format_ == rhs.format_ + && lhs.comment_ == rhs.comment_*/; +} + } // namespace detail } // namespace xlnt diff --git a/source/detail/implementations/conditional_format_impl.hpp b/source/detail/implementations/conditional_format_impl.hpp index 7ea9f3a9..2542d97d 100644 --- a/source/detail/implementations/conditional_format_impl.hpp +++ b/source/detail/implementations/conditional_format_impl.hpp @@ -20,8 +20,21 @@ struct worksheet_impl; struct conditional_format_impl { stylesheet *parent; + worksheet_impl *target_sheet; + + bool operator==(const conditional_format_impl& rhs) const + { + // not comparing parent or target sheet + return target_range == rhs.target_range + && priority == rhs.priority + && differential_format_id == rhs.differential_format_id + && when == rhs.when + && border_id == rhs.border_id + && fill_id == rhs.fill_id + && font_id == rhs.font_id; + } + range_reference target_range; - worksheet_impl *target_sheet; std::size_t priority; std::size_t differential_format_id; diff --git a/source/detail/implementations/hyperlink_impl.hpp b/source/detail/implementations/hyperlink_impl.hpp index 8f28670f..84172986 100644 --- a/source/detail/implementations/hyperlink_impl.hpp +++ b/source/detail/implementations/hyperlink_impl.hpp @@ -37,5 +37,12 @@ struct hyperlink_impl std::string display; }; +inline bool operator==(const hyperlink_impl &lhs, const hyperlink_impl &rhs) +{ + return lhs.relationship == rhs.relationship + && lhs.tooltip == rhs.tooltip + && lhs.display == rhs.display; +} + } // namespace detail } // namespace xlnt diff --git a/source/detail/implementations/style_impl.hpp b/source/detail/implementations/style_impl.hpp index b79785f4..c05ec889 100644 --- a/source/detail/implementations/style_impl.hpp +++ b/source/detail/implementations/style_impl.hpp @@ -22,6 +22,30 @@ struct style_impl { stylesheet *parent; + bool operator==(const style_impl& rhs) const + { + return name == rhs.name + && formatting_record_id == rhs.formatting_record_id + && custom_builtin == rhs.custom_builtin + && hidden_style == rhs.hidden_style + && builtin_id == rhs.builtin_id + && outline_style == rhs.outline_style + && alignment_id == rhs.alignment_id + && alignment_applied == rhs.alignment_applied + && border_id == rhs.border_id + && border_applied == rhs.border_applied + && fill_id == rhs.fill_id + && fill_applied == rhs.fill_applied + && font_id == rhs.font_id + && font_applied == rhs.font_applied + && number_format_id == rhs.number_format_id + && number_format_applied == number_format_applied + && protection_id == rhs.protection_id + && protection_applied == rhs.protection_applied + && pivot_button_ == rhs.pivot_button_ + && quote_prefix_ == rhs.quote_prefix_; + } + std::string name; std::size_t formatting_record_id; diff --git a/source/detail/implementations/stylesheet.hpp b/source/detail/implementations/stylesheet.hpp index 196933c0..8ccc5976 100644 --- a/source/detail/implementations/stylesheet.hpp +++ b/source/detail/implementations/stylesheet.hpp @@ -527,6 +527,25 @@ struct stylesheet } workbook *parent; + + bool operator==(const stylesheet& rhs) const + { + // no equality on parent as there is only 1 stylesheet per borkbook hence would always be false + return garbage_collection_enabled == rhs.garbage_collection_enabled + && known_fonts_enabled == rhs.known_fonts_enabled + && conditional_format_impls == rhs.conditional_format_impls + && format_impls == rhs.format_impls + && style_impls == rhs.style_impls + && style_names == rhs.style_names + && default_slicer_style == rhs.default_slicer_style + && alignments == rhs.alignments + && borders == rhs.borders + && fills == rhs.fills + && fonts == rhs.fonts + && number_formats == rhs.number_formats + && protections == rhs.protections + && colors == rhs.colors; + } bool garbage_collection_enabled = true; bool known_fonts_enabled = false; diff --git a/source/detail/implementations/workbook_impl.hpp b/source/detail/implementations/workbook_impl.hpp index 3ecdd6a8..38d099cb 100644 --- a/source/detail/implementations/workbook_impl.hpp +++ b/source/detail/implementations/workbook_impl.hpp @@ -89,6 +89,30 @@ struct workbook_impl return *this; } + bool operator==(const workbook_impl &other) + { + return active_sheet_index_ == other.active_sheet_index_ + && worksheets_ == other.worksheets_ + && shared_strings_ == other.shared_strings_ + && stylesheet_ == other.stylesheet_ + && base_date_ == other.base_date_ + && title_ == other.title_ + && manifest_ == other.manifest_ + && theme_ == other.theme_ + && images_ == other.images_ + && core_properties_ == other.core_properties_ + && extended_properties_ == other.extended_properties_ + && custom_properties_ == other.custom_properties_ + && sheet_title_rel_id_map_ == other.sheet_title_rel_id_map_ + && view_ == other.view_ + && code_name_ == other.code_name_ + && file_version_ == other.file_version_ + && calculation_properties_ == other.calculation_properties_ + && abs_path_ == other.abs_path_ + && arch_id_flags_ == other.arch_id_flags_ + && extensions_ == other.extensions_; + } + optional active_sheet_index_; std::list worksheets_; @@ -118,6 +142,14 @@ struct workbook_impl std::size_t last_edited; std::size_t lowest_edited; std::size_t rup_build; + + bool operator==(const file_version_t& rhs) const + { + return app_name == rhs.app_name + && last_edited == rhs.last_edited + && lowest_edited == rhs.lowest_edited + && rup_build == rhs.rup_build; + } }; optional file_version_; diff --git a/source/detail/implementations/worksheet_impl.hpp b/source/detail/implementations/worksheet_impl.hpp index aca394bd..cb3be2fb 100644 --- a/source/detail/implementations/worksheet_impl.hpp +++ b/source/detail/implementations/worksheet_impl.hpp @@ -99,6 +99,33 @@ struct worksheet_impl workbook *parent_; + bool operator==(const worksheet_impl& rhs) const + { + return id_ == rhs.id_ + && title_ == rhs.title_ + && format_properties_ == rhs.format_properties_ + && column_properties_ == rhs.column_properties_ + && row_properties_ == rhs.row_properties_ + && cell_map_ == rhs.cell_map_ + && page_setup_ == rhs.page_setup_ + && auto_filter_ == rhs.auto_filter_ + && page_margins_ == rhs.page_margins_ + && merged_cells_ == rhs.merged_cells_ + && named_ranges_ == rhs.named_ranges_ + && phonetic_properties_ == rhs.phonetic_properties_ + && header_footer_ == rhs.header_footer_ + && print_title_cols_ == rhs.print_title_cols_ + && print_title_rows_ == rhs.print_title_rows_ + && print_area_ == rhs.print_area_ + && views_ == rhs.views_ + && column_breaks_ == rhs.column_breaks_ + && row_breaks_ == rhs.row_breaks_ + && comments_ == rhs.comments_ + && print_options_ == rhs.print_options_ + && sheet_properties_ == rhs.sheet_properties_ + && extension_list_ == rhs.extension_list_; + } + std::size_t id_; std::string title_; diff --git a/source/packaging/ext_list.cpp b/source/packaging/ext_list.cpp index 5a109691..26988655 100644 --- a/source/packaging/ext_list.cpp +++ b/source/packaging/ext_list.cpp @@ -137,4 +137,9 @@ const std::vector &ext_list::extensions() const return extensions_; } +bool ext_list::operator==(const ext_list &rhs) const +{ + return std::equal(extensions_.begin(), extensions_.end(), rhs.extensions_.begin(), rhs.extensions_.end()); +} + } \ No newline at end of file diff --git a/source/packaging/manifest.cpp b/source/packaging/manifest.cpp index e6ecf582..d11c29eb 100644 --- a/source/packaging/manifest.cpp +++ b/source/packaging/manifest.cpp @@ -337,4 +337,11 @@ std::string manifest::override_type(const xlnt::path &part) const return override_content_types_.at(part); } +bool manifest::operator==(const manifest &other) const +{ + return default_content_types_ == other.default_content_types_ + && override_content_types_ == other.override_content_types_ + && relationships_ == other.relationships_; +} + } // namespace xlnt diff --git a/source/utils/variant.cpp b/source/utils/variant.cpp index 5743e4e5..44d349be 100644 --- a/source/utils/variant.cpp +++ b/source/utils/variant.cpp @@ -41,7 +41,7 @@ variant::variant(const char *value) : variant(std::string(value)) { } -variant::variant(int value) +variant::variant(std::int32_t value) : type_(type::i4), i4_value_(value) { @@ -125,6 +125,28 @@ variant::variant(const std::vector &value) } } +bool variant::operator==(const variant &rhs) const +{ + if (type_ != rhs.type_) + { + return false; + } + switch (type_) + { + case type::vector: + return vector_value_ == rhs.vector_value_; + case type::i4: + case type::boolean: + return i4_value_ == rhs.i4_value_; + case type::date: + case type::lpstr: + return lpstr_value_ == rhs.lpstr_value_; + case type::null: + return true; + } + return false; +} + bool variant::is(type t) const { return type_ == t; diff --git a/source/workbook/named_range.cpp b/source/workbook/named_range.cpp index d9927f94..9bf19732 100644 --- a/source/workbook/named_range.cpp +++ b/source/workbook/named_range.cpp @@ -123,4 +123,10 @@ named_range &named_range::operator=(const named_range &other) return *this; } +bool named_range::operator==(const named_range &rhs) const +{ + return name_ == rhs.name_ + && targets_ == rhs.targets_; +} + } // namespace xlnt diff --git a/source/workbook/workbook.cpp b/source/workbook/workbook.cpp index 77d628c8..55c460c4 100644 --- a/source/workbook/workbook.cpp +++ b/source/workbook/workbook.cpp @@ -1133,12 +1133,12 @@ void workbook::clear() bool workbook::operator==(const workbook &rhs) const { - return d_.get() == rhs.d_.get(); + return *d_ == *rhs.d_; } bool workbook::operator!=(const workbook &rhs) const { - return d_.get() != rhs.d_.get(); + return !operator==(rhs); } void workbook::swap(workbook &right) diff --git a/source/worksheet/header_footer.cpp b/source/worksheet/header_footer.cpp index 7fa76b89..b852da60 100644 --- a/source/worksheet/header_footer.cpp +++ b/source/worksheet/header_footer.cpp @@ -295,4 +295,17 @@ rich_text header_footer::even_footer(location where) const return even_footers_.at(where); } +bool header_footer::operator==(const header_footer &rhs) const +{ + return align_with_margins_ == rhs.align_with_margins_ + && different_odd_even_ == rhs.different_odd_even_ + && scale_with_doc_ == rhs.scale_with_doc_ + && odd_headers_ == rhs.odd_headers_ + && even_headers_ == rhs.even_headers_ + && first_headers_ == rhs.first_headers_ + && odd_footers_ == rhs.odd_footers_ + && even_footers_ == rhs.even_footers_ + && first_footers_ == rhs.first_footers_; +} + } // namespace xlnt diff --git a/source/worksheet/page_margins.cpp b/source/worksheet/page_margins.cpp index 82e4ba19..77faec5b 100644 --- a/source/worksheet/page_margins.cpp +++ b/source/worksheet/page_margins.cpp @@ -89,4 +89,13 @@ void page_margins::footer(double footer) footer_ = footer; } +bool page_margins::operator==(const page_margins &rhs) const +{ + return top_ == rhs.top_ + && left_ == rhs.left_ + && right_ == rhs.right_ + && header_ == rhs.header_ + && footer_ == rhs.footer_; +} + } // namespace xlnt diff --git a/source/worksheet/page_setup.cpp b/source/worksheet/page_setup.cpp index de7f9ec8..c832d5f4 100644 --- a/source/worksheet/page_setup.cpp +++ b/source/worksheet/page_setup.cpp @@ -106,4 +106,15 @@ double page_setup::scale() const return scale_; } +bool page_setup::operator==(const page_setup &rhs) const +{ + return break_ == rhs.break_ + && sheet_state_ == rhs.sheet_state_ + && paper_size_ == rhs.paper_size_ + && fit_to_page_ == rhs.fit_to_page_ + && fit_to_height_ == rhs.fit_to_height_ + && fit_to_width_ == rhs.fit_to_width_ + && scale_ == rhs.scale_; +} + } // namespace xlnt diff --git a/source/worksheet/phonetic_pr.cpp b/source/worksheet/phonetic_pr.cpp index f9e6b49f..97efe752 100644 --- a/source/worksheet/phonetic_pr.cpp +++ b/source/worksheet/phonetic_pr.cpp @@ -140,4 +140,11 @@ phonetic_pr::align phonetic_pr::alignment_from_string(const std::string &str) return align::no_control; } +bool phonetic_pr::operator==(const phonetic_pr &rhs) const +{ + return font_id_ == rhs.font_id_ + && type_ == rhs.type_ + && alignment_ == rhs.alignment_; +} + } // namespace xlnt \ No newline at end of file diff --git a/tests/workbook/workbook_test_suite.hpp b/tests/workbook/workbook_test_suite.hpp index cbd4bda3..9fe60ac1 100644 --- a/tests/workbook/workbook_test_suite.hpp +++ b/tests/workbook/workbook_test_suite.hpp @@ -26,9 +26,10 @@ #include #include +#include +#include #include #include -#include class workbook_test_suite : public test_suite { @@ -57,6 +58,7 @@ public: register_test(test_clear); register_test(test_comparison); register_test(test_id_gen); + register_test(test_load_file); } void test_active_sheet() @@ -105,12 +107,12 @@ public: xlnt_assert_equals(wb.sheet_titles().at(1), "Active"); xlnt_assert_equals(wb.sheet_by_index(1).cell("B3").value(), 2); } - + void test_remove_sheet() { xlnt::workbook wb, wb2; auto new_sheet = wb.create_sheet(0); - new_sheet.title("removed"); + new_sheet.title("removed"); wb.remove_sheet(new_sheet); xlnt_assert(!wb.contains("removed")); xlnt_assert_throws(wb.remove_sheet(wb2.active_sheet()), std::runtime_error); @@ -128,14 +130,14 @@ public: const auto &wb_const = wb; xlnt_assert_throws(wb_const.sheet_by_title("error"), xlnt::key_not_found); } - + void test_get_sheet_by_title_const() { xlnt::workbook wb; auto new_sheet = wb.create_sheet(); std::string title = "my sheet"; new_sheet.title(title); - const xlnt::workbook& wbconst = wb; + const xlnt::workbook &wbconst = wb; auto found_sheet = wbconst.sheet_by_title(title); xlnt_assert_equals(new_sheet, found_sheet); } @@ -153,17 +155,17 @@ public: xlnt_assert(wb.contains("Sheet1")); xlnt_assert(!wb.contains("NotThere")); } - + void test_iter() { xlnt::workbook wb; - - for(auto ws : wb) + + for (auto ws : wb) { xlnt_assert_equals(ws.title(), "Sheet1"); } } - + void test_get_index() { xlnt::workbook wb; @@ -182,11 +184,11 @@ public: xlnt::workbook wb; wb.create_sheet().title("test_get_sheet_titles"); - const std::vector expected_titles = { "Sheet1", "test_get_sheet_titles" }; + const std::vector expected_titles = {"Sheet1", "test_get_sheet_titles"}; - xlnt_assert_equals(wb.sheet_titles(), expected_titles); + xlnt_assert_equals(wb.sheet_titles(), expected_titles); } - + void test_add_named_range() { xlnt::workbook wb, wb2; @@ -196,7 +198,7 @@ public: xlnt_assert(wb.has_named_range("test_nr")); xlnt_assert_throws(wb2.create_named_range("test_nr", new_sheet, "A1"), std::runtime_error); } - + void test_get_named_range() { xlnt::workbook wb; @@ -251,7 +253,7 @@ public: wb.create_sheet().title("Sheet3"); auto iter = wb.begin(); - xlnt_assert_equals((*iter).title(), "Sheet1"); + xlnt_assert_equals((*iter).title(), "Sheet1"); iter++; xlnt_assert_equals((*iter).title(), "Sheet2"); @@ -269,10 +271,10 @@ public: xlnt_assert_equals((*iter).title(), "Sheet3"); xlnt_assert_equals(iter, copy); } - + void test_manifest() { - xlnt::manifest m; + xlnt::manifest m; xlnt_assert(!m.has_default_type("xml")); xlnt_assert_throws(m.default_type("xml"), xlnt::key_not_found); xlnt_assert(!m.has_relationship(xlnt::path("/"), xlnt::relationship_type::office_document)); @@ -299,7 +301,7 @@ public: xlnt_assert(wb.active_sheet().cell("B2").has_style()); wb.clear_styles(); xlnt_assert(!wb.active_sheet().cell("B2").has_style()); - xlnt::format format = wb.create_format(); + xlnt::format format = wb.create_format(); xlnt::font font; font.size(41); format.font(font, true); @@ -318,13 +320,13 @@ public: xlnt_assert(!(wb != wb)); xlnt_assert(!(wb == wb2)); xlnt_assert(wb != wb2); - + const auto &wb_const = wb; //TODO these aren't tests... wb_const.manifest(); - + xlnt_assert(wb.has_theme()); - + wb.create_style("style1"); wb.style("style1"); wb_const.style("style1"); @@ -339,4 +341,39 @@ public: wb.create_sheet(); xlnt_assert_differs(wb[1].id(), wb[2].id()); } + + void test_load_file() + { + xlnt::path file = path_helper::test_file("2_minimal.xlsx"); + xlnt::workbook wb_path(file); + // ctor from ifstream + std::ifstream file_reader(file.string(), std::ios::binary); + xlnt_assert_equals(wb_path, xlnt::workbook(file_reader)); + // load with string + xlnt::workbook wb_load1; + xlnt_assert_differs(wb_path, wb_load1); + wb_load1.load(file.string()); + xlnt_assert_equals(wb_path, wb_load1); + // load with wstring + xlnt::workbook wb_load2; + wb_load2.load(file.string()); + xlnt_assert_equals(wb_path, wb_load2); + // load with path + xlnt::workbook wb_load3; + wb_load3.load(file); + xlnt_assert_equals(wb_path, wb_load3); + // load with istream + xlnt::workbook wb_load4; + std::ifstream file_reader2(file.string(), std::ios::binary); + wb_load4.load(file_reader2); + xlnt_assert_equals(wb_path, wb_load4); + // load with vector + std::ifstream file_reader3(file.string(), std::ios::binary); + file_reader3.unsetf(std::ios::skipws); + std::vector data(std::istream_iterator{file_reader3}, + std::istream_iterator()); + xlnt::workbook wb_load5; + wb_load5.load(data); + xlnt_assert_equals(wb_path, wb_load5); + } }; From 99f20a014aaf36e2a65209ee4f34ec7899ce0bd1 Mon Sep 17 00:00:00 2001 From: sukoi26 Date: Mon, 9 Jul 2018 18:21:01 +0200 Subject: [PATCH 06/23] error what(): xl/sharedStrings.xml: error: duplicate attribute #290 --- source/detail/serialization/xlsx_producer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/detail/serialization/xlsx_producer.cpp b/source/detail/serialization/xlsx_producer.cpp index b7cae367..928c7e2c 100644 --- a/source/detail/serialization/xlsx_producer.cpp +++ b/source/detail/serialization/xlsx_producer.cpp @@ -868,7 +868,7 @@ void xlsx_producer::write_shared_string_table(const relationship & /*rel*/) { write_attribute(xml::qname(xmlns_xml, "space"), "preserve"); } - write_characters(string.plain_text(), has_trailing_whitespace(string.plain_text())); + write_characters(string.plain_text()); write_end_element(xmlns, "t"); write_end_element(xmlns, "si"); From 744dd0afbbe064dc7dc3d92c77b9306560cbe742 Mon Sep 17 00:00:00 2001 From: Joshua Date: Wed, 30 May 2018 07:52:54 +1200 Subject: [PATCH 07/23] Remove uses of std::iterator (deprecated in C++17) Detailed reasoning for the deprecation is provided by the paper proposing deprecation (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0174r2.html) and a related LWG issue (https://cplusplus.github.io/LWG/issue2438). This was the only issue preventing a clean compile with VS 15.7.2 with c++17/c++latest set as the target language The issue could be resolved in two ways. Providing a custom replacement to std::iterator (a very simple structure) or by providing the 5 required typedefs. The only functional difference from my reading is that the typedefs are not immediately available to the implementer with the inheritance. I find the inline typedefs to be clearer hence the selection in this commit --- include/xlnt/workbook/worksheet_iterator.hpp | 33 +++++++++++------- include/xlnt/worksheet/cell_iterator.hpp | 36 ++++++++++++-------- include/xlnt/worksheet/range_iterator.hpp | 34 ++++++++++-------- 3 files changed, 61 insertions(+), 42 deletions(-) diff --git a/include/xlnt/workbook/worksheet_iterator.hpp b/include/xlnt/workbook/worksheet_iterator.hpp index d0ac56e7..b2830f88 100644 --- a/include/xlnt/workbook/worksheet_iterator.hpp +++ b/include/xlnt/workbook/worksheet_iterator.hpp @@ -37,18 +37,22 @@ class worksheet; // because one needs to point at a const workbook and the other needs // to point at a non-const workbook stored as a member variable, respectively. -/// -/// Alias the parent class of this iterator to increase clarity. -/// -using ws_iter_type = std::iterator; /// /// An iterator which is used to iterate over the worksheets in a workbook. /// -class XLNT_API worksheet_iterator : public ws_iter_type +class XLNT_API worksheet_iterator { public: + /// + /// iterator tags required for use with standard algorithms and adapters + /// + using iterator_category = std::bidirectional_iterator_tag; + using value_type = worksheet; + using difference_type = std::ptrdiff_t; + using pointer = worksheet *; + using reference = worksheet; // intentionally value + /// /// Constructs a worksheet iterator from a workbook and sheet index. /// @@ -106,18 +110,21 @@ private: }; -/// -/// Alias the parent class of this iterator to increase clarity. -/// -using c_ws_iter_type = std::iterator; - /// /// An iterator which is used to iterate over the worksheets in a const workbook. /// -class XLNT_API const_worksheet_iterator : public c_ws_iter_type +class XLNT_API const_worksheet_iterator { public: + /// + /// iterator tags required for use with standard algorithms and adapters + /// + using iterator_category = std::bidirectional_iterator_tag; + using value_type = const worksheet; + using difference_type = std::ptrdiff_t; + using pointer = const worksheet *; + using reference = const worksheet; // intentionally value + /// /// Constructs a worksheet iterator from a workbook and sheet index. /// diff --git a/include/xlnt/worksheet/cell_iterator.hpp b/include/xlnt/worksheet/cell_iterator.hpp index 496dd6a9..5f7b24eb 100644 --- a/include/xlnt/worksheet/cell_iterator.hpp +++ b/include/xlnt/worksheet/cell_iterator.hpp @@ -40,18 +40,21 @@ class cell; class cell_reference; class range_reference; -/// -/// Alias the parent class of this iterator to increase clarity. -/// -using c_iter_type = std::iterator; - /// /// A cell iterator iterates over a 1D range by row or by column. /// -class XLNT_API cell_iterator : public c_iter_type +class XLNT_API cell_iterator { public: + /// + /// iterator tags required for use with standard algorithms and adapters + /// + using iterator_category = std::bidirectional_iterator_tag; + using value_type = cell; + using difference_type = std::ptrdiff_t; + using pointer = cell *; + using reference = cell; // intentionally value + /// /// Constructs a cell_iterator from a worksheet, range, and iteration settings. /// @@ -135,7 +138,7 @@ private: /// If true, cells that don't exist in the worksheet will be skipped during iteration. /// bool skip_null_; - + /// /// If true, when on the last column, the cursor will continue to the next row /// (and vice versa when iterating in column-major order) until reaching the @@ -144,18 +147,21 @@ private: bool wrap_; }; -/// -/// Alias the parent class of this iterator to increase clarity. -/// -using cc_iter_type = std::iterator; - /// /// A cell iterator iterates over a 1D range by row or by column. /// -class XLNT_API const_cell_iterator : public cc_iter_type +class XLNT_API const_cell_iterator { public: + /// + /// iterator tags required for use with standard algorithms and adapters + /// + using iterator_category = std::bidirectional_iterator_tag; + using value_type = const cell; + using difference_type = std::ptrdiff_t; + using pointer = const cell *; + using reference = const cell; // intentionally value + /// /// Constructs a cell_iterator from a worksheet, range, and iteration settings. /// diff --git a/include/xlnt/worksheet/range_iterator.hpp b/include/xlnt/worksheet/range_iterator.hpp index cdd51d84..351e257f 100644 --- a/include/xlnt/worksheet/range_iterator.hpp +++ b/include/xlnt/worksheet/range_iterator.hpp @@ -35,19 +35,22 @@ namespace xlnt { class cell_vector; -/// -/// Alias the parent class of this iterator to increase clarity. -/// -using r_iter_type = std::iterator; - /// /// An iterator used by worksheet and range for traversing /// a 2D grid of cells by row/column then across that row/column. /// -class XLNT_API range_iterator : public r_iter_type +class XLNT_API range_iterator { public: + /// + /// iterator tags required for use with standard algorithms and adapters + /// + using iterator_category = std::bidirectional_iterator_tag; + using value_type = cell_vector; + using difference_type = std::ptrdiff_t; + using pointer = cell_vector *; + using reference = cell_vector; // intentionally value + /// /// Constructs a range iterator on a worksheet, cell pointing to the current /// row or column, range bounds, an order, and whether or not to skip null column/rows. @@ -127,19 +130,22 @@ private: bool skip_null_; }; -/// -/// Alias the parent class of this iterator to increase clarity. -/// -using cr_iter_type = std::iterator; - /// /// A const version of range_iterator which does not allow modification /// to the dereferenced cell_vector. /// -class XLNT_API const_range_iterator : public cr_iter_type +class XLNT_API const_range_iterator { public: + /// + /// this iterator meets the interface requirements of bidirection_iterator + /// + using iterator_category = std::bidirectional_iterator_tag; + using value_type = const cell_vector; + using difference_type = std::ptrdiff_t; + using pointer = const cell_vector *; + using reference = const cell_vector; // intentionally value + /// /// Constructs a range iterator on a worksheet, cell pointing to the current /// row or column, range bounds, an order, and whether or not to skip null column/rows. From 92ac3a2a2ea25731561cb91650a10f16147ef389 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Thu, 31 May 2018 13:07:02 +1200 Subject: [PATCH 08/23] cell_iterator - rule of 5/0 For rule of 5/0, where no implementation is required, all 5 operations have been declared as defaulted. This is less likely to forget definitions for all 5 if required - removed forwarding of copy to assignment (which was defaulted already) in favour of defaulted copy ctor - added defaulted move assignment/ctor and destructor --- include/xlnt/worksheet/cell_iterator.hpp | 37 +++++++++++++++++++++--- source/worksheet/cell_iterator.cpp | 10 ------- 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/include/xlnt/worksheet/cell_iterator.hpp b/include/xlnt/worksheet/cell_iterator.hpp index 5f7b24eb..64e5c9b2 100644 --- a/include/xlnt/worksheet/cell_iterator.hpp +++ b/include/xlnt/worksheet/cell_iterator.hpp @@ -64,12 +64,27 @@ public: /// /// Constructs a cell_iterator as a copy of an existing cell_iterator. /// - cell_iterator(const cell_iterator &other); + cell_iterator(const cell_iterator &) = default; /// /// Assigns this iterator to match the data in rhs. /// - cell_iterator &operator=(const cell_iterator &rhs) = default; + cell_iterator &operator=(const cell_iterator &) = default; + + /// + /// Constructs a cell_iterator by moving from a cell_iterator temporary + /// + cell_iterator(const cell_iterator &&) = default; + + /// + /// Assigns this iterator to from a cell_iterator temporary + /// + cell_iterator &operator=(const cell_iterator &&) = default; + + /// + /// destructor for const_cell_iterator + /// + ~cell_iterator() = default; /// /// Dereferences this iterator to return the cell it points to. @@ -169,15 +184,29 @@ public: const range_reference &limits, major_order order, bool skip_null, bool wrap); /// - /// Constructs a cell_iterator as a copy of an existing cell_iterator. + /// Constructs a const_cell_iterator as a copy of an existing cell_iterator. /// - const_cell_iterator(const const_cell_iterator &other); + const_cell_iterator(const const_cell_iterator &) = default; /// /// Assigns this iterator to match the data in rhs. /// const_cell_iterator &operator=(const const_cell_iterator &) = default; + /// + /// Constructs a const_cell_iterator by moving from a const_cell_iterator temporary + /// + const_cell_iterator(const const_cell_iterator &&) = default; + + /// + /// Assigns this iterator to from a const_cell_iterator temporary + /// + const_cell_iterator &operator=(const const_cell_iterator &&) = default; + + /// + /// destructor for const_cell_iterator + /// + ~const_cell_iterator() = default; /// /// Dereferences this iterator to return the cell it points to. /// diff --git a/source/worksheet/cell_iterator.cpp b/source/worksheet/cell_iterator.cpp index bad750ba..25aa7196 100644 --- a/source/worksheet/cell_iterator.cpp +++ b/source/worksheet/cell_iterator.cpp @@ -58,16 +58,6 @@ const_cell_iterator::const_cell_iterator(worksheet ws, const cell_reference &cur } } -cell_iterator::cell_iterator(const cell_iterator &other) -{ - *this = other; -} - -const_cell_iterator::const_cell_iterator(const const_cell_iterator &other) -{ - *this = other; -} - bool cell_iterator::operator==(const cell_iterator &other) const { return ws_ == other.ws_ From 20e72d69e5aaf795a182b6e5fd94782b42734a88 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Thu, 31 May 2018 13:16:54 +1200 Subject: [PATCH 09/23] cell_iterator - operator* const overloads Users may still want to derederence a const iterator (note: not a const_iterator). Also use the "reference" typedef to ensure there is only 1 source of information --- include/xlnt/worksheet/cell_iterator.hpp | 14 +++++++++++++- source/worksheet/cell_iterator.cpp | 8 ++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/include/xlnt/worksheet/cell_iterator.hpp b/include/xlnt/worksheet/cell_iterator.hpp index 64e5c9b2..b11818f6 100644 --- a/include/xlnt/worksheet/cell_iterator.hpp +++ b/include/xlnt/worksheet/cell_iterator.hpp @@ -89,7 +89,13 @@ public: /// /// Dereferences this iterator to return the cell it points to. /// - cell operator*(); + reference operator*(); + + /// + /// Dereferences this iterator to return the cell it points to. + /// + const reference operator*() const; + /// /// Returns true if this iterator is equivalent to other. @@ -207,6 +213,12 @@ public: /// destructor for const_cell_iterator /// ~const_cell_iterator() = default; + + /// + /// Dereferences this iterator to return the cell it points to. + /// + const reference operator*() const; + /// /// Dereferences this iterator to return the cell it points to. /// diff --git a/source/worksheet/cell_iterator.cpp b/source/worksheet/cell_iterator.cpp index 25aa7196..e4c334e9 100644 --- a/source/worksheet/cell_iterator.cpp +++ b/source/worksheet/cell_iterator.cpp @@ -265,14 +265,18 @@ const_cell_iterator const_cell_iterator::operator++(int) return old; } -cell cell_iterator::operator*() +cell_iterator::reference cell_iterator::operator*() { return ws_.cell(cursor_); } -const cell const_cell_iterator::operator*() const +const cell_iterator::reference cell_iterator::operator*() const { return ws_.cell(cursor_); } +const const_cell_iterator::reference const_cell_iterator::operator*() const +{ + return ws_.cell(cursor_); +} } // namespace xlnt From 1c4f9c7ed2905722cf22dc1e85420fe5ec2143c9 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Thu, 31 May 2018 13:18:39 +1200 Subject: [PATCH 10/23] cell iterator - add operator-> Implementation just forwards to operator*, but this is a typical operator in the iterator API and is suprising and inconvenient that it is not present --- include/xlnt/worksheet/cell_iterator.hpp | 11 ++++++++++- source/worksheet/cell_iterator.cpp | 16 ++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/include/xlnt/worksheet/cell_iterator.hpp b/include/xlnt/worksheet/cell_iterator.hpp index b11818f6..9e179057 100644 --- a/include/xlnt/worksheet/cell_iterator.hpp +++ b/include/xlnt/worksheet/cell_iterator.hpp @@ -96,6 +96,15 @@ public: /// const reference operator*() const; + /// + /// Dereferences this iterator to return the cell it points to. + /// + reference operator->(); + + /// + /// Dereferences this iterator to return the cell it points to. + /// + const reference operator->() const; /// /// Returns true if this iterator is equivalent to other. @@ -222,7 +231,7 @@ public: /// /// Dereferences this iterator to return the cell it points to. /// - const cell operator*() const; + const reference operator->() const; /// /// Returns true if this iterator is equivalent to other. diff --git a/source/worksheet/cell_iterator.cpp b/source/worksheet/cell_iterator.cpp index e4c334e9..d1f66252 100644 --- a/source/worksheet/cell_iterator.cpp +++ b/source/worksheet/cell_iterator.cpp @@ -279,4 +279,20 @@ const const_cell_iterator::reference const_cell_iterator::operator*() const { return ws_.cell(cursor_); } + +cell_iterator::reference cell_iterator::operator->() +{ + return operator*(); +} + +const cell_iterator::reference cell_iterator::operator->() const +{ + return operator*(); +} + +const const_cell_iterator::reference const_cell_iterator::operator->() const +{ + return operator*(); +} + } // namespace xlnt From dc020622c055d5b1b2a2c5539d538ba8696ae385 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Thu, 31 May 2018 15:21:21 +1200 Subject: [PATCH 11/23] cell iterator - move parameters aren't const copy/paste error --- include/xlnt/worksheet/cell_iterator.hpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/xlnt/worksheet/cell_iterator.hpp b/include/xlnt/worksheet/cell_iterator.hpp index 9e179057..8dc53737 100644 --- a/include/xlnt/worksheet/cell_iterator.hpp +++ b/include/xlnt/worksheet/cell_iterator.hpp @@ -74,12 +74,12 @@ public: /// /// Constructs a cell_iterator by moving from a cell_iterator temporary /// - cell_iterator(const cell_iterator &&) = default; + cell_iterator(cell_iterator &&) = default; /// /// Assigns this iterator to from a cell_iterator temporary /// - cell_iterator &operator=(const cell_iterator &&) = default; + cell_iterator &operator=(cell_iterator &&) = default; /// /// destructor for const_cell_iterator @@ -211,12 +211,12 @@ public: /// /// Constructs a const_cell_iterator by moving from a const_cell_iterator temporary /// - const_cell_iterator(const const_cell_iterator &&) = default; + const_cell_iterator(const_cell_iterator &&) = default; /// /// Assigns this iterator to from a const_cell_iterator temporary /// - const_cell_iterator &operator=(const const_cell_iterator &&) = default; + const_cell_iterator &operator=(const_cell_iterator &&) = default; /// /// destructor for const_cell_iterator From 1e66824af06319a86f71555fd9476d15ed41479b Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Thu, 31 May 2018 15:27:20 +1200 Subject: [PATCH 12/23] range_iterator - rule of 5/0 For rule of 5/0, where no implementation is required, all 5 operations have been declared as defaulted. This is less likely to forget definitions for all 5 if required - removed forwarding of copy ctor to assignment (which was defaulted already) in favour of defaulted copy ctor - added defaulted move assignment/ctor and destructor --- include/xlnt/worksheet/range_iterator.hpp | 58 +++++++++++++++++------ source/worksheet/range_iterator.cpp | 10 ---- 2 files changed, 44 insertions(+), 24 deletions(-) diff --git a/include/xlnt/worksheet/range_iterator.hpp b/include/xlnt/worksheet/range_iterator.hpp index 351e257f..5e942f73 100644 --- a/include/xlnt/worksheet/range_iterator.hpp +++ b/include/xlnt/worksheet/range_iterator.hpp @@ -59,20 +59,35 @@ public: const range_reference &bounds, major_order order, bool skip_null); /// - /// Copy constructor. + /// Default copy constructor. /// - range_iterator(const range_iterator &other); - - /// - /// Dereference the iterator to return a column or row. - /// - cell_vector operator*() const; + range_iterator(const range_iterator &) = default; /// /// Default assignment operator. /// range_iterator &operator=(const range_iterator &) = default; + /// + /// Default move constructor. + /// + range_iterator(range_iterator &&) = default; + + /// + /// Default move assignment operator. + /// + range_iterator &operator=(range_iterator &&) = default; + + /// + /// Default destructor + /// + ~range_iterator() = default; + + /// + /// Dereference the iterator to return a column or row. + /// + cell_vector operator*() const; + /// /// Returns true if this iterator is equivalent to other. /// @@ -154,20 +169,35 @@ public: const range_reference &bounds, major_order order, bool skip_null); /// - /// Copy constructor. + /// Default copy constructor. /// - const_range_iterator(const const_range_iterator &other); - - /// - /// Dereferennce the iterator to return the current column/row. - /// - const cell_vector operator*() const; + const_range_iterator(const const_range_iterator &) = default; /// /// Default assignment operator. /// const_range_iterator &operator=(const const_range_iterator &) = default; + /// + /// Default move constructor. + /// + const_range_iterator(const_range_iterator &&) = default; + + /// + /// Default move assignment operator. + /// + const_range_iterator &operator=(const_range_iterator &&) = default; + + /// + /// Default destructor + /// + ~const_range_iterator() = default; + + /// + /// Dereferennce the iterator to return the current column/row. + /// + const cell_vector operator*() const; + /// /// Returns true if this iterator is equivalent to other. /// diff --git a/source/worksheet/range_iterator.cpp b/source/worksheet/range_iterator.cpp index 2da73d5c..57208d26 100644 --- a/source/worksheet/range_iterator.cpp +++ b/source/worksheet/range_iterator.cpp @@ -47,11 +47,6 @@ range_iterator::range_iterator(worksheet &ws, const cell_reference &cursor, } } -range_iterator::range_iterator(const range_iterator &other) -{ - *this = other; -} - bool range_iterator::operator==(const range_iterator &other) const { return ws_ == other.ws_ @@ -168,11 +163,6 @@ const_range_iterator::const_range_iterator(const worksheet &ws, const cell_refer } } -const_range_iterator::const_range_iterator(const const_range_iterator &other) -{ - *this = other; -} - bool const_range_iterator::operator==(const const_range_iterator &other) const { return ws_ == other.ws_ From bc8cd21d676d1f08dfeec228407c6fca595e0088 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Thu, 31 May 2018 15:37:10 +1200 Subject: [PATCH 13/23] range_iterator - operator* const overloads Users may still want to dereference a const iterator (note: not a const_iterator). Also use the "reference" typedef to ensure there is only 1 source of information --- include/xlnt/worksheet/range_iterator.hpp | 9 +++++++-- source/worksheet/range_iterator.cpp | 9 +++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/include/xlnt/worksheet/range_iterator.hpp b/include/xlnt/worksheet/range_iterator.hpp index 5e942f73..c6f43979 100644 --- a/include/xlnt/worksheet/range_iterator.hpp +++ b/include/xlnt/worksheet/range_iterator.hpp @@ -86,7 +86,12 @@ public: /// /// Dereference the iterator to return a column or row. /// - cell_vector operator*() const; + reference operator*(); + + /// + /// Dereference the iterator to return a column or row. + /// + const reference operator*() const; /// /// Returns true if this iterator is equivalent to other. @@ -196,7 +201,7 @@ public: /// /// Dereferennce the iterator to return the current column/row. /// - const cell_vector operator*() const; + const reference operator*() const; /// /// Returns true if this iterator is equivalent to other. diff --git a/source/worksheet/range_iterator.cpp b/source/worksheet/range_iterator.cpp index 57208d26..2c7a3a10 100644 --- a/source/worksheet/range_iterator.cpp +++ b/source/worksheet/range_iterator.cpp @@ -28,7 +28,12 @@ namespace xlnt { -cell_vector range_iterator::operator*() const +range_iterator::reference range_iterator::operator*() +{ + return cell_vector(ws_, cursor_, bounds_, order_, skip_null_, false); +} + +const range_iterator::reference range_iterator::operator*() const { return cell_vector(ws_, cursor_, bounds_, order_, skip_null_, false); } @@ -264,7 +269,7 @@ const_range_iterator const_range_iterator::operator++(int) return old; } -const cell_vector const_range_iterator::operator*() const +const const_range_iterator::reference const_range_iterator::operator*() const { return cell_vector(ws_, cursor_, bounds_, order_, skip_null_, false); } From 62e0a7dd178ae5c1b45d2524a6a0c926b65c2035 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Thu, 31 May 2018 15:41:57 +1200 Subject: [PATCH 14/23] Revert "cell iterator - add operator->" This reverts commit 1b705ae043ae1b43d0ecbc93da04c944383bb296. --- include/xlnt/worksheet/cell_iterator.hpp | 11 +---------- source/worksheet/cell_iterator.cpp | 16 ---------------- 2 files changed, 1 insertion(+), 26 deletions(-) diff --git a/include/xlnt/worksheet/cell_iterator.hpp b/include/xlnt/worksheet/cell_iterator.hpp index 8dc53737..c6f1fa12 100644 --- a/include/xlnt/worksheet/cell_iterator.hpp +++ b/include/xlnt/worksheet/cell_iterator.hpp @@ -96,15 +96,6 @@ public: /// const reference operator*() const; - /// - /// Dereferences this iterator to return the cell it points to. - /// - reference operator->(); - - /// - /// Dereferences this iterator to return the cell it points to. - /// - const reference operator->() const; /// /// Returns true if this iterator is equivalent to other. @@ -231,7 +222,7 @@ public: /// /// Dereferences this iterator to return the cell it points to. /// - const reference operator->() const; + const cell operator*() const; /// /// Returns true if this iterator is equivalent to other. diff --git a/source/worksheet/cell_iterator.cpp b/source/worksheet/cell_iterator.cpp index d1f66252..e4c334e9 100644 --- a/source/worksheet/cell_iterator.cpp +++ b/source/worksheet/cell_iterator.cpp @@ -279,20 +279,4 @@ const const_cell_iterator::reference const_cell_iterator::operator*() const { return ws_.cell(cursor_); } - -cell_iterator::reference cell_iterator::operator->() -{ - return operator*(); -} - -const cell_iterator::reference cell_iterator::operator->() const -{ - return operator*(); -} - -const const_cell_iterator::reference const_cell_iterator::operator->() const -{ - return operator*(); -} - } // namespace xlnt From eaf3c2a773d26337c52194273c13bc673e80dc1c Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Thu, 31 May 2018 15:43:58 +1200 Subject: [PATCH 15/23] Remove double definition --- include/xlnt/worksheet/cell_iterator.hpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/include/xlnt/worksheet/cell_iterator.hpp b/include/xlnt/worksheet/cell_iterator.hpp index c6f1fa12..415dee6f 100644 --- a/include/xlnt/worksheet/cell_iterator.hpp +++ b/include/xlnt/worksheet/cell_iterator.hpp @@ -219,11 +219,6 @@ public: /// const reference operator*() const; - /// - /// Dereferences this iterator to return the cell it points to. - /// - const cell operator*() const; - /// /// Returns true if this iterator is equivalent to other. /// From a47985b2dbe76e624da8b0ef8a7e27542a849d92 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Thu, 31 May 2018 15:56:07 +1200 Subject: [PATCH 16/23] worksheet_iterator - rule of 5/0 For rule of 5/0, where no implementation is required, all 5 operations have been declared as defaulted. This is less likely to forget definitions for all 5 if required - removed forwarding of copy ctor to assignment (which was defaulted already) in favour of defaulted copy ctor - added defaulted move assignment/ctor and destructor Changed workbook reference to a pointer to allow tests to compile (reference isn't rebindable so defaulted assignment is equivalent to deleted) --- include/xlnt/workbook/worksheet_iterator.hpp | 42 +++++++++++++++++--- source/workbook/worksheet_iterator.cpp | 28 +++---------- 2 files changed, 42 insertions(+), 28 deletions(-) diff --git a/include/xlnt/workbook/worksheet_iterator.hpp b/include/xlnt/workbook/worksheet_iterator.hpp index b2830f88..c1b24a78 100644 --- a/include/xlnt/workbook/worksheet_iterator.hpp +++ b/include/xlnt/workbook/worksheet_iterator.hpp @@ -61,12 +61,27 @@ public: /// /// Copy constructs a worksheet iterator from another iterator. /// - worksheet_iterator(const worksheet_iterator &); + worksheet_iterator(const worksheet_iterator &) = default; /// /// Assigns the iterator so that it points to the same worksheet in the same workbook. /// - worksheet_iterator &operator=(const worksheet_iterator &); + worksheet_iterator &operator=(const worksheet_iterator &) = default; + + /// + /// Copy constructs a worksheet iterator from another iterator. + /// + worksheet_iterator(worksheet_iterator &&) = default; + + /// + /// Assigns the iterator so that it points to the same worksheet in the same workbook. + /// + worksheet_iterator &operator=(worksheet_iterator &&) = default; + + /// + /// Default destructor + /// + ~worksheet_iterator() = default; /// /// Dereferences the iterator to return the worksheet it is pointing to. @@ -101,7 +116,7 @@ private: /// /// The target workbook of this iterator. /// - workbook &wb_; + workbook *wb_; /// /// The index of the worksheet in wb_ this iterator is currently pointing to. @@ -133,12 +148,27 @@ public: /// /// Copy constructs a worksheet iterator from another iterator. /// - const_worksheet_iterator(const const_worksheet_iterator &); + const_worksheet_iterator(const const_worksheet_iterator &) = default; /// /// Assigns the iterator so that it points to the same worksheet in the same workbook. /// - const_worksheet_iterator &operator=(const const_worksheet_iterator &); + const_worksheet_iterator &operator=(const const_worksheet_iterator &) = default; + + /// + /// Move constructs a worksheet iterator from another iterator. + /// + const_worksheet_iterator(const_worksheet_iterator &&) = default; + + /// + /// Assigns the iterator from a temporary + /// + const_worksheet_iterator &operator=(const_worksheet_iterator &&) = default; + + /// + /// Default destructor + /// + ~const_worksheet_iterator() = default; /// /// Dereferences the iterator to return the worksheet it is pointing to. @@ -173,7 +203,7 @@ private: /// /// The target workbook of this iterator. /// - const workbook &wb_; + const workbook *wb_; /// /// The index of the worksheet in wb_ this iterator is currently pointing to. diff --git a/source/workbook/worksheet_iterator.cpp b/source/workbook/worksheet_iterator.cpp index daee7a64..0487cf33 100644 --- a/source/workbook/worksheet_iterator.cpp +++ b/source/workbook/worksheet_iterator.cpp @@ -28,18 +28,13 @@ namespace xlnt { worksheet_iterator::worksheet_iterator(workbook &wb, std::size_t index) - : wb_(wb), index_(index) -{ -} - -worksheet_iterator::worksheet_iterator(const worksheet_iterator &rhs) - : wb_(rhs.wb_), index_(rhs.index_) + : wb_(&wb), index_(index) { } worksheet worksheet_iterator::operator*() { - return wb_[index_]; + return (*wb_)[index_]; } worksheet_iterator &worksheet_iterator::operator++() @@ -50,7 +45,7 @@ worksheet_iterator &worksheet_iterator::operator++() worksheet_iterator worksheet_iterator::operator++(int) { - worksheet_iterator old(wb_, index_); + worksheet_iterator old(*wb_, index_); ++*this; return old; } @@ -65,25 +60,14 @@ bool worksheet_iterator::operator!=(const worksheet_iterator &comparand) const return !(*this == comparand); } -worksheet_iterator &worksheet_iterator::operator=(const worksheet_iterator &other) -{ - index_ = other.index_; - return *this; -} - const_worksheet_iterator::const_worksheet_iterator(const workbook &wb, std::size_t index) - : wb_(wb), index_(index) -{ -} - -const_worksheet_iterator::const_worksheet_iterator(const const_worksheet_iterator &rhs) - : wb_(rhs.wb_), index_(rhs.index_) + : wb_(&wb), index_(index) { } const worksheet const_worksheet_iterator::operator*() { - return wb_.sheet_by_index(index_); + return wb_->sheet_by_index(index_); } const_worksheet_iterator &const_worksheet_iterator::operator++() @@ -94,7 +78,7 @@ const_worksheet_iterator &const_worksheet_iterator::operator++() const_worksheet_iterator const_worksheet_iterator::operator++(int) { - const_worksheet_iterator old(wb_, index_); + const_worksheet_iterator old(*wb_, index_); ++*this; return old; } From 04b50d9b8ee5ee920f86209d3e972e8efcacd436 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Thu, 31 May 2018 16:00:40 +1200 Subject: [PATCH 17/23] worksheet_iterator - operator* const overloads Users may still want to dereference a const iterator (note: not a const_iterator). Also use the "reference" typedef to ensure there is only 1 source of information --- include/xlnt/workbook/worksheet_iterator.hpp | 11 +++++++++-- source/workbook/worksheet_iterator.cpp | 9 +++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/include/xlnt/workbook/worksheet_iterator.hpp b/include/xlnt/workbook/worksheet_iterator.hpp index c1b24a78..444b792f 100644 --- a/include/xlnt/workbook/worksheet_iterator.hpp +++ b/include/xlnt/workbook/worksheet_iterator.hpp @@ -88,7 +88,14 @@ public: /// If the iterator points to one-past-the-end of the workbook, an invalid_parameter /// exception will be thrown. /// - worksheet operator*(); + reference operator*(); + + /// + /// Dereferences the iterator to return the worksheet it is pointing to. + /// If the iterator points to one-past-the-end of the workbook, an invalid_parameter + /// exception will be thrown. + /// + const reference operator*() const; /// /// Returns true if this iterator points to the same worksheet as comparand. @@ -175,7 +182,7 @@ public: /// If the iterator points to one-past-the-end of the workbook, an invalid_parameter /// exception will be thrown. /// - const worksheet operator*(); + const reference operator*() const; /// /// Returns true if this iterator points to the same worksheet as comparand. diff --git a/source/workbook/worksheet_iterator.cpp b/source/workbook/worksheet_iterator.cpp index 0487cf33..3c981339 100644 --- a/source/workbook/worksheet_iterator.cpp +++ b/source/workbook/worksheet_iterator.cpp @@ -32,7 +32,12 @@ worksheet_iterator::worksheet_iterator(workbook &wb, std::size_t index) { } -worksheet worksheet_iterator::operator*() +worksheet_iterator::reference worksheet_iterator::operator*() +{ + return (*wb_)[index_]; +} + +const worksheet_iterator::reference worksheet_iterator::operator*() const { return (*wb_)[index_]; } @@ -65,7 +70,7 @@ const_worksheet_iterator::const_worksheet_iterator(const workbook &wb, std::size { } -const worksheet const_worksheet_iterator::operator*() +const const_worksheet_iterator::reference const_worksheet_iterator::operator*() const { return wb_->sheet_by_index(index_); } From b95919323d779f99096705520e48602933d113b3 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Sat, 9 Jun 2018 11:57:45 +1200 Subject: [PATCH 18/23] add more tests for worksheet iterator, add operator-- * operator-- completes the naive bidirectional iterator requirements * tests for various construction and assignment behaviours * tests for iteration and dereferencing behaviours --- include/xlnt/workbook/worksheet_iterator.hpp | 24 +++++++ source/workbook/worksheet_iterator.cpp | 26 +++++++ tests/workbook/workbook_test_suite.hpp | 76 ++++++++++++++++++-- 3 files changed, 122 insertions(+), 4 deletions(-) diff --git a/include/xlnt/workbook/worksheet_iterator.hpp b/include/xlnt/workbook/worksheet_iterator.hpp index 444b792f..f575dc7c 100644 --- a/include/xlnt/workbook/worksheet_iterator.hpp +++ b/include/xlnt/workbook/worksheet_iterator.hpp @@ -119,6 +119,18 @@ public: /// worksheet_iterator &operator++(); + /// + /// Post-decrement the iterator's internal workseet index. Returns a copy of the + /// iterator as it was before being incremented. + /// + worksheet_iterator operator--(int); + + /// + /// Pre-decrement the iterator's internal workseet index. Returns a refernce + /// to the same iterator. + /// + worksheet_iterator &operator--(); + private: /// /// The target workbook of this iterator. @@ -206,6 +218,18 @@ public: /// const_worksheet_iterator &operator++(); + /// + /// Post-decrement the iterator's internal workseet index. Returns a copy of the + /// iterator as it was before being incremented. + /// + const_worksheet_iterator operator--(int); + + /// + /// Pre-decrement the iterator's internal workseet index. Returns a refernce + /// to the same iterator. + /// + const_worksheet_iterator &operator--(); + private: /// /// The target workbook of this iterator. diff --git a/source/workbook/worksheet_iterator.cpp b/source/workbook/worksheet_iterator.cpp index 3c981339..b1432e33 100644 --- a/source/workbook/worksheet_iterator.cpp +++ b/source/workbook/worksheet_iterator.cpp @@ -55,6 +55,19 @@ worksheet_iterator worksheet_iterator::operator++(int) return old; } +worksheet_iterator &worksheet_iterator::operator--() +{ + --index_; + return *this; +} + +worksheet_iterator worksheet_iterator::operator--(int) +{ + worksheet_iterator old(*wb_, index_); + --(*this); + return old; +} + bool worksheet_iterator::operator==(const worksheet_iterator &comparand) const { return index_ == comparand.index_ && wb_ == comparand.wb_; @@ -88,6 +101,19 @@ const_worksheet_iterator const_worksheet_iterator::operator++(int) return old; } +const_worksheet_iterator &const_worksheet_iterator::operator--() +{ + --index_; + return *this; +} + +const_worksheet_iterator const_worksheet_iterator::operator--(int) +{ + const_worksheet_iterator old(*wb_, index_); + --(*this); + return old; +} + bool const_worksheet_iterator::operator==(const const_worksheet_iterator &comparand) const { return index_ == comparand.index_ && wb_ == comparand.wb_; diff --git a/tests/workbook/workbook_test_suite.hpp b/tests/workbook/workbook_test_suite.hpp index cbd4bda3..8a69a9cd 100644 --- a/tests/workbook/workbook_test_suite.hpp +++ b/tests/workbook/workbook_test_suite.hpp @@ -45,6 +45,7 @@ public: register_test(test_index_operator); register_test(test_contains); register_test(test_iter); + register_test(test_const_iter); register_test(test_get_index); register_test(test_get_sheet_names); register_test(test_add_named_range); @@ -153,17 +154,84 @@ public: xlnt_assert(wb.contains("Sheet1")); xlnt_assert(!wb.contains("NotThere")); } - + void test_iter() { xlnt::workbook wb; - - for(auto ws : wb) + + for (auto ws : wb) { xlnt_assert_equals(ws.title(), "Sheet1"); } + + xlnt::workbook wb2; + auto iter = wb.begin(); + auto iter_copy(iter); // copy ctor + xlnt_assert_equals(iter, iter_copy); + auto begin_2(wb2.begin()); + xlnt_assert_differs(begin_2, iter); + iter = begin_2; // copy assign + xlnt_assert_equals(iter, begin_2); + iter = wb.begin(); + iter = std::move(begin_2); + xlnt_assert_equals(iter, wb2.begin()); + + auto citer = wb.cbegin(); + auto citer_copy(citer); // copy ctor + xlnt_assert_equals(citer, citer_copy); + auto cbegin_2(wb2.cbegin()); + xlnt_assert_differs(cbegin_2, citer); + citer = cbegin_2; // copy assign + xlnt_assert_equals(citer, cbegin_2); + citer = wb.cbegin(); + citer = std::move(cbegin_2); + xlnt_assert_equals(citer, wb2.cbegin()); + + wb2.create_sheet(); // wb2 iterators assumed invalidated + iter = wb2.begin(); + xlnt_assert_equals((*iter).title(), "Sheet1"); + ++iter; + xlnt_assert_differs(iter, wb2.begin()); + xlnt_assert_equals((*iter).title(), "Sheet2"); + --iter; + xlnt_assert_equals((*iter).title(), "Sheet1"); + std::advance(iter, 2); + xlnt_assert_equals(iter, wb2.end()); } - + + void test_const_iter() + { + const xlnt::workbook wb; + + for (auto ws : wb) + { + xlnt_assert_equals(ws.title(), "Sheet1"); + } + + xlnt::workbook wb2; + auto iter = wb.cbegin(); + auto iter_copy(iter); // copy ctor + xlnt_assert_equals(iter, iter_copy); + auto begin_2(wb2.cbegin()); + xlnt_assert_differs(begin_2, iter); + iter = begin_2; // copy assign + xlnt_assert_equals(iter, begin_2); + iter = wb.cbegin(); + iter = std::move(begin_2); + xlnt_assert_equals(iter, wb2.cbegin()); + + wb2.create_sheet(); // wb2 iterators assumed invalidated + iter = wb2.cbegin(); + xlnt_assert_equals((*iter).title(), "Sheet1"); + ++iter; + xlnt_assert_differs(iter, wb2.cbegin()); + xlnt_assert_equals((*iter).title(), "Sheet2"); + --iter; + xlnt_assert_equals((*iter).title(), "Sheet1"); + std::advance(iter, 2); + xlnt_assert_equals(iter, wb2.cend()); + } + void test_get_index() { xlnt::workbook wb; From 9a82c4fab715f801173813e91c37d36863398438 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Sat, 9 Jun 2018 12:39:58 +1200 Subject: [PATCH 19/23] Comment fixes and clarifications --- include/xlnt/workbook/worksheet_iterator.hpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/include/xlnt/workbook/worksheet_iterator.hpp b/include/xlnt/workbook/worksheet_iterator.hpp index f575dc7c..3e0165fb 100644 --- a/include/xlnt/workbook/worksheet_iterator.hpp +++ b/include/xlnt/workbook/worksheet_iterator.hpp @@ -64,17 +64,17 @@ public: worksheet_iterator(const worksheet_iterator &) = default; /// - /// Assigns the iterator so that it points to the same worksheet in the same workbook. + /// Copy assigns the iterator so that it points to the same worksheet in the same workbook. /// worksheet_iterator &operator=(const worksheet_iterator &) = default; /// - /// Copy constructs a worksheet iterator from another iterator. + /// Move constructs a worksheet iterator from a temporary iterator. /// worksheet_iterator(worksheet_iterator &&) = default; /// - /// Assigns the iterator so that it points to the same worksheet in the same workbook. + /// Move assign the iterator from a temporary iterator /// worksheet_iterator &operator=(worksheet_iterator &&) = default; @@ -170,17 +170,17 @@ public: const_worksheet_iterator(const const_worksheet_iterator &) = default; /// - /// Assigns the iterator so that it points to the same worksheet in the same workbook. + /// Copy assigns the iterator so that it points to the same worksheet in the same workbook. /// const_worksheet_iterator &operator=(const const_worksheet_iterator &) = default; /// - /// Move constructs a worksheet iterator from another iterator. + /// Move constructs a worksheet iterator from a temporary iterator. /// const_worksheet_iterator(const_worksheet_iterator &&) = default; /// - /// Assigns the iterator from a temporary + /// Move assigns the iterator from a temporary iterator /// const_worksheet_iterator &operator=(const_worksheet_iterator &&) = default; From 4afc0963c6af750db1bb75c2f74372f772846282 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Sat, 9 Jun 2018 12:49:54 +1200 Subject: [PATCH 20/23] add post (in/de)crement tests --- tests/workbook/workbook_test_suite.hpp | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/tests/workbook/workbook_test_suite.hpp b/tests/workbook/workbook_test_suite.hpp index 8a69a9cd..531ca22c 100644 --- a/tests/workbook/workbook_test_suite.hpp +++ b/tests/workbook/workbook_test_suite.hpp @@ -193,10 +193,18 @@ public: ++iter; xlnt_assert_differs(iter, wb2.begin()); xlnt_assert_equals((*iter).title(), "Sheet2"); - --iter; + auto temp = --iter; + xlnt_assert_equals((*temp).title(), "Sheet1"); xlnt_assert_equals((*iter).title(), "Sheet1"); - std::advance(iter, 2); + iter++; + xlnt_assert_equals((*iter).title(), "Sheet2"); + temp = iter++; + xlnt_assert_equals((*temp).title(), "Sheet2"); xlnt_assert_equals(iter, wb2.end()); + + iter = temp--; + xlnt_assert_equals((*iter).title(), "Sheet2"); + xlnt_assert_equals((*temp).title(), "Sheet1"); } void test_const_iter() @@ -226,10 +234,18 @@ public: ++iter; xlnt_assert_differs(iter, wb2.cbegin()); xlnt_assert_equals((*iter).title(), "Sheet2"); - --iter; + auto temp = --iter; + xlnt_assert_equals((*temp).title(), "Sheet1"); xlnt_assert_equals((*iter).title(), "Sheet1"); - std::advance(iter, 2); + iter++; + xlnt_assert_equals((*iter).title(), "Sheet2"); + temp = iter++; + xlnt_assert_equals((*temp).title(), "Sheet2"); xlnt_assert_equals(iter, wb2.cend()); + + iter = temp--; + xlnt_assert_equals((*iter).title(), "Sheet2"); + xlnt_assert_equals((*temp).title(), "Sheet1"); } void test_get_index() From cffb1ce3452b9ce8738bd28b8d1153dea0996d00 Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Tue, 10 Jul 2018 14:12:26 +1200 Subject: [PATCH 21/23] restart CI --- tests/workbook/workbook_test_suite.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/workbook/workbook_test_suite.hpp b/tests/workbook/workbook_test_suite.hpp index 531ca22c..e85a7a7e 100644 --- a/tests/workbook/workbook_test_suite.hpp +++ b/tests/workbook/workbook_test_suite.hpp @@ -227,8 +227,8 @@ public: iter = wb.cbegin(); iter = std::move(begin_2); xlnt_assert_equals(iter, wb2.cbegin()); - - wb2.create_sheet(); // wb2 iterators assumed invalidated + // wb2 iterators assumed invalidated + wb2.create_sheet(); iter = wb2.cbegin(); xlnt_assert_equals((*iter).title(), "Sheet1"); ++iter; From 399b5e3775158c8c23f2fb163ed536bc5a46202c Mon Sep 17 00:00:00 2001 From: Crzyrndm Date: Tue, 10 Jul 2018 13:18:21 +1200 Subject: [PATCH 22/23] Resolve CI build failure --- include/xlnt/worksheet/range.hpp | 5 +++++ source/packaging/ext_list.cpp | 2 +- source/worksheet/range.cpp | 9 ++++++--- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/include/xlnt/worksheet/range.hpp b/include/xlnt/worksheet/range.hpp index a8c5ffd7..0abebbcf 100644 --- a/include/xlnt/worksheet/range.hpp +++ b/include/xlnt/worksheet/range.hpp @@ -117,6 +117,11 @@ public: /// const class cell cell(const cell_reference &ref) const; + /// + /// The worksheet this range targets + /// + const worksheet & target_worksheet() const; + /// /// Returns the reference defining the bounds of this range. /// diff --git a/source/packaging/ext_list.cpp b/source/packaging/ext_list.cpp index 26988655..ac06f795 100644 --- a/source/packaging/ext_list.cpp +++ b/source/packaging/ext_list.cpp @@ -139,7 +139,7 @@ const std::vector &ext_list::extensions() const bool ext_list::operator==(const ext_list &rhs) const { - return std::equal(extensions_.begin(), extensions_.end(), rhs.extensions_.begin(), rhs.extensions_.end()); + return extensions_ == rhs.extensions_; } } \ No newline at end of file diff --git a/source/worksheet/range.cpp b/source/worksheet/range.cpp index bcbf9309..fa8e8eb2 100644 --- a/source/worksheet/range.cpp +++ b/source/worksheet/range.cpp @@ -39,9 +39,7 @@ range::range(worksheet ws, const range_reference &reference, major_order order, { } -range::~range() -{ -} +range::~range() = default; void range::clear_cells() { @@ -71,6 +69,11 @@ cell_vector range::operator[](std::size_t index) return vector(index); } +const worksheet &range::target_worksheet() const +{ + return ws_; +} + range_reference range::reference() const { return ref_; From 28a71572cf220b3835745456a98c03192e4edd22 Mon Sep 17 00:00:00 2001 From: sukoi26 Date: Tue, 10 Jul 2018 09:32:34 +0200 Subject: [PATCH 23/23] update #290 suggested by Crzyrndm --- source/detail/serialization/xlsx_producer.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/source/detail/serialization/xlsx_producer.cpp b/source/detail/serialization/xlsx_producer.cpp index 928c7e2c..aef2a050 100644 --- a/source/detail/serialization/xlsx_producer.cpp +++ b/source/detail/serialization/xlsx_producer.cpp @@ -864,11 +864,7 @@ void xlsx_producer::write_shared_string_table(const relationship & /*rel*/) { write_start_element(xmlns, "si"); write_start_element(xmlns, "t"); - if (string.runs().front().preserve_space) - { - write_attribute(xml::qname(xmlns_xml, "space"), "preserve"); - } - write_characters(string.plain_text()); + write_characters(string.plain_text(),string.runs().front().preserve_space); write_end_element(xmlns, "t"); write_end_element(xmlns, "si");