improve relationships<=>worksheets handling, fixes #59

include/xlnt/workbook/workbook.hpp

@@ -124,6 +124,7 @@ public:
     worksheet create_sheet(std::size_t index);
     worksheet create_sheet(const std::string &title);
     worksheet create_sheet(std::size_t index, const std::string &title);
+    worksheet create_sheet(const std::string &title, const relationship &rel);
 
     void copy_sheet(worksheet worksheet);
     void copy_sheet(worksheet worksheet, std::size_t index);

include/xlnt/worksheet/worksheet.hpp

@@ -79,7 +79,10 @@ public:
 	const workbook &get_workbook() const;
     void garbage_collect();
 
-    // title
+    // identification
+    std::size_t get_id() const;
+    void set_id(std::size_t id);
+
     std::string get_title() const;
     void set_title(const std::string &title);
     std::string make_unique_sheet_name(const std::string &value);

source/detail/excel_serializer.cpp

@@ -158,7 +158,8 @@ bool load_workbook(xlnt::zip_file &archive, bool guess_types, bool data_only, xl
             continue;
         }
 
-        auto ws = wb.create_sheet(sheet_node.attribute("name").value());
+        auto ws = wb.create_sheet(sheet_node.attribute("name").value(), rel);
+        ws.set_id(sheet_node.attribute("sheetId").as_ullong());
         xlnt::worksheet_serializer worksheet_serializer(ws);
 		pugi::xml_document worksheet_xml;
 		worksheet_xml.load(archive.read("xl/" + rel.get_target_uri()).c_str());

source/detail/workbook_serializer.cpp

@@ -278,28 +278,18 @@ void workbook_serializer::write_workbook(pugi::xml_document &xml) const
     auto sheets_node = root_node.append_child("sheets");
     auto defined_names_node = root_node.append_child("definedNames");
 
-    for (const auto &relationship : workbook_.get_relationships())
+    for (const auto ws : workbook_)
     {
-        if (relationship.get_type() == relationship::type::worksheet)
-        {
-            // TODO: this is ugly
-            std::string sheet_index_string = relationship.get_target_uri();
-            sheet_index_string = sheet_index_string.substr(0, sheet_index_string.find('.'));
-            sheet_index_string = sheet_index_string.substr(sheet_index_string.find_last_of('/'));
-            auto iter = sheet_index_string.end();
-            iter--;
-            while (isdigit(*iter))
-                iter--;
-            auto first_digit = iter - sheet_index_string.begin();
-            sheet_index_string = sheet_index_string.substr(static_cast<std::string::size_type>(first_digit + 1));
-            std::size_t sheet_index = static_cast<std::size_t>(std::stoll(sheet_index_string) - 1);
+        auto target = "worksheets/sheet" + std::to_string(ws.get_id()) + ".xml";
 
-            auto ws = workbook_.get_sheet_by_index(sheet_index);
+        for (const auto &rel : workbook_.get_relationships())
+        {
+            if (rel.get_target_uri() != target) continue;
 
             auto sheet_node = sheets_node.append_child("sheet");
             sheet_node.append_attribute("name").set_value(ws.get_title().c_str());
-            sheet_node.append_attribute("sheetId").set_value(std::to_string(sheet_index + 1).c_str());
-            sheet_node.append_attribute("r:id").set_value(relationship.get_id().c_str());
+            sheet_node.append_attribute("sheetId").set_value(std::to_string(ws.get_id()).c_str());
+            sheet_node.append_attribute("r:id").set_value(rel.get_id().c_str());
 
             if (ws.has_auto_filter())
             {

source/detail/worksheet_impl.hpp

@@ -43,8 +43,9 @@ namespace detail {
 
 struct worksheet_impl
 {
-    worksheet_impl(workbook *parent_workbook, const std::string &title)
+    worksheet_impl(workbook *parent_workbook, std::size_t id, const std::string &title)
         : parent_(parent_workbook),
+          id_(id),
           title_(title),
           comment_count_(0)
     {
@@ -58,9 +59,10 @@ struct worksheet_impl
     void operator=(const worksheet_impl &other)
     {
         parent_ = other.parent_;
+        id_ = other.id_;
+        title_ = other.title_;
         column_properties_ = other.column_properties_;
         row_properties_ = other.row_properties_;
-        title_ = other.title_;
         cell_map_ = other.cell_map_;
         for (auto &row : cell_map_)
         {
@@ -84,9 +86,10 @@ struct worksheet_impl
     }
 
     workbook *parent_;
+    std::size_t id_;
+    std::string title_;
     std::unordered_map<column_t, column_properties> column_properties_;
     std::unordered_map<row_t, row_properties> row_properties_;
-    std::string title_;
     std::unordered_map<row_t, std::unordered_map<column_t, cell_impl>> cell_map_;
     std::vector<relationship> relationships_;
     page_setup page_setup_;

source/workbook/workbook.cpp

@@ -158,14 +158,16 @@ worksheet workbook::create_sheet()
         title = "Sheet" + std::to_string(++index);
     }
 
-    std::string sheet_filename = "sheet" + std::to_string(d_->worksheets_.size() + 1) + ".xml";
+    auto sheet_id = d_->worksheets_.size() + 1;
+    std::string sheet_filename = "sheet" + std::to_string(sheet_id) + ".xml";
 
-    d_->worksheets_.push_back(detail::worksheet_impl(this, title));
-    create_relationship("rId" + std::to_string(d_->relationships_.size() + 1),
+    d_->worksheets_.push_back(detail::worksheet_impl(this, sheet_id, title));
+    create_relationship("rId" + std::to_string(sheet_id),
                         "worksheets/" + sheet_filename,
                         relationship::type::worksheet);
-    
-    d_->manifest_.add_override_type("/" + constants::package_worksheets() + "/" + sheet_filename, "application/vnd.openxmlformats-officedocument.spreadsheetml.worksheet+xml");
+    d_->manifest_.add_override_type("/" + constants::package_worksheets()
+        + "/" + sheet_filename,
+        "application/vnd.openxmlformats-officedocument.spreadsheetml.worksheet+xml");
 
     return worksheet(&d_->worksheets_.back());
 }
@@ -389,6 +391,14 @@ worksheet workbook::create_sheet(const std::string &title)
     return ws;
 }
 
+worksheet workbook::create_sheet(const std::string &title, const relationship &rel)
+{
+    auto sheet_id = d_->worksheets_.size() + 1;
+    d_->worksheets_.push_back(detail::worksheet_impl(this, sheet_id, title));
+
+    return worksheet(&d_->worksheets_.back());
+}
+
 workbook::iterator workbook::begin()
 {
     return iterator(*this, 0);
@@ -690,10 +700,24 @@ const std::vector<text> &workbook::get_shared_strings() const
 void workbook::add_shared_string(const text &shared, bool allow_duplicates)
 {
     if (d_->shared_strings_.empty())
+    {
+        bool has_shared_strings = false;
+
+        for (const auto &rel : get_relationships())
+        {
+            if (rel.get_type() == relationship::type::shared_strings)
+            {
+                has_shared_strings = true;
+                break;
+            }
+        }
+
+        if (!has_shared_strings)
         {
             create_relationship(next_relationship_id(), "sharedStrings.xml", relationship::type::shared_strings);
             d_->manifest_.add_override_type("/" + constants::part_shared_strings(), "application/vnd.openxmlformats-officedocument.spreadsheetml.sharedStrings+xml");
         }
+    }
 
 	if (!allow_duplicates)
 	{

source/worksheet/worksheet.cpp

@@ -207,12 +207,23 @@ void worksheet::garbage_collect()
     }
 }
 
+void worksheet::set_id(std::size_t id)
+{
+    d_->id_ = id;
+}
+
+std::size_t worksheet::get_id() const
+{
+    return d_->id_;
+}
+
 std::string worksheet::get_title() const
 {
     if (d_ == nullptr)
     {
         throw std::runtime_error("null worksheet");
     }
+
     return d_->title_;
 }